[libfuzzer] fixing collected pc addresses for coverage

[aizatsky]

Summary

The causes google/ossfuzz#84

[kcc]

LGTM with a nit

lib/Fuzzer/FuzzerTracePC.cpp

29

move to FuzzerDefs.h

Or maybe just not do it. inline should be enough for GetPreviousInstructionPc

[aizatsky]

moved macro to FuzzerDefs.

[kcc]

LGTM

lib/Fuzzer/FuzzerTracePC.h

 class TracePC {
  public:
   static const size_t kFeatureSetSize = ValueBitMap::kNumberOfItems;
 
   void HandleTrace(uint32_t *guard, uintptr_t PC);
   void HandleInit(uint32_t *start, uint32_t *stop);
   void HandleCallerCallee(uintptr_t Caller, uintptr_t Callee);
   void HandleValueProfile(size_t Value) { ValueProfileMap.AddValue(Value); }
-  template <class T> void HandleCmp(void *PC, T Arg1, T Arg2);
+  template <class T> void HandleCmp(uintptr_t PC, T Arg1, T Arg2);
   size_t GetTotalPCCoverage();
   void SetUseCounters(bool UC) { UseCounters = UC; }
   void SetUseValueProfile(bool VP) { UseValueProfile = VP; }
   void SetPrintNewPCs(bool P) { DoPrintNewPCs = P; }
   template <class Callback> size_t CollectFeatures(Callback CB);
   bool UpdateValueProfileMap(ValueBitMap *MaxValueProfileMap) {
     return UseValueProfile && MaxValueProfileMap->MergeFrom(ValueProfileMap);
   }

lib/Fuzzer/FuzzerTracePC.cpp

 #include "FuzzerValueBitMap.h"
 #include <map>
 #include <sanitizer/coverage_interface.h>
 #include <set>
 #include <sstream>
 
 namespace fuzzer {
 
+#ifdef __clang__
+# define ALWAYS_INLINE __attribute__((always_inline))
+#else
+# define ALWAYS_INLINE
+#endif  // __clang__
+
 TracePC TPC;
 
 void TracePC::HandleTrace(uint32_t *Guard, uintptr_t PC) {
   uint32_t Idx = *Guard;
   if (!Idx) return;
   PCs[Idx % kNumPCs] = PC;
   Counters[Idx % kNumCounters]++;
 }
void TracePC::AddValueForStrcmp(void *caller_pc, const char *s1, const char *s2,
   size_t PC = reinterpret_cast<size_t>(caller_pc);
   size_t Idx = I;
   // if (I < Len && A1[I])
   //  Idx += __builtin_popcountl((A1[I] ^ A2[I])) - 1;
   TPC.HandleValueProfile((PC & 4095) | (Idx << 12));
 }
 
 template <class T>
-ATTRIBUTE_TARGET_POPCNT
-#ifdef __clang__  // g++ can't handle this __attribute__ here :(
-__attribute__((always_inline))
-#endif  // __clang__
-void TracePC::HandleCmp(void *PC, T Arg1, T Arg2) {
-  uintptr_t PCuint = reinterpret_cast<uintptr_t>(PC);
+ATTRIBUTE_TARGET_POPCNT ALWAYS_INLINE
+void TracePC::HandleCmp(uintptr_t PC, T Arg1, T Arg2) {
   uint64_t ArgXor = Arg1 ^ Arg2;
   uint64_t ArgDistance = __builtin_popcountl(ArgXor) + 1; // [1,65]
-  uintptr_t Idx = ((PCuint & 4095) + 1) * ArgDistance;
+  uintptr_t Idx = ((PC & 4095) + 1) * ArgDistance;
   if (sizeof(T) == 4)
       TORC4.Insert(ArgXor, Arg1, Arg2);
   else if (sizeof(T) == 8)
       TORC8.Insert(ArgXor, Arg1, Arg2);
   HandleValueProfile(Idx);
 }
 
+inline ALWAYS_INLINE uintptr_t GetPreviousInstructionPc(void* pc) {
+  // TODO: this implementation is x86 only.
+  // see sanitizer_common GetPreviousInstructionPc for full implementation.
+  return reinterpret_cast<uintptr_t>(pc) - 1;
+}
+
 } // namespace fuzzer
 
 extern "C" {
 __attribute__((visibility("default")))
 void __sanitizer_cov_trace_pc_guard(uint32_t *Guard) {
-  uintptr_t PC = (uintptr_t)__builtin_return_address(0);
+  uintptr_t PC = fuzzer::GetPreviousInstructionPc(__builtin_return_address(0));
   fuzzer::TPC.HandleTrace(Guard, PC);
 }
 
 __attribute__((visibility("default")))
 void __sanitizer_cov_trace_pc_guard_init(uint32_t *Start, uint32_t *Stop) {
   fuzzer::TPC.HandleInit(Start, Stop);
 }
 
 __attribute__((visibility("default")))
 void __sanitizer_cov_trace_pc_indir(uintptr_t Callee) {
-  uintptr_t PC = (uintptr_t)__builtin_return_address(0);
+  uintptr_t PC = fuzzer::GetPreviousInstructionPc(__builtin_return_address(0));
   fuzzer::TPC.HandleCallerCallee(PC, Callee);
 }
 
 __attribute__((visibility("default")))
 void __sanitizer_cov_trace_cmp8(uint64_t Arg1, uint64_t Arg2) {
-  fuzzer::TPC.HandleCmp(__builtin_return_address(0), Arg1, Arg2);
+  uintptr_t PC = fuzzer::GetPreviousInstructionPc(__builtin_return_address(0));
+  fuzzer::TPC.HandleCmp(PC, Arg1, Arg2);
 }
 __attribute__((visibility("default")))
 void __sanitizer_cov_trace_cmp4(uint32_t Arg1, uint32_t Arg2) {
-  fuzzer::TPC.HandleCmp(__builtin_return_address(0), Arg1, Arg2);
+  uintptr_t PC = fuzzer::GetPreviousInstructionPc(__builtin_return_address(0));
+  fuzzer::TPC.HandleCmp(PC, Arg1, Arg2);
 }
 __attribute__((visibility("default")))
 void __sanitizer_cov_trace_cmp2(uint16_t Arg1, uint16_t Arg2) {
-  fuzzer::TPC.HandleCmp(__builtin_return_address(0), Arg1, Arg2);
+  uintptr_t PC = fuzzer::GetPreviousInstructionPc(__builtin_return_address(0));
+  fuzzer::TPC.HandleCmp(PC, Arg1, Arg2);
 }
 __attribute__((visibility("default")))
 void __sanitizer_cov_trace_cmp1(uint8_t Arg1, uint8_t Arg2) {
-  fuzzer::TPC.HandleCmp(__builtin_return_address(0), Arg1, Arg2);
+  uintptr_t PC = fuzzer::GetPreviousInstructionPc(__builtin_return_address(0));
+  fuzzer::TPC.HandleCmp(PC, Arg1, Arg2);
 }
 
 __attribute__((visibility("default")))
 void __sanitizer_cov_trace_switch(uint64_t Val, uint64_t *Cases) {
   uint64_t N = Cases[0];
   uint64_t ValSizeInBits = Cases[1];
   uint64_t *Vals = Cases + 2;
   // Skip the most common and the most boring case.
   if (Vals[N - 1]  < 256 && Val < 256)
     return;
-  char *PC = (char*)__builtin_return_address(0);
+  uintptr_t PC = fuzzer::GetPreviousInstructionPc(__builtin_return_address(0));
   size_t i;
   uint64_t Token = 0;
   for (i = 0; i < N; i++) {
     Token = Val ^ Vals[i];
     if (Val < Vals[i])
       break;
   }
 
   if (ValSizeInBits == 16)
     fuzzer::TPC.HandleCmp(PC + i, static_cast<uint16_t>(Token), (uint16_t)(0));
   else if (ValSizeInBits == 32)
     fuzzer::TPC.HandleCmp(PC + i, static_cast<uint32_t>(Token), (uint32_t)(0));
   else
     fuzzer::TPC.HandleCmp(PC + i, Token, (uint64_t)(0));
 }
 
 __attribute__((visibility("default")))
 void __sanitizer_cov_trace_div4(uint32_t Val) {
-  fuzzer::TPC.HandleCmp(__builtin_return_address(0), Val, (uint32_t)0);
+  uintptr_t PC = fuzzer::GetPreviousInstructionPc(__builtin_return_address(0));
+  fuzzer::TPC.HandleCmp(PC, Val, (uint32_t)0);
 }
 __attribute__((visibility("default")))
 void __sanitizer_cov_trace_div8(uint64_t Val) {
-  fuzzer::TPC.HandleCmp(__builtin_return_address(0), Val, (uint64_t)0);
+  uintptr_t PC = fuzzer::GetPreviousInstructionPc(__builtin_return_address(0));
+  fuzzer::TPC.HandleCmp(PC, Val, (uint64_t)0);
 }
 __attribute__((visibility("default")))
 void __sanitizer_cov_trace_gep(uintptr_t Idx) {
-  fuzzer::TPC.HandleCmp(__builtin_return_address(0), Idx, (uintptr_t)0);
+  uintptr_t PC = fuzzer::GetPreviousInstructionPc(__builtin_return_address(0));
+  fuzzer::TPC.HandleCmp(PC, Idx, (uintptr_t)0);
 }
 
 }  // extern "C"

lib/Fuzzer/test/CMakeLists.txt

 else()
   set(HAS_LSAN 1)
 endif()
 
 foreach(Test ${Tests})
   add_libfuzzer_test(${Test} SOURCES ${Test}.cpp)
 endforeach()
 
-
 ###############################################################################
 # Unit tests
 ###############################################################################
 
 add_executable(LLVMFuzzer-Unittest
   FuzzerUnittest.cpp
   )
 

 
 configure_lit_site_cfg(
   ${CMAKE_CURRENT_SOURCE_DIR}/unit/lit.site.cfg.in
   ${CMAKE_CURRENT_BINARY_DIR}/unit/lit.site.cfg
   )
 
 add_lit_testsuite(check-fuzzer "Running Fuzzer tests"
     ${CMAKE_CURRENT_BINARY_DIR}
-    DEPENDS ${TestBinaries} FileCheck not
+    DEPENDS ${TestBinaries} FileCheck sancov not
     )

lib/Fuzzer/test/dump_coverage.test

 RUN: DIR=%t_workdir
 RUN: BUILD_DIR=$(pwd)
 RUN: rm -rf $DIR && mkdir -p $DIR && cd $DIR
 RUN: not $BUILD_DIR/LLVMFuzzer-NullDerefTest -dump_coverage=1 2>&1 | FileCheck %s
+RUN: sancov -covered-functions *.sancov $BUILD_DIR/LLVMFuzzer-NullDerefTest | FileCheck %s --check-prefix=SANCOV
 RUN: $BUILD_DIR/LLVMFuzzer-DSOTest -dump_coverage=1 -runs=0 2>&1 | FileCheck %s --check-prefix=DSO
 RUN: not $BUILD_DIR/LLVMFuzzer-NullDerefTest -dump_coverage=0 2>&1 | FileCheck %s --check-prefix=NOCOV
 RUN: rm -rf $DIR
 
 
 CHECK: SanitizerCoverage: ./LLVMFuzzer-NullDerefTest.{{.*}}.sancov {{.*}} PCs written
+SANCOV: LLVMFuzzerTestOneInput
 
 DSO: SanitizerCoverage: ./LLVMFuzzer-DSOTest.{{.*}}.sancov {{.*}} PCs written
 DSO-DAG: SanitizerCoverage: ./libLLVMFuzzer-DSO1.{{.*}}.sancov {{.*}} PCs written
 DSO-DAG: SanitizerCoverage: ./libLLVMFuzzer-DSO2.{{.*}}.sancov {{.*}} PCs written
 
 NOCOV-NOT: SanitizerCoverage: {{.*}} PCs written