[libFuzzer] Generalize the code for getting the previous offset for different architectures

[george.karpenkov]

Summary

Without this change, tests in coverage.test and dump_coverage.test are failing on non-x86_64 platforms.
The diff is copied from sanitizer_common library, an alternative would be to link it together with libFuzzer.

[george.karpenkov]

@morehouse After this and the linked change, it should be safe to re-enable coverage.test on AArch64.

[Dor1s]

I should admit that I'm not familiar with ARM offsets, but since the change fixes the test, LGTM.

compiler-rt/lib/fuzzer/FuzzerTracePC.cpp

202

Are you sure you need -3 here, not -2? I'm definitely not an expert here, so it's more like a sanity check question :)

>>> hex((0x122 - 3) & (~1))
'0x11e'
>>> hex((0x122 - 2) & (~1))
'0x120'

213

some comment here might be useful too

[Dor1s]

Btw, @george.karpenkov, have you every experienced anything similar to https://bugs.chromium.org/p/chromium/issues/detail?id=892167 ?

[george.karpenkov]

compiler-rt/lib/fuzzer/FuzzerTracePC.cpp

202

I've copied this code from sanitizer_common/sanitizer_stracktrace.h.

Another possibility is to link to sanitizer_common instead, but I think at one point @kcc was against that (has that changed?)

[george.karpenkov]

@Dor1s I have definitely seen this error, but at some point it disappeared. I do not recall exactly the circumstances, maybe it appeared when the compiler version did not match exactly the runtime version.

[george.karpenkov]

@Dor1s this can't land without D53039, does that look fine as well?

[morehouse]

compiler-rt/lib/fuzzer/FuzzerTracePC.cpp

221

Could we avoid putting ifdefs here? Maybe they could go in FuzzerDefs.h.

[george.karpenkov]

compiler-rt/lib/fuzzer/FuzzerTracePC.cpp

221

I don't think this is a good idea.

1. FuzzerDefs contains constants, this is not a constant.
2. As the code is cloned, it would be harder to maintain parity with the code in sanitizer_common if it's significantly changed.

[morehouse]

compiler-rt/lib/fuzzer/FuzzerTracePC.cpp

221

Ok, then let's add a comment saying where this code came from.

[george.karpenkov]

compiler-rt/lib/fuzzer/FuzzerTracePC.cpp

221

OK!

compiler-rt/lib/fuzzer/FuzzerTracePC.cpp

void TracePC::HandleCallerCallee(uintptr_t Caller, uintptr_t Callee) {
   ValueProfileMap.AddValueModPrime(Idx);
 }
 
 void TracePC::UpdateObservedPCs() {
   Vector<uintptr_t> CoveredFuncs;
   auto ObservePC = [&](uintptr_t PC) {
     if (ObservedPCs.insert(PC).second && DoPrintNewPCs) {
       PrintPC("\tNEW_PC: %p %F %L", "\tNEW_PC: %p", PC + 1);
       Printf("\n");
     }
   };
 
   auto Observe = [&](const PCTableEntry &TE) {
     if (TE.PCFlags & 1)
       if (++ObservedFuncs[TE.PC] == 1 && NumPrintNewFuncs)
         CoveredFuncs.push_back(TE.PC);
     ObservePC(TE.PC);
   };
 
   if (NumPCsInPCTables) {
     if (NumInline8bitCounters == NumPCsInPCTables) {
       IterateInline8bitCounters([&](int i, int j, int CounterIdx) {
         if (ModuleCounters[i].Start[j])
           Observe(ModulePCTable[i].Start[j]);
       });
     } else if (NumGuards == NumPCsInPCTables) {
       size_t GuardIdx = 1;
       for (size_t i = 0; i < NumModules; i++) {
         uint32_t *Beg = Modules[i].Start;
         size_t Size = Modules[i].Stop - Beg;
         assert(Size ==
                (size_t)(ModulePCTable[i].Stop - ModulePCTable[i].Start));
         for (size_t j = 0; j < Size; j++, GuardIdx++)
           if (Counters()[GuardIdx])
             Observe(ModulePCTable[i].Start[j]);
       }
     }
   }
 
   for (size_t i = 0, N = Min(CoveredFuncs.size(), NumPrintNewFuncs); i < N;
        i++) {
     Printf("\tNEW_FUNC[%zd/%zd]: ", i + 1, CoveredFuncs.size());
     PrintPC("%p %F %L", "%p", CoveredFuncs[i] + 1);
     Printf("\n");
   }
 }
 
 inline ALWAYS_INLINE uintptr_t GetPreviousInstructionPc(uintptr_t PC) {
-  // TODO: this implementation is x86 only.
-  // see sanitizer_common GetPreviousInstructionPc for full implementation.
+#if defined(__arm__)
+  // T32 (Thumb) branch instructions might be 16 or 32 bit long,
+  // so we return (pc-2) in that case in order to be safe.
+  // For A32 mode we return (pc-4) because all instructions are 32 bit long.
+  return (PC - 3) & (~1);
+#elif defined(__powerpc__) || defined(__powerpc64__) || defined(__aarch64__)
+  // PCs are always 4 byte aligned.
+  return PC - 4;
+#elif defined(__sparc__) || defined(__mips__)
+  return PC - 8;
+#else
   return PC - 1;
+#endif
 }
 
 inline ALWAYS_INLINE uintptr_t GetNextInstructionPc(uintptr_t PC) {
-  // TODO: this implementation is x86 only.
-  // see sanitizer_common GetPreviousInstructionPc for full implementation.
+#if defined(__mips__)
+  return PC + 8;
+#elif defined(__powerpc__) || defined(__sparc__) || defined(__arm__) || \
+    defined(__aarch64__)
+  return PC + 4;
+#else
   return PC + 1;
+#endif
 }
 
 static std::string GetModuleName(uintptr_t PC) {
   char ModulePathRaw[4096] = "";  // What's PATH_MAX in portable C++?
   void *OffsetRaw = nullptr;
   if (!EF->__sanitizer_get_module_and_offset_for_pc(
       reinterpret_cast<void *>(PC), ModulePathRaw,
       sizeof(ModulePathRaw), &OffsetRaw))