diff mbox series

[1/2,libsanitizer] hwasan: Remove testsuite check for a complaint message [PR112644]

Message ID patch-18223-tamar@arm.com
State New
Headers show
Series [1/2,libsanitizer] hwasan: Remove testsuite check for a complaint message [PR112644] | expand

Commit Message

Tamar Christina Jan. 31, 2024, 2:17 p.m. UTC
Hi All,

Recent libhwasan updates[1] intercept various string and memory functions.
These functions have checking in them, which means there's no need to
inline the checking.

This patch marks said functions as intercepted, and adjusts a testcase
to handle the difference.  It also looks for HWASAN in a check in
expand_builtin.  This check originally is there to avoid using expand to
inline the behaviour of builtins like memset which are intercepted by
ASAN and hence which we rely on the function call staying as a function
call.  With the new reliance on function calls in HWASAN we need to do
the same thing for HWASAN too.

HWASAN and ASAN don't seem to however instrument the same functions.

Looking into libsanitizer/sanitizer_common/sanitizer_common_interceptors_memintrinsics.inc
it looks like the common ones are memset, memmove and memcpy.

The rest of the routines for asan seem to be defined in
compiler-rt/lib/asan/asan_interceptors.h however compiler-rt/lib/hwasan/
does not have such a file but it does have
compiler-rt/lib/hwasan/hwasan_platform_interceptors.h which it looks like is
forcing off everything but memset, memmove, memcpy, memcmp and bcmp.

As such I've taken those as the final list that hwasan currently supports.
This also means that on future updates this list should be cross checked.

[1] https://discourse.llvm.org/t/hwasan-question-about-the-recent-interceptors-being-added/75351

Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.

Ok for master?

Thanks,
Tamar

gcc/ChangeLog:

	PR sanitizer/112644
	* asan.h (asan_intercepted_p): Incercept memset, memmove, memcpy and
	memcmp.
	* builtins.cc (expand_builtin): Include HWASAN when checking for
	builtin inlining.

gcc/testsuite/ChangeLog:

	PR sanitizer/112644
	* c-c++-common/hwasan/builtin-special-handling.c: Update testcase.

Co-Authored-By: Matthew Malcomson <matthew.malcomson@arm.com>

--- inline copy of patch -- 
diff --git a/gcc/asan.h b/gcc/asan.h
index 82811bdbe697665652aba89f2ee1c3ac07970df9..d1bf8b1e701b15525c6a900d324f2aebfb778cba 100644




--
diff --git a/gcc/asan.h b/gcc/asan.h
index 82811bdbe697665652aba89f2ee1c3ac07970df9..d1bf8b1e701b15525c6a900d324f2aebfb778cba 100644
--- a/gcc/asan.h
+++ b/gcc/asan.h
@@ -185,8 +185,13 @@ extern hash_set<tree> *asan_handled_variables;
 inline bool
 asan_intercepted_p (enum built_in_function fcode)
 {
+  /* This list should be kept up-to-date with upstream's version at
+     compiler-rt/lib/hwasan/hwasan_platform_interceptors.h.  */
   if (hwasan_sanitize_p ())
-    return false;
+    return fcode == BUILT_IN_MEMCMP
+	 || fcode == BUILT_IN_MEMCPY
+	 || fcode == BUILT_IN_MEMMOVE
+	 || fcode == BUILT_IN_MEMSET;
 
   return fcode == BUILT_IN_INDEX
 	 || fcode == BUILT_IN_MEMCHR
diff --git a/gcc/builtins.cc b/gcc/builtins.cc
index a0bd82c7981c05caf2764de70c62fe83bef9ad29..12cc7a54e99555d0f4b21fa2cc32ffa7bb548f18 100644
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -7792,7 +7792,8 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
       default:
 	break;
       }
-  if (sanitize_flags_p (SANITIZE_ADDRESS) && asan_intercepted_p (fcode))
+  if (sanitize_flags_p (SANITIZE_ADDRESS | SANITIZE_HWADDRESS)
+		  && asan_intercepted_p (fcode))
     return expand_call (exp, target, ignore);
 
   /* When not optimizing, generate calls to library functions for a certain
diff --git a/gcc/testsuite/c-c++-common/hwasan/builtin-special-handling.c b/gcc/testsuite/c-c++-common/hwasan/builtin-special-handling.c
index a7a6d91693ae48c20f33ab28f28d27b01af4722c..f975baaaa1cc397bc0d6fd475dbfed5ccc8ac386 100644
--- a/gcc/testsuite/c-c++-common/hwasan/builtin-special-handling.c
+++ b/gcc/testsuite/c-c++-common/hwasan/builtin-special-handling.c
@@ -8,24 +8,24 @@
 /* { dg-skip-if "" { *-*-* }  { "-flto" } { "-flto-partition=none" } } */
 
 typedef __SIZE_TYPE__ size_t;
-/* Functions to observe that HWASAN instruments memory builtins in the expected
-   manner.  */
+/* HWASAN used to instrument calls to memset, memcpy, and memmove.  It no
+   longer does this.  Many other string and memory builtins are intercepted by
+   the runtime (and hence the codegen need not do anything).  */
 void * __attribute__((noinline))
 memset_builtin (void *dest, int value, size_t len)
 {
   return __builtin_memset (dest, value, len);
 }
 
-/* HWASAN avoids strlen because it doesn't know the size of the memory access
-   until *after* the function call.  */
 size_t __attribute__ ((noinline))
 strlen_builtin (char *element)
 {
   return __builtin_strlen (element);
 }
 
-/* First test ensures that the HWASAN_CHECK was emitted before the
-   memset.  Second test ensures there was only HWASAN_CHECK (which demonstrates
-   that strlen was not instrumented).  */
-/* { dg-final { scan-tree-dump-times "HWASAN_CHECK.*memset" 1 "asan1" } } */
-/* { dg-final { scan-tree-dump-times "HWASAN_CHECK" 1 "asan1" } } */
+/* First check here ensures there is no inline instrumentation generated for
+   these builtins.  Second checks that we end up calling memset (i.e. that it's
+   not optimised into an inline operation, which would happen without the
+   instrumentation).  */
+/* { dg-final { scan-tree-dump-not "HWASAN_CHECK" "asan1" } } */
+/* { dg-final { scan-assembler-times "\tmemset\\M" 1 } } */

Comments

Jakub Jelinek Jan. 31, 2024, 2:27 p.m. UTC | #1
On Wed, Jan 31, 2024 at 02:17:00PM +0000, Tamar Christina wrote:
> gcc/ChangeLog:
> 
> 	PR sanitizer/112644
> 	* asan.h (asan_intercepted_p): Incercept memset, memmove, memcpy and
> 	memcmp.
> 	* builtins.cc (expand_builtin): Include HWASAN when checking for
> 	builtin inlining.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR sanitizer/112644
> 	* c-c++-common/hwasan/builtin-special-handling.c: Update testcase.
> 
> Co-Authored-By: Matthew Malcomson <matthew.malcomson@arm.com>

LGTM

	Jakub
diff mbox series

Patch

--- a/gcc/asan.h
+++ b/gcc/asan.h
@@ -185,8 +185,13 @@  extern hash_set<tree> *asan_handled_variables;
 inline bool
 asan_intercepted_p (enum built_in_function fcode)
 {
+  /* This list should be kept up-to-date with upstream's version at
+     compiler-rt/lib/hwasan/hwasan_platform_interceptors.h.  */
   if (hwasan_sanitize_p ())
-    return false;
+    return fcode == BUILT_IN_MEMCMP
+	 || fcode == BUILT_IN_MEMCPY
+	 || fcode == BUILT_IN_MEMMOVE
+	 || fcode == BUILT_IN_MEMSET;
 
   return fcode == BUILT_IN_INDEX
 	 || fcode == BUILT_IN_MEMCHR
diff --git a/gcc/builtins.cc b/gcc/builtins.cc
index a0bd82c7981c05caf2764de70c62fe83bef9ad29..12cc7a54e99555d0f4b21fa2cc32ffa7bb548f18 100644
--- a/gcc/builtins.cc
+++ b/gcc/builtins.cc
@@ -7792,7 +7792,8 @@  expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
       default:
 	break;
       }
-  if (sanitize_flags_p (SANITIZE_ADDRESS) && asan_intercepted_p (fcode))
+  if (sanitize_flags_p (SANITIZE_ADDRESS | SANITIZE_HWADDRESS)
+		  && asan_intercepted_p (fcode))
     return expand_call (exp, target, ignore);
 
   /* When not optimizing, generate calls to library functions for a certain
diff --git a/gcc/testsuite/c-c++-common/hwasan/builtin-special-handling.c b/gcc/testsuite/c-c++-common/hwasan/builtin-special-handling.c
index a7a6d91693ae48c20f33ab28f28d27b01af4722c..f975baaaa1cc397bc0d6fd475dbfed5ccc8ac386 100644
--- a/gcc/testsuite/c-c++-common/hwasan/builtin-special-handling.c
+++ b/gcc/testsuite/c-c++-common/hwasan/builtin-special-handling.c
@@ -8,24 +8,24 @@ 
 /* { dg-skip-if "" { *-*-* }  { "-flto" } { "-flto-partition=none" } } */
 
 typedef __SIZE_TYPE__ size_t;
-/* Functions to observe that HWASAN instruments memory builtins in the expected
-   manner.  */
+/* HWASAN used to instrument calls to memset, memcpy, and memmove.  It no
+   longer does this.  Many other string and memory builtins are intercepted by
+   the runtime (and hence the codegen need not do anything).  */
 void * __attribute__((noinline))
 memset_builtin (void *dest, int value, size_t len)
 {
   return __builtin_memset (dest, value, len);
 }
 
-/* HWASAN avoids strlen because it doesn't know the size of the memory access
-   until *after* the function call.  */
 size_t __attribute__ ((noinline))
 strlen_builtin (char *element)
 {
   return __builtin_strlen (element);
 }
 
-/* First test ensures that the HWASAN_CHECK was emitted before the
-   memset.  Second test ensures there was only HWASAN_CHECK (which demonstrates
-   that strlen was not instrumented).  */
-/* { dg-final { scan-tree-dump-times "HWASAN_CHECK.*memset" 1 "asan1" } } */
-/* { dg-final { scan-tree-dump-times "HWASAN_CHECK" 1 "asan1" } } */
+/* First check here ensures there is no inline instrumentation generated for
+   these builtins.  Second checks that we end up calling memset (i.e. that it's
+   not optimised into an inline operation, which would happen without the
+   instrumentation).  */
+/* { dg-final { scan-tree-dump-not "HWASAN_CHECK" "asan1" } } */
+/* { dg-final { scan-assembler-times "\tmemset\\M" 1 } } */