Add sanitizer support on FreeBSD #74576
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @oli-obk (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
That std patch by itself shouldn't be necessary, rust/src/libstd/os/freebsd/raw.rs Line 78 in d8bdb3f |
That's what I'm wondering how to do..
(just excluding the method would be fine too, almost nobody really seems to use that ext thing anyway) I guess the only way is to repeat what libc's build.rs does with |
Oh — the thing is, the accessor does not use the Maybe that should be changed, and then if we detect the |
|
If the libc crate can figure this out, maybe we can, too? Or is this done in a build script? |
|
while implementation wise we can definitely add more things to the libstd build script, I don't know anything about this area and how to proceed best. |
|
Unfortunately I'm not sure I'll be able to help much here. It sounds like this is tied up in the question of what FreeBSD version is targeted for targets like Overally I don't have a great answer here, sorry :(. I think it would work to duplicate libc's build script into libstd, but that's not exactly a great solution unfortunately. |
|
The great solution would be to have target versions ( |
Hmm... I couldn't find any preexisting versioning like this in https://github.com/rust-lang/rust/tree/e59b08e62ea691916d2f063cac5aab4634128022/src/librustc_target/spec so yea, that would be a major undertaking. How about going with something really obvious like |
valpackett
force-pushed
the
freebsd-sanitizers
branch
from
71b5b4c
to
6343f06
Compare
|
☔ The latest upstream changes (presumably #74844) made this pull request unmergeable. Please resolve the merge conflicts. |
oli-obk
reviewed
Jul 31, 2020
oli-obk
reviewed
Jul 31, 2020
oli-obk
reviewed
Jul 31, 2020
|
sorry, I didn't see your force push |
valpackett
force-pushed
the
freebsd-sanitizers
branch
from
6343f06
to
baaf084
Compare
|
@bors r+ |
|
📌 Commit baaf084 has been approved by |
bors
added
S-waiting-on-bors
Manishearth
added a commit
to Manishearth/rust
that referenced
this pull request
Jul 31, 2020
…-obk Add sanitizer support on FreeBSD Restarting rust-lang#47337. Everything is better now, no more weird llvm problems, well not everything: Unfortunately, the sanitizers don't have proper support for versioned symbols (google/sanitizers#628), so `libc`'s usage of `stat@FBSD_1.0` and so on explodes, e.g. in calling `std::fs::metadata`. Building std (now easy thanks to cargo `-Zbuild-std`) and libc with `freebsd12/13` config via the `LIBC_CI=1` env variable is a good workaround… ``` LIBC_CI=1 RUSTFLAGS="-Z sanitizer=address" cargo +san-test -Zbuild-std run --target x86_64-unknown-freebsd --verbose ``` …*except* std won't build because there's no `st_lspare` in the ino64 version of the struct, so an std patch is required: ```diff --- i/src/libstd/os/freebsd/fs.rs +++ w/src/libstd/os/freebsd/fs.rs @@ -66,8 +66,6 @@ pub trait MetadataExt { fn st_flags(&self) -> u32; #[stable(feature = "metadata_ext2", since = "1.8.0")] fn st_gen(&self) -> u32; - #[stable(feature = "metadata_ext2", since = "1.8.0")] - fn st_lspare(&self) -> u32; } #[stable(feature = "metadata_ext", since = "1.1.0")] @@ -136,7 +134,4 @@ impl MetadataExt for Metadata { fn st_flags(&self) -> u32 { self.as_inner().as_inner().st_flags as u32 } - fn st_lspare(&self) -> u32 { - self.as_inner().as_inner().st_lspare as u32 - } } ``` I guess std could like.. detect that `libc` isn't built for the old ABI, and replace the implementation of `st_lspare` with a panic?
bors
added
S-waiting-on-author
valpackett
force-pushed
the
freebsd-sanitizers
branch
from
baaf084
to
224552f
Compare
bors
added
S-waiting-on-bors
|
⌛ Testing commit ddbc456 with merge e9d65c53543dee07c4dd26407716f6083c43c4c1... |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
Spurious network error: |
|
@bors retry |
bors
added
S-waiting-on-bors
|
⌛ Testing commit ddbc456 with merge c18def13276b39d8270d559735e824f4167c307b... |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
tmandry
added a commit
to tmandry/rust
that referenced
this pull request
Aug 15, 2020
…imulacrum Set CMAKE_SYSTEM_NAME when cross-compiling Configure CMAKE_SYSTEM_NAME when cross-compiling in `configure_cmake`, to tell CMake about target system. Previously this was done only for LLVM step and now applies more generally to steps using cmake. Helps with rust-lang#74576.
|
Can we retry this? The #75376 should address tsan build failure. |
|
@bors retry |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions, checks-azure |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Restarting #47337. Everything is better now, no more weird llvm problems, well not everything:
Unfortunately, the sanitizers don't have proper support for versioned symbols (google/sanitizers#628), so
libc's usage ofstat@FBSD_1.0and so on explodes, e.g. in callingstd::fs::metadata.Building std (now easy thanks to cargo
-Zbuild-std) and libc withfreebsd12/13config via theLIBC_CI=1env variable is a good workaround……except std won't build because there's no
st_lsparein the ino64 version of the struct, so an std patch is required:I guess std could like.. detect that
libcisn't built for the old ABI, and replace the implementation ofst_lsparewith a panic?