[PATCH] D127544: Add no_sanitize('hwaddress') (and 'memtag', but that's a no-op).

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for this, can you also add a release note for the fix?



================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:8849
     break;
-  
   // HLSL attributes:
----------------
Spurious whitespace change?


================
Comment at: clang/test/CodeGen/hwasan-globals.cpp:1-2
+// RUN: echo "int extra_global;" > %t.extra-source.cpp
+// RUN: echo "global:*ignorelisted_global*" > %t.ignorelist
+// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=hwaddress -fsanitize-ignorelist=%t.ignorelist -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK
----------------
Are these files automatically deleted when the test is done because we're using %t, or do we need to clean those up manually?


================
Comment at: clang/test/CodeGen/hwasan-globals.cpp:9
+int global;
+int __attribute__((no_sanitize("hwaddress"))) attributed_global;
+int __attribute__((disable_sanitizer_instrumentation)) disable_instrumentation_global;
----------------
Should we add a `memtag` test as well given that also changed in this patch?


================
Comment at: compiler-rt/test/hwasan/TestCases/global-with-reduction.c:25
 
-int x = 1;
+#include <stdlib.h>
 
----------------
I'm not a compiler-rt expert, but is this valid? I assume this is using the system stdlib.h which is not something we usually want in lit tests.

I think that's why `atoi` was previously being forward declared; then we don't need to include the whole header file.


================
Comment at: compiler-rt/test/hwasan/TestCases/global-with-reduction.c:50
+  f()[atoi(argv[1])] = 1;
+  return 0;
 }
----------------
This is unnecessary -- falling off `main` already returns 0.


================
Comment at: compiler-rt/test/hwasan/TestCases/global.c:47
   (&x)[atoi(argv[1])] = 1;
+  return 0;
 }
----------------
This is unnecessary -- falling off `main` already returns 0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127544