Building LLVM's fuzzers

Project Infrastructure LLVM Dev List Archives
1

(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=On, 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
  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?

2

Should -DCMAKE_CXX_COMPILER be also specified?

3

George Karpenkov <ekarpenkov@apple.com> writes:

Should -DCMAKE_CXX_COMPILER be also specified?

CMake is smart enough to infer that from C_COMPILER:

  % grep CMAKE_CXX_COMPILER CMakeCache.txt
  CMAKE_CXX_COMPILER:FILEPATH=/Users/bogner/llvm-lkgc/bin/clang++

4

Right.
One difference is that I was previously building a debug build, trying release one now.

5

Indeed, I can reproduce in release build. Looking into it.
As a workaround, for now you could try to use debug build of fuzzers.

6

+mascasa@ FYI

Also, I am going to file a bug to implement some improvements in the way we build and use fuzz targets in LLVM.

And, take a look at the last night’s trophies: https://bugs.chromium.org/p/oss-fuzz/issues/list?q=label:Proj-llvm

7

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)

8

This is quite unexpected.
Do you have a minimized example of dead stripping eliminating the coverage instrumentation?

–kcc

9

Yeah, at least on mac basically nothing works:

george@/Volumes/Transcend/code/llvm (✱✖master)✘≻ ./release-build/bin/clang -O3 -isysroot (xcrun --show-sdk-path) -fsanitize=fuzzer,address -Wl,-dead_strip projects/compiler-rt/test/fuzzer/StrcmpTest.cpp
george@/Volumes/Transcend/code/llvm (✱✖master)≻ ./a.out
INFO: Seed: 3036650336
INFO: Loaded 1 modules (8 guards): 8 [0x106c2a440, 0x106c2a460),
INFO: Loaded 1 PC tables (0 PCs): 0 [0x106c2a5d0,0x106c2a5d0),
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.
numguards=8, NumPCsInPCTables=0, NumInline8bitCounters=0

10

George Karpenkov <ekarpenkov@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?

11

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.
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?

–kcc

12

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()

13

Kostya Serebryany <kcc@google.com> writes:

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.

It looks like the tests were passing, and I *think* I'd tried my fuzzer
since that change was in place. Maybe something about how we're linking
in compiler-rt makes it more obvious to the linker that these symbols
look dead?

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?

I think the closest thing is -Wl,--gc-sections but that might be less
aggressive about it than macOS's linker is.

14

Kostya Serebryany <kcc@google.com> writes:

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.

It looks like the tests were passing, and I *think* I'd tried my fuzzer
since that change was in place.

that wouldn’t have helped: it was specifying coverage flags explicitly,
which did not include the new flag in question.

15

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’

16

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'

This happens only with 'ld'.
lld and gold are fine.

17

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::CreateSecStartEnd(Module &M, const char *Section,
       new GlobalVariable(M, Ty, false, GlobalVariable::ExternalLinkage,
                          nullptr, getSectionEnd(Section));
   SecEnd->setVisibility(GlobalValue::HiddenVisibility);
+ appendToUsed(M, {SecStart, SecEnd});

   return std::make_pair(SecStart, SecEnd);
}

I'm trying it out now.

Kostya Serebryany <kcc@google.com> writes:

18

+pcc (for the discussion on how to preserve a section when -Wl,-gc-sections is used)

19

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::CreateSecStartEnd(Module &M,
const char *Section,
       new GlobalVariable(M, Ty, false, GlobalVariable::ExternalLinkage,
                          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!

20

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::CreateSecStartEnd(Module
&M, const char *Section,
       new GlobalVariable(M, Ty, false, GlobalVariable::ExternalLinkage,
                          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?

Peter