[sancov][sanitizer-common] Correct sanitizer coverage point

[XiaodongLoong]

Summary

Sanitizer coverage point should be the previous instruction PC of the
caller and the offset to the previous instruction might be different
on each CPU architecture.

[XiaodongLoong]

compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cpp

13–20

clang-format might make a mistake as following:

clang-format suggested style edits found:

          14 #include "sanitizer_allocator_internal.h"
          15 #include "sanitizer_atomic.h"
          16 #include "sanitizer_common.h"
>>> -     17 #include "sanitizer_common/sanitizer_stacktrace.h"
    -     18 #include "sanitizer_file.h"
    +        #  include "sanitizer_common/sanitizer_stacktrace.h"
    +        #  include "sanitizer_file.h"
          19 
          20 using namespace __sanitizer;
          21 
          22 using AddressRange = LoadedModule::AddressRange;

What should I do for this issue?

[XiaodongLoong]

A gentle ping.

[XiaodongLoong]

rebase code

[XiaodongLoong]

compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cpp

13–20

After rebased code, it is OK now.

It seems that the building failure is not caused by this patch.

[MaskRay]

See D120362: I think we should refactor the -1/-4 thing first.

This patch should focus on fixing the offset for __sanitizer_cov_trace_pc_guard.

compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cpp

227

I have confirmed that aarch64 needs this as well.

llvm/include/llvm/ADT/Triple.h

821 ↗

(On Diff #410492)

Upload a separate patch for the Triple.h change. You may add ro as a reviewer.

[MaskRay]

llvm/include/llvm/ADT/Triple.h

821 ↗

(On Diff #410492)

Maybe for @ro to add this. @ro may have some idea whether this can be used elsewhere.

[XiaodongLoong]

rebase code

[XiaodongLoong]

@MaskRay Thanks for your review. I have rebased the code and split Triple.h to other patch D120381.

[MaskRay]

llvm/tools/sancov/sancov.cpp

697 ↗

(On Diff #410699)

Drop this unrelated change.

@ro can decide what to do with isSparc. There are three places to update if - 8 is wrong:

3de5322b5f719d9414423d4237a6533fe43cd7f8
fc0bd3c2cee929ffbd75b5cca486f4c77f7d5c59
1ec9dd3aae0b8c90a91f845ad629ef7d199986c0

[XiaodongLoong]

drop isSparc part

compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cpp

 //===-- sanitizer_coverage_libcdep_new.cpp --------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 // Sanitizer Coverage Controller for Trace PC Guard.
 
 #include "sanitizer_platform.h"
 
 #if !SANITIZER_FUCHSIA
 #  include "sancov_flags.h"
 #  include "sanitizer_allocator_internal.h"
 #  include "sanitizer_atomic.h"
 #  include "sanitizer_common.h"
+#  include "sanitizer_common/sanitizer_stacktrace.h"
 #  include "sanitizer_file.h"
 #  include "sanitizer_interface_internal.h"
 
 using namespace __sanitizer;
 
 using AddressRange = LoadedModule::AddressRange;
 
 namespace __sancov {
 namespace {
 
 static const u64 Magic64 = 0xC0BFFFFFFFFFFF64ULL;

 extern "C" {
 SANITIZER_INTERFACE_ATTRIBUTE void __sanitizer_dump_coverage(const uptr* pcs,
                                                              uptr len) {
   return __sancov::SanitizerDumpCoverage(pcs, len);
 }
 
 SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_trace_pc_guard, u32* guard) {
   if (!*guard) return;
-  __sancov::pc_guard_controller.TracePcGuard(guard, GET_CALLER_PC() - 1);
+  __sancov::pc_guard_controller.TracePcGuard(
+      guard, StackTrace::GetPreviousInstructionPc(GET_CALLER_PC()));
 }
 
 SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_trace_pc_guard_init,
                              u32* start, u32* end) {
   if (start == end || *start) return;
   __sancov::pc_guard_controller.InitTracePcGuard(start, end);
 }