| Message ID | 20170614171014.GC2123@tucnak |
|---|---|
| State | New |
| Headers | show |
On Wed, 14 Jun 2017, Jakub Jelinek wrote: > Hi! > > -fsanitize=object-size is yet another sanitization that ignored aggregate > function arguments. Fixed thusly (plus some small cleanup for > instrument_null), bootstrapped/regtested on x86_64-linux and i686-linux, ok > for trunk? Ok. Richard. > 2017-06-14 Jakub Jelinek <jakub@redhat.com> > > PR sanitizer/81094 > * ubsan.c (instrument_null): Add T argument, use it instead > of computing it based on IS_LHS. > (instrument_object_size): Likewise. > (pass_ubsan::execute): Adjust instrument_null and > instrument_object_size callers to pass gimple_get_lhs or > gimple_assign_rhs1 result to it. Use instrument_null instead of > calling get_base_address and instrument_mem_ref. Handle > aggregate call arguments for object-size sanitization. > > * c-c++-common/ubsan/object-size-11.c: New test. > > --- gcc/ubsan.c.jj 2017-06-14 14:40:39.000000000 +0200 > +++ gcc/ubsan.c 2017-06-14 14:49:17.702131958 +0200 > @@ -1204,10 +1204,8 @@ instrument_mem_ref (tree mem, tree base, > /* Perform the pointer instrumentation. */ > > static void > -instrument_null (gimple_stmt_iterator gsi, bool is_lhs) > +instrument_null (gimple_stmt_iterator gsi, tree t, bool is_lhs) > { > - gimple *stmt = gsi_stmt (gsi); > - tree t = is_lhs ? gimple_get_lhs (stmt) : gimple_assign_rhs1 (stmt); > /* Handle also e.g. &s->i. */ > if (TREE_CODE (t) == ADDR_EXPR) > t = TREE_OPERAND (t, 0); > @@ -1754,11 +1752,10 @@ instrument_nonnull_return (gimple_stmt_i > points to an out-of-bounds location. */ > > static void > -instrument_object_size (gimple_stmt_iterator *gsi, bool is_lhs) > +instrument_object_size (gimple_stmt_iterator *gsi, tree t, bool is_lhs) > { > gimple *stmt = gsi_stmt (*gsi); > location_t loc = gimple_location (stmt); > - tree t = is_lhs ? gimple_get_lhs (stmt) : gimple_assign_rhs1 (stmt); > tree type; > tree index = NULL_TREE; > HOST_WIDE_INT size_in_bytes; > @@ -1989,9 +1986,9 @@ pass_ubsan::execute (function *fun) > if (sanitize_flags_p (SANITIZE_NULL | SANITIZE_ALIGNMENT, fun->decl)) > { > if (gimple_store_p (stmt)) > - instrument_null (gsi, true); > + instrument_null (gsi, gimple_get_lhs (stmt), true); > if (gimple_assign_single_p (stmt)) > - instrument_null (gsi, false); > + instrument_null (gsi, gimple_assign_rhs1 (stmt), false); > if (is_gimple_call (stmt)) > { > unsigned args_num = gimple_call_num_args (stmt); > @@ -2000,10 +1997,7 @@ pass_ubsan::execute (function *fun) > 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); > + instrument_null (gsi, arg, false); > } > } > } > @@ -2033,9 +2027,21 @@ pass_ubsan::execute (function *fun) > if (sanitize_flags_p (SANITIZE_OBJECT_SIZE, fun->decl)) > { > if (gimple_store_p (stmt)) > - instrument_object_size (&gsi, true); > + instrument_object_size (&gsi, gimple_get_lhs (stmt), true); > if (gimple_assign_load_p (stmt)) > - instrument_object_size (&gsi, false); > + instrument_object_size (&gsi, gimple_assign_rhs1 (stmt), > + 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; > + instrument_object_size (&gsi, arg, false); > + } > + } > } > > gsi_next (&gsi); > --- gcc/testsuite/c-c++-common/ubsan/object-size-11.c.jj 2017-06-14 16:16:43.192137010 +0200 > +++ gcc/testsuite/c-c++-common/ubsan/object-size-11.c 2017-06-14 16:16:22.000000000 +0200 > @@ -0,0 +1,53 @@ > +/* PR sanitizer/81094 */ > +/* { dg-do run } */ > +/* { dg-skip-if "" { *-*-* } { "*" } { "-O2" } } */ > +/* { dg-options "-fsanitize=object-size" } */ > + > +#define N 20 > + > +struct S { int i; }; > + > +__attribute__((noinline, noclone)) void > +f0 (struct S s) > +{ > + asm volatile ("" : : "r" (s.i) : "memory"); > +} > + > +__attribute__((noinline, noclone)) void > +f1 (int i) > +{ > + char *orig; > + struct S *p; > + orig = (char *) __builtin_calloc (N, sizeof (struct S)); > + p = (struct S *) orig; > + f0 (*(p + i)); > + f0 (p[i]); > + p++; > + f0 (p[i - 1]); > + f0 (*(p + i - 1)); > + __builtin_free (orig); > +} > + > +/* { dg-output "load of address \[^\n\r]* with insufficient space for an object of type 'struct S'\[^\n\r]*(\n|\r\n|\r)" } */ > +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ > +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ > +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ > +/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'struct S'\[^\n\r]*(\n|\r\n|\r)" } */ > +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ > +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ > +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ > +/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'struct S'\[^\n\r]*(\n|\r\n|\r)" } */ > +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ > +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ > +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ > +/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'struct S'\[^\n\r]*(\n|\r\n|\r)" } */ > +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ > +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ > +/* { dg-output "\[^\n\r]*\\^" } */ > + > +int > +main () > +{ > + f1 (N); > + return 0; > +} > > Jakub > >
--- gcc/ubsan.c.jj 2017-06-14 14:40:39.000000000 +0200 +++ gcc/ubsan.c 2017-06-14 14:49:17.702131958 +0200 @@ -1204,10 +1204,8 @@ instrument_mem_ref (tree mem, tree base, /* Perform the pointer instrumentation. */ static void -instrument_null (gimple_stmt_iterator gsi, bool is_lhs) +instrument_null (gimple_stmt_iterator gsi, tree t, bool is_lhs) { - gimple *stmt = gsi_stmt (gsi); - tree t = is_lhs ? gimple_get_lhs (stmt) : gimple_assign_rhs1 (stmt); /* Handle also e.g. &s->i. */ if (TREE_CODE (t) == ADDR_EXPR) t = TREE_OPERAND (t, 0); @@ -1754,11 +1752,10 @@ instrument_nonnull_return (gimple_stmt_i points to an out-of-bounds location. */ static void -instrument_object_size (gimple_stmt_iterator *gsi, bool is_lhs) +instrument_object_size (gimple_stmt_iterator *gsi, tree t, bool is_lhs) { gimple *stmt = gsi_stmt (*gsi); location_t loc = gimple_location (stmt); - tree t = is_lhs ? gimple_get_lhs (stmt) : gimple_assign_rhs1 (stmt); tree type; tree index = NULL_TREE; HOST_WIDE_INT size_in_bytes; @@ -1989,9 +1986,9 @@ pass_ubsan::execute (function *fun) if (sanitize_flags_p (SANITIZE_NULL | SANITIZE_ALIGNMENT, fun->decl)) { if (gimple_store_p (stmt)) - instrument_null (gsi, true); + instrument_null (gsi, gimple_get_lhs (stmt), true); if (gimple_assign_single_p (stmt)) - instrument_null (gsi, false); + instrument_null (gsi, gimple_assign_rhs1 (stmt), false); if (is_gimple_call (stmt)) { unsigned args_num = gimple_call_num_args (stmt); @@ -2000,10 +1997,7 @@ pass_ubsan::execute (function *fun) 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); + instrument_null (gsi, arg, false); } } } @@ -2033,9 +2027,21 @@ pass_ubsan::execute (function *fun) if (sanitize_flags_p (SANITIZE_OBJECT_SIZE, fun->decl)) { if (gimple_store_p (stmt)) - instrument_object_size (&gsi, true); + instrument_object_size (&gsi, gimple_get_lhs (stmt), true); if (gimple_assign_load_p (stmt)) - instrument_object_size (&gsi, false); + instrument_object_size (&gsi, gimple_assign_rhs1 (stmt), + 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; + instrument_object_size (&gsi, arg, false); + } + } } gsi_next (&gsi); --- gcc/testsuite/c-c++-common/ubsan/object-size-11.c.jj 2017-06-14 16:16:43.192137010 +0200 +++ gcc/testsuite/c-c++-common/ubsan/object-size-11.c 2017-06-14 16:16:22.000000000 +0200 @@ -0,0 +1,53 @@ +/* PR sanitizer/81094 */ +/* { dg-do run } */ +/* { dg-skip-if "" { *-*-* } { "*" } { "-O2" } } */ +/* { dg-options "-fsanitize=object-size" } */ + +#define N 20 + +struct S { int i; }; + +__attribute__((noinline, noclone)) void +f0 (struct S s) +{ + asm volatile ("" : : "r" (s.i) : "memory"); +} + +__attribute__((noinline, noclone)) void +f1 (int i) +{ + char *orig; + struct S *p; + orig = (char *) __builtin_calloc (N, sizeof (struct S)); + p = (struct S *) orig; + f0 (*(p + i)); + f0 (p[i]); + p++; + f0 (p[i - 1]); + f0 (*(p + i - 1)); + __builtin_free (orig); +} + +/* { dg-output "load of address \[^\n\r]* with insufficient space for an object of type 'struct S'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'struct S'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'struct S'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*load of address \[^\n\r]* with insufficient space for an object of type 'struct S'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*note: pointer points here\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\\^" } */ + +int +main () +{ + f1 (N); + return 0; +}