-
Notifications
You must be signed in to change notification settings - Fork 46
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
Update libFuzzer to llvm/llvm-project@60e32a1 #89
Conversation
bors r+ never remember if we have this set up here or not... (we also will need to update our test suite sometime down the line for new pass names I bet) |
Configuration problem: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ci fails
Need to switch |
9351a54
to
55faf5a
Compare
Linking issues in CI. Not totally sure what's going on here. https://github.com/rust-fuzz/libfuzzer/runs/4930210184?check_suite_focus=true#step:5:74
|
@fitzgen I get the same error when trying to compile master. A workaround is to add |
Which means we would probably need to add it to And that is going to slow down build times too. Very much not ideal. Would prefer to figure out how to fix these linker errors without resorting to a single codegen unit, but I don't have time to dig into this myself right now. |
I tried to take a look but linker errors are a bit outside my area of expertise. But I did found another strange bug, see #90. |
This is not ideal, but it does provide two things: 1. It fixes bizarre linker errors about missing `sancov` symbols. 2. It allows LLVM to do inlining that it otherwise refuses to do. For some reason, when sanitizers are enabled, LLVM refuses to inline across codegen units. This is a problem because trivial methods like `Vec::len` won't be inlined, resulting in 100x slowdowns. `cargo fuzz` already restricts its builds to a single codegen unit, so we might as well do the same thing in CI here.
3c309da
to
d4d1c46
Compare
Okay I pushed a commit to do This is not ideal, but it does provide two things:
|
Success! |
This commit migrates CI to using the `cargo fuzz` binary with its settings for compiling Rust code to avoid mismatches like the codegen unit issue found in rust-fuzz#89
This commit migrates CI to using the `cargo fuzz` binary with its settings for compiling Rust code to avoid mismatches like the codegen unit issue found in rust-fuzz#89
FWIW there's some more info about the 1 cgu issue at rust-fuzz/cargo-fuzz#215 |
Supercedes #88