On Fri, Jun 27, 2014 at 09:13:07AM +0200, Jakub Jelinek wrote: > The patch adds two new (trivial handlers) to libubsan, as it is maintained > in llvm's compiler-rt, will talk to them if they are interested in those > and what exact wording and form (AFAIK clang also added the gcc > {,returns_}nonnull attributes). If they wouldn't be interested, guess > we could add them in a separate, gcc owned, source file in ubsan (like we > own Makefile*).
And here is the actual new version of the patch, including cherry-picked libsanitizer changes. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2014-09-09 Jakub Jelinek <ja...@redhat.com> gcc/ * flag-types.h (enum sanitize_code): Add SANITIZE_NONNULL_ATTRIBUTE and SANITIZE_RETURNS_NONNULL_ATTRIBUTE, or them into SANITIZE_UNDEFINED. * opts.c (common_handle_option): Handle SANITIZE_NONNULL_ATTRIBUTE and SANITIZE_RETURNS_NONNULL_ATTRIBUTE and disable flag_delete_null_pointer_checks for them. * sanitizer.def (BUILT_IN_UBSAN_HANDLE_NONNULL_ARG, BUILT_IN_UBSAN_HANDLE_NONNULL_ARG_ABORT, BUILT_IN_UBSAN_HANDLE_NONNULL_RETURN, BUILT_IN_UBSAN_HANDLE_NONNULL_RETURN_ABORT): New. * ubsan.c (instrument_bool_enum_load): Set *gsi back to stmt's iterator. (instrument_nonnull_arg, instrument_nonnull_return): New functions. (pass_ubsan::gate): Return true even for SANITIZE_NONNULL_ATTRIBUTE or SANITIZE_RETURNS_NONNULL_ATTRIBUTE. (pass_ubsan::execute): Call instrument_nonnull_{arg,return}. * doc/invoke.texi (-fsanitize=nonnull-attribute, -fsanitize=returns-nonnull-attribute): Document. gcc/testsuite/ * c-c++-common/ubsan/attrib-3.c: New test. * c-c++-common/ubsan/nonnull-1.c: New test. * c-c++-common/ubsan/nonnull-2.c: New test. * c-c++-common/ubsan/nonnull-3.c: New test. * c-c++-common/ubsan/nonnull-4.c: New test. * c-c++-common/ubsan/nonnull-5.c: New test. libsanitizer/ * ubsan/ubsan_handlers.cc, ubsan/ubsan_handlers.h: Cherry pick upstream r215485, r217389, r217391 and r217400. --- gcc/sanitizer.def.jj 2014-09-08 22:12:27.671740863 +0200 +++ gcc/sanitizer.def 2014-09-09 11:59:18.694180701 +0200 @@ -417,3 +417,19 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_UBSAN_HAN "__ubsan_handle_out_of_bounds_abort", BT_FN_VOID_PTR_PTR, ATTR_COLD_NORETURN_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_UBSAN_HANDLE_NONNULL_ARG, + "__ubsan_handle_nonnull_arg", + BT_FN_VOID_PTR_PTRMODE, + ATTR_COLD_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_UBSAN_HANDLE_NONNULL_ARG_ABORT, + "__ubsan_handle_nonnull_arg_abort", + BT_FN_VOID_PTR_PTRMODE, + ATTR_COLD_NORETURN_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_UBSAN_HANDLE_NONNULL_RETURN, + "__ubsan_handle_nonnull_return", + BT_FN_VOID_PTR, + ATTR_COLD_NOTHROW_LEAF_LIST) +DEF_SANITIZER_BUILTIN(BUILT_IN_UBSAN_HANDLE_NONNULL_RETURN_ABORT, + "__ubsan_handle_nonnull_return_abort", + BT_FN_VOID_PTR, + ATTR_COLD_NORETURN_NOTHROW_LEAF_LIST) --- gcc/flag-types.h.jj 2014-09-09 11:59:18.693180712 +0200 +++ gcc/flag-types.h 2014-09-09 12:00:26.780842334 +0200 @@ -234,10 +234,14 @@ enum sanitize_code { SANITIZE_FLOAT_CAST = 1 << 15, SANITIZE_BOUNDS = 1 << 16, SANITIZE_ALIGNMENT = 1 << 17, + SANITIZE_NONNULL_ATTRIBUTE = 1 << 18, + SANITIZE_RETURNS_NONNULL_ATTRIBUTE = 1 << 19, SANITIZE_UNDEFINED = SANITIZE_SHIFT | SANITIZE_DIVIDE | SANITIZE_UNREACHABLE | SANITIZE_VLA | SANITIZE_NULL | SANITIZE_RETURN | SANITIZE_SI_OVERFLOW | SANITIZE_BOOL | SANITIZE_ENUM - | SANITIZE_BOUNDS | SANITIZE_ALIGNMENT, + | SANITIZE_BOUNDS | SANITIZE_ALIGNMENT + | SANITIZE_NONNULL_ATTRIBUTE + | SANITIZE_RETURNS_NONNULL_ATTRIBUTE, SANITIZE_NONDEFAULT = SANITIZE_FLOAT_DIVIDE | SANITIZE_FLOAT_CAST }; --- gcc/ubsan.c.jj 2014-09-09 14:08:22.938445806 +0200 +++ gcc/ubsan.c 2014-09-09 13:58:14.381504541 +0200 @@ -1090,6 +1090,7 @@ instrument_bool_enum_load (gimple_stmt_i } gimple_set_location (g, loc); gsi_insert_before (&gsi2, g, GSI_SAME_STMT); + *gsi = gsi_for_stmt (stmt); } /* Instrument float point-to-integer conversion. TYPE is an integer type of @@ -1215,6 +1216,122 @@ ubsan_instrument_float_cast (location_t fn, integer_zero_node); } +/* Instrument values passed to function arguments with nonnull attribute. */ + +static void +instrument_nonnull_arg (gimple_stmt_iterator *gsi) +{ + gimple stmt = gsi_stmt (*gsi); + location_t loc[2]; + /* infer_nonnull_range needs flag_delete_null_pointer_checks set, + while for nonnull sanitization it is clear. */ + int save_flag_delete_null_pointer_checks = flag_delete_null_pointer_checks; + flag_delete_null_pointer_checks = 1; + loc[0] = gimple_location (stmt); + loc[1] = UNKNOWN_LOCATION; + for (unsigned int i = 0; i < gimple_call_num_args (stmt); i++) + { + tree arg = gimple_call_arg (stmt, i); + if (POINTER_TYPE_P (TREE_TYPE (arg)) + && infer_nonnull_range (stmt, arg, false, true)) + { + gimple g; + if (!is_gimple_val (arg)) + { + g = gimple_build_assign (make_ssa_name (TREE_TYPE (arg), NULL), + arg); + gimple_set_location (g, loc[0]); + gsi_insert_before (gsi, g, GSI_SAME_STMT); + arg = gimple_assign_lhs (g); + } + + basic_block then_bb, fallthru_bb; + *gsi = create_cond_insert_point (gsi, true, false, true, + &then_bb, &fallthru_bb); + g = gimple_build_cond (EQ_EXPR, arg, + build_zero_cst (TREE_TYPE (arg)), + NULL_TREE, NULL_TREE); + gimple_set_location (g, loc[0]); + gsi_insert_after (gsi, g, GSI_NEW_STMT); + + *gsi = gsi_after_labels (then_bb); + if (flag_sanitize_undefined_trap_on_error) + g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); + else + { + tree data = ubsan_create_data ("__ubsan_nonnull_arg_data", + 2, loc, NULL_TREE, + build_int_cst (integer_type_node, + i + 1), + NULL_TREE); + data = build_fold_addr_expr_loc (loc[0], data); + enum built_in_function bcode + = flag_sanitize_recover + ? BUILT_IN_UBSAN_HANDLE_NONNULL_ARG + : BUILT_IN_UBSAN_HANDLE_NONNULL_ARG_ABORT; + tree fn = builtin_decl_explicit (bcode); + + g = gimple_build_call (fn, 1, data); + } + gimple_set_location (g, loc[0]); + gsi_insert_before (gsi, g, GSI_SAME_STMT); + } + *gsi = gsi_for_stmt (stmt); + } + flag_delete_null_pointer_checks = save_flag_delete_null_pointer_checks; +} + +/* Instrument returns in functions with returns_nonnull attribute. */ + +static void +instrument_nonnull_return (gimple_stmt_iterator *gsi) +{ + gimple stmt = gsi_stmt (*gsi); + location_t loc[2]; + tree arg = gimple_return_retval (stmt); + /* infer_nonnull_range needs flag_delete_null_pointer_checks set, + while for nonnull return sanitization it is clear. */ + int save_flag_delete_null_pointer_checks = flag_delete_null_pointer_checks; + flag_delete_null_pointer_checks = 1; + loc[0] = gimple_location (stmt); + loc[1] = UNKNOWN_LOCATION; + if (arg + && POINTER_TYPE_P (TREE_TYPE (arg)) + && is_gimple_val (arg) + && infer_nonnull_range (stmt, arg, false, true)) + { + basic_block then_bb, fallthru_bb; + *gsi = create_cond_insert_point (gsi, true, false, true, + &then_bb, &fallthru_bb); + gimple g = gimple_build_cond (EQ_EXPR, arg, + build_zero_cst (TREE_TYPE (arg)), + NULL_TREE, NULL_TREE); + gimple_set_location (g, loc[0]); + gsi_insert_after (gsi, g, GSI_NEW_STMT); + + *gsi = gsi_after_labels (then_bb); + if (flag_sanitize_undefined_trap_on_error) + g = gimple_build_call (builtin_decl_explicit (BUILT_IN_TRAP), 0); + else + { + tree data = ubsan_create_data ("__ubsan_nonnull_return_data", + 2, loc, NULL_TREE, NULL_TREE); + data = build_fold_addr_expr_loc (loc[0], data); + enum built_in_function bcode + = flag_sanitize_recover + ? BUILT_IN_UBSAN_HANDLE_NONNULL_RETURN + : BUILT_IN_UBSAN_HANDLE_NONNULL_RETURN_ABORT; + tree fn = builtin_decl_explicit (bcode); + + g = gimple_build_call (fn, 1, data); + } + gimple_set_location (g, loc[0]); + gsi_insert_before (gsi, g, GSI_SAME_STMT); + *gsi = gsi_for_stmt (stmt); + } + flag_delete_null_pointer_checks = save_flag_delete_null_pointer_checks; +} + namespace { const pass_data pass_data_ubsan = @@ -1242,7 +1359,9 @@ public: { return flag_sanitize & (SANITIZE_NULL | SANITIZE_SI_OVERFLOW | SANITIZE_BOOL | SANITIZE_ENUM - | SANITIZE_ALIGNMENT) + | SANITIZE_ALIGNMENT + | SANITIZE_NONNULL_ATTRIBUTE + | SANITIZE_RETURNS_NONNULL_ATTRIBUTE) && current_function_decl != NULL_TREE && !lookup_attribute ("no_sanitize_undefined", DECL_ATTRIBUTES (current_function_decl)); @@ -1285,7 +1404,25 @@ pass_ubsan::execute (function *fun) if (flag_sanitize & (SANITIZE_BOOL | SANITIZE_ENUM) && gimple_assign_load_p (stmt)) - instrument_bool_enum_load (&gsi); + { + instrument_bool_enum_load (&gsi); + bb = gimple_bb (stmt); + } + + if ((flag_sanitize & SANITIZE_NONNULL_ATTRIBUTE) + && is_gimple_call (stmt) + && !gimple_call_internal_p (stmt)) + { + instrument_nonnull_arg (&gsi); + bb = gimple_bb (stmt); + } + + if ((flag_sanitize & SANITIZE_RETURNS_NONNULL_ATTRIBUTE) + && gimple_code (stmt) == GIMPLE_RETURN) + { + instrument_nonnull_return (&gsi); + bb = gimple_bb (stmt); + } gsi_next (&gsi); } --- gcc/doc/invoke.texi.jj 2014-09-08 22:12:42.643667108 +0200 +++ gcc/doc/invoke.texi 2014-09-09 11:59:18.702180660 +0200 @@ -5578,6 +5578,20 @@ This option enables floating-point type We check that the result of the conversion does not overflow. This option does not work well with @code{FE_INVALID} exceptions enabled. +@item -fsanitize=nonnull-attribute +@opindex fsanitize=nonnull-attribute + +This option enables instrumentation of calls, checking whether null values +are not passed to arguments marked as requiring a non-null value by the +@code{nonnull} function attribute. + +@item -fsanitize=returns-nonnull-attribute +@opindex fsanitize=returns-nonnull-attribute + +This option enables instrumentation of return statements in functions +marked with @code{returns_nonnull} function attribute, to detect returning +of null values from such functions. + @end table While @option{-ftrapv} causes traps for signed overflows to be emitted, --- gcc/testsuite/c-c++-common/ubsan/nonnull-4.c.jj 2014-09-09 11:59:18.704180650 +0200 +++ gcc/testsuite/c-c++-common/ubsan/nonnull-4.c 2014-09-09 11:59:18.704180650 +0200 @@ -0,0 +1,34 @@ +/* { dg-do run } */ +/* { dg-shouldfail "ubsan" } */ +/* { dg-options "-fsanitize=undefined -fsanitize-undefined-trap-on-error" } */ + +int q, r; +void *a, *b, *c = (void *) &q, *d, *e, *f = (void *) &q, *g, *h; + +__attribute__((returns_nonnull, nonnull (1, 3))) +void * +foo (void *p, void *q, void *r) +{ + a = p; + b = r; + return q; +} + +int +bar (const void *a, const void *b) +{ + int c = *(const int *) a; + int d = *(const int *) b; + return c - d; +} + +int +main () +{ + asm volatile ("" : : : "memory"); + d = foo (c, b, c); + e = foo (e, c, f); + g = foo (c, f, g); + __builtin_memset (d, '\0', q); + return 0; +} --- gcc/testsuite/c-c++-common/ubsan/attrib-3.c.jj 2014-09-09 11:59:18.703180655 +0200 +++ gcc/testsuite/c-c++-common/ubsan/attrib-3.c 2014-09-09 11:59:18.703180655 +0200 @@ -0,0 +1,23 @@ +/* { dg-do compile } */ +/* { dg-options "-fsanitize=undefined" } */ + +/* Test that we don't instrument functions marked with + no_sanitize_undefined attribute. */ + +__attribute__((no_sanitize_undefined, returns_nonnull)) +char * +foo (char *x) +{ + return x; +} + +__attribute__((nonnull)) void bar (char *, int, char *); + +__attribute__((no_sanitize_undefined)) +void +baz (char *x, int y, char *z) +{ + bar (x, y, z); +} + +/* { dg-final { scan-assembler-not "__ubsan_handle" } } */ --- gcc/testsuite/c-c++-common/ubsan/nonnull-5.c.jj 2014-09-09 11:59:18.704180650 +0200 +++ gcc/testsuite/c-c++-common/ubsan/nonnull-5.c 2014-09-09 11:59:18.704180650 +0200 @@ -0,0 +1,34 @@ +/* { dg-do run } */ +/* { dg-shouldfail "ubsan" } */ +/* { dg-options "-fsanitize=undefined -fsanitize-undefined-trap-on-error" } */ + +int q, r; +void *a, *b, *c = (void *) &q, *d, *e, *f = (void *) &q, *g, *h; + +__attribute__((returns_nonnull, nonnull (1, 3))) +void * +foo (void *p, void *q, void *r) +{ + a = p; + b = r; + return q; +} + +int +bar (const void *a, const void *b) +{ + int c = *(const int *) a; + int d = *(const int *) b; + return c - d; +} + +int +main () +{ + asm volatile ("" : : : "memory"); + d = foo (c, (void *) &r, c); + e = foo (e, c, f); + g = foo (c, f, g); + __builtin_memset (d, '\0', q); + return 0; +} --- gcc/testsuite/c-c++-common/ubsan/nonnull-1.c.jj 2014-09-09 11:59:18.703180655 +0200 +++ gcc/testsuite/c-c++-common/ubsan/nonnull-1.c 2014-09-09 14:55:26.778264963 +0200 @@ -0,0 +1,38 @@ +/* { dg-do run } */ +/* { dg-options "-fsanitize=nonnull-attribute,returns-nonnull-attribute" } */ + +int q, r; +void *a, *b, *c = (void *) &q, *d, *e, *f = (void *) &q, *g, *h; + +__attribute__((returns_nonnull, nonnull (1, 3))) +void * +foo (void *p, void *q, void *r) +{ + a = p; + b = r; + return q; +} + +int +bar (const void *a, const void *b) +{ + int c = *(const int *) a; + int d = *(const int *) b; + return c - d; +} + +int +main () +{ + asm volatile ("" : : : "memory"); + d = foo (c, b, c); + e = foo (e, c, f); + g = foo (c, f, g); + __builtin_memset (d, '\0', q); + return 0; +} + +/* { dg-output "\.c:13:\[0-9]*:\[^\n\r]*null pointer returned from function declared to never return null\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\.c:29:\[0-9]*:\[^\n\r]*null pointer passed as argument 1, which is declared to never be null\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\.c:30:\[0-9]*:\[^\n\r]*null pointer passed as argument 3, which is declared to never be null\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*\.c:31:\[0-9]*:\[^\n\r]*null pointer passed as argument 1, which is declared to never be null" } */ --- gcc/testsuite/c-c++-common/ubsan/nonnull-2.c.jj 2014-09-09 11:59:18.703180655 +0200 +++ gcc/testsuite/c-c++-common/ubsan/nonnull-2.c 2014-09-09 14:54:11.202642791 +0200 @@ -0,0 +1,36 @@ +/* { dg-do run } */ +/* { dg-shouldfail "ubsan" } */ +/* { dg-options "-fsanitize=undefined -fno-sanitize-recover" } */ + +int q, r; +void *a, *b, *c = (void *) &q, *d, *e, *f = (void *) &q, *g, *h; + +__attribute__((returns_nonnull, nonnull (1, 3))) +void * +foo (void *p, void *q, void *r) +{ + a = p; + b = r; + return q; +} + +int +bar (const void *a, const void *b) +{ + int c = *(const int *) a; + int d = *(const int *) b; + return c - d; +} + +int +main () +{ + asm volatile ("" : : : "memory"); + d = foo (c, b, c); + e = foo (e, c, f); + g = foo (c, f, g); + __builtin_memset (d, '\0', q); + return 0; +} + +/* { dg-output "\.c:14:\[0-9]*:\[^\n\r]*null pointer returned from function declared to never return null" } */ --- gcc/testsuite/c-c++-common/ubsan/nonnull-3.c.jj 2014-09-09 11:59:18.704180650 +0200 +++ gcc/testsuite/c-c++-common/ubsan/nonnull-3.c 2014-09-09 14:55:45.110173336 +0200 @@ -0,0 +1,36 @@ +/* { dg-do run } */ +/* { dg-shouldfail "ubsan" } */ +/* { dg-options "-fsanitize=undefined -fno-sanitize-recover" } */ + +int q, r; +void *a, *b, *c = (void *) &q, *d, *e, *f = (void *) &q, *g, *h; + +__attribute__((returns_nonnull, nonnull (1, 3))) +void * +foo (void *p, void *q, void *r) +{ + a = p; + b = r; + return q; +} + +int +bar (const void *a, const void *b) +{ + int c = *(const int *) a; + int d = *(const int *) b; + return c - d; +} + +int +main () +{ + asm volatile ("" : : : "memory"); + d = foo (c, (void *) &r, c); + e = foo (e, c, f); + g = foo (c, f, g); + __builtin_memset (d, '\0', q); + return 0; +} + +/* { dg-output "\.c:30:\[0-9]*:\[^\n\r]*null pointer passed as argument 1, which is declared to never be null" } */ --- gcc/opts.c.jj 2014-09-08 22:12:52.797617097 +0200 +++ gcc/opts.c 2014-09-09 13:41:10.725662283 +0200 @@ -1499,6 +1499,11 @@ common_handle_option (struct gcc_options sizeof "float-cast-overflow" - 1 }, { "bounds", SANITIZE_BOUNDS, sizeof "bounds" - 1 }, { "alignment", SANITIZE_ALIGNMENT, sizeof "alignment" - 1 }, + { "nonnull-attribute", SANITIZE_NONNULL_ATTRIBUTE, + sizeof "nonnull-attribute" - 1 }, + { "returns-nonnull-attribute", + SANITIZE_RETURNS_NONNULL_ATTRIBUTE, + sizeof "returns-nonnull-attribute" - 1 }, { NULL, 0, 0 } }; const char *comma; @@ -1542,7 +1547,8 @@ common_handle_option (struct gcc_options /* When instrumenting the pointers, we don't want to remove the null pointer checks. */ - if (flag_sanitize & SANITIZE_NULL) + if (flag_sanitize & (SANITIZE_NULL | SANITIZE_NONNULL_ATTRIBUTE + | SANITIZE_RETURNS_NONNULL_ATTRIBUTE)) opts->x_flag_delete_null_pointer_checks = 0; /* Kernel ASan implies normal ASan but does not yet support --- libsanitizer/ubsan/ubsan_handlers.cc.jj 2014-06-30 16:15:11.139941236 +0200 +++ libsanitizer/ubsan/ubsan_handlers.cc 2014-09-09 13:18:23.449511672 +0200 @@ -277,3 +277,43 @@ void __ubsan::__ubsan_handle_function_ty __ubsan_handle_function_type_mismatch(Data, Function); Die(); } + +static void handleNonnullReturn(NonNullReturnData *Data) { + SourceLocation Loc = Data->Loc.acquire(); + if (Loc.isDisabled()) + return; + + Diag(Loc, DL_Error, "null pointer returned from function declared to never " + "return null"); + if (!Data->AttrLoc.isInvalid()) + Diag(Data->AttrLoc, DL_Note, "returns_nonnull attribute specified here"); +} + +void __ubsan::__ubsan_handle_nonnull_return(NonNullReturnData *Data) { + handleNonnullReturn(Data); +} + +void __ubsan::__ubsan_handle_nonnull_return_abort(NonNullReturnData *Data) { + handleNonnullReturn(Data); + Die(); +} + +static void handleNonNullArg(NonNullArgData *Data) { + SourceLocation Loc = Data->Loc.acquire(); + if (Loc.isDisabled()) + return; + + Diag(Loc, DL_Error, "null pointer passed as argument %0, which is declared to " + "never be null") << Data->ArgIndex; + if (!Data->AttrLoc.isInvalid()) + Diag(Data->AttrLoc, DL_Note, "nonnull attribute specified here"); +} + +void __ubsan::__ubsan_handle_nonnull_arg(NonNullArgData *Data) { + handleNonNullArg(Data); +} + +void __ubsan::__ubsan_handle_nonnull_arg_abort(NonNullArgData *Data) { + handleNonNullArg(Data); + Die(); +} --- libsanitizer/ubsan/ubsan_handlers.h.jj 2014-06-30 16:15:11.128941292 +0200 +++ libsanitizer/ubsan/ubsan_handlers.h 2014-09-09 13:48:42.686387487 +0200 @@ -119,6 +119,23 @@ RECOVERABLE(function_type_mismatch, FunctionTypeMismatchData *Data, ValueHandle Val) +struct NonNullReturnData { + SourceLocation Loc; + SourceLocation AttrLoc; +}; + +/// \brief Handle returning null from function with returns_nonnull attribute. +RECOVERABLE(nonnull_return, NonNullReturnData *Data) + +struct NonNullArgData { + SourceLocation Loc; + SourceLocation AttrLoc; + int ArgIndex; +}; + +/// \brief Handle passing null pointer to function with nonnull attribute. +RECOVERABLE(nonnull_arg, NonNullArgData *Data) + } #endif // UBSAN_HANDLERS_H Jakub