Re: [PATCH] Add -fsanitize={null,alignment} sanitization of aggregate arguments of calls (PR middle-end/81005)

[Richard Biener <rguenther at suse dot de>]

On June 8, 2017 8:15:28 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>I've noticed we don't sanitize aggregate arguments of calls.
>
>Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
>ok for trunk? 

OK.

 For asms I'm not 100% sure, the arguments might be
>unused
>in the asm, or used only conditionally etc.

Valid points.

Richard.

>2017-06-07  Jakub Jelinek  <jakub@redhat.com>
>
>	PR middle-end/81005
>	* ubsan.c (instrument_null): Avoid pointless code temporary.
>	(pass_ubsan::execute): Instrument aggregate arguments of calls.
>
>	* c-c++-common/ubsan/align-10.c: New test.
>	* c-c++-common/ubsan/null-13.c: New test.
>
>--- gcc/ubsan.c.jj	2017-05-21 15:46:13.000000000 +0200
>+++ gcc/ubsan.c	2017-06-07 14:14:27.883227570 +0200
>@@ -1212,8 +1212,7 @@ instrument_null (gimple_stmt_iterator gs
>   if (TREE_CODE (t) == ADDR_EXPR)
>     t = TREE_OPERAND (t, 0);
>   tree base = get_base_address (t);
>-  const enum tree_code code = TREE_CODE (base);
>-  if (code == MEM_REF
>+  if (TREE_CODE (base) == MEM_REF
>       && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME)
>     instrument_mem_ref (t, base, &gsi, is_lhs);
> }
>@@ -2003,6 +2002,20 @@ pass_ubsan::execute (function *fun)
> 		instrument_null (gsi, true);
> 	      if (gimple_assign_single_p (stmt))
> 		instrument_null (gsi, false);
>+	      if (is_gimple_call (stmt))
>+		{
>+		  unsigned args_num = gimple_call_num_args (stmt);
>+		  for (unsigned i = 0; i < args_num; ++i)
>+		    {
>+		      tree arg = gimple_call_arg (stmt, i);
>+		      if (is_gimple_reg (arg) || is_gimple_min_invariant (arg))
>+			continue;
>+		      tree base = get_base_address (arg);
>+		      if (TREE_CODE (base) == MEM_REF
>+			  && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME)
>+			instrument_mem_ref (arg, base, &gsi, false);
>+		    }
>+		}
> 	    }
> 
> 	  if (flag_sanitize & (SANITIZE_BOOL | SANITIZE_ENUM)
>--- gcc/testsuite/c-c++-common/ubsan/align-10.c.jj	2017-06-07
>15:32:18.545409340 +0200
>+++ gcc/testsuite/c-c++-common/ubsan/align-10.c	2017-06-07
>17:06:44.000000000 +0200
>@@ -0,0 +1,39 @@
>+/* Limit this to known non-strict alignment targets.  */
>+/* { dg-do run { target { i?86-*-linux* x86_64-*-linux* } } } */
>+/* { dg-options "-O -fsanitize=alignment -fsanitize-recover=alignment"
>} */
>+
>+struct R { int a; } r;
>+struct S { struct R a; char b; long long c; short d[10]; };
>+struct T { char a; long long b; };
>+struct U { char a; int b; int c; long long d; struct S e; struct T f;
>} __attribute__((packed));
>+struct V { long long a; struct S b; struct T c; struct U u; } v;
>+
>+__attribute__((noinline, noclone)) int
>+bar (int x, struct R y, struct R z)
>+{
>+  return x + y.a;
>+}
>+
>+__attribute__((noinline, noclone)) int
>+foo (struct S *p, struct S *q)
>+{
>+  int i = bar (0, r, r);
>+  i += bar (1, p->a, r);
>+  i += bar (2, r, q->a);
>+  return i;
>+}
>+
>+int
>+main ()
>+{
>+  char *p = (char *) &v.u.e;
>+  struct S *q, *r;
>+  asm volatile ("" : "=r" (q) : "0" (p));
>+  asm volatile ("" : "=r" (r) : "0" (p));
>+  if (foo (q, r) != 3)
>+    __builtin_abort ();
>+  return 0;
>+}
>+
>+/* { dg-output "\.c:21:\[0-9]*: \[^\n\r]*member access within
>misaligned address 0x\[0-9a-fA-F]* for type 'struct S', which requires
>\[48] byte alignment.*" } */
>+/* { dg-output "\.c:22:\[0-9]*: \[^\n\r]*member access within
>misaligned address 0x\[0-9a-fA-F]* for type 'struct S', which requires
>\[48] byte alignment" } */
>--- gcc/testsuite/c-c++-common/ubsan/null-13.c.jj	2017-06-07
>14:21:27.460145351 +0200
>+++ gcc/testsuite/c-c++-common/ubsan/null-13.c	2017-06-07
>16:54:18.576027478 +0200
>@@ -0,0 +1,37 @@
>+/* { dg-do run } */
>+/* { dg-options "-fsanitize=null -fno-sanitize-recover=null -w" } */
>+/* { dg-shouldfail "ubsan" } */
>+
>+struct S {
>+  int i;
>+  long long j;
>+  long long m;
>+};
>+union U {
>+  int k;
>+  struct S l;
>+};
>+
>+__attribute__((noinline, noclone)) int
>+foo (struct S s)
>+{
>+  return s.i + s.j + s.m;
>+}
>+
>+__attribute__((noinline, noclone)) int
>+bar (union U *u)
>+{
>+  foo (u->l);
>+}
>+
>+union U v;
>+
>+int
>+main (void)
>+{
>+  union U *u = 0;
>+  asm volatile ("" : "+r" (u) : "r" (&v) : "memory");
>+  return bar (u);
>+}
>+
>+/* { dg-output "member access within null pointer of type 'union U'" }
>*/
>
>	Jakub