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.

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 AFL++ commit id #9461

Merged
merged 40 commits into from
May 25, 2023
Merged

Update AFL++ commit id #9461

merged 40 commits into from
May 25, 2023

Conversation

vanhauser-thc
Copy link
Contributor

minor fixes and enhancements. does not depend on the PR in clusterfuzz.

@jonathanmetzman
Copy link
Contributor

/gcbrun trial_build.py all --sanitizer coverage --fuzzing-engine afl

@jonathanmetzman
Copy link
Contributor

/gcbrun trial_build.py skcms --sanitizer address --fuzzing-engine libfuzzer

@jonathanmetzman
Copy link
Contributor

/gcbrun trial_build.py all --sanitizer address --fuzzing-engine afl

@jonathanmetzman
Copy link
Contributor

This breaks three projects. Because the breakage is a runtime, I'm a little nervous about merging this. maybe worth looking into why they break:

Step #20 - "build-check-afl-address-x86_64": [*] Attempting dry run with 'id:000000,time:0,execs:0,orig:input'...
Step #20 - "build-check-afl-address-x86_64": 
Step #20 - "build-check-afl-address-x86_64": [-] Oops, the program crashed with one of the test cases provided. There are
Step #20 - "build-check-afl-address-x86_64":     several possible explanations:
Step #20 - "build-check-afl-address-x86_64": 
Step #20 - "build-check-afl-address-x86_64":     - The test case causes known crashes under normal working conditions. If
Step #20 - "build-check-afl-address-x86_64":       so, please remove it. The fuzzer should be seeded with interesting
Step #20 - "build-check-afl-address-x86_64":       inputs - but not ones that cause an outright crash.
Step #20 - "build-check-afl-address-x86_64": 
Step #20 - "build-check-afl-address-x86_64":     - In QEMU persistent mode the selected address(es) for the loop are not
Step #20 - "build-check-afl-address-x86_64":       properly cleaning up variables and memory. Try adding
Step #20 - "build-check-afl-address-x86_64":       AFL_QEMU_PERSISTENT_GPR=1 or select better addresses in the binary.
Step #20 - "build-check-afl-address-x86_64": 
Step #20 - "build-check-afl-address-x86_64":     - Least likely, there is a horrible bug in the fuzzer. If other options
Step #20 - "build-check-afl-address-x86_64":       fail, poke <afl-users@googlegroups.com> for troubleshooting tips.
Step #20 - "build-check-afl-address-x86_64": [!] WARNING: Test case 'id:000000,time:0,execs:0,orig:input' results in a crash, skipping
Step #20 - "build-check-afl-address-x86_64": [+] All test cases processed.
Step #20 - "build-check-afl-address-x86_64": �
Step #20 - "build-check-afl-address-x86_64": [-] PROGRAM ABORT : We need at least one valid input seed that does not crash!
Step #20 - "build-check-afl-address-x86_64":          Location : main(), src/afl-fuzz.c:2228
Step #20 - "build-check-afl-address-x86_64": 
Step #20 - "build-check-afl-address-x86_64": 
Step #20 - "build-check-afl-address-x86_64": ERROR: 100.0% of fuzz targets seem to be broken. See the list above for a detailed information.

tor, woff2, pcre2

To repro:
docker pull gcr.io/oss-fuzz-base/base-builder-testing-master
docker tag gcr.io/oss-fuzz-base/base-builder-testing-master gcr.io/oss-fuzz-base/base-builder # Let's use your update AFL++
python infra/helper.py build_fuzzers --engine afl
python infra/helper.py check_build --engine afl

@DonggeLiu
Copy link
Contributor

what do you mean? that is what this PR is about, right?

Oh, sorry if I misunderstood.
Does this PR update AFL++ to a version with the regression fix (e.g., later than d822181467ec41f1ee2d840c3c5b1918c72ffc86)?

@vanhauser-thc
Copy link
Contributor Author

yes it is the most current state :)

@jonathanmetzman
Copy link
Contributor

poppler seems to fail again but it's a different issue and it passed once. I'm going to ignore that. immer - is broken outside of this PR.failed to parse default search paths from compiler output

@@ -16,8 +16,8 @@
################################################################################

# Temporarily disable randomization and enforce a safe and sane setup
export AFL_SKIP_OSSFUZZ=1
Copy link
Contributor

@jonathanmetzman jonathanmetzman May 24, 2023

Choose a reason for hiding this comment

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

Is this reenabling randomization? I don't want to do this, sorry

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does not but however set this made a m mistake which cost afl++ a lot of effectiveness. In the future it is better to talk to me about changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I'm not great at bash, but this change seems to the block here to be taken and thus randomization being reenabled. Am I misunderstanding something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah you talk about the ossfuzz skip, you are right that this enables randomization. When I enabled this it was because nobody told me that this was necessary. It looked like a mistake because the line below is. Classic instrumentation is a very bad idea. In cases where targets do asm stuff a target specific “native” should be used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This pr is now 4 months old and I am not at my computer so I have little memory on this … ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure. Let me skip the randomization and try again.

@jonathanmetzman
Copy link
Contributor

jonathanmetzman commented May 24, 2023

If this doesn't reenable randomization, I'm fine with it, and will merge. I don't want randomization reenabled because it's too frustrating for devs to debug builds.

@jonathanmetzman
Copy link
Contributor

/gcbrun trial_build.py all --fuzzing-engine afl --sanitizer address

@DonggeLiu
Copy link
Contributor

yes it is the most current state :)

Amazing, thanks!

@jonathanmetzman
Copy link
Contributor

All of these failures look spurious.

Copy link
Contributor

@jonathanmetzman jonathanmetzman left a comment

Choose a reason for hiding this comment

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

LGTM

@jonathanmetzman jonathanmetzman merged commit 10473b7 into google:master May 25, 2023
@vanhauser-thc
Copy link
Contributor Author

yay! :) sorry for the mix up about the random compile option feature.
please ping me for any changes you make to the afl++ setup in the future.

@maflcko
Copy link
Contributor

maflcko commented May 30, 2023

Looks like this has issues building libevent for us? libfuzzer works, afl doesn't. To reproduce:

OSS_FUZZ_CI=1 python3 infra/helper.py build_fuzzers --sanitizer undefined  --engine afl bitcoin-core
...
Configuring libevent...
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /usr/bin/mkdir -p
checking for gawk... no
checking for mawk... mawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
checking whether make supports nested variables... (cached) yes
checking whether make supports the include directive... yes (GNU style)
checking for x86_64-pc-linux-gnu-gcc... /src/aflplusplus/afl-clang-fast
checking whether the C compiler works... no
configure: error: in `/src/bitcoin-core/depends/work/build/x86_64-pc-linux-gnu/libevent/2.1.12-stable-4246a966c2d':
configure: error: C compiler cannot create executables
See `config.log' for more details
make: *** [funcs.mk:291: /src/bitcoin-core/depends/x86_64-pc-linux-gnu/.libevent_stamp_configured] Error 77

Not sure how to get the config log out of there. cc @dergoegge @fanquake

As a workaround we can revert thin LTO.

DavidKorczynski pushed a commit that referenced this pull request May 30, 2023
LTO=1 temporarily disabled due to
#9461 (comment)

Also, avoid wasting storage space on a docker layer by merging the two
layers.

cc @dergoegge @fanquake
@fanquake
Copy link
Contributor

Not sure how to get the config log out of there. cc @dergoegge @fanquake

configure:3759: checking whether the C compiler works
configure:3781: /src/aflplusplus/afl-clang-fast  -O1 -fno-omit-frame-pointer -gline-tables-only -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fsanitize=array-bounds,bool,builtin,enum,float-divide-by-zero,function,integer-divide-by-zero,null,object-size,return,returns-nonnull-attribute,shift,signed-integer-overflow,unsigned-integer-overflow,unreachable,vla-bound,vptr -fno-sanitize-recover=array-bounds,bool,builtin,enum,float-divide-by-zero,function,integer-divide-by-zero,null,object-size,return,returns-nonnull-attribute,shift,signed-integer-overflow,unreachable,vla-bound,vptr        -I/src/bitcoin-core/depends/x86_64-pc-linux-gnu/include -D_FORTIFY_SOURCE=3     -flto=thin  -L/src/bitcoin-core/depends/x86_64-pc-linux-gnu/lib     conftest.c  >&5
/usr/bin/ld: error: LLVM gold plugin: Invalid record (Producer: 'LLVM15.0.0git' Reader: 'LLVM 15.0.0git')
clang-15: error: linker command failed with exit code 1 (use -v to see invocation)
configure:3785: $? = 1
configure:3823: result: no

@vanhauser-thc
Copy link
Contributor Author

an alternative solution is to set export AFL_LLVM_INSTRUMENT=LLVM-NATIVE in the build.sh script.

the problem is that pcguard 15+ produces code specific to an llvm version as it seems, and we do not build our pcguard to the specific variant of what llvm 15+ expects this breaks.

this will be quite an effort to have an own pcguard that changes behaviour based on the llvm version used ... for the time being I will make a PR to oss-fuzz that switches to llvm native instrumentation. that will be a 3%+ performance decrease though as ours is quite a bit better.

@fanquake
Copy link
Contributor

@vanhauser-thc thanks for following up.

an alternative solution is to set export AFL_LLVM_INSTRUMENT=LLVM-NATIVE in the build.sh script.

I can confirm that using AFL_LLVM_INSTRUMENT=LLVM-NATIVE fixes configuring in our build (after re-enabling ThinLTO).

fanquake added a commit to fanquake/oss-fuzz that referenced this pull request Jul 14, 2023
Only for merge after google#10427,
which fixes the issue we saw:
google#9461 (comment).
DavidKorczynski pushed a commit that referenced this pull request Jul 14, 2023
For merge after #10427, which
fixes the issue we saw with AFL++:
#9461 (comment).

cc @MarcoFalke @dergoegge
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants