Kostya Serebryany <kcc at google.com> writes:> Justin,> Calling appendToUsed has horrible complexity and if we call it in > every function clang consumes tons of memory (6Gb when compiling one > of the clang's source files). This killed my machine today :) > > The solution is to call appendToUsed once per module, instead of once > per function.Oh right, updating lists in metadata is O(n), so doing it per function is quadratic. This slipped my mind - sorry!> Also, since this does not seem to be required for linux, I've put this > under if TargetTriple.isOSBinFormatMachO Submitted r312855, I'll see > if this breaks Mac (there seem to be no llvm tests for this, only > compiler-rt tests) but please also check if this looks ok.This looks fine, though I'd rather if we just did it on all platforms for consistency / clear semantic intent. Running appendToUsed once should be cheap, and we do it unguarded in our other instrumentation (like InstrProfiling.cpp).> But this all still sounds bad on linux at least: > * with the old bfd linker and -ffunction-sections -Wl,-gc-sections these > arrays get removed (as discussed here)This is sad, and I don't think we have any particularly good ideas to fix this.> * with newer linkers the sanitizer coverage essentially disables > gc-sectionsI'm not sure I follow. We're making sure the global arrays / instrumentation data doesn't get dead stripped here, but dead stripping of functions should work as normal.> --kcc > > > On Thu, Aug 24, 2017 at 6:43 PM, Peter Collingbourne <peter at pcc.me.uk> > wrote: > >> >> >> On Thu, Aug 24, 2017 at 6:30 PM, Justin Bogner <mail at justinbogner.com> >> wrote: >> >>> Peter Collingbourne <peter at pcc.me.uk> writes: >>> > On Thu, Aug 24, 2017 at 3:38 PM, Kostya Serebryany <kcc at google.com> >>> wrote: >>> > >>> >> >>> >> >>> >> On Thu, Aug 24, 2017 at 3:35 PM, Peter Collingbourne <peter at pcc.me.uk> >>> >> wrote: >>> >> >>> >>> On Thu, Aug 24, 2017 at 3:21 PM, Kostya Serebryany via llvm-dev < >>> >>> llvm-dev at lists.llvm.org> wrote: >>> >>> >>> >>>> >>> >>>> >>> >>>> On Thu, Aug 24, 2017 at 3:20 PM, Justin Bogner < >>> mail at justinbogner.com> >>> >>>> wrote: >>> >>>> >>> >>>>> I think the simplest fix is something like this: >>> >>>>> >>> >>>>> diff --git a/lib/Transforms/Instrumentation/SanitizerCoverage.cpp >>> >>>>> b/lib/Transforms/Instrumentation/SanitizerCoverage.cpp >>> >>>>> index c6f0d17f8fe..e81957ab80a 100644 >>> >>>>> --- a/lib/Transforms/Instrumentation/SanitizerCoverage.cpp >>> >>>>> +++ b/lib/Transforms/Instrumentation/SanitizerCoverage.cpp >>> >>>>> @@ -256,6 +256,7 @@ SanitizerCoverageModule::Creat >>> eSecStartEnd(Module >>> >>>>> &M, const char *Section, >>> >>>>> new GlobalVariable(M, Ty, false, >>> GlobalVariable::ExternalLinkag >>> >>>>> e, >>> >>>>> nullptr, getSectionEnd(Section)); >>> >>>>> SecEnd->setVisibility(GlobalValue::HiddenVisibility); >>> >>>>> + appendToUsed(M, {SecStart, SecEnd}); >>> >>>>> >>> >>>>> return std::make_pair(SecStart, SecEnd); >>> >>>>> } >>> >>>>> >>> >>>>> I'm trying it out now. >>> >>>>> >>> >>>> >>> >>>> LGTM (if this works), thanks! >>> >>>> >>> >>> >>> >>> I wouldn't expect that to work because for ELF targets llvm.used has >>> no >>> >>> effect on the object file (only on the optimizer). >>> >>> >>> >>> Is there a simple way to reproduce the link failure? >>> >>> >>> >> >>> >> >>> >> ninja compiler-rt >>> >> echo 'extern "C" int LLVMFuzzerTestOneInput(const unsigned char *a, >>> >> unsigned long b){return 0; } ' > test.cc >>> >> clang -O3 test.cc -fsanitize=fuzzer # works >>> >> clang -O3 test.cc -Wl,-gc-sections -fsanitize=fuzzer # fails >>> >> >>> > >>> > It seems that the issue is that older versions of ld.bfd have a bug >>> which >>> > causes it not to define __start_ and __stop_ symbols if the only >>> reference >>> > to those symbols is from a constructor. >>> >>> It looks like this is a different problem from the one on macOS (and I >>> wasn't able to reproduce it with any bfd ld I had available, they were >>> all too new) >>> >>> I've gone ahead and fixed the issue on macOS in r311742. >>> >>> > If I add an artificial reference to the start symbol from libfuzzer's >>> main >>> > function, the program links correctly. >>> > >>> > diff --git a/compiler-rt/lib/fuzzer/FuzzerMain.cpp >>> > b/compiler-rt/lib/fuzzer/FuzzerMain.cpp >>> > index af8657200be2..c41e28e012db 100644 >>> > --- a/compiler-rt/lib/fuzzer/FuzzerMain.cpp >>> > +++ b/compiler-rt/lib/fuzzer/FuzzerMain.cpp >>> > @@ -16,6 +16,10 @@ extern "C" { >>> > int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size); >>> > } // extern "C" >>> > >>> > +__attribute__((weak)) void nop(void *p) {} >>> > +extern void *__start___sancov_pcs; >>> > + >>> > int main(int argc, char **argv) { >>> > + nop(__start___sancov_pcs); >>> > return fuzzer::FuzzerDriver(&argc, &argv, LLVMFuzzerTestOneInput); >>> > } >>> >>> If we were to do this, we'd have to guard it appropriately - not all >>> platforms name the __start symbols like this. >>> >> >> Of course. There's also the issue of how to keep the symbols alive in DSOs. >> >> > The problem also goes away if I use "GNU ld (GNU Binutils) >>> > 2.28.51.20170105". >>> >>> 2.27 also doesn't have the issue. I don't know what our minimum version >>> of binutils is, and I'm under the impression most people use gold or lld >>> to link LLVM these days, so it isn't clear to me how big of a problem >>> this is. >>> >> >> For the record, the problem reproduces under 2.24, which is shipped by >> Ubuntu 14.04 LTS, which isn't that old. My view is that if we can find an >> unintrusive enough workaround, we should deploy it (with a comment to >> remove it after N years). >> >> Peter >> >> >>> > Peter >>> > >>> > >>> > >>> >> >>> >> >>> >> >>> >> >>> >> >>> >>> >>> >>> Peter >>> >>> >>> >>> >>> >>>> >>> >>>>> >>> >>>>> Kostya Serebryany <kcc at google.com> writes: >>> >>>>> > With -Wl,-gc-sections I get this: >>> >>>>> > SimpleTest.cpp:(.text.sancov.module_ctor[sancov.module_ctor] >>> +0x1b): >>> >>>>> > undefined reference to `__start___sancov_pcs' >>> >>>>> > SimpleTest.cpp:(.text.sancov.module_ctor[sancov.module_ctor] >>> +0x20): >>> >>>>> > undefined reference to `__stop___sancov_pcs' >>> >>>>> > >>> >>>>> > >>> >>>>> > >>> >>>>> > On Thu, Aug 24, 2017 at 3:07 PM, George Karpenkov < >>> >>>>> ekarpenkov at apple.com> >>> >>>>> > wrote: >>> >>>>> > >>> >>>>> >> >>> >>>>> >> On Aug 24, 2017, at 2:55 PM, Kostya Serebryany <kcc at google.com> >>> >>>>> wrote: >>> >>>>> >> >>> >>>>> >> Interesting. >>> >>>>> >> This is a relatively new addition (fsanitize-coverage=pc-tables, >>> >>>>> which is >>> >>>>> >> now a part of -fsanitize=fuzzer). >>> >>>>> >> The tests worked (did they? On Mac?) so I thought everything is >>> ok. >>> >>>>> >> >>> >>>>> >> >>> >>>>> >> For tests we never compile the tested target with -O3 (and that >>> >>>>> wouldn’t >>> >>>>> >> be sufficient), >>> >>>>> >> and for testing fuzzers I was always building them in debug >>> >>>>> >> >>> >>>>> >> Yea, we need to make sure the pc-tables are not stripped (this >>> is a >>> >>>>> >> separate section with globals). >>> >>>>> >> (I still haven't documented pc-tables, will do soon) >>> >>>>> >> >>> >>>>> >> >>> >>>>> >> Do you know what's the analog of Wl,-dead_strip on Linux? >>> >>>>> >> >>> >>>>> >> >>> >>>>> >> Apparently -Wl,—gc-sections. >>> >>>>> >> For some reason LLVM does not do it for gold, even though it >>> seems to >>> >>>>> >> support this flag as well. >>> >>>>> >> (that could be another reason why you don’t see the failure on >>> Linux) >>> >>>>> >> >>> >>>>> >> 1 *if*(NOT LLVM_NO_DEAD_STRIP) >>> >>>>> >> 2 *if*(${CMAKE_SYSTEM_NAME} MATCHES "Darwin") >>> >>>>> >> 3 # ld64's implementation of -dead_strip breaks tools that >>> use >>> >>>>> >> plugins. >>> >>>>> >> 4 set_property(TARGET ${target_name} APPEND_STRING PROPERTY >>> >>>>> >> 5 LINK_FLAGS " -Wl,-dead_strip") >>> >>>>> >> 6 *elseif*(${CMAKE_SYSTEM_NAME} MATCHES "SunOS") >>> >>>>> >> 7 set_property(TARGET ${target_name} APPEND_STRING PROPERTY >>> >>>>> >> 8 LINK_FLAGS " -Wl,-z >>> -Wl,discard-unused=sections") >>> >>>>> >> 9 *elseif*(NOT WIN32 AND NOT LLVM_LINKER_IS_GOLD) >>> >>>>> >> 10 # Object files are compiled with -ffunction-data-sections. >>> >>>>> >> 11 # Versions of bfd ld < 2.23.1 have a bug in --gc-sections >>> that >>> >>>>> >> breaks >>> >>>>> >> 12 # tools that use plugins. Always pass --gc-sections once >>> we >>> >>>>> require >>> >>>>> >> 13 # a newer linker. >>> >>>>> >> 14 set_property(TARGET ${target_name} APPEND_STRING PROPERTY >>> >>>>> >> 15 LINK_FLAGS " -Wl,--gc-sections") >>> >>>>> >> 16 *endif*() >>> >>>>> >> 17 *endif*() >>> >>>>> >> >>> >>>>> >> >>> >>>>> >> >>> >>>>> >> --kcc >>> >>>>> >> >>> >>>>> >> >>> >>>>> >> >>> >>>>> >> On Thu, Aug 24, 2017 at 2:49 PM, Justin Bogner < >>> >>>>> mail at justinbogner.com> >>> >>>>> >> wrote: >>> >>>>> >> >>> >>>>> >>> George Karpenkov <ekarpenkov at apple.com> writes: >>> >>>>> >>> > OK so with Kuba’s help I’ve found the error: with >>> optimization, >>> >>>>> dead >>> >>>>> >>> > stripping of produced libraries is enabled, >>> >>>>> >>> > which removes coverage instrumentation. >>> >>>>> >>> > >>> >>>>> >>> > However, this has nothing to do with the move to compiler-rt, >>> so >>> >>>>> I’m >>> >>>>> >>> > quite skeptical on whether it has worked >>> >>>>> >>> > beforehand. >>> >>>>> >>> > >>> >>>>> >>> > A trivial fix is to do: >>> >>>>> >>> > >>> >>>>> >>> > diff --git a/cmake/modules/HandleLLVMOptions.cmake >>> >>>>> >>> b/cmake/modules/HandleLLVMOptions.cmake >>> >>>>> >>> > index 04596a6ff63..5465d8d95ba 100644 >>> >>>>> >>> > --- a/cmake/modules/HandleLLVMOptions.cmake >>> >>>>> >>> > +++ b/cmake/modules/HandleLLVMOptions.cmake >>> >>>>> >>> > @@ -665,6 +665,9 @@ if(LLVM_USE_SANITIZER) >>> >>>>> >>> > endif() >>> >>>>> >>> > if (LLVM_USE_SANITIZE_COVERAGE) >>> >>>>> >>> > append("-fsanitize=fuzzer-no-link" CMAKE_C_FLAGS >>> >>>>> CMAKE_CXX_FLAGS) >>> >>>>> >>> > + >>> >>>>> >>> > + # Dead stripping messes up coverage instrumentation. >>> >>>>> >>> > + set(LLVM_NO_DEAD_STRIP ON) >>> >>>>> >>> > endif() >>> >>>>> >>> > endif() >>> >>>>> >>> > >>> >>>>> >>> > Any arguments against that? >>> >>>>> >>> >>> >>>>> >>> We shouldn't do this. We really only want to prevent dead >>> stripping >>> >>>>> of >>> >>>>> >>> the counters themselves - disabling it completely isn't very >>> nice. >>> >>>>> >>> >>> >>>>> >>> > Apparently, a better way is to follow ASAN instrumentation >>> pass, >>> >>>>> >>> > which uses some magic to protect against dead-stripping. >>> >>>>> >>> >>> >>>>> >>> I thought this was already being done - how else did it work >>> before? >>> >>>>> >>> >>> >>>>> >>> >> On Aug 24, 2017, at 11:29 AM, Justin Bogner < >>> >>>>> mail at justinbogner.com> >>> >>>>> >>> wrote: >>> >>>>> >>> >> >>> >>>>> >>> >> (kcc, george: sorry for the re-send, the first was from a >>> >>>>> non-list >>> >>>>> >>> email >>> >>>>> >>> >> address) >>> >>>>> >>> >> >>> >>>>> >>> >> My configuration for building the fuzzers in the LLVM tree >>> >>>>> doesn't >>> >>>>> >>> seem to >>> >>>>> >>> >> work any more (possibly as of moving libFuzzer to >>> compiler-rt, >>> >>>>> but >>> >>>>> >>> there >>> >>>>> >>> >> have been a few other changes in the last week or so that >>> may be >>> >>>>> >>> related). >>> >>>>> >>> >> >>> >>>>> >>> >> I'm building with a fresh top-of-tree clang and setting >>> >>>>> >>> >> -DLLVM_USE_SANITIZER=Address and >>> -DLLVM_USE_SANITIZE_COVERAGE=O >>> >>>>> n, >>> >>>>> >>> which >>> >>>>> >>> >> was working before: >>> >>>>> >>> >> >>> >>>>> >>> >> % cmake -GNinja \ >>> >>>>> >>> >> -DCMAKE_BUILD_TYPE=Release >>> -DLLVM_ENABLE_ASSERTIONS=On \ >>> >>>>> >>> >> -DLLVM_ENABLE_WERROR=On \ >>> >>>>> >>> >> -DLLVM_USE_SANITIZER=Address >>> >>>>> -DLLVM_USE_SANITIZE_COVERAGE=On >>> >>>>> >>> \ >>> >>>>> >>> >> -DCMAKE_C_COMPILER=$HOME/llvm-lkgc/bin/clang \ >>> >>>>> >>> >> $HOME/code/llvm-src >>> >>>>> >>> >> >>> >>>>> >>> >> But when I run any of the fuzzers, it looks like the >>> sanitizer >>> >>>>> coverage >>> >>>>> >>> >> hasn't been set up correctly: >>> >>>>> >>> >> >>> >>>>> >>> >> % ./bin/llvm-as-fuzzer >>> >>>>> >>> 2017-08-24 11:14:33 >>> >>>>> >>> >> INFO: Seed: 4089166883 <(408)%20916-6883> >>> >>>>> >>> >> INFO: Loaded 1 modules (50607 guards): 50607 [0x10e14ef80, >>> >>>>> >>> 0x10e18063c), >>> >>>>> >>> >> INFO: Loaded 1 PC tables (0 PCs): 0 >>> [0x10e2870a8,0x10e2870a8), >>> >>>>> >>> >> ERROR: The size of coverage PC tables does not match the >>> number >>> >>>>> of >>> >>>>> >>> instrumented PCs. This might be a bug in the compiler, please >>> >>>>> contact the >>> >>>>> >>> libFuzzer developers. >>> >>>>> >>> >> >>> >>>>> >>> >> From the build logs, it looks like we're now building objects >>> >>>>> with >>> >>>>> >>> these >>> >>>>> >>> >> sanitizer flags: >>> >>>>> >>> >> >>> >>>>> >>> >> -fsanitize=address >>> >>>>> >>> >> -fsanitize-address-use-after-scope >>> >>>>> >>> >> -fsanitize=fuzzer-no-link >>> >>>>> >>> >> >>> >>>>> >>> >> We're then linking the fuzzer binaries with these: >>> >>>>> >>> >> >>> >>>>> >>> >> -fsanitize=address >>> >>>>> >>> >> -fsanitize-address-use-after-scope >>> >>>>> >>> >> -fsanitize=fuzzer-no-link >>> >>>>> >>> >> -fsanitize=fuzzer >>> >>>>> >>> >> >>> >>>>> >>> >> Any idea what's wrong or where to start looking? >>> >>>>> >>> >>> >>>>> >> >>> >>>>> >> >>> >>>>> >> >>> >>>>> >>> >>>> >>> >>>> >>> >>>> _______________________________________________ >>> >>>> LLVM Developers mailing list >>> >>>> llvm-dev at lists.llvm.org >>> >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>> >>>> >>> >>>> >>> >>> >>> >>> >>> >>> -- >>> >>> -- >>> >>> Peter >>> >>> >>> >> >>> >> >>> > >>> > >>> > -- >>> >> >> >> >> -- >> -- >> Peter >>
On Mon, Sep 11, 2017 at 11:52 AM, Justin Bogner <mail at justinbogner.com> wrote:> Kostya Serebryany <kcc at google.com> writes: > > Justin, > > > Calling appendToUsed has horrible complexity and if we call it in > > every function clang consumes tons of memory (6Gb when compiling one > > of the clang's source files). This killed my machine today :) > > > > The solution is to call appendToUsed once per module, instead of once > > per function. > > Oh right, updating lists in metadata is O(n), so doing it per function > is quadratic. This slipped my mind - sorry! >Yea. Very unexpected (although I've stumbled on it a few times, I still don't remember about it)> > > Also, since this does not seem to be required for linux, I've put this > > under if TargetTriple.isOSBinFormatMachO Submitted r312855, I'll see > > if this breaks Mac (there seem to be no llvm tests for this, only > > compiler-rt tests) but please also check if this looks ok. > > This looks fine, though I'd rather if we just did it on all platforms > for consistency / clear semantic intent. Running appendToUsed once > should be cheap, and we do it unguarded in our other instrumentation > (like InstrProfiling.cpp). >It's unclear if it's going to hurt things on linux. If I see it doesn't -- I'll remove the if TargetTriple.isOSBinFormatMachO> > > But this all still sounds bad on linux at least: > > * with the old bfd linker and -ffunction-sections -Wl,-gc-sections > these > > arrays get removed (as discussed here) > > This is sad, and I don't think we have any particularly good ideas to > fix this. > > > * with newer linkers the sanitizer coverage essentially disables > > gc-sections > > I'm not sure I follow. We're making sure the global arrays / > instrumentation data doesn't get dead stripped here, but dead stripping > of functions should work as normal. >Ah, that's now my confusion. inline-8bit-counters and trace-pc-guard seems to work just fine. pc-table actually does break gc-sections as there is no a reference to every function in the pc-table section. +eugenis, who dealt with a similar issue in asan (although it's probably a separate topic now). --kcc> > > --kcc > > > > > > On Thu, Aug 24, 2017 at 6:43 PM, Peter Collingbourne <peter at pcc.me.uk> > > wrote: > > > >> > >> > >> On Thu, Aug 24, 2017 at 6:30 PM, Justin Bogner <mail at justinbogner.com> > >> wrote: > >> > >>> Peter Collingbourne <peter at pcc.me.uk> writes: > >>> > On Thu, Aug 24, 2017 at 3:38 PM, Kostya Serebryany <kcc at google.com> > >>> wrote: > >>> > > >>> >> > >>> >> > >>> >> On Thu, Aug 24, 2017 at 3:35 PM, Peter Collingbourne < > peter at pcc.me.uk> > >>> >> wrote: > >>> >> > >>> >>> On Thu, Aug 24, 2017 at 3:21 PM, Kostya Serebryany via llvm-dev < > >>> >>> llvm-dev at lists.llvm.org> wrote: > >>> >>> > >>> >>>> > >>> >>>> > >>> >>>> On Thu, Aug 24, 2017 at 3:20 PM, Justin Bogner < > >>> mail at justinbogner.com> > >>> >>>> wrote: > >>> >>>> > >>> >>>>> I think the simplest fix is something like this: > >>> >>>>> > >>> >>>>> diff --git a/lib/Transforms/Instrumentati > on/SanitizerCoverage.cpp > >>> >>>>> b/lib/Transforms/Instrumentation/SanitizerCoverage.cpp > >>> >>>>> index c6f0d17f8fe..e81957ab80a 100644 > >>> >>>>> --- a/lib/Transforms/Instrumentation/SanitizerCoverage.cpp > >>> >>>>> +++ b/lib/Transforms/Instrumentation/SanitizerCoverage.cpp > >>> >>>>> @@ -256,6 +256,7 @@ SanitizerCoverageModule::Creat > >>> eSecStartEnd(Module > >>> >>>>> &M, const char *Section, > >>> >>>>> new GlobalVariable(M, Ty, false, > >>> GlobalVariable::ExternalLinkag > >>> >>>>> e, > >>> >>>>> nullptr, getSectionEnd(Section)); > >>> >>>>> SecEnd->setVisibility(GlobalValue::HiddenVisibility); > >>> >>>>> + appendToUsed(M, {SecStart, SecEnd}); > >>> >>>>> > >>> >>>>> return std::make_pair(SecStart, SecEnd); > >>> >>>>> } > >>> >>>>> > >>> >>>>> I'm trying it out now. > >>> >>>>> > >>> >>>> > >>> >>>> LGTM (if this works), thanks! > >>> >>>> > >>> >>> > >>> >>> I wouldn't expect that to work because for ELF targets llvm.used > has > >>> no > >>> >>> effect on the object file (only on the optimizer). > >>> >>> > >>> >>> Is there a simple way to reproduce the link failure? > >>> >>> > >>> >> > >>> >> > >>> >> ninja compiler-rt > >>> >> echo 'extern "C" int LLVMFuzzerTestOneInput(const unsigned char *a, > >>> >> unsigned long b){return 0; } ' > test.cc > >>> >> clang -O3 test.cc -fsanitize=fuzzer # works > >>> >> clang -O3 test.cc -Wl,-gc-sections -fsanitize=fuzzer # fails > >>> >> > >>> > > >>> > It seems that the issue is that older versions of ld.bfd have a bug > >>> which > >>> > causes it not to define __start_ and __stop_ symbols if the only > >>> reference > >>> > to those symbols is from a constructor. > >>> > >>> It looks like this is a different problem from the one on macOS (and I > >>> wasn't able to reproduce it with any bfd ld I had available, they were > >>> all too new) > >>> > >>> I've gone ahead and fixed the issue on macOS in r311742. > >>> > >>> > If I add an artificial reference to the start symbol from libfuzzer's > >>> main > >>> > function, the program links correctly. > >>> > > >>> > diff --git a/compiler-rt/lib/fuzzer/FuzzerMain.cpp > >>> > b/compiler-rt/lib/fuzzer/FuzzerMain.cpp > >>> > index af8657200be2..c41e28e012db 100644 > >>> > --- a/compiler-rt/lib/fuzzer/FuzzerMain.cpp > >>> > +++ b/compiler-rt/lib/fuzzer/FuzzerMain.cpp > >>> > @@ -16,6 +16,10 @@ extern "C" { > >>> > int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size); > >>> > } // extern "C" > >>> > > >>> > +__attribute__((weak)) void nop(void *p) {} > >>> > +extern void *__start___sancov_pcs; > >>> > + > >>> > int main(int argc, char **argv) { > >>> > + nop(__start___sancov_pcs); > >>> > return fuzzer::FuzzerDriver(&argc, &argv, LLVMFuzzerTestOneInput); > >>> > } > >>> > >>> If we were to do this, we'd have to guard it appropriately - not all > >>> platforms name the __start symbols like this. > >>> > >> > >> Of course. There's also the issue of how to keep the symbols alive in > DSOs. > >> > >> > The problem also goes away if I use "GNU ld (GNU Binutils) > >>> > 2.28.51.20170105". > >>> > >>> 2.27 also doesn't have the issue. I don't know what our minimum version > >>> of binutils is, and I'm under the impression most people use gold or > lld > >>> to link LLVM these days, so it isn't clear to me how big of a problem > >>> this is. > >>> > >> > >> For the record, the problem reproduces under 2.24, which is shipped by > >> Ubuntu 14.04 LTS, which isn't that old. My view is that if we can find > an > >> unintrusive enough workaround, we should deploy it (with a comment to > >> remove it after N years). > >> > >> Peter > >> > >> > >>> > Peter > >>> > > >>> > > >>> > > >>> >> > >>> >> > >>> >> > >>> >> > >>> >> > >>> >>> > >>> >>> Peter > >>> >>> > >>> >>> > >>> >>>> > >>> >>>>> > >>> >>>>> Kostya Serebryany <kcc at google.com> writes: > >>> >>>>> > With -Wl,-gc-sections I get this: > >>> >>>>> > SimpleTest.cpp:(.text.sancov.module_ctor[sancov.module_ctor] > >>> +0x1b): > >>> >>>>> > undefined reference to `__start___sancov_pcs' > >>> >>>>> > SimpleTest.cpp:(.text.sancov.module_ctor[sancov.module_ctor] > >>> +0x20): > >>> >>>>> > undefined reference to `__stop___sancov_pcs' > >>> >>>>> > > >>> >>>>> > > >>> >>>>> > > >>> >>>>> > On Thu, Aug 24, 2017 at 3:07 PM, George Karpenkov < > >>> >>>>> ekarpenkov at apple.com> > >>> >>>>> > wrote: > >>> >>>>> > > >>> >>>>> >> > >>> >>>>> >> On Aug 24, 2017, at 2:55 PM, Kostya Serebryany < > kcc at google.com> > >>> >>>>> wrote: > >>> >>>>> >> > >>> >>>>> >> Interesting. > >>> >>>>> >> This is a relatively new addition > (fsanitize-coverage=pc-tables, > >>> >>>>> which is > >>> >>>>> >> now a part of -fsanitize=fuzzer). > >>> >>>>> >> The tests worked (did they? On Mac?) so I thought everything > is > >>> ok. > >>> >>>>> >> > >>> >>>>> >> > >>> >>>>> >> For tests we never compile the tested target with -O3 (and > that > >>> >>>>> wouldn’t > >>> >>>>> >> be sufficient), > >>> >>>>> >> and for testing fuzzers I was always building them in debug > >>> >>>>> >> > >>> >>>>> >> Yea, we need to make sure the pc-tables are not stripped (this > >>> is a > >>> >>>>> >> separate section with globals). > >>> >>>>> >> (I still haven't documented pc-tables, will do soon) > >>> >>>>> >> > >>> >>>>> >> > >>> >>>>> >> Do you know what's the analog of Wl,-dead_strip on Linux? > >>> >>>>> >> > >>> >>>>> >> > >>> >>>>> >> Apparently -Wl,—gc-sections. > >>> >>>>> >> For some reason LLVM does not do it for gold, even though it > >>> seems to > >>> >>>>> >> support this flag as well. > >>> >>>>> >> (that could be another reason why you don’t see the failure on > >>> Linux) > >>> >>>>> >> > >>> >>>>> >> 1 *if*(NOT LLVM_NO_DEAD_STRIP) > >>> >>>>> >> 2 *if*(${CMAKE_SYSTEM_NAME} MATCHES "Darwin") > >>> >>>>> >> 3 # ld64's implementation of -dead_strip breaks tools > that > >>> use > >>> >>>>> >> plugins. > >>> >>>>> >> 4 set_property(TARGET ${target_name} APPEND_STRING > PROPERTY > >>> >>>>> >> 5 LINK_FLAGS " -Wl,-dead_strip") > >>> >>>>> >> 6 *elseif*(${CMAKE_SYSTEM_NAME} MATCHES "SunOS") > >>> >>>>> >> 7 set_property(TARGET ${target_name} APPEND_STRING > PROPERTY > >>> >>>>> >> 8 LINK_FLAGS " -Wl,-z > >>> -Wl,discard-unused=sections") > >>> >>>>> >> 9 *elseif*(NOT WIN32 AND NOT LLVM_LINKER_IS_GOLD) > >>> >>>>> >> 10 # Object files are compiled with > -ffunction-data-sections. > >>> >>>>> >> 11 # Versions of bfd ld < 2.23.1 have a bug in > --gc-sections > >>> that > >>> >>>>> >> breaks > >>> >>>>> >> 12 # tools that use plugins. Always pass --gc-sections > once > >>> we > >>> >>>>> require > >>> >>>>> >> 13 # a newer linker. > >>> >>>>> >> 14 set_property(TARGET ${target_name} APPEND_STRING > PROPERTY > >>> >>>>> >> 15 LINK_FLAGS " -Wl,--gc-sections") > >>> >>>>> >> 16 *endif*() > >>> >>>>> >> 17 *endif*() > >>> >>>>> >> > >>> >>>>> >> > >>> >>>>> >> > >>> >>>>> >> --kcc > >>> >>>>> >> > >>> >>>>> >> > >>> >>>>> >> > >>> >>>>> >> On Thu, Aug 24, 2017 at 2:49 PM, Justin Bogner < > >>> >>>>> mail at justinbogner.com> > >>> >>>>> >> wrote: > >>> >>>>> >> > >>> >>>>> >>> George Karpenkov <ekarpenkov at apple.com> writes: > >>> >>>>> >>> > OK so with Kuba’s help I’ve found the error: with > >>> optimization, > >>> >>>>> dead > >>> >>>>> >>> > stripping of produced libraries is enabled, > >>> >>>>> >>> > which removes coverage instrumentation. > >>> >>>>> >>> > > >>> >>>>> >>> > However, this has nothing to do with the move to > compiler-rt, > >>> so > >>> >>>>> I’m > >>> >>>>> >>> > quite skeptical on whether it has worked > >>> >>>>> >>> > beforehand. > >>> >>>>> >>> > > >>> >>>>> >>> > A trivial fix is to do: > >>> >>>>> >>> > > >>> >>>>> >>> > diff --git a/cmake/modules/HandleLLVMOptions.cmake > >>> >>>>> >>> b/cmake/modules/HandleLLVMOptions.cmake > >>> >>>>> >>> > index 04596a6ff63..5465d8d95ba 100644 > >>> >>>>> >>> > --- a/cmake/modules/HandleLLVMOptions.cmake > >>> >>>>> >>> > +++ b/cmake/modules/HandleLLVMOptions.cmake > >>> >>>>> >>> > @@ -665,6 +665,9 @@ if(LLVM_USE_SANITIZER) > >>> >>>>> >>> > endif() > >>> >>>>> >>> > if (LLVM_USE_SANITIZE_COVERAGE) > >>> >>>>> >>> > append("-fsanitize=fuzzer-no-link" CMAKE_C_FLAGS > >>> >>>>> CMAKE_CXX_FLAGS) > >>> >>>>> >>> > + > >>> >>>>> >>> > + # Dead stripping messes up coverage instrumentation. > >>> >>>>> >>> > + set(LLVM_NO_DEAD_STRIP ON) > >>> >>>>> >>> > endif() > >>> >>>>> >>> > endif() > >>> >>>>> >>> > > >>> >>>>> >>> > Any arguments against that? > >>> >>>>> >>> > >>> >>>>> >>> We shouldn't do this. We really only want to prevent dead > >>> stripping > >>> >>>>> of > >>> >>>>> >>> the counters themselves - disabling it completely isn't very > >>> nice. > >>> >>>>> >>> > >>> >>>>> >>> > Apparently, a better way is to follow ASAN instrumentation > >>> pass, > >>> >>>>> >>> > which uses some magic to protect against dead-stripping. > >>> >>>>> >>> > >>> >>>>> >>> I thought this was already being done - how else did it work > >>> before? > >>> >>>>> >>> > >>> >>>>> >>> >> On Aug 24, 2017, at 11:29 AM, Justin Bogner < > >>> >>>>> mail at justinbogner.com> > >>> >>>>> >>> wrote: > >>> >>>>> >>> >> > >>> >>>>> >>> >> (kcc, george: sorry for the re-send, the first was from a > >>> >>>>> non-list > >>> >>>>> >>> email > >>> >>>>> >>> >> address) > >>> >>>>> >>> >> > >>> >>>>> >>> >> My configuration for building the fuzzers in the LLVM tree > >>> >>>>> doesn't > >>> >>>>> >>> seem to > >>> >>>>> >>> >> work any more (possibly as of moving libFuzzer to > >>> compiler-rt, > >>> >>>>> but > >>> >>>>> >>> there > >>> >>>>> >>> >> have been a few other changes in the last week or so that > >>> may be > >>> >>>>> >>> related). > >>> >>>>> >>> >> > >>> >>>>> >>> >> I'm building with a fresh top-of-tree clang and setting > >>> >>>>> >>> >> -DLLVM_USE_SANITIZER=Address and > >>> -DLLVM_USE_SANITIZE_COVERAGE=O > >>> >>>>> n, > >>> >>>>> >>> which > >>> >>>>> >>> >> was working before: > >>> >>>>> >>> >> > >>> >>>>> >>> >> % cmake -GNinja \ > >>> >>>>> >>> >> -DCMAKE_BUILD_TYPE=Release > >>> -DLLVM_ENABLE_ASSERTIONS=On \ > >>> >>>>> >>> >> -DLLVM_ENABLE_WERROR=On \ > >>> >>>>> >>> >> -DLLVM_USE_SANITIZER=Address > >>> >>>>> -DLLVM_USE_SANITIZE_COVERAGE=On > >>> >>>>> >>> \ > >>> >>>>> >>> >> -DCMAKE_C_COMPILER=$HOME/llvm-lkgc/bin/clang \ > >>> >>>>> >>> >> $HOME/code/llvm-src > >>> >>>>> >>> >> > >>> >>>>> >>> >> But when I run any of the fuzzers, it looks like the > >>> sanitizer > >>> >>>>> coverage > >>> >>>>> >>> >> hasn't been set up correctly: > >>> >>>>> >>> >> > >>> >>>>> >>> >> % ./bin/llvm-as-fuzzer > >>> >>>>> >>> 2017-08-24 11:14:33 > >>> >>>>> >>> >> INFO: Seed: 4089166883 <(408)%20916-6883> > >>> >>>>> >>> >> INFO: Loaded 1 modules (50607 guards): 50607 > [0x10e14ef80, > >>> >>>>> >>> 0x10e18063c), > >>> >>>>> >>> >> INFO: Loaded 1 PC tables (0 PCs): 0 > >>> [0x10e2870a8,0x10e2870a8), > >>> >>>>> >>> >> ERROR: The size of coverage PC tables does not match the > >>> number > >>> >>>>> of > >>> >>>>> >>> instrumented PCs. This might be a bug in the compiler, please > >>> >>>>> contact the > >>> >>>>> >>> libFuzzer developers. > >>> >>>>> >>> >> > >>> >>>>> >>> >> From the build logs, it looks like we're now building > objects > >>> >>>>> with > >>> >>>>> >>> these > >>> >>>>> >>> >> sanitizer flags: > >>> >>>>> >>> >> > >>> >>>>> >>> >> -fsanitize=address > >>> >>>>> >>> >> -fsanitize-address-use-after-scope > >>> >>>>> >>> >> -fsanitize=fuzzer-no-link > >>> >>>>> >>> >> > >>> >>>>> >>> >> We're then linking the fuzzer binaries with these: > >>> >>>>> >>> >> > >>> >>>>> >>> >> -fsanitize=address > >>> >>>>> >>> >> -fsanitize-address-use-after-scope > >>> >>>>> >>> >> -fsanitize=fuzzer-no-link > >>> >>>>> >>> >> -fsanitize=fuzzer > >>> >>>>> >>> >> > >>> >>>>> >>> >> Any idea what's wrong or where to start looking? > >>> >>>>> >>> > >>> >>>>> >> > >>> >>>>> >> > >>> >>>>> >> > >>> >>>>> > >>> >>>> > >>> >>>> > >>> >>>> _______________________________________________ > >>> >>>> LLVM Developers mailing list > >>> >>>> llvm-dev at lists.llvm.org > >>> >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > >>> >>>> > >>> >>>> > >>> >>> > >>> >>> > >>> >>> -- > >>> >>> -- > >>> >>> Peter > >>> >>> > >>> >> > >>> >> > >>> > > >>> > > >>> > -- > >>> > >> > >> > >> > >> -- > >> -- > >> Peter > >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170911/9b754cbc/attachment-0001.html>
Two wrap up this discussion I've filed https://bugs.llvm.org/show_bug.cgi?id=34636 (for Matt) On Mon, Sep 11, 2017 at 6:57 PM, Kostya Serebryany <kcc at google.com> wrote:> > > On Mon, Sep 11, 2017 at 11:52 AM, Justin Bogner <mail at justinbogner.com> > wrote: > >> Kostya Serebryany <kcc at google.com> writes: >> > Justin, >> >> > Calling appendToUsed has horrible complexity and if we call it in >> > every function clang consumes tons of memory (6Gb when compiling one >> > of the clang's source files). This killed my machine today :) >> > >> > The solution is to call appendToUsed once per module, instead of once >> > per function. >> >> Oh right, updating lists in metadata is O(n), so doing it per function >> is quadratic. This slipped my mind - sorry! >> > > Yea. Very unexpected (although I've stumbled on it a few times, I still > don't remember about it) > > >> >> > Also, since this does not seem to be required for linux, I've put this >> > under if TargetTriple.isOSBinFormatMachO Submitted r312855, I'll see >> > if this breaks Mac (there seem to be no llvm tests for this, only >> > compiler-rt tests) but please also check if this looks ok. >> >> This looks fine, though I'd rather if we just did it on all platforms >> for consistency / clear semantic intent. Running appendToUsed once >> should be cheap, and we do it unguarded in our other instrumentation >> (like InstrProfiling.cpp). >> > > It's unclear if it's going to hurt things on linux. > If I see it doesn't -- I'll remove the if TargetTriple.isOSBinFormatMachO > > >> >> > But this all still sounds bad on linux at least: >> > * with the old bfd linker and -ffunction-sections -Wl,-gc-sections >> these >> > arrays get removed (as discussed here) >> >> This is sad, and I don't think we have any particularly good ideas to >> fix this. >> >> > * with newer linkers the sanitizer coverage essentially disables >> > gc-sections >> >> I'm not sure I follow. We're making sure the global arrays / >> instrumentation data doesn't get dead stripped here, but dead stripping >> of functions should work as normal. >> > > Ah, that's now my confusion. > inline-8bit-counters and trace-pc-guard seems to work just fine. > pc-table actually does break gc-sections as there is no a reference to > every function in the pc-table section. > > +eugenis, who dealt with a similar issue in asan (although it's probably a > separate topic now). > > --kcc > > > > >> >> > --kcc >> > >> > >> > On Thu, Aug 24, 2017 at 6:43 PM, Peter Collingbourne <peter at pcc.me.uk> >> > wrote: >> > >> >> >> >> >> >> On Thu, Aug 24, 2017 at 6:30 PM, Justin Bogner <mail at justinbogner.com> >> >> wrote: >> >> >> >>> Peter Collingbourne <peter at pcc.me.uk> writes: >> >>> > On Thu, Aug 24, 2017 at 3:38 PM, Kostya Serebryany <kcc at google.com> >> >>> wrote: >> >>> > >> >>> >> >> >>> >> >> >>> >> On Thu, Aug 24, 2017 at 3:35 PM, Peter Collingbourne < >> peter at pcc.me.uk> >> >>> >> wrote: >> >>> >> >> >>> >>> On Thu, Aug 24, 2017 at 3:21 PM, Kostya Serebryany via llvm-dev < >> >>> >>> llvm-dev at lists.llvm.org> wrote: >> >>> >>> >> >>> >>>> >> >>> >>>> >> >>> >>>> On Thu, Aug 24, 2017 at 3:20 PM, Justin Bogner < >> >>> mail at justinbogner.com> >> >>> >>>> wrote: >> >>> >>>> >> >>> >>>>> I think the simplest fix is something like this: >> >>> >>>>> >> >>> >>>>> diff --git a/lib/Transforms/Instrumentati >> on/SanitizerCoverage.cpp >> >>> >>>>> b/lib/Transforms/Instrumentation/SanitizerCoverage.cpp >> >>> >>>>> index c6f0d17f8fe..e81957ab80a 100644 >> >>> >>>>> --- a/lib/Transforms/Instrumentation/SanitizerCoverage.cpp >> >>> >>>>> +++ b/lib/Transforms/Instrumentation/SanitizerCoverage.cpp >> >>> >>>>> @@ -256,6 +256,7 @@ SanitizerCoverageModule::Creat >> >>> eSecStartEnd(Module >> >>> >>>>> &M, const char *Section, >> >>> >>>>> new GlobalVariable(M, Ty, false, >> >>> GlobalVariable::ExternalLinkag >> >>> >>>>> e, >> >>> >>>>> nullptr, getSectionEnd(Section)); >> >>> >>>>> SecEnd->setVisibility(GlobalValue::HiddenVisibility); >> >>> >>>>> + appendToUsed(M, {SecStart, SecEnd}); >> >>> >>>>> >> >>> >>>>> return std::make_pair(SecStart, SecEnd); >> >>> >>>>> } >> >>> >>>>> >> >>> >>>>> I'm trying it out now. >> >>> >>>>> >> >>> >>>> >> >>> >>>> LGTM (if this works), thanks! >> >>> >>>> >> >>> >>> >> >>> >>> I wouldn't expect that to work because for ELF targets llvm.used >> has >> >>> no >> >>> >>> effect on the object file (only on the optimizer). >> >>> >>> >> >>> >>> Is there a simple way to reproduce the link failure? >> >>> >>> >> >>> >> >> >>> >> >> >>> >> ninja compiler-rt >> >>> >> echo 'extern "C" int LLVMFuzzerTestOneInput(const unsigned char *a, >> >>> >> unsigned long b){return 0; } ' > test.cc >> >>> >> clang -O3 test.cc -fsanitize=fuzzer # works >> >>> >> clang -O3 test.cc -Wl,-gc-sections -fsanitize=fuzzer # fails >> >>> >> >> >>> > >> >>> > It seems that the issue is that older versions of ld.bfd have a bug >> >>> which >> >>> > causes it not to define __start_ and __stop_ symbols if the only >> >>> reference >> >>> > to those symbols is from a constructor. >> >>> >> >>> It looks like this is a different problem from the one on macOS (and I >> >>> wasn't able to reproduce it with any bfd ld I had available, they were >> >>> all too new) >> >>> >> >>> I've gone ahead and fixed the issue on macOS in r311742. >> >>> >> >>> > If I add an artificial reference to the start symbol from >> libfuzzer's >> >>> main >> >>> > function, the program links correctly. >> >>> > >> >>> > diff --git a/compiler-rt/lib/fuzzer/FuzzerMain.cpp >> >>> > b/compiler-rt/lib/fuzzer/FuzzerMain.cpp >> >>> > index af8657200be2..c41e28e012db 100644 >> >>> > --- a/compiler-rt/lib/fuzzer/FuzzerMain.cpp >> >>> > +++ b/compiler-rt/lib/fuzzer/FuzzerMain.cpp >> >>> > @@ -16,6 +16,10 @@ extern "C" { >> >>> > int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size); >> >>> > } // extern "C" >> >>> > >> >>> > +__attribute__((weak)) void nop(void *p) {} >> >>> > +extern void *__start___sancov_pcs; >> >>> > + >> >>> > int main(int argc, char **argv) { >> >>> > + nop(__start___sancov_pcs); >> >>> > return fuzzer::FuzzerDriver(&argc, &argv, >> LLVMFuzzerTestOneInput); >> >>> > } >> >>> >> >>> If we were to do this, we'd have to guard it appropriately - not all >> >>> platforms name the __start symbols like this. >> >>> >> >> >> >> Of course. There's also the issue of how to keep the symbols alive in >> DSOs. >> >> >> >> > The problem also goes away if I use "GNU ld (GNU Binutils) >> >>> > 2.28.51.20170105". >> >>> >> >>> 2.27 also doesn't have the issue. I don't know what our minimum >> version >> >>> of binutils is, and I'm under the impression most people use gold or >> lld >> >>> to link LLVM these days, so it isn't clear to me how big of a problem >> >>> this is. >> >>> >> >> >> >> For the record, the problem reproduces under 2.24, which is shipped by >> >> Ubuntu 14.04 LTS, which isn't that old. My view is that if we can find >> an >> >> unintrusive enough workaround, we should deploy it (with a comment to >> >> remove it after N years). >> >> >> >> Peter >> >> >> >> >> >>> > Peter >> >>> > >> >>> > >> >>> > >> >>> >> >> >>> >> >> >>> >> >> >>> >> >> >>> >> >> >>> >>> >> >>> >>> Peter >> >>> >>> >> >>> >>> >> >>> >>>> >> >>> >>>>> >> >>> >>>>> Kostya Serebryany <kcc at google.com> writes: >> >>> >>>>> > With -Wl,-gc-sections I get this: >> >>> >>>>> > SimpleTest.cpp:(.text.sancov.module_ctor[sancov.module_ctor] >> >>> +0x1b): >> >>> >>>>> > undefined reference to `__start___sancov_pcs' >> >>> >>>>> > SimpleTest.cpp:(.text.sancov.module_ctor[sancov.module_ctor] >> >>> +0x20): >> >>> >>>>> > undefined reference to `__stop___sancov_pcs' >> >>> >>>>> > >> >>> >>>>> > >> >>> >>>>> > >> >>> >>>>> > On Thu, Aug 24, 2017 at 3:07 PM, George Karpenkov < >> >>> >>>>> ekarpenkov at apple.com> >> >>> >>>>> > wrote: >> >>> >>>>> > >> >>> >>>>> >> >> >>> >>>>> >> On Aug 24, 2017, at 2:55 PM, Kostya Serebryany < >> kcc at google.com> >> >>> >>>>> wrote: >> >>> >>>>> >> >> >>> >>>>> >> Interesting. >> >>> >>>>> >> This is a relatively new addition >> (fsanitize-coverage=pc-tables, >> >>> >>>>> which is >> >>> >>>>> >> now a part of -fsanitize=fuzzer). >> >>> >>>>> >> The tests worked (did they? On Mac?) so I thought everything >> is >> >>> ok. >> >>> >>>>> >> >> >>> >>>>> >> >> >>> >>>>> >> For tests we never compile the tested target with -O3 (and >> that >> >>> >>>>> wouldn’t >> >>> >>>>> >> be sufficient), >> >>> >>>>> >> and for testing fuzzers I was always building them in debug >> >>> >>>>> >> >> >>> >>>>> >> Yea, we need to make sure the pc-tables are not stripped >> (this >> >>> is a >> >>> >>>>> >> separate section with globals). >> >>> >>>>> >> (I still haven't documented pc-tables, will do soon) >> >>> >>>>> >> >> >>> >>>>> >> >> >>> >>>>> >> Do you know what's the analog of Wl,-dead_strip on Linux? >> >>> >>>>> >> >> >>> >>>>> >> >> >>> >>>>> >> Apparently -Wl,—gc-sections. >> >>> >>>>> >> For some reason LLVM does not do it for gold, even though it >> >>> seems to >> >>> >>>>> >> support this flag as well. >> >>> >>>>> >> (that could be another reason why you don’t see the failure >> on >> >>> Linux) >> >>> >>>>> >> >> >>> >>>>> >> 1 *if*(NOT LLVM_NO_DEAD_STRIP) >> >>> >>>>> >> 2 *if*(${CMAKE_SYSTEM_NAME} MATCHES "Darwin") >> >>> >>>>> >> 3 # ld64's implementation of -dead_strip breaks tools >> that >> >>> use >> >>> >>>>> >> plugins. >> >>> >>>>> >> 4 set_property(TARGET ${target_name} APPEND_STRING >> PROPERTY >> >>> >>>>> >> 5 LINK_FLAGS " -Wl,-dead_strip") >> >>> >>>>> >> 6 *elseif*(${CMAKE_SYSTEM_NAME} MATCHES "SunOS") >> >>> >>>>> >> 7 set_property(TARGET ${target_name} APPEND_STRING >> PROPERTY >> >>> >>>>> >> 8 LINK_FLAGS " -Wl,-z >> >>> -Wl,discard-unused=sections") >> >>> >>>>> >> 9 *elseif*(NOT WIN32 AND NOT LLVM_LINKER_IS_GOLD) >> >>> >>>>> >> 10 # Object files are compiled with >> -ffunction-data-sections. >> >>> >>>>> >> 11 # Versions of bfd ld < 2.23.1 have a bug in >> --gc-sections >> >>> that >> >>> >>>>> >> breaks >> >>> >>>>> >> 12 # tools that use plugins. Always pass --gc-sections >> once >> >>> we >> >>> >>>>> require >> >>> >>>>> >> 13 # a newer linker. >> >>> >>>>> >> 14 set_property(TARGET ${target_name} APPEND_STRING >> PROPERTY >> >>> >>>>> >> 15 LINK_FLAGS " -Wl,--gc-sections") >> >>> >>>>> >> 16 *endif*() >> >>> >>>>> >> 17 *endif*() >> >>> >>>>> >> >> >>> >>>>> >> >> >>> >>>>> >> >> >>> >>>>> >> --kcc >> >>> >>>>> >> >> >>> >>>>> >> >> >>> >>>>> >> >> >>> >>>>> >> On Thu, Aug 24, 2017 at 2:49 PM, Justin Bogner < >> >>> >>>>> mail at justinbogner.com> >> >>> >>>>> >> wrote: >> >>> >>>>> >> >> >>> >>>>> >>> George Karpenkov <ekarpenkov at apple.com> writes: >> >>> >>>>> >>> > OK so with Kuba’s help I’ve found the error: with >> >>> optimization, >> >>> >>>>> dead >> >>> >>>>> >>> > stripping of produced libraries is enabled, >> >>> >>>>> >>> > which removes coverage instrumentation. >> >>> >>>>> >>> > >> >>> >>>>> >>> > However, this has nothing to do with the move to >> compiler-rt, >> >>> so >> >>> >>>>> I’m >> >>> >>>>> >>> > quite skeptical on whether it has worked >> >>> >>>>> >>> > beforehand. >> >>> >>>>> >>> > >> >>> >>>>> >>> > A trivial fix is to do: >> >>> >>>>> >>> > >> >>> >>>>> >>> > diff --git a/cmake/modules/HandleLLVMOptions.cmake >> >>> >>>>> >>> b/cmake/modules/HandleLLVMOptions.cmake >> >>> >>>>> >>> > index 04596a6ff63..5465d8d95ba 100644 >> >>> >>>>> >>> > --- a/cmake/modules/HandleLLVMOptions.cmake >> >>> >>>>> >>> > +++ b/cmake/modules/HandleLLVMOptions.cmake >> >>> >>>>> >>> > @@ -665,6 +665,9 @@ if(LLVM_USE_SANITIZER) >> >>> >>>>> >>> > endif() >> >>> >>>>> >>> > if (LLVM_USE_SANITIZE_COVERAGE) >> >>> >>>>> >>> > append("-fsanitize=fuzzer-no-link" CMAKE_C_FLAGS >> >>> >>>>> CMAKE_CXX_FLAGS) >> >>> >>>>> >>> > + >> >>> >>>>> >>> > + # Dead stripping messes up coverage instrumentation. >> >>> >>>>> >>> > + set(LLVM_NO_DEAD_STRIP ON) >> >>> >>>>> >>> > endif() >> >>> >>>>> >>> > endif() >> >>> >>>>> >>> > >> >>> >>>>> >>> > Any arguments against that? >> >>> >>>>> >>> >> >>> >>>>> >>> We shouldn't do this. We really only want to prevent dead >> >>> stripping >> >>> >>>>> of >> >>> >>>>> >>> the counters themselves - disabling it completely isn't very >> >>> nice. >> >>> >>>>> >>> >> >>> >>>>> >>> > Apparently, a better way is to follow ASAN instrumentation >> >>> pass, >> >>> >>>>> >>> > which uses some magic to protect against dead-stripping. >> >>> >>>>> >>> >> >>> >>>>> >>> I thought this was already being done - how else did it work >> >>> before? >> >>> >>>>> >>> >> >>> >>>>> >>> >> On Aug 24, 2017, at 11:29 AM, Justin Bogner < >> >>> >>>>> mail at justinbogner.com> >> >>> >>>>> >>> wrote: >> >>> >>>>> >>> >> >> >>> >>>>> >>> >> (kcc, george: sorry for the re-send, the first was from a >> >>> >>>>> non-list >> >>> >>>>> >>> email >> >>> >>>>> >>> >> address) >> >>> >>>>> >>> >> >> >>> >>>>> >>> >> My configuration for building the fuzzers in the LLVM >> tree >> >>> >>>>> doesn't >> >>> >>>>> >>> seem to >> >>> >>>>> >>> >> work any more (possibly as of moving libFuzzer to >> >>> compiler-rt, >> >>> >>>>> but >> >>> >>>>> >>> there >> >>> >>>>> >>> >> have been a few other changes in the last week or so that >> >>> may be >> >>> >>>>> >>> related). >> >>> >>>>> >>> >> >> >>> >>>>> >>> >> I'm building with a fresh top-of-tree clang and setting >> >>> >>>>> >>> >> -DLLVM_USE_SANITIZER=Address and >> >>> -DLLVM_USE_SANITIZE_COVERAGE=O >> >>> >>>>> n, >> >>> >>>>> >>> which >> >>> >>>>> >>> >> was working before: >> >>> >>>>> >>> >> >> >>> >>>>> >>> >> % cmake -GNinja \ >> >>> >>>>> >>> >> -DCMAKE_BUILD_TYPE=Release >> >>> -DLLVM_ENABLE_ASSERTIONS=On \ >> >>> >>>>> >>> >> -DLLVM_ENABLE_WERROR=On \ >> >>> >>>>> >>> >> -DLLVM_USE_SANITIZER=Address >> >>> >>>>> -DLLVM_USE_SANITIZE_COVERAGE=On >> >>> >>>>> >>> \ >> >>> >>>>> >>> >> -DCMAKE_C_COMPILER=$HOME/llvm-lkgc/bin/clang \ >> >>> >>>>> >>> >> $HOME/code/llvm-src >> >>> >>>>> >>> >> >> >>> >>>>> >>> >> But when I run any of the fuzzers, it looks like the >> >>> sanitizer >> >>> >>>>> coverage >> >>> >>>>> >>> >> hasn't been set up correctly: >> >>> >>>>> >>> >> >> >>> >>>>> >>> >> % ./bin/llvm-as-fuzzer >> >>> >>>>> >>> 2017-08-24 11:14:33 >> >>> >>>>> >>> >> INFO: Seed: 4089166883 <(408)%20916-6883> >> >>> >>>>> >>> >> INFO: Loaded 1 modules (50607 guards): 50607 >> [0x10e14ef80, >> >>> >>>>> >>> 0x10e18063c), >> >>> >>>>> >>> >> INFO: Loaded 1 PC tables (0 PCs): 0 >> >>> [0x10e2870a8,0x10e2870a8), >> >>> >>>>> >>> >> ERROR: The size of coverage PC tables does not match the >> >>> number >> >>> >>>>> of >> >>> >>>>> >>> instrumented PCs. This might be a bug in the compiler, >> please >> >>> >>>>> contact the >> >>> >>>>> >>> libFuzzer developers. >> >>> >>>>> >>> >> >> >>> >>>>> >>> >> From the build logs, it looks like we're now building >> objects >> >>> >>>>> with >> >>> >>>>> >>> these >> >>> >>>>> >>> >> sanitizer flags: >> >>> >>>>> >>> >> >> >>> >>>>> >>> >> -fsanitize=address >> >>> >>>>> >>> >> -fsanitize-address-use-after-scope >> >>> >>>>> >>> >> -fsanitize=fuzzer-no-link >> >>> >>>>> >>> >> >> >>> >>>>> >>> >> We're then linking the fuzzer binaries with these: >> >>> >>>>> >>> >> >> >>> >>>>> >>> >> -fsanitize=address >> >>> >>>>> >>> >> -fsanitize-address-use-after-scope >> >>> >>>>> >>> >> -fsanitize=fuzzer-no-link >> >>> >>>>> >>> >> -fsanitize=fuzzer >> >>> >>>>> >>> >> >> >>> >>>>> >>> >> Any idea what's wrong or where to start looking? >> >>> >>>>> >>> >> >>> >>>>> >> >> >>> >>>>> >> >> >>> >>>>> >> >> >>> >>>>> >> >>> >>>> >> >>> >>>> >> >>> >>>> _______________________________________________ >> >>> >>>> LLVM Developers mailing list >> >>> >>>> llvm-dev at lists.llvm.org >> >>> >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> >>> >>>> >> >>> >>>> >> >>> >>> >> >>> >>> >> >>> >>> -- >> >>> >>> -- >> >>> >>> Peter >> >>> >>> >> >>> >> >> >>> >> >> >>> > >> >>> > >> >>> > -- >> >>> >> >> >> >> >> >> >> >> -- >> >> -- >> >> Peter >> >> >> > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170915/c54e3eb4/attachment.html>