Add initial support for DataFlowSanitizer#120761
Conversation
|
This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp. These commits modify compiler targets. |
|
r? @cuviper |
This comment has been minimized.
This comment has been minimized.
|
These commits modify compiler targets. This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp. |
This comment has been minimized.
This comment has been minimized.
browneee
left a comment
There was a problem hiding this comment.
Hello,
(I am DFSan maintainer in LLVM.)
Thank you for working on DFSan support for Rust!
DFSan support will also need DFSan APIs so the program can make DFSan do something useful. Imo it would be best to do this in this commit, so we can properly test the DFSan pass is working correctly.
There was a problem hiding this comment.
Building with DFSan enabled is a good step, but DFSan needs a bit more than other sanitizers. DFSan doesn't know when it should start taint tracking, and when it should stop taint tracking. There is an API for the program to tell DFSan what it should track and check where it went:
https://github.com/llvm/llvm-project/blob/main/compiler-rt/include/sanitizer/dfsan_interface.h
Should have a basic test like this:
https://github.com/llvm/llvm-project/blob/main/compiler-rt/test/dfsan/basic.c
Other sanitizers have similar interface headers: https://github.com/llvm/llvm-project/blob/main/compiler-rt/include/sanitizer/msan_interface.h
but they are less important (msan will do its thing without that header - you only need the header in rare cases where something unusual is happening and you need to override the behavior). DFSan needs the header to do anything useful.
Looking around rust the closest thing I found for other sanitizers was
but it is not clear to me if this is a useful example.There was a problem hiding this comment.
I don't think the Rust compiler currently has any FFI bindings for any of the sanitizers interfaces. Since those are user interfaces, I think the best way to provide them would be to create and provide a sanitizers crate with those bindings. What do you think?
There was a problem hiding this comment.
Ideally these user interfaces would be kept in sync with the compiler.
I don't know how this should best fit together for Rust repos/build/release.
I guess simplest thing (external crate) is probably fine for getting it working.
Would probably be best to ask Rust people how it should fit together.
There was a problem hiding this comment.
Another possibility is to have this crate in the library/ directory along with core, std, and others. (Note that this crate is different than the rustc_sanitizers planned to be in the compiler/ directory because those are user interfaces.) @cuviper @wesleywiser What do you think?
There was a problem hiding this comment.
I think it will make it a much bigger lift if we have to add to the standard library.
Can you sketch out what kind of calls the user code would need to make?
There was a problem hiding this comment.
As pointed out to me by @cuviper and @wesleywiser, some of the GitHub workflows don't use the src/llvm-project LLVM, and since this test requires the new dfsan runtime library which is enabled by this PR to be built when the build sanitizers option is enabled for bootstrap, it will fail to link with the dfsan runtime library as it doesn't seem to be present in the LLVM used by the GitHub workflow.
This test passes locally using the src/llvm-project LLVM and is also part of the sanitizers crate so I'll remove it from this PR.
There was a problem hiding this comment.
I'm not sure I fully understand the problem, but maybe you just need to update src/bootstrap/download-ci-llvm-stamp to force a rebuild?
There was a problem hiding this comment.
Ahhh, I didn't know about that. Let me give it a try.
There was a problem hiding this comment.
It worked! (It seems it also needed needs-sanitizer-support in the test file.) Thank you! I'll also adjust the path and file names to conform to #121811 which is in the queue to be merged. Then it should be good to go. I'll let you know! Thanks again!
There was a problem hiding this comment.
#121811 was merged and I adjusted the path and file names. It should be good to go. Thanks again!
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
c243d42 to
cec8233
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Do you have some specific use case in mind for this, or is this more for the sake of completeness? I'm always a bit fuzzy on what people actually use dfsan for. |
It helps us to do C++ and Rust interop. If a C++ codebase (that already uses DFSan) starts interoperating with Rust, the DFSan build will break - unless the Rust part can also support DFSan. |
|
☔ The latest upstream changes (presumably #121327) made this pull request unmergeable. Please resolve the merge conflicts. |
tests/ui/sanitize/sanitizer-dataflow-abilist-rust-no-core-no-std.txt
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Adds initial support for DataFlowSanitizer to the Rust compiler. It currently supports `-Zsanitizer-dataflow-abilist`. Additional options for it can be passed to LLVM command line argument processor via LLVM arguments using `llvm-args` codegen option (e.g., `-Cllvm-args=-dfsan-combine-pointer-labels-on-load=false`).
|
@bors r+ |
Rollup of 5 pull requests Successful merges: - rust-lang#120761 (Add initial support for DataFlowSanitizer) - rust-lang#121622 (Preserve same vtable pointer when cloning raw waker, to fix Waker::will_wake) - rust-lang#121716 (match lowering: Lower bindings in a predictable order) - rust-lang#121731 (Now that inlining, mir validation and const eval all use reveal-all, we won't be constraining hidden types here anymore) - rust-lang#121841 (`f16` and `f128` step 2: intrinsics) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#120761 - rcvalle:rust-dfsan, r=nikic Add initial support for DataFlowSanitizer Adds initial support for DataFlowSanitizer to the Rust compiler. It currently supports `-Zsanitizer-dataflow-abilist`. Additional options for it can be passed to LLVM command line argument processor via LLVM arguments using `llvm-args` codegen option (e.g., `-Cllvm-args=-dfsan-combine-pointer-labels-on-load=false`).
Adds initial support for DataFlowSanitizer to the Rust compiler. It currently supports
-Zsanitizer-dataflow-abilist. Additional options for it can be passed to LLVM command line argument processor via LLVM arguments usingllvm-argscodegen option (e.g.,-Cllvm-args=-dfsan-combine-pointer-labels-on-load=false).