[PATCH] D91466: [WIP][clang][Fuchsia] Support HWASan for Fuchsia

[Petr Hosek via Phabricator via llvm-commits]

phosek added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:100-101
       Dyld += "tsan/";
+    if (SanArgs.needsHwasanRt() && SanArgs.needsSharedRt())
+      Dyld += "hwasan/";
     Dyld += "ld.so.1";
----------------
Nit: can you move this above TSan so it's alphabetically sorted?


================
Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:222
                           .flag("+fno-exceptions"));
+  // ASan has higher priority because we always want the instrumentated version.
+  Multilibs.push_back(Multilib("hwasan", {}, {}, 6)
----------------



================
Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:222-229
+  // ASan has higher priority because we always want the instrumentated version.
+  Multilibs.push_back(Multilib("hwasan", {}, {}, 6)
+                          .flag("+fsanitize=hwaddress"));
+  // Use the asan+noexcept variant with ASan and -fno-exceptions.
+  Multilibs.push_back(Multilib("hwasan+noexcept", {}, {}, 7)
+                          .flag("+fsanitize=hwaddress")
+                          .flag("-fexceptions")
----------------
phosek wrote:
> 
Can you move this above relative-vtables so we keep all sanitizers together?


================
Comment at: clang/lib/Driver/ToolChains/Fuchsia.cpp:225
+                          .flag("+fsanitize=hwaddress"));
+  // Use the asan+noexcept variant with ASan and -fno-exceptions.
+  Multilibs.push_back(Multilib("hwasan+noexcept", {}, {}, 7)
----------------



================
Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:122-124
+  // we wish to ignore. This (currently) only occurs on AArch64, as x64
+  // implementations use SIGTRAP to implement the failure, and thus do not go
+  // through the stack saver.
----------------
I don't think this applies to Fuchsia.


================
Comment at: compiler-rt/lib/hwasan/hwasan_poisoning.cpp:22
 
 uptr TagMemoryAligned(uptr p, uptr size, tag_t tag) {
   CHECK(IsAligned(p, kShadowAlignment));
----------------
It might be better to move this function to a per-platform file rather than conditionally compiling the entire body.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91466