fuzz-introspector: remove use of LDFLAGS

[DavidKorczynski]

The use of LDFLAGS does not really follow the policy of OSS-Fuzz. This
moves the linker flags into the sanitizer flags.

Ref:
#7540 (comment)

[evverx]

I think by analogy with 3c6e2bc -Wno-unused-command-line-argument should also be passed to let projects keep passing -Werror.

[DavidKorczynski]

I think by analogy with 3c6e2bc -Wno-unused-command-line-argument should also be passed to let projects keep passing -Werror.

Agreed, thanks @evverx !

[evverx]

@DavidKorczynski I'm just curious as to why gold and lto should be forced to make the fuzz introspector work. As far as I can remember that combination is known to fail to compile some parts of systemd: systemd/systemd#21960 (comment)

[DavidKorczynski]

It's because fuzz-introspector relies on an LTO pass to extract control flow graphs of a program, and the reason it's an LTO pass is because LTO is convenient for extracting program-wide CFGs.

LTO is not ideal from a user experience perspective though, however, it has been good for getting fuzz introspector off the ground. We have an issue open for alternatives: ossf/fuzz-introspector#4

[evverx]

Got it. Thanks!

Just to clarify, that particular issue was a gold/binutils bug and it might have been fixed since then but the systemd build system defaults to bfd in some places and CFLAGS/LDFLAGS can't change that unless gold is specified explicitly. I don't think that part of systemd is built when the fuzz targets are built so there it isn't an issue but I can imagine a scenario where CFLAGS and LDFLAGS aren't enough.

[DavidKorczynski]

Thanks for the details @evverx

One option is also to have a dual use mode. For example, I'm trying to push towards building some analyses that do not rely on LTO, e.g. reasoning about runtime coverage and non-program-wide program details, which could be enabled without LTO. It would be nice to have it so that projects that are incompatible/partly incompatible with LTO can still get some analysis output, and there should be room for developing such analysis. But this is more thinking ahead a bit.

[evverx]

It would be nice to have it so that projects that are incompatible/partly incompatible with LTO can still get some analysis output, and there should be room for developing such analysis

I think it would be great. lto tends to break in mysterious ways like mesonbuild/meson#7360 (where I still have no idea what happened) so if it was possible to switch between the two modes if would be even better.

[DavidKorczynski]

I think it would be great.

Thanks for letting me know! I opened an issue on fuzz-introspector for this: ossf/fuzz-introspector#209

[evverx]

@DavidKorczynski thanks! One last thing (unrelated to this PR) looking at #7190 (comment) it seems the fuzz introspector is going to be used to get the coverage of fuzz targets that can then be used by CIFuzz to exclude unaffected fuzz targets. That's another place where systemd is likely to hit yet another corner case like #7011. It would be great if the fuzz introspector data can be compatible with CIFuzz out of the box.

[DavidKorczynski]

@DavidKorczynski thanks! One last thing (unrelated to this PR) looking at #7190 (comment) it seems the fuzz introspector is going to be used to get the coverage of fuzz targets that can then be used by CIFuzz to exclude unaffected fuzz targets. That's another place where systemd is likely to hit yet another corner case like #7011. It would be great if the fuzz introspector data can be compatible with CIFuzz out of the box.

Interesting, I don't think the plan was to use fuzz-introspector here. However, there may be some room, e.g. one thought I have had is to turn this module https://github.com/ossf/fuzz-introspector/blob/main/post-processing/fuzz_cov_load.py into a library that allows easy reasoning about coverage reports, this could perhaps make it easy to quickly do a check along the lines of

if lines_changed_by_pr in coverage.all_covered_lines():
    print("the changed code is hit!")
else:
    print("This pr introduces code that is not hit by the current fuzzers")

This could be done without LTO.

[evverx]

Interesting, I don't think the plan was to use fuzz-introspector here

I think I assumed it was fuzz-introspector because that "granular" coverage was mentioned in a fuzz-introspector report I had taken a look at a couple days ago. Anyway, I have to admit I'm not sure how it all works so I'll show myself out.