This is an archive of the discontinued LLVM Phabricator instance.

[libFuzzer] replace Vector/Set with std::vector/std::set. The custom names are not required any more since we now build with a private version of libc++. Fix some of the 81+ character lines. Mechanical change, NFC expected.
ClosedPublic

Authored by kcc on Aug 3 2021, 10:11 AM.

Details

Summary

[libFuzzer] replace Vector/Set with std::vector/std::set.

Diff Detail

Event Timeline

kcc requested review of this revision.Aug 3 2021, 10:11 AM
kcc created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptAug 3 2021, 10:11 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript
This revision is now accepted and ready to land.Aug 3 2021, 10:44 AM
This revision was landed with ongoing or failed builds.Aug 3 2021, 11:31 AM
This revision was automatically updated to reflect the committed changes.

Bear with me, this report is a bit strange but I'm sure we can find a good explanation for it. The code looks fine, I think it has uncovered something else.

Since this commit, our 2 stage aarch64 bots have been failing on one particular test:
https://lab.llvm.org/buildbot/#/builders/185/builds/260

I haven't seen this on any other bots. The odd thing is that stage 1, which we build with clang-12, is fine.

The error itself is:

/tmp/FlagsTest-19213f.o: In function `sancov.module_ctor_8bit_counters':
FlagsTest.cpp:(.text.sancov.module_ctor_8bit_counters[sancov.module_ctor_8bit_counters]+0x14): undefined reference to `__start___sancov_cntrs'
FlagsTest.cpp:(.text.sancov.module_ctor_8bit_counters[sancov.module_ctor_8bit_counters]+0x18): undefined reference to `__stop___sancov_cntrs'
<...>

I looked at the IR for the test file and it was identical with this commit reverted. (which makes a lot of sense but just for sanity) I saw that these _sancov_cntrs etc. are defined in the IR. (I thought they might be in the fuzzer lib instead for example)

So the weird thing is, why doesn't the linker just find the definitions inside the file?

If I add -c to the command lit runs, and examine the pre-link objects:

$ objdump -h /tmp/not_working | grep sancov
 18 .text.sancov.module_ctor_8bit_counters 00000048  0000000000000000  0000000000000000  00002bb8  2**2
 23 __sancov_cntrs 00000008  0000000000000000  0000000000000000  00002e08  2**0
 24 __sancov_pcs  00000080  0000000000000000  0000000000000000  00002e10  2**3
 25 __sancov_cntrs 0000002d  0000000000000000  0000000000000000  00002e90  2**0
 26 __sancov_pcs  000002d0  0000000000000000  0000000000000000  00002ec0  2**3
 27 __sancov_cntrs 0000000a  0000000000000000  0000000000000000  00003190  2**0
 28 __sancov_pcs  000000a0  0000000000000000  0000000000000000  000031a0  2**3
 29 __sancov_cntrs 0000001e  0000000000000000  0000000000000000  00003240  2**0
 30 __sancov_pcs  000001e0  0000000000000000  0000000000000000  00003260  2**3
 31 __sancov_cntrs 0000001a  0000000000000000  0000000000000000  00003440  2**0
 32 __sancov_pcs  000001a0  0000000000000000  0000000000000000  00003460  2**3
 33 __sancov_cntrs 00000001  0000000000000000  0000000000000000  00003600  2**0
 34 __sancov_pcs  00000010  0000000000000000  0000000000000000  00003608  2**3
$ objdump -h /tmp/working | grep sancov
 14 __sancov_pcs  00000780  0000000000520340  0000000000520340  00120340  2**3
 27 __sancov_cntrs 00000078  00000000005520d0  00000000005520d0  001420d0  2**0

It seems that module_ctor_8bit_counters in the working object has been placed into the .text section. Where in the not working object it's got its own section, plus we have way more __sancov_.... (whether that's a good thing or not)

Our buildbot is using lld, I tried this using ld as well and got the same results. So I'm assuming that it's not both linkers mishandling the same input, but something before that going wrong. (which would be why clang-12 stage 1 was fine but clang-14 stage 2 was not)

If we look at the symbols:

$ nm /tmp/not_working | grep sancov
                 U __sancov_lowest_stack
                 w __start___sancov_cntrs
                 w __start___sancov_pcs
                 w __stop___sancov_cntrs
                 w __stop___sancov_pcs
0000000000000000 t sancov.module_ctor_8bit_counters
$ nm /tmp/working | grep sancov
00000000004f08a0 T _ZN8__sancov11SancovFlags11SetDefaultsEv
00000000004f10d0 t _ZN8__sancov12_GLOBAL__N_119WriteModuleCoverageEPcPKcPKmm
00000000006ec278 b _ZN8__sancov12_GLOBAL__N_119pc_guard_controllerE
000000000051c768 r _ZN8__sancov12_GLOBAL__N_15MagicE
00000000004f08b0 T _ZN8__sancov21InitializeSancovFlagsEv
00000000006ec270 B _ZN8__sancov30sancov_flags_dont_use_directlyE
0000000000452410 W _ZTW21__sancov_lowest_stack
00000000004f0890 W __sancov_default_options
0000000000000008 B __sancov_lowest_stack
00000000005520d0 D __start___sancov_cntrs
0000000000520340 R __start___sancov_pcs
0000000000552148 D __stop___sancov_cntrs
0000000000520ac0 R __stop___sancov_pcs
000000000050704c t sancov.module_ctor_8bit_counters

The working object has resolved the _start__/_stop__ symbols before we go to the linker. I don't understand at the moment, how it does that and what might be going wrong in the failing case.

@MaskRay is this anything to do with https://reviews.llvm.org/D106246 ? (either way, you have been in this area do you have any pointers of how to investigate?)

The failure has resolved itself somehow. I'm going to see if I can find anything more but don't expect to.

I spoke to soon and it's back again. I've xfailed the test in https://reviews.llvm.org/rG683147ff11cf115b071a0367bb7814db053b9c54.

FYI, the test failure totally wasn't anything to do with your patch.

For anyone else who might hit this, I've left the details on https://reviews.llvm.org/rGd4b193ca64e91c54487c52ac2f6eee5bd40e41ef. It's some issue with GNU ld <2.32 when handling objects created by a clang with the new pass manager enabled.