diff mbox series

PR target/96260 - KASAN should work even back-end not porting anything.

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

Commit Message

Kito Cheng July 22, 2020, 8:53 a.m. UTC 
- 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?

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

Comments

Richard Biener July 22, 2020, 10 a.m. UTC | #1 
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.  */
>
Jakub Jelinek July 22, 2020, 10:13 a.m. UTC | #2 
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
Kito Cheng July 23, 2020, 7:21 a.m. UTC | #3 
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
>
Kito Cheng July 23, 2020, 7:49 a.m. UTC | #4 
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 mbox series

Patch

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.  */