diff mbox series

[libsanitizer] : Sync fixes for asan interceptors from upstream [PR112644]

Message ID patch-18219-tamar@arm.com
State Unresolved
Headers
Series [libsanitizer] : Sync fixes for asan interceptors from upstream [PR112644] |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Tamar Christina Jan. 29, 2024, 3:03 p.m. UTC 
  Hi All,

This cherry-picks and squashes the differences between commits

d3e5c20ab846303874a2a25e5877c72271fc798b..76e1e45922e6709392fb82aac44bebe3dbc2ea63
from LLVM upstream from compiler-rt/lib/hwasan/ to GCC on the changes relevant
for GCC.

This is required to fix the linked PR.

As mentioned in the PR the last sync brought in a bug from upstream[1] where
operations became non-recoverable and as such the tests in AArch64 started
failing.  This cherry picks the fix and there are minor updates needed to GCC
after this to fix the cases.

[1] https://github.com/llvm/llvm-project/pull/74000

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

Ok for master?

Thanks,
Tamar

libsanitizer/ChangeLog:

	PR sanitizer/112644
	* hwasan/hwasan_interceptors.cpp (ACCESS_MEMORY_RANGE,
	HWASAN_READ_RANGE, HWASAN_WRITE_RANGE, COMMON_SYSCALL_PRE_READ_RANGE,
	COMMON_SYSCALL_PRE_WRITE_RANGE, COMMON_INTERCEPTOR_WRITE_RANGE,
	COMMON_INTERCEPTOR_READ_RANGE): Make recoverable.

--- inline copy of patch -- 
diff --git a/libsanitizer/hwasan/hwasan_interceptors.cpp b/libsanitizer/hwasan/hwasan_interceptors.cpp
index d9237cf9b8e3bf982cf213123ef22e73ec027c9e..96df4dd0c24d7d3db28fa2557cf63da0f295e33f 100644




--
diff --git a/libsanitizer/hwasan/hwasan_interceptors.cpp b/libsanitizer/hwasan/hwasan_interceptors.cpp
index d9237cf9b8e3bf982cf213123ef22e73ec027c9e..96df4dd0c24d7d3db28fa2557cf63da0f295e33f 100644
--- a/libsanitizer/hwasan/hwasan_interceptors.cpp
+++ b/libsanitizer/hwasan/hwasan_interceptors.cpp
@@ -36,16 +36,16 @@ struct HWAsanInterceptorContext {
   const char *interceptor_name;
 };
 
-#  define ACCESS_MEMORY_RANGE(ctx, offset, size, access)                    \
-    do {                                                                    \
-      __hwasan::CheckAddressSized<ErrorAction::Abort, access>((uptr)offset, \
-                                                              size);        \
+#  define ACCESS_MEMORY_RANGE(offset, size, access)                           \
+    do {                                                                      \
+      __hwasan::CheckAddressSized<ErrorAction::Recover, access>((uptr)offset, \
+                                                                size);        \
     } while (0)
 
-#  define HWASAN_READ_RANGE(ctx, offset, size) \
-    ACCESS_MEMORY_RANGE(ctx, offset, size, AccessType::Load)
-#  define HWASAN_WRITE_RANGE(ctx, offset, size) \
-    ACCESS_MEMORY_RANGE(ctx, offset, size, AccessType::Store)
+#  define HWASAN_READ_RANGE(offset, size) \
+    ACCESS_MEMORY_RANGE(offset, size, AccessType::Load)
+#  define HWASAN_WRITE_RANGE(offset, size) \
+    ACCESS_MEMORY_RANGE(offset, size, AccessType::Store)
 
 #  if !SANITIZER_APPLE
 #    define HWASAN_INTERCEPT_FUNC(name)                                        \
@@ -74,9 +74,8 @@ struct HWAsanInterceptorContext {
 
 #  if HWASAN_WITH_INTERCEPTORS
 
-#    define COMMON_SYSCALL_PRE_READ_RANGE(p, s) __hwasan_loadN((uptr)p, (uptr)s)
-#    define COMMON_SYSCALL_PRE_WRITE_RANGE(p, s) \
-      __hwasan_storeN((uptr)p, (uptr)s)
+#    define COMMON_SYSCALL_PRE_READ_RANGE(p, s) HWASAN_READ_RANGE(p, s)
+#    define COMMON_SYSCALL_PRE_WRITE_RANGE(p, s) HWASAN_WRITE_RANGE(p, s)
 #    define COMMON_SYSCALL_POST_READ_RANGE(p, s) \
       do {                                       \
         (void)(p);                               \
@@ -91,10 +90,10 @@ struct HWAsanInterceptorContext {
 #    include "sanitizer_common/sanitizer_syscalls_netbsd.inc"
 
 #    define COMMON_INTERCEPTOR_WRITE_RANGE(ctx, ptr, size) \
-      HWASAN_WRITE_RANGE(ctx, ptr, size)
+      HWASAN_WRITE_RANGE(ptr, size)
 
 #    define COMMON_INTERCEPTOR_READ_RANGE(ctx, ptr, size) \
-      HWASAN_READ_RANGE(ctx, ptr, size)
+      HWASAN_READ_RANGE(ptr, size)
 
 #    define COMMON_INTERCEPTOR_ENTER(ctx, func, ...) \
       HWAsanInterceptorContext _ctx = {#func};       \

Comments

Jakub Jelinek Jan. 29, 2024, 3:07 p.m. UTC | #1 
On Mon, Jan 29, 2024 at 03:03:46PM +0000, Tamar Christina wrote:
> Hi All,
> 
> This cherry-picks and squashes the differences between commits
> 
> d3e5c20ab846303874a2a25e5877c72271fc798b..76e1e45922e6709392fb82aac44bebe3dbc2ea63
> from LLVM upstream from compiler-rt/lib/hwasan/ to GCC on the changes relevant
> for GCC.
> 
> This is required to fix the linked PR.
> 
> As mentioned in the PR the last sync brought in a bug from upstream[1] where
> operations became non-recoverable and as such the tests in AArch64 started
> failing.  This cherry picks the fix and there are minor updates needed to GCC
> after this to fix the cases.
> 
> [1] https://github.com/llvm/llvm-project/pull/74000
> 
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> 
> Ok for master?
> 
> Thanks,
> Tamar
> 
> libsanitizer/ChangeLog:
> 
> 	PR sanitizer/112644
> 	* hwasan/hwasan_interceptors.cpp (ACCESS_MEMORY_RANGE,
> 	HWASAN_READ_RANGE, HWASAN_WRITE_RANGE, COMMON_SYSCALL_PRE_READ_RANGE,
> 	COMMON_SYSCALL_PRE_WRITE_RANGE, COMMON_INTERCEPTOR_WRITE_RANGE,
> 	COMMON_INTERCEPTOR_READ_RANGE): Make recoverable.

The normal ChangeLog entry for this would be
	Cherry-pick llvm-project revision XYZ and UVW.

Ok for trunk with that changed.

	Jakub
Andrew Pinski Jan. 29, 2024, 9:54 p.m. UTC | #2 
On Mon, Jan 29, 2024 at 7:04 AM Tamar Christina <tamar.christina@arm.com> wrote:
>
> Hi All,
>
> This cherry-picks and squashes the differences between commits
>
> d3e5c20ab846303874a2a25e5877c72271fc798b..76e1e45922e6709392fb82aac44bebe3dbc2ea63
> from LLVM upstream from compiler-rt/lib/hwasan/ to GCC on the changes relevant
> for GCC.
>
> This is required to fix the linked PR.
>
> As mentioned in the PR the last sync brought in a bug from upstream[1] where
> operations became non-recoverable and as such the tests in AArch64 started
> failing.  This cherry picks the fix and there are minor updates needed to GCC
> after this to fix the cases.
>
> [1] https://github.com/llvm/llvm-project/pull/74000
>
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
>
> Ok for master?

Thanks for handling this; though I wonder how this slipped through
testing upstream in LLVM. I see they added some new testcases for
this. I Know GCC's testsuite for sanitizer is slightly different from
LLVM's. Is it the case, GCC has more tests in this area? Is someone
adding the testcases that GCC has in this area upstream to LLVM;
basically so merging won't bring in regressions like this in the
future?

Thanks,
Andrew

>
> Thanks,
> Tamar
>
> libsanitizer/ChangeLog:
>
>         PR sanitizer/112644
>         * hwasan/hwasan_interceptors.cpp (ACCESS_MEMORY_RANGE,
>         HWASAN_READ_RANGE, HWASAN_WRITE_RANGE, COMMON_SYSCALL_PRE_READ_RANGE,
>         COMMON_SYSCALL_PRE_WRITE_RANGE, COMMON_INTERCEPTOR_WRITE_RANGE,
>         COMMON_INTERCEPTOR_READ_RANGE): Make recoverable.
>
> --- inline copy of patch --
> diff --git a/libsanitizer/hwasan/hwasan_interceptors.cpp b/libsanitizer/hwasan/hwasan_interceptors.cpp
> index d9237cf9b8e3bf982cf213123ef22e73ec027c9e..96df4dd0c24d7d3db28fa2557cf63da0f295e33f 100644
> --- a/libsanitizer/hwasan/hwasan_interceptors.cpp
> +++ b/libsanitizer/hwasan/hwasan_interceptors.cpp
> @@ -36,16 +36,16 @@ struct HWAsanInterceptorContext {
>    const char *interceptor_name;
>  };
>
> -#  define ACCESS_MEMORY_RANGE(ctx, offset, size, access)                    \
> -    do {                                                                    \
> -      __hwasan::CheckAddressSized<ErrorAction::Abort, access>((uptr)offset, \
> -                                                              size);        \
> +#  define ACCESS_MEMORY_RANGE(offset, size, access)                           \
> +    do {                                                                      \
> +      __hwasan::CheckAddressSized<ErrorAction::Recover, access>((uptr)offset, \
> +                                                                size);        \
>      } while (0)
>
> -#  define HWASAN_READ_RANGE(ctx, offset, size) \
> -    ACCESS_MEMORY_RANGE(ctx, offset, size, AccessType::Load)
> -#  define HWASAN_WRITE_RANGE(ctx, offset, size) \
> -    ACCESS_MEMORY_RANGE(ctx, offset, size, AccessType::Store)
> +#  define HWASAN_READ_RANGE(offset, size) \
> +    ACCESS_MEMORY_RANGE(offset, size, AccessType::Load)
> +#  define HWASAN_WRITE_RANGE(offset, size) \
> +    ACCESS_MEMORY_RANGE(offset, size, AccessType::Store)
>
>  #  if !SANITIZER_APPLE
>  #    define HWASAN_INTERCEPT_FUNC(name)                                        \
> @@ -74,9 +74,8 @@ struct HWAsanInterceptorContext {
>
>  #  if HWASAN_WITH_INTERCEPTORS
>
> -#    define COMMON_SYSCALL_PRE_READ_RANGE(p, s) __hwasan_loadN((uptr)p, (uptr)s)
> -#    define COMMON_SYSCALL_PRE_WRITE_RANGE(p, s) \
> -      __hwasan_storeN((uptr)p, (uptr)s)
> +#    define COMMON_SYSCALL_PRE_READ_RANGE(p, s) HWASAN_READ_RANGE(p, s)
> +#    define COMMON_SYSCALL_PRE_WRITE_RANGE(p, s) HWASAN_WRITE_RANGE(p, s)
>  #    define COMMON_SYSCALL_POST_READ_RANGE(p, s) \
>        do {                                       \
>          (void)(p);                               \
> @@ -91,10 +90,10 @@ struct HWAsanInterceptorContext {
>  #    include "sanitizer_common/sanitizer_syscalls_netbsd.inc"
>
>  #    define COMMON_INTERCEPTOR_WRITE_RANGE(ctx, ptr, size) \
> -      HWASAN_WRITE_RANGE(ctx, ptr, size)
> +      HWASAN_WRITE_RANGE(ptr, size)
>
>  #    define COMMON_INTERCEPTOR_READ_RANGE(ctx, ptr, size) \
> -      HWASAN_READ_RANGE(ctx, ptr, size)
> +      HWASAN_READ_RANGE(ptr, size)
>
>  #    define COMMON_INTERCEPTOR_ENTER(ctx, func, ...) \
>        HWAsanInterceptorContext _ctx = {#func};       \
>
>
>
>
> --
Tamar Christina Jan. 31, 2024, 2:15 p.m. UTC | #3 
> -----Original Message-----
> From: Andrew Pinski <pinskia@gmail.com>
> Sent: Monday, January 29, 2024 9:55 PM
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; jakub@redhat.com;
> dodji@redhat.com; kcc@google.com; dvyukov@google.com
> Subject: Re: [PATCH][libsanitizer]: Sync fixes for asan interceptors from upstream
> [PR112644]
> 
> On Mon, Jan 29, 2024 at 7:04 AM Tamar Christina <tamar.christina@arm.com>
> wrote:
> >
> > Hi All,
> >
> > This cherry-picks and squashes the differences between commits
> >
> >
> d3e5c20ab846303874a2a25e5877c72271fc798b..76e1e45922e6709392fb82aa
> c44bebe3dbc2ea63
> > from LLVM upstream from compiler-rt/lib/hwasan/ to GCC on the changes
> relevant
> > for GCC.
> >
> > This is required to fix the linked PR.
> >
> > As mentioned in the PR the last sync brought in a bug from upstream[1] where
> > operations became non-recoverable and as such the tests in AArch64 started
> > failing.  This cherry picks the fix and there are minor updates needed to GCC
> > after this to fix the cases.
> >
> > [1] https://github.com/llvm/llvm-project/pull/74000
> >
> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> >
> > Ok for master?
> 
> Thanks for handling this; though I wonder how this slipped through
> testing upstream in LLVM. I see they added some new testcases for
> this. I Know GCC's testsuite for sanitizer is slightly different from
> LLVM's. Is it the case, GCC has more tests in this area? Is someone
> adding the testcases that GCC has in this area upstream to LLVM;
> basically so merging won't bring in regressions like this in the
> future?

There were two parts here.  The first one is that their testsuite didn't have any
test for the recovery case.  Which they've now added.

But the second parts (which I'm not posting patches for) is that the change
In hwasan means that the runtime can now instrument some additional
library methods which it couldn't before.  And GCC now needs to not inline
these anymore.

This does mean that on future updates one needs to take a look at the
Instrumentation list and make sure to keep it in sync with GCC's otherwise
we'll lose instrumentation.

Regards,
Tamar
> 
> Thanks,
> Andrew
> 
> >
> > Thanks,
> > Tamar
> >
> > libsanitizer/ChangeLog:
> >
> >         PR sanitizer/112644
> >         * hwasan/hwasan_interceptors.cpp (ACCESS_MEMORY_RANGE,
> >         HWASAN_READ_RANGE, HWASAN_WRITE_RANGE,
> COMMON_SYSCALL_PRE_READ_RANGE,
> >         COMMON_SYSCALL_PRE_WRITE_RANGE,
> COMMON_INTERCEPTOR_WRITE_RANGE,
> >         COMMON_INTERCEPTOR_READ_RANGE): Make recoverable.
> >
> > --- inline copy of patch --
> > diff --git a/libsanitizer/hwasan/hwasan_interceptors.cpp
> b/libsanitizer/hwasan/hwasan_interceptors.cpp
> > index
> d9237cf9b8e3bf982cf213123ef22e73ec027c9e..96df4dd0c24d7d3db28fa2557
> cf63da0f295e33f 100644
> > --- a/libsanitizer/hwasan/hwasan_interceptors.cpp
> > +++ b/libsanitizer/hwasan/hwasan_interceptors.cpp
> > @@ -36,16 +36,16 @@ struct HWAsanInterceptorContext {
> >    const char *interceptor_name;
> >  };
> >
> > -#  define ACCESS_MEMORY_RANGE(ctx, offset, size, access)                    \
> > -    do {                                                                    \
> > -      __hwasan::CheckAddressSized<ErrorAction::Abort, access>((uptr)offset, \
> > -                                                              size);        \
> > +#  define ACCESS_MEMORY_RANGE(offset, size, access)                           \
> > +    do {                                                                      \
> > +      __hwasan::CheckAddressSized<ErrorAction::Recover, access>((uptr)offset, \
> > +                                                                size);        \
> >      } while (0)
> >
> > -#  define HWASAN_READ_RANGE(ctx, offset, size) \
> > -    ACCESS_MEMORY_RANGE(ctx, offset, size, AccessType::Load)
> > -#  define HWASAN_WRITE_RANGE(ctx, offset, size) \
> > -    ACCESS_MEMORY_RANGE(ctx, offset, size, AccessType::Store)
> > +#  define HWASAN_READ_RANGE(offset, size) \
> > +    ACCESS_MEMORY_RANGE(offset, size, AccessType::Load)
> > +#  define HWASAN_WRITE_RANGE(offset, size) \
> > +    ACCESS_MEMORY_RANGE(offset, size, AccessType::Store)
> >
> >  #  if !SANITIZER_APPLE
> >  #    define HWASAN_INTERCEPT_FUNC(name)                                        \
> > @@ -74,9 +74,8 @@ struct HWAsanInterceptorContext {
> >
> >  #  if HWASAN_WITH_INTERCEPTORS
> >
> > -#    define COMMON_SYSCALL_PRE_READ_RANGE(p, s)
> __hwasan_loadN((uptr)p, (uptr)s)
> > -#    define COMMON_SYSCALL_PRE_WRITE_RANGE(p, s) \
> > -      __hwasan_storeN((uptr)p, (uptr)s)
> > +#    define COMMON_SYSCALL_PRE_READ_RANGE(p, s)
> HWASAN_READ_RANGE(p, s)
> > +#    define COMMON_SYSCALL_PRE_WRITE_RANGE(p, s)
> HWASAN_WRITE_RANGE(p, s)
> >  #    define COMMON_SYSCALL_POST_READ_RANGE(p, s) \
> >        do {                                       \
> >          (void)(p);                               \
> > @@ -91,10 +90,10 @@ struct HWAsanInterceptorContext {
> >  #    include "sanitizer_common/sanitizer_syscalls_netbsd.inc"
> >
> >  #    define COMMON_INTERCEPTOR_WRITE_RANGE(ctx, ptr, size) \
> > -      HWASAN_WRITE_RANGE(ctx, ptr, size)
> > +      HWASAN_WRITE_RANGE(ptr, size)
> >
> >  #    define COMMON_INTERCEPTOR_READ_RANGE(ctx, ptr, size) \
> > -      HWASAN_READ_RANGE(ctx, ptr, size)
> > +      HWASAN_READ_RANGE(ptr, size)
> >
> >  #    define COMMON_INTERCEPTOR_ENTER(ctx, func, ...) \
> >        HWAsanInterceptorContext _ctx = {#func};       \
> >
> >
> >
> >
> > --
diff mbox series

Patch

--- a/libsanitizer/hwasan/hwasan_interceptors.cpp
+++ b/libsanitizer/hwasan/hwasan_interceptors.cpp
@@ -36,16 +36,16 @@  struct HWAsanInterceptorContext {
   const char *interceptor_name;
 };
 
-#  define ACCESS_MEMORY_RANGE(ctx, offset, size, access)                    \
-    do {                                                                    \
-      __hwasan::CheckAddressSized<ErrorAction::Abort, access>((uptr)offset, \
-                                                              size);        \
+#  define ACCESS_MEMORY_RANGE(offset, size, access)                           \
+    do {                                                                      \
+      __hwasan::CheckAddressSized<ErrorAction::Recover, access>((uptr)offset, \
+                                                                size);        \
     } while (0)
 
-#  define HWASAN_READ_RANGE(ctx, offset, size) \
-    ACCESS_MEMORY_RANGE(ctx, offset, size, AccessType::Load)
-#  define HWASAN_WRITE_RANGE(ctx, offset, size) \
-    ACCESS_MEMORY_RANGE(ctx, offset, size, AccessType::Store)
+#  define HWASAN_READ_RANGE(offset, size) \
+    ACCESS_MEMORY_RANGE(offset, size, AccessType::Load)
+#  define HWASAN_WRITE_RANGE(offset, size) \
+    ACCESS_MEMORY_RANGE(offset, size, AccessType::Store)
 
 #  if !SANITIZER_APPLE
 #    define HWASAN_INTERCEPT_FUNC(name)                                        \
@@ -74,9 +74,8 @@  struct HWAsanInterceptorContext {
 
 #  if HWASAN_WITH_INTERCEPTORS
 
-#    define COMMON_SYSCALL_PRE_READ_RANGE(p, s) __hwasan_loadN((uptr)p, (uptr)s)
-#    define COMMON_SYSCALL_PRE_WRITE_RANGE(p, s) \
-      __hwasan_storeN((uptr)p, (uptr)s)
+#    define COMMON_SYSCALL_PRE_READ_RANGE(p, s) HWASAN_READ_RANGE(p, s)
+#    define COMMON_SYSCALL_PRE_WRITE_RANGE(p, s) HWASAN_WRITE_RANGE(p, s)
 #    define COMMON_SYSCALL_POST_READ_RANGE(p, s) \
       do {                                       \
         (void)(p);                               \
@@ -91,10 +90,10 @@  struct HWAsanInterceptorContext {
 #    include "sanitizer_common/sanitizer_syscalls_netbsd.inc"
 
 #    define COMMON_INTERCEPTOR_WRITE_RANGE(ctx, ptr, size) \
-      HWASAN_WRITE_RANGE(ctx, ptr, size)
+      HWASAN_WRITE_RANGE(ptr, size)
 
 #    define COMMON_INTERCEPTOR_READ_RANGE(ctx, ptr, size) \
-      HWASAN_READ_RANGE(ctx, ptr, size)
+      HWASAN_READ_RANGE(ptr, size)
 
 #    define COMMON_INTERCEPTOR_ENTER(ctx, func, ...) \
       HWAsanInterceptorContext _ctx = {#func};       \