Message ID | 20200722085300.5563-1-kito.cheng@sifive.com |
---|---|
State | New |
Headers | show |
Series | PR target/96260 - KASAN should work even back-end not porting anything. | expand |
On Wed, 22 Jul 2020, Kito Cheng wrote: > - Most KASAN function don't need any porting anything in back-end > except asan stack protection. > > - However kernel will given shadow offset when enable asan stack > protection, so eveything in KASAN can work if shadow offset is given. > > - Verified with x86 and risc-v. > > - Verified with RISC-V linux kernel. > > OK for trunk and GCC 10 branch? If it is approved please wait until after the GCC 10.2 release tomorrow. Richard. > gcc/ChangeLog: > > PR target/96260 > * asan.c (asan_shadow_offset_set_p): New. > * asan.h (asan_shadow_offset_set_p): Ditto. > * toplev.c (process_options): Allow -fsanitize=kernel-address > even TARGET_ASAN_SHADOW_OFFSET not implemented, only check when > asan stack protection is enabled. > > gcc/testsuite/ChangeLog: > > PR target/96260 > * gcc.target/riscv/pr91441.c: Update warning message. > * gcc.target/riscv/pr96260.c: New. > --- > gcc/asan.c | 6 ++++++ > gcc/asan.h | 2 ++ > gcc/testsuite/gcc.target/riscv/pr91441.c | 2 +- > gcc/testsuite/gcc.target/riscv/pr96260.c | 9 +++++++++ > gcc/toplev.c | 21 ++++++++++++++++++++- > 5 files changed, 38 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/riscv/pr96260.c > > diff --git a/gcc/asan.c b/gcc/asan.c > index 9c9aa4cae358..2e759540246f 100644 > --- a/gcc/asan.c > +++ b/gcc/asan.c > @@ -344,6 +344,12 @@ asan_shadow_offset () > return asan_shadow_offset_value; > } > > +/* Returns Asan shadow offset has been set. */ > +bool asan_shadow_offset_set_p () > +{ > + return asan_shadow_offset_computed; > +} > + > alias_set_type asan_shadow_set = -1; > > /* Pointer types to 1, 2 or 4 byte integers in shadow memory. A separate > diff --git a/gcc/asan.h b/gcc/asan.h > index 9efd33f9b86b..114b457ef91c 100644 > --- a/gcc/asan.h > +++ b/gcc/asan.h > @@ -129,6 +129,8 @@ asan_var_and_redzone_size (unsigned HOST_WIDE_INT size) > > extern bool set_asan_shadow_offset (const char *); > > +extern bool asan_shadow_offset_set_p (); > + > extern void set_sanitized_sections (const char *); > > extern bool asan_sanitize_stack_p (void); > diff --git a/gcc/testsuite/gcc.target/riscv/pr91441.c b/gcc/testsuite/gcc.target/riscv/pr91441.c > index 593a2972a0f0..2403c98bb703 100644 > --- a/gcc/testsuite/gcc.target/riscv/pr91441.c > +++ b/gcc/testsuite/gcc.target/riscv/pr91441.c > @@ -7,4 +7,4 @@ int *f( int a) > { > return bar(&a); > } > -/* { dg-warning ".'-fsanitize=address' and '-fsanitize=kernel-address' are not supported for this target" "" { target *-*-* } 0 } */ > +/* { dg-warning ".'-fsanitize=kernel-address' with stack protection is not supported without '-fasan-shadow-offset=' for this target." "" { target *-*-* } 0 } */ > diff --git a/gcc/testsuite/gcc.target/riscv/pr96260.c b/gcc/testsuite/gcc.target/riscv/pr96260.c > new file mode 100644 > index 000000000000..229997f877b7 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/riscv/pr96260.c > @@ -0,0 +1,9 @@ > +/* PR target/96260 */ > +/* { dg-do compile } */ > +/* { dg-options "--param asan-stack=1 -fsanitize=kernel-address -fasan-shadow-offset=0x100000" } */ > + > +int *bar(int *); > +int *f( int a) > +{ > + return bar(&a); > +} > diff --git a/gcc/toplev.c b/gcc/toplev.c > index 95eea63380f6..48f13d282c52 100644 > --- a/gcc/toplev.c > +++ b/gcc/toplev.c > @@ -1835,7 +1835,7 @@ process_options (void) > /* Address Sanitizer needs porting to each target architecture. */ > > if ((flag_sanitize & SANITIZE_ADDRESS) > - && (!FRAME_GROWS_DOWNWARD || targetm.asan_shadow_offset == NULL)) > + && !FRAME_GROWS_DOWNWARD) > { > warning_at (UNKNOWN_LOCATION, 0, > "%<-fsanitize=address%> and %<-fsanitize=kernel-address%> " > @@ -1843,6 +1843,25 @@ process_options (void) > flag_sanitize &= ~SANITIZE_ADDRESS; > } > > + if ((flag_sanitize & SANITIZE_USER_ADDRESS) > + && targetm.asan_shadow_offset == NULL) > + { > + warning_at (UNKNOWN_LOCATION, 0, > + "%<-fsanitize=address%> not supported for this target"); > + flag_sanitize &= ~SANITIZE_ADDRESS; > + } > + > + if ((flag_sanitize & SANITIZE_KERNEL_ADDRESS) > + && (targetm.asan_shadow_offset == NULL && param_asan_stack > + && !asan_shadow_offset_set_p ())) > + { > + warning_at (UNKNOWN_LOCATION, 0, > + "%<-fsanitize=kernel-address%> with stack protection " > + "is not supported without %<-fasan-shadow-offset=%> " > + "for this target."); > + flag_sanitize &= ~SANITIZE_ADDRESS; > + } > + > /* Do not use IPA optimizations for register allocation if profiler is active > or patchable function entries are inserted for run-time instrumentation > or port does not emit prologue and epilogue as RTL. */ >
On Wed, Jul 22, 2020 at 04:53:00PM +0800, Kito Cheng wrote: > --- a/gcc/asan.c > +++ b/gcc/asan.c > @@ -344,6 +344,12 @@ asan_shadow_offset () > return asan_shadow_offset_value; > } > > +/* Returns Asan shadow offset has been set. */ > +bool asan_shadow_offset_set_p () Formatting. Should be bool asan_shadow_offset_set_p () > +{ > + return asan_shadow_offset_computed; > +} > + > alias_set_type asan_shadow_set = -1; > > /* Pointer types to 1, 2 or 4 byte integers in shadow memory. A separate > -/* { dg-warning ".'-fsanitize=address' and '-fsanitize=kernel-address' are not supported for this target" "" { target *-*-* } 0 } */ > +/* { dg-warning ".'-fsanitize=kernel-address' with stack protection is not supported without '-fasan-shadow-offset=' for this target." "" { target *-*-* } 0 } */ Please adjust, see below. > index 95eea63380f6..48f13d282c52 100644 > --- a/gcc/toplev.c > +++ b/gcc/toplev.c > @@ -1835,7 +1835,7 @@ process_options (void) > /* Address Sanitizer needs porting to each target architecture. */ > > if ((flag_sanitize & SANITIZE_ADDRESS) > - && (!FRAME_GROWS_DOWNWARD || targetm.asan_shadow_offset == NULL)) > + && !FRAME_GROWS_DOWNWARD) > { > warning_at (UNKNOWN_LOCATION, 0, > "%<-fsanitize=address%> and %<-fsanitize=kernel-address%> " > @@ -1843,6 +1843,25 @@ process_options (void) > flag_sanitize &= ~SANITIZE_ADDRESS; > } > > + if ((flag_sanitize & SANITIZE_USER_ADDRESS) > + && targetm.asan_shadow_offset == NULL) > + { > + warning_at (UNKNOWN_LOCATION, 0, > + "%<-fsanitize=address%> not supported for this target"); > + flag_sanitize &= ~SANITIZE_ADDRESS; > + } > + > + if ((flag_sanitize & SANITIZE_KERNEL_ADDRESS) > + && (targetm.asan_shadow_offset == NULL && param_asan_stack > + && !asan_shadow_offset_set_p ())) Formatting. If there are several &&s (or ||s) and it doesn't fit on one line, each of them should be on a separate line, so there should be a newline and indentation instead of space before "&& param_asan_stack". > + { > + warning_at (UNKNOWN_LOCATION, 0, > + "%<-fsanitize=kernel-address%> with stack protection " > + "is not supported without %<-fasan-shadow-offset=%> " > + "for this target."); No full stop at the end of diagnostics (plus adjust testcase for it). Otherwise LGTM for trunk and 10.3 (see Richi's mail). Jakub
Hi Jakub: Thanks for your review, committed with formatting fixes. Hi Richard: Thanks, I'll commit that after gcc 10.2 release :) On Wed, Jul 22, 2020 at 6:14 PM Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > On Wed, Jul 22, 2020 at 04:53:00PM +0800, Kito Cheng wrote: > > --- a/gcc/asan.c > > +++ b/gcc/asan.c > > @@ -344,6 +344,12 @@ asan_shadow_offset () > > return asan_shadow_offset_value; > > } > > > > +/* Returns Asan shadow offset has been set. */ > > +bool asan_shadow_offset_set_p () > > Formatting. Should be > bool > asan_shadow_offset_set_p () > > > +{ > > + return asan_shadow_offset_computed; > > +} > > + > > alias_set_type asan_shadow_set = -1; > > > > /* Pointer types to 1, 2 or 4 byte integers in shadow memory. A separate > > > -/* { dg-warning ".'-fsanitize=address' and '-fsanitize=kernel-address' are not supported for this target" "" { target *-*-* } 0 } */ > > +/* { dg-warning ".'-fsanitize=kernel-address' with stack protection is not supported without '-fasan-shadow-offset=' for this target." "" { target *-*-* } 0 } */ > > Please adjust, see below. > > index 95eea63380f6..48f13d282c52 100644 > > --- a/gcc/toplev.c > > +++ b/gcc/toplev.c > > @@ -1835,7 +1835,7 @@ process_options (void) > > /* Address Sanitizer needs porting to each target architecture. */ > > > > if ((flag_sanitize & SANITIZE_ADDRESS) > > - && (!FRAME_GROWS_DOWNWARD || targetm.asan_shadow_offset == NULL)) > > + && !FRAME_GROWS_DOWNWARD) > > { > > warning_at (UNKNOWN_LOCATION, 0, > > "%<-fsanitize=address%> and %<-fsanitize=kernel-address%> " > > @@ -1843,6 +1843,25 @@ process_options (void) > > flag_sanitize &= ~SANITIZE_ADDRESS; > > } > > > > + if ((flag_sanitize & SANITIZE_USER_ADDRESS) > > + && targetm.asan_shadow_offset == NULL) > > + { > > + warning_at (UNKNOWN_LOCATION, 0, > > + "%<-fsanitize=address%> not supported for this target"); > > + flag_sanitize &= ~SANITIZE_ADDRESS; > > + } > > + > > + if ((flag_sanitize & SANITIZE_KERNEL_ADDRESS) > > + && (targetm.asan_shadow_offset == NULL && param_asan_stack > > + && !asan_shadow_offset_set_p ())) > > Formatting. If there are several &&s (or ||s) and it doesn't fit on one > line, each of them should be on a separate line, so there should be a > newline and indentation instead of space before "&& param_asan_stack". > > + { > > + warning_at (UNKNOWN_LOCATION, 0, > > + "%<-fsanitize=kernel-address%> with stack protection " > > + "is not supported without %<-fasan-shadow-offset=%> " > > + "for this target."); > > No full stop at the end of diagnostics (plus adjust testcase for it). > > Otherwise LGTM for trunk and 10.3 (see Richi's mail). > > Jakub >
GCC 10.2 released, committed to GCC 10 branch :) On Thu, Jul 23, 2020 at 3:21 PM Kito Cheng <kito.cheng@gmail.com> wrote: > > Hi Jakub: > > Thanks for your review, committed with formatting fixes. > > Hi Richard: > > Thanks, I'll commit that after gcc 10.2 release :) > > On Wed, Jul 22, 2020 at 6:14 PM Jakub Jelinek via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > On Wed, Jul 22, 2020 at 04:53:00PM +0800, Kito Cheng wrote: > > > --- a/gcc/asan.c > > > +++ b/gcc/asan.c > > > @@ -344,6 +344,12 @@ asan_shadow_offset () > > > return asan_shadow_offset_value; > > > } > > > > > > +/* Returns Asan shadow offset has been set. */ > > > +bool asan_shadow_offset_set_p () > > > > Formatting. Should be > > bool > > asan_shadow_offset_set_p () > > > > > +{ > > > + return asan_shadow_offset_computed; > > > +} > > > + > > > alias_set_type asan_shadow_set = -1; > > > > > > /* Pointer types to 1, 2 or 4 byte integers in shadow memory. A separate > > > > > -/* { dg-warning ".'-fsanitize=address' and '-fsanitize=kernel-address' are not supported for this target" "" { target *-*-* } 0 } */ > > > +/* { dg-warning ".'-fsanitize=kernel-address' with stack protection is not supported without '-fasan-shadow-offset=' for this target." "" { target *-*-* } 0 } */ > > > > Please adjust, see below. > > > index 95eea63380f6..48f13d282c52 100644 > > > --- a/gcc/toplev.c > > > +++ b/gcc/toplev.c > > > @@ -1835,7 +1835,7 @@ process_options (void) > > > /* Address Sanitizer needs porting to each target architecture. */ > > > > > > if ((flag_sanitize & SANITIZE_ADDRESS) > > > - && (!FRAME_GROWS_DOWNWARD || targetm.asan_shadow_offset == NULL)) > > > + && !FRAME_GROWS_DOWNWARD) > > > { > > > warning_at (UNKNOWN_LOCATION, 0, > > > "%<-fsanitize=address%> and %<-fsanitize=kernel-address%> " > > > @@ -1843,6 +1843,25 @@ process_options (void) > > > flag_sanitize &= ~SANITIZE_ADDRESS; > > > } > > > > > > + if ((flag_sanitize & SANITIZE_USER_ADDRESS) > > > + && targetm.asan_shadow_offset == NULL) > > > + { > > > + warning_at (UNKNOWN_LOCATION, 0, > > > + "%<-fsanitize=address%> not supported for this target"); > > > + flag_sanitize &= ~SANITIZE_ADDRESS; > > > + } > > > + > > > + if ((flag_sanitize & SANITIZE_KERNEL_ADDRESS) > > > + && (targetm.asan_shadow_offset == NULL && param_asan_stack > > > + && !asan_shadow_offset_set_p ())) > > > > Formatting. If there are several &&s (or ||s) and it doesn't fit on one > > line, each of them should be on a separate line, so there should be a > > newline and indentation instead of space before "&& param_asan_stack". > > > + { > > > + warning_at (UNKNOWN_LOCATION, 0, > > > + "%<-fsanitize=kernel-address%> with stack protection " > > > + "is not supported without %<-fasan-shadow-offset=%> " > > > + "for this target."); > > > > No full stop at the end of diagnostics (plus adjust testcase for it). > > > > Otherwise LGTM for trunk and 10.3 (see Richi's mail). > > > > Jakub > >
diff --git a/gcc/asan.c b/gcc/asan.c index 9c9aa4cae358..2e759540246f 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -344,6 +344,12 @@ asan_shadow_offset () return asan_shadow_offset_value; } +/* Returns Asan shadow offset has been set. */ +bool asan_shadow_offset_set_p () +{ + return asan_shadow_offset_computed; +} + alias_set_type asan_shadow_set = -1; /* Pointer types to 1, 2 or 4 byte integers in shadow memory. A separate diff --git a/gcc/asan.h b/gcc/asan.h index 9efd33f9b86b..114b457ef91c 100644 --- a/gcc/asan.h +++ b/gcc/asan.h @@ -129,6 +129,8 @@ asan_var_and_redzone_size (unsigned HOST_WIDE_INT size) extern bool set_asan_shadow_offset (const char *); +extern bool asan_shadow_offset_set_p (); + extern void set_sanitized_sections (const char *); extern bool asan_sanitize_stack_p (void); diff --git a/gcc/testsuite/gcc.target/riscv/pr91441.c b/gcc/testsuite/gcc.target/riscv/pr91441.c index 593a2972a0f0..2403c98bb703 100644 --- a/gcc/testsuite/gcc.target/riscv/pr91441.c +++ b/gcc/testsuite/gcc.target/riscv/pr91441.c @@ -7,4 +7,4 @@ int *f( int a) { return bar(&a); } -/* { dg-warning ".'-fsanitize=address' and '-fsanitize=kernel-address' are not supported for this target" "" { target *-*-* } 0 } */ +/* { dg-warning ".'-fsanitize=kernel-address' with stack protection is not supported without '-fasan-shadow-offset=' for this target." "" { target *-*-* } 0 } */ diff --git a/gcc/testsuite/gcc.target/riscv/pr96260.c b/gcc/testsuite/gcc.target/riscv/pr96260.c new file mode 100644 index 000000000000..229997f877b7 --- /dev/null +++ b/gcc/testsuite/gcc.target/riscv/pr96260.c @@ -0,0 +1,9 @@ +/* PR target/96260 */ +/* { dg-do compile } */ +/* { dg-options "--param asan-stack=1 -fsanitize=kernel-address -fasan-shadow-offset=0x100000" } */ + +int *bar(int *); +int *f( int a) +{ + return bar(&a); +} diff --git a/gcc/toplev.c b/gcc/toplev.c index 95eea63380f6..48f13d282c52 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -1835,7 +1835,7 @@ process_options (void) /* Address Sanitizer needs porting to each target architecture. */ if ((flag_sanitize & SANITIZE_ADDRESS) - && (!FRAME_GROWS_DOWNWARD || targetm.asan_shadow_offset == NULL)) + && !FRAME_GROWS_DOWNWARD) { warning_at (UNKNOWN_LOCATION, 0, "%<-fsanitize=address%> and %<-fsanitize=kernel-address%> " @@ -1843,6 +1843,25 @@ process_options (void) flag_sanitize &= ~SANITIZE_ADDRESS; } + if ((flag_sanitize & SANITIZE_USER_ADDRESS) + && targetm.asan_shadow_offset == NULL) + { + warning_at (UNKNOWN_LOCATION, 0, + "%<-fsanitize=address%> not supported for this target"); + flag_sanitize &= ~SANITIZE_ADDRESS; + } + + if ((flag_sanitize & SANITIZE_KERNEL_ADDRESS) + && (targetm.asan_shadow_offset == NULL && param_asan_stack + && !asan_shadow_offset_set_p ())) + { + warning_at (UNKNOWN_LOCATION, 0, + "%<-fsanitize=kernel-address%> with stack protection " + "is not supported without %<-fasan-shadow-offset=%> " + "for this target."); + flag_sanitize &= ~SANITIZE_ADDRESS; + } + /* Do not use IPA optimizations for register allocation if profiler is active or patchable function entries are inserted for run-time instrumentation or port does not emit prologue and epilogue as RTL. */