https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79265

            Bug ID: 79265
           Summary: -fsanitize=undefined inserts unnecessary null pointer
                    tests
           Product: gcc
           Version: 7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: sanitizer
          Assignee: unassigned at gcc dot gnu.org
          Reporter: msebor at gcc dot gnu.org
                CC: dodji at gcc dot gnu.org, dvyukov at gcc dot gnu.org,
                    jakub at gcc dot gnu.org, kcc at gcc dot gnu.org
  Target Milestone: ---

UBSan inserts a null pointer test for each pointer argument to a function whose
corresponding formal argument is declared attribute nonnull.  That makes sense
except for expressions that are known/guaranteed no to be null, such as arrays,
VLAs, or pointers obtained from calls to GCC built-in functions
known/guaranteed not to return null such as alloca.  I'm inclined to suggest
it's also unnecessary for ordinary (non-builtin) functions declared with
attribute returns_nonnull since those, too, guarantee not to return a null
pointer, but since possible for such a function to break that guarantee there
may be some value in UBSan emitting tests for those (unless those tests are or
can be defeated by subsequent optimization passes).  While some of these
redundant test are removed by subsequent optimizations they tend to interfere
with them and lead to suboptimal code (as discussed in bug 79257), and can
trigger warnings.

The following test case shows the unnecessary tests. 

$ cat b.c && gcc -O2 -c -Wall -fdump-tree-ubsan=/dev/stdout
-fsanitize=undefined b.c
void* __attribute__ ((alloc_size (1), returns_nonnull))
myalloc (unsigned);

void sink (void*);

void f0 (void)
{
  char a[4];
  __builtin_sprintf (a, "%%");
  sink (a);
}

void f1 (unsigned n)
{
  char a[n];
  __builtin_sprintf (a, "%%");
  sink (a);
}

void f2 (unsigned n)
{
  void *p = __builtin_alloca (n);
  __builtin_sprintf (p, "%%");
  sink (p);
}

void f3 (unsigned n)
{
  void *p = myalloc(n);
  __builtin_sprintf (p, "%%");
  sink (p);
}


;; Function f0 (f0, funcdef_no=0, decl_uid=2121, cgraph_uid=0, symbol_order=0)

Introduced new external node (__builtin___ubsan_handle_nonnull_arg/15).

Symbols to be put in SSA form
{ D.2162 }
Incremental SSA update started at block: 0
Number of blocks in CFG: 7
Number of blocks to update: 6 ( 86%)


f0 ()
{
  char a[4];

  <bb 2> [0.00%]:
  if (&a == 0B)
    goto <bb 4>; [0.04%]
  else
    goto <bb 3>; [99.96%]

  <bb 4> [0.00%]:
  __builtin___ubsan_handle_nonnull_arg (&*.Lubsan_data1);

  <bb 3> [0.00%]:
  if ("%%" == 0B)
    goto <bb 6>; [0.04%]
  else
    goto <bb 5>; [99.96%]

  <bb 6> [0.00%]:
  __builtin___ubsan_handle_nonnull_arg (&*.Lubsan_data2);

  <bb 5> [0.00%]:
  __builtin_sprintf (&a, "%%");
  sink (&a);
  a ={v} {CLOBBER};
  return;

}



;; Function f1 (f1, funcdef_no=1, decl_uid=2125, cgraph_uid=1, symbol_order=3)


Symbols to be put in SSA form
{ D.2173 }
Incremental SSA update started at block: 0
Number of blocks in CFG: 9
Number of blocks to update: 8 ( 89%)


f1 (unsigned int n)
{
  char[0:D.2155] * a.2;
  sizetype D.2155;
  unsigned long _1;
  long int _2;
  long int _3;
  bitsizetype _5;
  void * saved_stack.5_6;
  bitsizetype _8;
  char[0:D.2155] * a.3_11;
  char[0:D.2155] * a.4_12;
  unsigned int n.0_13;
  unsigned int n.1_15;
  bitsizetype _17;
  sizetype _18;
  bitsizetype _19;
  sizetype _20;

  <bb 2> [0.00%]:
  saved_stack.5_6 = __builtin_stack_save ();
  n.0_13 = n_4(D);
  if (n.0_13 == 0)
    goto <bb 3>; [0.00%]
  else
    goto <bb 4>; [0.00%]

  <bb 3> [0.00%]:
  _1 = (unsigned long) n.0_13;
  __builtin___ubsan_handle_vla_bound_not_positive (&*.Lubsan_data0, _1);

  <bb 4> [0.00%]:
  n.1_15 = n.0_13;
  _2 = (long int) n.1_15;
  _3 = UBSAN_CHECK_SUB (_2, 1);
  _16 = (sizetype) _3;
  _5 = (bitsizetype) n.1_15;
  _17 = _5 * 8;
  _18 = (sizetype) n.1_15;
  _8 = (bitsizetype) n.1_15;
  _19 = _8 * 8;
  _20 = (sizetype) n.1_15;
  a.2_22 = __builtin_alloca_with_align (_20, 8);
  a.3_11 = a.2_22;
  if (a.3_11 == 0B)
    goto <bb 6>; [0.04%]
  else
    goto <bb 5>; [99.96%]

  <bb 6> [0.00%]:
  __builtin___ubsan_handle_nonnull_arg (&*.Lubsan_data3);

  <bb 5> [0.00%]:
  if ("%%" == 0B)
    goto <bb 8>; [0.04%]
  else
    goto <bb 7>; [99.96%]

  <bb 8> [0.00%]:
  __builtin___ubsan_handle_nonnull_arg (&*.Lubsan_data4);

  <bb 7> [0.00%]:
  __builtin_sprintf (a.3_11, "%%");
  a.4_12 = a.2_22;
  sink (a.4_12);
  __builtin_stack_restore (saved_stack.5_6);
  return;

}



;; Function f2 (f2, funcdef_no=2, decl_uid=2142, cgraph_uid=2, symbol_order=4)


Symbols to be put in SSA form
{ D.2184 }
Incremental SSA update started at block: 0
Number of blocks in CFG: 7
Number of blocks to update: 6 ( 86%)


f2 (unsigned int n)
{
  void * p;
  long unsigned int _1;

  <bb 2> [0.00%]:
  _1 = (long unsigned int) n_2(D);
  p_5 = __builtin_alloca (_1);
  if (p_5 == 0B)
    goto <bb 4>; [0.04%]
  else
    goto <bb 3>; [99.96%]

  <bb 4> [0.00%]:
  __builtin___ubsan_handle_nonnull_arg (&*.Lubsan_data5);

  <bb 3> [0.00%]:
  if ("%%" == 0B)
    goto <bb 6>; [0.04%]
  else
    goto <bb 5>; [99.96%]

  <bb 6> [0.00%]:
  __builtin___ubsan_handle_nonnull_arg (&*.Lubsan_data6);

  <bb 5> [0.00%]:
  __builtin_sprintf (p_5, "%%");
  sink (p_5);
  return;

}



;; Function f3 (f3, funcdef_no=3, decl_uid=2146, cgraph_uid=3, symbol_order=5)


Symbols to be put in SSA form
{ D.2195 }
Incremental SSA update started at block: 0
Number of blocks in CFG: 7
Number of blocks to update: 6 ( 86%)


f3 (unsigned int n)
{
  void * p;

  <bb 2> [0.00%]:
  p_4 = myalloc (n_2(D));
  if (p_4 == 0B)
    goto <bb 4>; [0.04%]
  else
    goto <bb 3>; [99.96%]

  <bb 4> [0.00%]:
  __builtin___ubsan_handle_nonnull_arg (&*.Lubsan_data7);

  <bb 3> [0.00%]:
  if ("%%" == 0B)
    goto <bb 6>; [0.04%]
  else
    goto <bb 5>; [99.96%]

  <bb 6> [0.00%]:
  __builtin___ubsan_handle_nonnull_arg (&*.Lubsan_data8);

  <bb 5> [0.00%]:
  __builtin_sprintf (p_4, "%%");
  sink (p_4);
  return;

}

Reply via email to