On Tue, Jun 07 2022, Josh Steadmon wrote: > On 2022.06.02 14:25, Ævar Arnfjörð Bjarmason wrote: >> Change the exit() wrapper added in ee4512ed481 (trace2: create new >> combined trace facility, 2019-02-22) so that we'll split up the trace2 >> logging concerns from wanting to wrap the "exit()" function itself for >> other purposes. >> >> This makes more sense structurally, as we won't seem to conflate >> non-trace2 behavior with the trace2 code. I'd previously added an >> explanation for this in 368b5843158 (common-main.c: call exit(), don't >> return, 2021-12-07), that comment is being adjusted here. >> >> Now the only thing we'll do if we're not using trace2 is to truncate >> the "code" argument to the lowest 8 bits. >> >> We only need to do that truncation on non-POSIX systems, but in >> ee4512ed481 that "if defined(__MINGW32__)" code added in >> 47e3de0e796 (MinGW: truncate exit()'s argument to lowest 8 bits, >> 2009-07-05) was made to run everywhere. It might be good for clarify >> to narrow that down by an "ifdef" again, but I'm not certain that in >> the interim we haven't had some other non-POSIX systems rely the >> behavior. On a POSIX system taking the lowest 8 bits is implicit, see >> exit(3)[1] and wait(2)[2]. Let's leave a comment about that instead. >> >> 1. https://man7.org/linux/man-pages/man3/exit.3.html >> 2. https://man7.org/linux/man-pages/man2/wait.2.html >> >> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> >> --- >> common-main.c | 20 ++++++++++++++++---- >> git-compat-util.h | 4 ++-- >> trace2.c | 8 ++------ >> trace2.h | 8 +------- >> 4 files changed, 21 insertions(+), 19 deletions(-) > > Just realized that this unexpectedly breaks the fuzzer builds: > > $ git checkout ab/bug-if-bug > $ make CC=clang CXX=clang++ \ > CFLAGS="-fsanitize=fuzzer-no-link,address" \ > LIB_FUZZING_ENGINE="-fsanitize=fuzzer" \ > fuzz-all > > [...] > LINK fuzz-pack-headers > LINK fuzz-pack-idx > /usr/bin/ld: archive.o: in function `parse_archive_args': > archive.c:(.text.parse_archive_args[parse_archive_args]+0x2cb8): undefined reference to `common_exit' > [...] Hrm, that's a pain, sorry about that. > The issue is that we don't link the fuzzers against common-main.o > because the fuzzing engine provides its own main(). Would it be too much > of a pain to move this out to a new common-exit.c file? I could do that, I actually did that in a WIP copy, but figured it was too much boilerplate to pass the list. But why it is that we can't just link to common-main.o? Yeah the linker error, but we already depend on modern clang here, so can't we just use -Wl,--allow-multiple-definition? This works for me with my local clang & GNU ld: make CC=clang CXX=clang++ \ CFLAGS="-fsanitize=fuzzer-no-link,address" \ FUZZ_CXXFLAGS="-fsanitize=fuzzer-no-link,address -Wl,--allow-multiple-definition" \ LIB_FUZZING_ENGINE="common-main.o -fsanitize=fuzzer" fuzz-all It seems to me that the $(FUZZ_PROGRAMS) rule is mostly trying to work around that by size, i.e. re-declaring most of $(OBJECTS). But maybe it's a no-go for some reason, maybe due to OSX ld?
