Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect use of MemorySanitizer without using Nightly-only features #571

Merged
merged 2 commits into from
Dec 18, 2024

Conversation

josephlr
Copy link
Member

This allows msan detection to "just-work" whenever someone passes -Zsanitizer=memory. Users no longer need to do any getrandom-specific configuration.

This will also continue working once rust-lang/rust#123615 is merged, which stabilizes some sanitizers (but not MemorySanitizer).

This is the approch taken by other low-level crates:

The only downside is that this adds a build-script, but it's as small as possible, doesn't seem to impact build times, and is only a temporary workaround.

@josephlr josephlr requested a review from newpavlov December 18, 2024 21:11
Copy link
Member

@newpavlov newpavlov left a comment

Choose a reason for hiding this comment

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

Addition of a build script is quite unfortunate, but since this approach is used by other fundamental crates I guess it's acceptable.

README.md Outdated Show resolved Hide resolved
This allows msan detection to "just-work" whenever someone passes
`-Zsanitizer=memory`. Users no longer need to do any
`getrandom`-specific configuration.

This will also continue working once
rust-lang/rust#123615 is merged which
stabilizes some sanitizers (but not MemorySanitizer).

This is the approch taken by other low-level crates:
  - [`parking_lot_core`](https://github.com/Amanieu/parking_lot/blob/ca920b31312839013b4455aba1d53a4aede21b2f/core/build.rs)
  - [`crossbeam-utils`](https://github.com/crossbeam-rs/crossbeam/blob/00283fb1818174c25b02d7f1c883c5e19f8506a4/crossbeam-utils/build.rs#L42)

The only downside is that this adds a build-script, but it's as small as
possible, doesn't seem to impact build times, and is only a temporary
workaround.

Signed-off-by: Joe Richey <[email protected]>
@newpavlov
Copy link
Member

newpavlov commented Dec 18, 2024

It's worth to update the changelog and change the configuration flag mentioned in it.

Signed-off-by: Joe Richey <[email protected]>
@josephlr josephlr merged commit 9b902af into master Dec 18, 2024
60 checks passed
@josephlr josephlr deleted the msan branch December 18, 2024 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants