-
Notifications
You must be signed in to change notification settings - Fork 107
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 for new LLVM pass manager #193
Conversation
So, as expected, this change breaks afl.rs on stable. Steps to reproduce:
So I am wondering what the best way to handle this is. Merge this once I'm not sure we want to support the latest nightly at the expense of stable. Thanks for doing this, BTW, @taiki-e. |
57554a7
to
32dcb7c
Compare
If the latest commit works, where we conditionally include the new flag with the right version, then this LGTM! |
Updated PR to check for rustc and LLVM versions. The new flag is used when Rust 1.57+ that rust-lang/rust#88243 is merged and LLVM13+. Since rust-lang/rust#88243 is available on nightly-2021-09-26+, this drops support for the old 1.57-nightly, but I think it is fine because only nightly is affected. (Using the way described by @smoelius, I confirmed that it works for stable(1.55.0), beta(1.56.0), and nightly(1.57.0).) |
Hmm. I'm having some trouble with the new flag on Linux: https://github.com/smoelius/afl.rs/runs/3779272226?check_suite_focus=true |
Adding Adding I'm curious as to why |
It appears In particular, if you run the following in a temporary directory with the latest version of
What's more, if you repeat #193 (comment) with |
@taiki-e, what do you think about adding If you agree, could you also include a short comment about why the option is there? Thank you again for going to this trouble. |
@smoelius: Seems
I read a PR that forces 1 CGU and a related issue, and I feel it's fine to always force 1 CGU on release mode.
Done in eb50593. |
Perfect! Thanks, @taiki-e. I'll push a new release soon. |
Fixes #192
This drops support for the old nightly, but that should be fine. (See discussion in cargo-fuzz: rust-fuzz/cargo-fuzz#277)