> -----Original Message-----
> From: Andrew Pinski <pins...@gmail.com>
> Sent: Monday, January 29, 2024 9:55 PM
> To: Tamar Christina <tamar.christ...@arm.com>
> Cc: gcc-patches@gcc.gnu.org; nd <n...@arm.com>; ja...@redhat.com;
> do...@redhat.com; k...@google.com; dvyu...@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.christ...@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};       \
> >
> >
> >
> >
> > --

Reply via email to