ASAN: do not rewrite param for DECL_NOT_GIMPLE_REG_P.

Message ID 64a9f3de-d345-a8df-9cad-becbbf60a4e1@suse.cz
State New
Headers show
Series
  • ASAN: do not rewrite param for DECL_NOT_GIMPLE_REG_P.
Related show

Commit Message

Martin Liška May 11, 2020, 11:26 a.m.
Hi.

Starting from r11-165-geb72dc663e9070b2 we should not rewrite parameters that
have DECL_NOT_GIMPLE_REG_P set to true.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2020-05-11  Martin Liska  <mliska@suse.cz>

	PR sanitizer/95033
	* sanopt.c (sanitize_rewrite_addressable_params):
	Do not rewrite for DECL_NOT_GIMPLE_REG_P.

gcc/testsuite/ChangeLog:

2020-05-11  Martin Liska  <mliska@suse.cz>

	PR sanitizer/95033
	* g++.dg/asan/function-argument-4.C: New test.
	* gcc.dg/asan/pr95033.c: New test.
---
  gcc/sanopt.c                                  |  1 +
  .../g++.dg/asan/function-argument-4.C         | 26 +++++++++++++++++++
  gcc/testsuite/gcc.dg/asan/pr95033.c           | 13 ++++++++++
  3 files changed, 40 insertions(+)
  create mode 100644 gcc/testsuite/g++.dg/asan/function-argument-4.C
  create mode 100644 gcc/testsuite/gcc.dg/asan/pr95033.c

Comments

Bill Schmidt via Gcc-patches May 11, 2020, 12:44 p.m. | #1
On Mon, May 11, 2020 at 1:26 PM Martin Liška <mliska@suse.cz> wrote:
>

> Hi.

>

> Starting from r11-165-geb72dc663e9070b2 we should not rewrite parameters that

> have DECL_NOT_GIMPLE_REG_P set to true.

>

> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

>

> Ready to be installed?


Hmm, I think the fix is to clear DECL_NOT_GIMPLE_REG_P instead
where the code clears TREE_ADDRESSABLE of 'arg'

> Thanks,

> Martin

>

> gcc/ChangeLog:

>

> 2020-05-11  Martin Liska  <mliska@suse.cz>

>

>         PR sanitizer/95033

>         * sanopt.c (sanitize_rewrite_addressable_params):

>         Do not rewrite for DECL_NOT_GIMPLE_REG_P.

>

> gcc/testsuite/ChangeLog:

>

> 2020-05-11  Martin Liska  <mliska@suse.cz>

>

>         PR sanitizer/95033

>         * g++.dg/asan/function-argument-4.C: New test.

>         * gcc.dg/asan/pr95033.c: New test.

> ---

>   gcc/sanopt.c                                  |  1 +

>   .../g++.dg/asan/function-argument-4.C         | 26 +++++++++++++++++++

>   gcc/testsuite/gcc.dg/asan/pr95033.c           | 13 ++++++++++

>   3 files changed, 40 insertions(+)

>   create mode 100644 gcc/testsuite/g++.dg/asan/function-argument-4.C

>   create mode 100644 gcc/testsuite/gcc.dg/asan/pr95033.c

>

>
Martin Liška May 12, 2020, 7:04 a.m. | #2
On 5/11/20 2:44 PM, Richard Biener wrote:
> Hmm, I think the fix is to clear DECL_NOT_GIMPLE_REG_P instead

> where the code clears TREE_ADDRESSABLE of 'arg'


Ah, you are right. There's a patch that I've just tested.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin
From d321e69665a930ef8f10d27db16a8e1200326a1c Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>

Date: Mon, 11 May 2020 11:11:17 +0200
Subject: [PATCH] ASAN: clear DECL_NOT_GIMPLE_REG_P.

gcc/ChangeLog:

2020-05-11  Martin Liska  <mliska@suse.cz>

	PR sanitizer/95033
	PR sanitizer/95051
	* sanopt.c (sanitize_rewrite_addressable_params):
	Clear DECL_NOT_GIMPLE_REG_P for argument.

gcc/testsuite/ChangeLog:

2020-05-11  Martin Liska  <mliska@suse.cz>

	PR sanitizer/95033
	PR sanitizer/95051
	* g++.dg/asan/function-argument-4.C: New test.
	* gcc.dg/asan/pr95033.c: New test.
	* gcc.dg/asan/pr95051.c: New test.
---
 gcc/sanopt.c                                  |  1 +
 .../g++.dg/asan/function-argument-4.C         | 26 +++++++++++++++++++
 gcc/testsuite/gcc.dg/asan/pr95033.c           | 13 ++++++++++
 gcc/testsuite/gcc.dg/asan/pr95051.c           | 22 ++++++++++++++++
 4 files changed, 62 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/asan/function-argument-4.C
 create mode 100644 gcc/testsuite/gcc.dg/asan/pr95033.c
 create mode 100644 gcc/testsuite/gcc.dg/asan/pr95051.c

diff --git a/gcc/sanopt.c b/gcc/sanopt.c
index 86180e32c7e..6c3bce92378 100644
--- a/gcc/sanopt.c
+++ b/gcc/sanopt.c
@@ -1158,6 +1158,7 @@ sanitize_rewrite_addressable_params (function *fun)
 	  && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST)
 	{
 	  TREE_ADDRESSABLE (arg) = 0;
+	  DECL_NOT_GIMPLE_REG_P (arg) = 0;
 	  /* The parameter is no longer addressable.  */
 	  has_any_addressable_param = true;
 
diff --git a/gcc/testsuite/g++.dg/asan/function-argument-4.C b/gcc/testsuite/g++.dg/asan/function-argument-4.C
new file mode 100644
index 00000000000..cec1f1d788f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/asan/function-argument-4.C
@@ -0,0 +1,26 @@
+// { dg-do run }
+// { dg-shouldfail "asan" }
+
+#include <complex.h>
+
+static __attribute__ ((noinline)) long double
+goo (long double _Complex *a)
+{
+  return crealf(*(volatile _Complex long double *)a);
+}
+
+__attribute__ ((noinline)) float
+foo (float _Complex arg)
+{
+  return goo ((long double _Complex *)&arg);
+}
+
+int
+main ()
+{
+  return foo (3 + 2 * I);
+}
+
+// { dg-output "ERROR: AddressSanitizer: stack-buffer-overflow on address.*(\n|\r\n|\r)" }
+// { dg-output "READ of size \[0-9\]* at.*" }
+// { dg-output ".*'arg' \\(line 13\\) <== Memory access at offset \[0-9\]* partially overflows this variable.*" }
diff --git a/gcc/testsuite/gcc.dg/asan/pr95033.c b/gcc/testsuite/gcc.dg/asan/pr95033.c
new file mode 100644
index 00000000000..1228b7edcdb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asan/pr95033.c
@@ -0,0 +1,13 @@
+/* PR sanitizer/95033 */
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=address" } */
+
+struct a
+{
+  int b;
+};
+
+struct a c(_Complex d)
+{
+  return *(struct a *)&d;
+}
diff --git a/gcc/testsuite/gcc.dg/asan/pr95051.c b/gcc/testsuite/gcc.dg/asan/pr95051.c
new file mode 100644
index 00000000000..ec41a831299
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asan/pr95051.c
@@ -0,0 +1,22 @@
+/* PR sanitizer/95051 */
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=kernel-address --param=asan-stack=1 -O2" } */
+
+struct a {
+  struct {
+    struct {
+      int b;
+    } c;
+  };
+};
+struct d {
+  struct {
+    int e;
+  } f;
+}
+
+g(int h) {
+  struct a *i;
+  struct d *j = (struct d*)&h;
+  i->c.b = j->f.e;
+}
-- 
2.26.2
Bill Schmidt via Gcc-patches May 12, 2020, 7:18 a.m. | #3
On Tue, May 12, 2020 at 9:04 AM Martin Liška <mliska@suse.cz> wrote:
>

> On 5/11/20 2:44 PM, Richard Biener wrote:

> > Hmm, I think the fix is to clear DECL_NOT_GIMPLE_REG_P instead

> > where the code clears TREE_ADDRESSABLE of 'arg'

>

> Ah, you are right. There's a patch that I've just tested.

>

> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

>

> Ready to be installed?


OK.

Richard.

> Thanks,

> Martin
Bill Schmidt via Gcc-patches May 12, 2020, 6:57 p.m. | #4
On Tue, May 12, 2020 at 1:12 AM Richard Biener via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>

> On Tue, May 12, 2020 at 9:04 AM Martin Liška <mliska@suse.cz> wrote:

> >

> > On 5/11/20 2:44 PM, Richard Biener wrote:

> > > Hmm, I think the fix is to clear DECL_NOT_GIMPLE_REG_P instead

> > > where the code clears TREE_ADDRESSABLE of 'arg'

> >

> > Ah, you are right. There's a patch that I've just tested.

> >

> > Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

> >

> > Ready to be installed?

>

> OK.

>


I got

FAIL: gcc.dg/asan/pr95051.c   -O0  (test for excess errors)
FAIL: gcc.dg/asan/pr95051.c   -O1  (test for excess errors)
FAIL: gcc.dg/asan/pr95051.c   -O2 -flto -fno-use-linker-plugin
-flto-partition=none  (test for excess errors)
FAIL: gcc.dg/asan/pr95051.c   -O2 -flto -fuse-linker-plugin
-fno-fat-lto-objects  (test for excess errors)
FAIL: gcc.dg/asan/pr95051.c   -O2  (test for excess errors)
FAIL: gcc.dg/asan/pr95051.c   -O3 -g  (test for excess errors)
FAIL: gcc.dg/asan/pr95051.c   -Os  (test for excess errors)

on Linux/x86.

-- 
H.J.
Bill Schmidt via Gcc-patches May 12, 2020, 7:06 p.m. | #5
On Tue, May 12, 2020 at 11:57 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>

> On Tue, May 12, 2020 at 1:12 AM Richard Biener via Gcc-patches

> <gcc-patches@gcc.gnu.org> wrote:

> >

> > On Tue, May 12, 2020 at 9:04 AM Martin Liška <mliska@suse.cz> wrote:

> > >

> > > On 5/11/20 2:44 PM, Richard Biener wrote:

> > > > Hmm, I think the fix is to clear DECL_NOT_GIMPLE_REG_P instead

> > > > where the code clears TREE_ADDRESSABLE of 'arg'

> > >

> > > Ah, you are right. There's a patch that I've just tested.

> > >

> > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

> > >

> > > Ready to be installed?

> >

> > OK.

> >

>

> I got

>

> FAIL: gcc.dg/asan/pr95051.c   -O0  (test for excess errors)

> FAIL: gcc.dg/asan/pr95051.c   -O1  (test for excess errors)

> FAIL: gcc.dg/asan/pr95051.c   -O2 -flto -fno-use-linker-plugin

> -flto-partition=none  (test for excess errors)

> FAIL: gcc.dg/asan/pr95051.c   -O2 -flto -fuse-linker-plugin

> -fno-fat-lto-objects  (test for excess errors)

> FAIL: gcc.dg/asan/pr95051.c   -O2  (test for excess errors)

> FAIL: gcc.dg/asan/pr95051.c   -O3 -g  (test for excess errors)

> FAIL: gcc.dg/asan/pr95051.c   -Os  (test for excess errors)

>

> on Linux/x86.

>


Excess errors:
cc1: error: '-fsanitize=address' is incompatible with
'-fsanitize=kernel-address'


-- 
H.J.

Patch

diff --git a/gcc/sanopt.c b/gcc/sanopt.c
index 86180e32c7e..28a63442f4d 100644
--- a/gcc/sanopt.c
+++ b/gcc/sanopt.c
@@ -1155,6 +1155,7 @@  sanitize_rewrite_addressable_params (function *fun)
       if (TREE_ADDRESSABLE (arg)
 	  && !TREE_ADDRESSABLE (type)
 	  && !TREE_THIS_VOLATILE (arg)
+	  && !DECL_NOT_GIMPLE_REG_P (arg)
 	  && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST)
 	{
 	  TREE_ADDRESSABLE (arg) = 0;
diff --git a/gcc/testsuite/g++.dg/asan/function-argument-4.C b/gcc/testsuite/g++.dg/asan/function-argument-4.C
new file mode 100644
index 00000000000..cec1f1d788f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/asan/function-argument-4.C
@@ -0,0 +1,26 @@ 
+// { dg-do run }
+// { dg-shouldfail "asan" }
+
+#include <complex.h>
+
+static __attribute__ ((noinline)) long double
+goo (long double _Complex *a)
+{
+  return crealf(*(volatile _Complex long double *)a);
+}
+
+__attribute__ ((noinline)) float
+foo (float _Complex arg)
+{
+  return goo ((long double _Complex *)&arg);
+}
+
+int
+main ()
+{
+  return foo (3 + 2 * I);
+}
+
+// { dg-output "ERROR: AddressSanitizer: stack-buffer-overflow on address.*(\n|\r\n|\r)" }
+// { dg-output "READ of size \[0-9\]* at.*" }
+// { dg-output ".*'arg' \\(line 13\\) <== Memory access at offset \[0-9\]* partially overflows this variable.*" }
diff --git a/gcc/testsuite/gcc.dg/asan/pr95033.c b/gcc/testsuite/gcc.dg/asan/pr95033.c
new file mode 100644
index 00000000000..1228b7edcdb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asan/pr95033.c
@@ -0,0 +1,13 @@ 
+/* PR sanitizer/95033 */
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=address" } */
+
+struct a
+{
+  int b;
+};
+
+struct a c(_Complex d)
+{
+  return *(struct a *)&d;
+}