Skip to content

GH-48094: [C++] Restrict SecureString capacity tail check to Linux - #49906

Merged
pitrou merged 1 commit into
apache:mainfrom
ArnavBalyan:arnavb/secure-string-msvc-asan
May 5, 2026
Merged

GH-48094: [C++] Restrict SecureString capacity tail check to Linux#49906
pitrou merged 1 commit into
apache:mainfrom
ArnavBalyan:arnavb/secure-string-msvc-asan

Conversation

@ArnavBalyan

@ArnavBalyanArnavBalyan commented May 1, 2026•
edited
Loading

Copy link
Copy Markdown
Member

Rationale for this change

What changes are included in this PR?

  • Guard for platform specific behavior.

Are these changes tested?

  • Yes

Are there any user-facing changes?

  • Yes

@github-actions

github-actionsBot commented May 1, 2026

Copy link
Copy Markdown

⚠️ GitHub issue #48094has been automatically assigned in GitHub to PR creator.

@ArnavBalyan

ArnavBalyan commented May 1, 2026

Copy link
Copy Markdown
MemberAuthor

cc @pitrou thanks

@pitrou

pitrou commented May 5, 2026

Copy link
Copy Markdown
Member

Can we condition this on MSVC rather than Linux? The test can still be used under macOS and other platforms.

@ArnavBalyan
ArnavBalyanforce-pushed the arnavb/secure-string-msvc-asan branch from 69d0341 to 1416cc9CompareMay 5, 2026 09:53
@ArnavBalyan
ArnavBalyanforce-pushed the arnavb/secure-string-msvc-asan branch from 1416cc9 to 1279689CompareMay 5, 2026 09:56
@ArnavBalyan

ArnavBalyan commented May 5, 2026

Copy link
Copy Markdown
MemberAuthor

done thanks

pitrou
pitrou approved these changes May 5, 2026
View reviewed changes

@pitroupitrou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, thank you @ArnavBalyan

@pitrou
pitrou merged commit 25e4fdb into apache:mainMay 5, 2026
54 of 56 checks passed
@pitroupitrou removed the awaiting review Awaiting review label May 5, 2026
@pitroupitrou mentioned this pull request May 5, 2026
Closed
@pitrou

pitrou commented May 5, 2026

Copy link
Copy Markdown
Member

Oops, I should have changed the PR title before merging. Too late.

@github-actionsgithub-actionsBot added the awaiting committer review Awaiting committer review label May 5, 2026
@conbench-apache-arrow

conbench-apache-arrowBot commented May 5, 2026

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 0 benchmarking runs that have been run so far on merge-commit 25e4fdb.

None of the specified runs were found on the Conbench server.

The full Conbench report has more details.

@conbench-apache-arrow

conbench-apache-arrowBot commented May 14, 2026

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 25e4fdb.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them.

Mottl pushed a commit to Mottl/arrow that referenced this pull request May 26, 2026
@ArnavBalyan@Mottl
apacheGH-48094: [C++] Restrict SecureString capacity tail check to Li…
23a5580
…nux (apache#49906)
### Rationale for this change
- The test reads the unused tail of string past size(), which is undefined. libstdc++ keeps that range addressable, but MSVC does not.
- Ensure the check is not run for MSVC. Existing assertions still run, the unused capacity is not asserted on non linux platforms.
- Closesapache#48094.
### What changes are included in this PR?
- Guard for platform specific behavior.
### Are these changes tested?
- Yes
### Are there any user-facing changes?
- Yes
* GitHub Issue: apache#48094
Authored-by: Arnav Balyan <arnavbalyan1@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pitroupitroupitrou approved these changes

Assignees

No one assigned

Labels

awaiting committer reviewAwaiting committer reviewComponent: C++

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[C++] Arrow test 'arrow-utility-test' contains heap-buffer-overflow error

2 participants

@ArnavBalyan@pitrou