[PATCH] D114558: [ASan] Shared optimized callbacks implementation.

[Matt Morehouse via Phabricator via llvm-commits]

morehouse added inline comments.


================
Comment at: compiler-rt/lib/asan/asan_rtl_x86_64.S:18
+#define BEGINF(reg, op, s, i) \
+.globl  FNAME(reg, op, s, i) ;\
+ASM_TYPE_FUNCTION(FNAME(reg, op, s, i)) ;\
----------------
Nit:  Can we omit the semicolons?


================
Comment at: compiler-rt/lib/asan/asan_rtl_x86_64.S:88
+
+// Access check functions for 8 and 16 bytwe types: no extra checks required.
+#define ASAN_MEMORY_ACCESS_CHECK_ADD(reg, op, s, c) \
----------------



================
Comment at: compiler-rt/lib/asan/asan_rtl_x86_64.S:99
+        mov    %##reg,%rdi ;\
+        jmp    __asan_report_##op##8 ;\
+
----------------
I think we also need to handle the 16 byte case.


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_shadow_defines.h:32
+#  if SANITIZER_RISCV64
+#    define SHADOW_OFFSET_CONST 0x55550000
+#  elif defined(__aarch64__)
----------------
Looks like in asan_mapping.h this value is 0xd55550000.


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_shadow_defines.h:46
+#  elif defined(__mips64)
+#    define SHADOW_OFFSET_CONST 0
+#  else
----------------
Why is the offset 0 for all these platforms?  It looks like they have non-zero in asan_mapping.h.


================
Comment at: compiler-rt/test/asan/TestCases/Linux/interface_symbols_linux.cpp:4
+// RUN: %clangxx -x c++-header -o - -E %p/../../../../lib/asan/asan_interface.inc  \
+// RUN:  >  %t.asan_interface.inc
 // RUN: %clangxx_asan -O2 %s -o %t.exe
----------------
Why is this step required now?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114558/new/

https://reviews.llvm.org/D114558