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; }