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.

Sign up for GitHub

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

Do not pass -C passes=... when nightly is used and plugins are compiled #449

Merged
merged 6 commits into from
Jan 16, 2024
Merged

Do not pass -C passes=... when nightly is used and plugins are compiled #449

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
87c0dbf
fix cargo-afl
vanhauser-thc Jan 15, 2024
c92e10a
Merge branch 'master' into master
vanhauser-thc Jan 15, 2024
2bd2d7a
revert afl++ commit
vanhauser-thc Jan 16, 2024
456144b
Merge branch 'master' into master
vanhauser-thc Jan 16, 2024
431b888
Remove added `is_nightly` check
smoelius Jan 16, 2024
ea716de
Address Clippy warning
smoelius Jan 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cargo-afl/AFLplusplus
Open in desktop
Submodule AFLplusplus updated 36 files
+4 −3 GNUmakefile
+7 −4 GNUmakefile.llvm
+1 −1 README.md
+1 −0 TODO.md
+1 −1 custom_mutators/grammar_mutator/GRAMMAR_VERSION
+1 −1 custom_mutators/grammar_mutator/grammar_mutator
+24 −1 docs/Changelog.md
+3 −3 docs/custom_mutators.md
+13 −0 docs/env_variables.md
+1 −7 docs/fuzzing_binary-only_targets.md
+2 −2 frida_mode/src/main.c
+1 −1 frida_mode/src/ranges.c
+18 −3 include/afl-mutations.h
+1 −1 include/config.h
+85 −231 include/envs.h
+7 −0 injections.dic
+48 −0 instrumentation/README.injections.md
+31 −23 instrumentation/SanitizerCoverageLTO.so.cc
+5 −0 instrumentation/SanitizerCoveragePCGUARD.so.cc
+49 −0 instrumentation/afl-compiler-rt.o.c
+18 −15 instrumentation/afl-llvm-dict2file.so.cc
+8 −7 instrumentation/cmplog-routines-pass.cc
+16 −14 instrumentation/compare-transform-pass.so.cc
+366 −0 instrumentation/injection-pass.cc
+50 −31 instrumentation/split-compares-pass.so.cc
+1 −1 nyx_mode/QEMU-Nyx
+1 −1 nyx_mode/libnyx
+1 −1 nyx_mode/packer
+1 −1 qemu_mode/QEMUAFL_VERSION
+30 −3 qemu_mode/README.md
+3 −1 qemu_mode/build_qemu_support.sh
+1 −1 qemu_mode/qemuafl
+1,853 −1,407 src/afl-cc.c
+9 −32 src/afl-fuzz-run.c
+1 −2 src/afl-fuzz-state.c
+29 −1 src/afl-fuzz.c
10 changes: 5 additions & 5 deletions cargo-afl/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,6 @@ where
let mut rustflags = format!(
"-C debug-assertions \
-C overflow_checks \
-C passes={passes} \
-C codegen-units=1 \
-C opt-level=3 \
-C target-cpu=native "
Expand All @@ -253,7 +252,7 @@ where
environment_variables.insert("ASAN_OPTIONS", asan_options);
environment_variables.insert("TSAN_OPTIONS", tsan_options);

if plugins_available() {
if plugins_available() && is_nightly() {
Copy link
Member

Choose a reason for hiding this comment

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

Why was is_nightly added here? Note that the assert two lines below checks that the compiler is nightly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested something else and forgot to remove it, on the other hand it does not hurt either :)

Copy link
Member

Choose a reason for hiding this comment

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

on the other hand it does not hurt either :)

I'm not sure I agree with this.

The is_nightly check is to ensure that a case not expected to occur in principle does not occur in practice. That case is that the plugins were built with a non-nightly compiler.

Suppose that case were to occur in practice. Without the change, a panic occurs. With the change, cargo-afl silently reverts to the non-nightly (i.e., non-plugin) behavior.

To me, the former sounds preferable. But you don't agree?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

both are good and bad.
with the panic - does a user know what to do to fix it? if not then silently taking the alternative is IMHO better. If the error message clearly states how ti fix it, then panic is better.
as I said, no time currently :(

// Make sure we are on nightly for the -Z flags
assert!(
rustc_version::version_meta().unwrap().channel == rustc_version::Channel::Nightly,
Expand All @@ -271,13 +270,14 @@ where

environment_variables.insert("AFL_QUIET", "1".to_string());
} else {
rustflags.push_str(
"-C llvm-args=-sanitizer-coverage-level=3 \
rustflags.push_str(&format!(
smoelius marked this conversation as resolved.
Show resolved Hide resolved
"-C passes={passes} \
-C llvm-args=-sanitizer-coverage-level=3 \
smoelius marked this conversation as resolved.
Show resolved Hide resolved
-C llvm-args=-sanitizer-coverage-trace-pc-guard \
-C llvm-args=-sanitizer-coverage-prune-blocks=0 \
-C llvm-args=-sanitizer-coverage-trace-compares
",
);
));
}

let no_cfg_fuzzing = env::var("AFL_NO_CFG_FUZZING").is_ok();
Expand Down
Loading