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

infra: fix fuzz-introspector linker flags #7583

Merged

Conversation

DavidKorczynski
Copy link
Collaborator

Moves -fuse-ld=gold to compile flags and removes -flto from linker
flags.

Should fix a number of the projects
#7540 (comment)

Ref:
#7540 (comment)

Ref:
#7540 (comment)

Moves -fuse-ld=gold to compile flags and removes -flto from linker
flags.

Should fix a number of the projects
google#7540 (comment)

Ref:
google#7540 (comment)

Ref:
google#7540 (comment)
@evverx
Copy link
Contributor

evverx commented Apr 19, 2022

I passed these flags to the systemd build script and it appears it isn't compatible with meson:

Compiler stderr:
 clang-13: error: argument unused during compilation: '-fuse-ld=gold' [-Werror,-Wunused-command-line-argument]

Compiler for C supports arguments -fsanitize=fuzzer-no-link: NO

meson.build:315:16: ERROR: Problem encountered: Looks like neither libFuzzer nor -fsanitize=fuzzer-no-link is supported
+ exit 1

It seems to be another variation on mesonbuild/meson#4542

@DavidKorczynski DavidKorczynski marked this pull request as draft April 19, 2022 10:55
@DavidKorczynski
Copy link
Collaborator Author

DavidKorczynski commented Apr 19, 2022

hmm, I just tried successfully running tinygltf (https://github.com/google/oss-fuzz/blob/master/projects/tinygltf/build.sh) through this set up that also uses meson. Could this be of something related to systemd's use of meson that we may be able to avoid when compiling with fuzz-introspector?

@evverx
Copy link
Contributor

evverx commented Apr 19, 2022

The systemd build system supports three different modes of building the fuzz targets. I ran it locally without the OSS-Fuzz toolchain. On OSS-Fuzz that particular check isn't triggered because it's always assumed that -fsanitize=fuzzer-no-link is supported there so in theory it should probably work on OSS-Fuzz. Then again if it works it's just kind of a coincidence and can be broken if systemd decides to call cc.has_argument unrelated to the fuzzers somewhere else.

@evverx
Copy link
Contributor

evverx commented Apr 19, 2022

@dcbaker @eli-schwartz I wonder what the best way to pass -fuse-ld=gold and -flto via CFLAGS/LDFLAGS would be to avoid issues like mesonbuild/meson#4542? It was pointed out there that

The core problem here is that OSS-Fuzz seems to treat all build systems as dumb and just throws compiler flags en masse at them. Meson does not really work like that. We aim to provide higher level interface to our users. We also exploit that to do things internally. This meshes badly with random linker flags injected in from the outside, especially for things that we have an internal option for.

The correct solution for this is that we provide the knobs necessary (most or possibly all of which we already do) so OSS-Fuzz and everyone else can just use those. This means we have one proper working solution and everyone and their dog does not have to reinvent the wheel and do the magic guessing game of "what linker flags should I inject in this case".

and I kind of agree with that but on the other hand OSS-Fuzz has to support all kinds of build systems and CFLAGS has worked fine so far.

@evverx
Copy link
Contributor

evverx commented Apr 20, 2022

This PR is going to break systemd but as discussed in #7540 (comment) I have no problem with that.

@DavidKorczynski DavidKorczynski marked this pull request as ready for review April 20, 2022 08:18
@DavidKorczynski
Copy link
Collaborator Author

DavidKorczynski commented Apr 20, 2022

@Navidem could please take a look? This one is ready to go

@evverx
Copy link
Contributor

evverx commented Apr 20, 2022

FWIW turns out CFLAGS=-fuse-ld=... breaking compiler checks was reported back in 2020 in mesonbuild/meson#6377 (comment). Passing LDFLAGS=-fuse-ld= doesn't work as expected either: mesonbuild/meson#6377 so the only way to set the linker to gold more or less reliably using meson is CC_LD=gold and CXX_LD=gold.

@evverx
Copy link
Contributor

evverx commented Apr 20, 2022

Before I forget, it's possible to make systemd (and probably any other project using meson) compile with fuzz-introspector with popsicle sticks and duct tape:

+    if [[ "$SANITIZER" == introspector ]]; then
+        # -fuse-ld=gold can't be passed via CFLAGS/CXXFLAGS/LDFLAGS due to
+        # https://github.com/mesonbuild/meson/issues/6377 and
+        # https://github.com/mesonbuild/meson/issues/6377#issuecomment-575977919
+        CFLAGS="${CFLAGS//-fuse-ld=gold/ }"
+        CXXFLAGS="${CXXFLAGS//-fuse-ld=gold/ }"
+        LDFLAGS="${LDFLAGS//-fuse-ld=gold/ }"
+        export CC_LD=gold
+        export CXX_LD=gold
+
+        # OSS-Fuzz passes -flto via CFLAGS/CXXFLAGS. Let's append it to LDFLAGS
+        # as well just in case. Another option would be to use -Db_lto* but it
+        # doesn't always mix well with CFLAGS/CXXFLAGS
+        LDFLAGS+=" -flto"
+    fi

It's fragile and unmaintainble but it should do the trick. I'd wait until fuzz-introspector is more or less settled before adding this kludge to the systemd build script.

@DavidKorczynski on a somewhat unrelated note I wonder if there are links to the latest fuzz-introspector reports by analogy with https://oss-fuzz.com/coverage-report/job/libfuzzer_asan_systemd/latest . I tried replacing dates with "latest" but it didn't work out.

@Navidem
Copy link
Contributor

Navidem commented Apr 20, 2022

on a somewhat unrelated note I wonder if there are links to the latest fuzz-introspector reports

@evverx you can check this introspector report for systemd, if that's what you are looking for:
https://storage.googleapis.com/oss-fuzz-introspector/systemd/inspector-report/20220419/fuzz_report.html

@evverx
Copy link
Contributor

evverx commented Apr 20, 2022

@Navidem I was looking for a link that could always point to the latest fuzz-introspector report without having to specify dates. For example, currently https://oss-fuzz.com/coverage-report/job/libfuzzer_asan_systemd/latest redirects to https://storage.googleapis.com/oss-fuzz-coverage/systemd/reports/20220419/linux/report.html and tomorrow it will change accordingly.

@DavidKorczynski DavidKorczynski merged commit dbdcb8f into google:master Apr 20, 2022
@DavidKorczynski
Copy link
Collaborator Author

Thanks for showing the fix on meson @evverx -- to my knowledge there is no latest link on fuzz-introspector reports atm.

@Navidem
Copy link
Contributor

Navidem commented Apr 20, 2022

@Navidem I was looking for a link that could always point to the latest fuzz-introspector report without having to specify dates. For example, currently https://oss-fuzz.com/coverage-report/job/libfuzzer_asan_systemd/latest redirects to https://storage.googleapis.com/oss-fuzz-coverage/systemd/reports/20220419/linux/report.html and tomorrow it will change accordingly.

Ah, now I see your point, currently we don't have the link to the latest reports as such for coverage.

@oliverchang
Copy link
Collaborator

Right, we don't have this latest link redirection for fuzz introspector right now.

In the meantine, there is a continuously generated index on https://oss-fuzz-introspector.storage.googleapis.com/index.html that links to the latest generated reports for each project.

@evverx
Copy link
Contributor

evverx commented Apr 21, 2022

@oliverchang thanks! It appears those links point to incomplete reports: #7599

evverx added a commit to evverx/systemd that referenced this pull request Apr 21, 2022
fuzz-introspector passes -fuse-ld=gold and -flto using CFLAGS/LDFLAGS and due to
mesonbuild/meson#6377 (comment) and
mesonbuild/meson#6377 it doesn't mix well with meson.
It's possible to build systemd with duct tape there using something like
google/oss-fuzz#7583 (comment) but
apparently even with gold and lto some parts of systemd are missing from
reports (presumably due to google/oss-fuzz#7598).
Let's just fail here for now to make it clear that fuzz-introspector isn't supported.
yuwata pushed a commit to systemd/systemd that referenced this pull request Apr 22, 2022
fuzz-introspector passes -fuse-ld=gold and -flto using CFLAGS/LDFLAGS and due to
mesonbuild/meson#6377 (comment) and
mesonbuild/meson#6377 it doesn't mix well with meson.
It's possible to build systemd with duct tape there using something like
google/oss-fuzz#7583 (comment) but
apparently even with gold and lto some parts of systemd are missing from
reports (presumably due to google/oss-fuzz#7598).
Let's just fail here for now to make it clear that fuzz-introspector isn't supported.
MartinPetkov pushed a commit to MartinPetkov/oss-fuzz that referenced this pull request Aug 15, 2022
Moves -fuse-ld=gold to compile flags and removes -flto from linker
flags.

Should fix a number of the projects
google#7540 (comment)

Ref:
google#7540 (comment)

Ref:
google#7540 (comment)
kasper93 added a commit to kasper93/oss-fuzz that referenced this pull request Jun 18, 2024
Workarounds the issue where compile tests would fail with
`-Werror=ignored-optimization-argument` because Meson doesn't allow
linker flags in `CFLAGS` or `CXXFLAGS`.

See: mesonbuild/meson#6377 (comment)

Thanks to @evverx for the idea:
google#7583 (comment)

This is a fragile workaround, but it looks like there isn't much else we
can do.
jonathanmetzman pushed a commit that referenced this pull request Jun 18, 2024
Workarounds the issue where compile tests would fail with
`-Werror=ignored-optimization-argument` because Meson doesn't allow
linker flags in `CFLAGS` or `CXXFLAGS`.

See:
mesonbuild/meson#6377 (comment)

Thanks to @evverx for the idea:
#7583 (comment)

This is a fragile workaround, but it looks like there isn't much else we
can do.
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.

None yet

4 participants