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

oss-fuzz clang issues #9989

Closed
amodra opened this issue Mar 24, 2023 · 23 comments
Closed

oss-fuzz clang issues #9989

amodra opened this issue Mar 24, 2023 · 23 comments

Comments

@amodra
Copy link
Contributor

amodra commented Mar 24, 2023

Bugs in clang version 15.0.0 (https://github.com/llvm/llvm-project.git bf7f8d6fa6f460bf0a16ffec319cd71592216bf4) used by oss-fuzz are the cause of binutils issues 57127, 57050, 57042, and 57041. An update to clang from a release branch would be welcome.

@DonggeLiu
Copy link
Contributor

@oliverchang, I guess it is safer to update Clang for this project only, if the current Clang works fine with other projects?
Is there a better way to resolve this?

@oliverchang
Copy link
Collaborator

It's possible, but probably something we should avoid. @jonathanmetzman thoughts?

@amodra do you have any details on what particular bug/issue with with Clang 15 causes these?

@amodra
Copy link
Contributor Author

amodra commented Mar 29, 2023

The binutils fuzzer issues that I mentioned are all in the setjmp return path after a longjmp. The source being compiled, i386-dis.c, does have a problem in that local variable accessed on that path are not volatile, but making them volatile doesn't help with the version of clang used by oss-fuzz. See https://sourceware.org/pipermail/binutils/2023-March/126757.html

My logs show the problem i386-dis.c compile as:
clang -DHAVE_CONFIG_H -I. -I. -I. -I../bfd -I./../include -I./../bfd -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Wshadow -O1 -fno-omit-frame-pointer -g -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION -fno-builtin -fsanitize-coverage=trace-pc-guard,pc-table,trace-cmp -O2 -g -Wno-unused-command-line-argument -Wl,-ldl -Wl,-lrt -Wl,-lpthread -Wl,/src/centipede/weak.o -fsanitize=address -fsanitize-address-use-after-scope -MT i386-dis.lo -MD -MP -MF .deps/i386-dis.Tpo -c i386-dis.c -o i386-dis.o

(Yes, I have some local mods to projects/binutils/build.sh to substitute -gline-tables-only with -g in CFLAGS and to pass V=1 to make in order to show the compiler command line.)

@oliverchang
Copy link
Collaborator

Thanks! Can you confirm if this is fixed in more recent Clang versions?

@amodra
Copy link
Contributor Author

amodra commented Mar 29, 2023

I can confirm that Ubuntu clang version 15.0.6 compiles binutils OK using
CC=clang-15 CXX=clang++-15
CFLAGS="-g -O2 -fno-omit-frame-pointer -fno-builtin -fsanitize-coverage=trace-pc-guard,pc-table,trace-cmp -fsanitize=address -fsanitize-address-use-after-scope"
CXXFLAGS="-g -O2 -fno-omit-frame-pointer -fno-builtin -fsanitize-coverage=trace-pc-guard,pc-table,trace-cmp -fsanitize=address -fsanitize-address-use-after-scope"
~/src/binutils-gdb/configure --build=x86_64-linux --enable-targets=all
--disable-gold --enable-threads
--disable-gdb --disable-gdbserver --disable-sim --disable-gprofng
--disable-libbacktrace --disable-readline --disable-libdecnumber
--disable-x86-used-note
--enable-plugins --disable-nls

and does not segfault on the problem oss-fuzz testcases. That isn't the same as saying that oss-fuzz will work fine with clang-15.0.6 of course, and I may have missed something critical about the centipede fuzzer build. I'm a complete newbie when it comes to using docker and the oss-fuzz build scripts..

@catenacyber
Copy link
Contributor

cf #9397 Alan ;-)

@jonathanmetzman
Copy link
Contributor

I'm fine with a single project using their own clang.

@ADKaster
Copy link
Contributor

ADKaster commented May 7, 2023

Clang 15.0.0 rather than Clang 15.0.x (where X is > 3) is also the cause of the current serenity oss-fuzz build failures.

https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=58457&sort=-opened&can=1&q=proj%3Aserenity

Is updating the version of clang used in https://github.com/google/oss-fuzz/blob/f9401119d859e7837e2bf0b7c7c4b73b339dd6d2/projects/serenity/Dockerfile as easy as opening a PR, or are there other infra steps required?

@DonggeLiu
Copy link
Contributor

I think that should be fine, WDYT @jonathanmetzman?

@ADKaster
Copy link
Contributor

ADKaster commented May 9, 2023

We were able to work around the issue with SerenityOS/serenity@872e18f (apparently Apple Clang from Xcode 14.3 has the same issue), so I don't think it'll be required for serenity right this second. The question remains open on how to customize the llvm version in our docker container if Apple clang's C++20 support surpasses the llvm commit used by oss-fuzz though (or how to help push through the llvm updates more globally :) )

@jonathanmetzman
Copy link
Contributor

Yeah we're fine with you update your own clang, but you are in warranty void territory (not that there's othewise a warranty :-)

@ligurio
Copy link
Contributor

ligurio commented Jun 6, 2023

clang 15 segfaults on linking fuzzing binaries, see #10448 (comment)
What is a right way to bump clang in a docker image?

@jonathanmetzman
Copy link
Contributor

There's no right way because we don't encourage [ ;-) ] this. But I think using the ppa works well

@maflcko
Copy link
Contributor

maflcko commented Jun 13, 2023

But I think using the ppa works well

I presume one would need to link to the oss-fuzz libfuzzer and libc++, or does using libfuzzer/libc++ from the llvm-16 apt work in theory as well?

cc @fanquake

@maflcko maflcko mentioned this issue Jun 13, 2023
@fanquake
Copy link
Contributor

cc @fanquake

I'll have a look

@LebedevRI
Copy link
Contributor

Is there a plan/timeline for the OUR_LLVM_REVISION bump?

Also, has an epoch solution been considered?
Requiring all projects to work with the newer OUR_LLVM_REVISION
seems like a rather large PITA, but perhaps having gcr.io/oss-fuzz-base/base-builder:15,
gcr.io/oss-fuzz-base/base-builder:16, gcr.io/oss-fuzz-base/base-builder:latest
and so on, and just pinning problematic projects to lower epoch of base container
would result in smoother expirience?

@nathaniel-brough
Copy link
Contributor

Also piling on as I'm unable to get google/pigweed to build using oss-fuzz's system clang-15 toolchain. At the moment, pigweed is using a vendored clang-18 version from cipd, but it's hard to get this to work with AFL as the clang wrappers aren't vendored into the project.

#11238

@oliverchang
Copy link
Collaborator

Is there a plan/timeline for the OUR_LLVM_REVISION bump?

Also, has an epoch solution been considered? Requiring all projects to work with the newer OUR_LLVM_REVISION seems like a rather large PITA, but perhaps having gcr.io/oss-fuzz-base/base-builder:15, gcr.io/oss-fuzz-base/base-builder:16, gcr.io/oss-fuzz-base/base-builder:latest and so on, and just pinning problematic projects to lower epoch of base container would result in smoother expirience?

I think we are going to have to consider something like this. @jonathanmetzman WDYT?

@jonathanmetzman
Copy link
Contributor

There will be quite a few hiccups with doing versioning, such as having fuzzers that are out of date if we are using an old base-builder. Maybe we should do something different like upload clang builds to a bucket for later downloading and installation.

@LebedevRI
Copy link
Contributor

Food for thought:

macos 15 will probably be released this summer, which will probably, if the trend continues,
mean that macOS 12 will become EOL, and macOS 13 will become the oldest (supported) macOS release.
XCode 14.2 (LLVM14-based) is the newest one that can be used on macOS 12,
while on macOS 13, it's XCode 15.2 (LLVM16-based).

Currently, oss-fuzz comes with LLVM15-ish. While it would be nice for oss-fuzz
to closely track LLVM tip-of-trunk, it would be a rather awesome miracle
if oss-fuzz could at least be bumped to LLVM16 around the same timeframe.

LebedevRI added a commit to LebedevRI/rawspeed that referenced this issue Feb 16, 2024
oss-fuzz comes with LLVM15-ish, but that (aside from macOS,
which comes with LLVM16), is the only libc++-based distro,
so it looks like we can get away with requiring libc++-16.

It is a pity that we have to support clang-15,
but that is much more invasive of a change.

Refs. google/oss-fuzz#9989
LebedevRI added a commit to LebedevRI/rawspeed that referenced this issue Feb 16, 2024
oss-fuzz comes with LLVM15-ish, but that (aside from macOS,
which comes with LLVM16), is the only libc++-based distro,
so it looks like we can get away with requiring libc++-16.

It is a pity that we have to support clang-15,
but that is much more invasive of a change.

Refs. google/oss-fuzz#9989
LebedevRI added a commit to LebedevRI/rawspeed that referenced this issue Feb 16, 2024
oss-fuzz comes with LLVM15-ish, but that (aside from macOS,
which comes with LLVM16), is the only libc++-based distro,
so it looks like we can get away with requiring libc++-16.

It is a pity that we have to support clang-15,
but that is much more invasive of a change.

Refs. google/oss-fuzz#9989
@ADKaster
Copy link
Contributor

With #8108 now closed, what is the best way to track progress on bumping OUR_LLVM_REVISION to an llvm-17 or llvm-18 based commit?

As an aside I'm still confused why oss-fuzz uses a chromium-managed commit instead of using a public release tag. The llvm-project releases have a lot of back ports for bugs discovered after the initial -init for a branch. Not using them makes it tricky to ensure that the compiler used for fuzzing matches other CI.

@maflcko
Copy link
Contributor

maflcko commented Apr 30, 2024

This was fixed today in #11714 and can be closed?

@oliverchang
Copy link
Collaborator

Indeed, thank you very much once again @maflcko!

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

No branches or pull requests