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 <ja...@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
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
