[RFC PATCH] -fsanitize=nonnull and -fsanitize=returns-nonnull support
Jakub Jelinek
jakub@redhat.com
Fri Jun 27 07:13:00 GMT 2014
Hi!
This patch implements sanitization for nonnull and returns_nonnull
attributes.
Similarly to -fsanitize=null this option disables
-fdelete-null-pointer-checks silently, and adds checks before calling
functions with declared nonnull arguments if we don't pass NULL there,
and before return in functions declared with returns_nonnull also inserts
check whether the return value is not NULL.
As GCC 4.9.0+ now aggressively optimizes based on these attributes and we've
seen several issues in real world apps, I think this is really needed.
Bootstrapped/regtested on x86_64-linux and i686-linux normally (yes,rtl
checking) and c,c++,fortran release checking bootstrap-ubsan.
Will post bugs in gcc discovered by it later.
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*).
2014-06-27 Jakub Jelinek <jakub@redhat.com>
* flag-types.h (enum sanitize_code): Add SANITIZE_NONNULL and
SANITIZE_RETURNS_NONNULL, or them into SANITIZE_UNDEFINED.
* opts.c (common_handle_option): Handle SANITIZE_NONNULL and
SANITIZE_RETURNS_NONNULL 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
or SANITIZE_RETURNS_NONNULL.
(pass_ubsan::execute): Call instrument_nonnull_{arg,return}.
* 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.
--- gcc/flag-types.h.jj 2014-06-20 23:26:24.000000000 +0200
+++ gcc/flag-types.h 2014-06-26 14:39:44.133970103 +0200
@@ -231,10 +231,13 @@ enum sanitize_code {
SANITIZE_FLOAT_DIVIDE = 1 << 12,
SANITIZE_FLOAT_CAST = 1 << 13,
SANITIZE_BOUNDS = 1 << 14,
+ SANITIZE_NONNULL = 1 << 15,
+ SANITIZE_RETURNS_NONNULL = 1 << 16,
SANITIZE_UNDEFINED = SANITIZE_SHIFT | SANITIZE_DIVIDE | SANITIZE_UNREACHABLE
| SANITIZE_VLA | SANITIZE_NULL | SANITIZE_RETURN
| SANITIZE_SI_OVERFLOW | SANITIZE_BOOL | SANITIZE_ENUM
- | SANITIZE_BOUNDS,
+ | SANITIZE_BOUNDS | SANITIZE_NONNULL
+ | SANITIZE_RETURNS_NONNULL,
SANITIZE_NONDEFAULT = SANITIZE_FLOAT_DIVIDE | SANITIZE_FLOAT_CAST
};
--- gcc/opts.c.jj 2014-06-20 23:26:23.000000000 +0200
+++ gcc/opts.c 2014-06-26 14:43:51.620652007 +0200
@@ -1468,6 +1468,9 @@ common_handle_option (struct gcc_options
{ "float-cast-overflow", SANITIZE_FLOAT_CAST,
sizeof "float-cast-overflow" - 1 },
{ "bounds", SANITIZE_BOUNDS, sizeof "bounds" - 1 },
+ { "nonnull", SANITIZE_NONNULL, sizeof "nonnull" - 1 },
+ { "returns-nonnull", SANITIZE_RETURNS_NONNULL,
+ sizeof "returns-nonnull" - 1 },
{ NULL, 0, 0 }
};
const char *comma;
@@ -1511,7 +1514,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
+ | SANITIZE_RETURNS_NONNULL))
opts->x_flag_delete_null_pointer_checks = 0;
break;
}
--- gcc/sanitizer.def.jj 2014-06-20 23:26:23.000000000 +0200
+++ gcc/sanitizer.def 2014-06-26 16:52:34.644801331 +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/ubsan.c.jj 2014-06-20 23:26:24.000000000 +0200
+++ gcc/ubsan.c 2014-06-26 18:23:28.190191706 +0200
@@ -997,6 +997,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
@@ -1121,6 +1122,117 @@ 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 = gimple_location (stmt);
+ /* 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;
+ 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);
+ 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);
+ 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",
+ &loc, NULL, NULL_TREE);
+ data = build_fold_addr_expr_loc (loc, 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, 2, data,
+ build_int_cst (pointer_sized_int_node,
+ i + 1));
+ }
+ gimple_set_location (g, loc);
+ 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 = gimple_location (stmt);
+ 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;
+ 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);
+ 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",
+ &loc, NULL, NULL_TREE);
+ data = build_fold_addr_expr_loc (loc, 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);
+ 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 =
@@ -1148,7 +1260,8 @@ public:
virtual bool gate (function *)
{
return flag_sanitize & (SANITIZE_NULL | SANITIZE_SI_OVERFLOW
- | SANITIZE_BOOL | SANITIZE_ENUM);
+ | SANITIZE_BOOL | SANITIZE_ENUM
+ | SANITIZE_NONNULL | SANITIZE_RETURNS_NONNULL);
}
virtual unsigned int execute (function *);
@@ -1188,7 +1301,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)
+ && is_gimple_call (stmt)
+ && !gimple_call_internal_p (stmt))
+ {
+ instrument_nonnull_arg (&gsi);
+ bb = gimple_bb (stmt);
+ }
+
+ if ((flag_sanitize & SANITIZE_RETURNS_NONNULL)
+ && gimple_code (stmt) == GIMPLE_RETURN)
+ {
+ instrument_nonnull_return (&gsi);
+ bb = gimple_bb (stmt);
+ }
gsi_next (&gsi);
}
--- gcc/testsuite/c-c++-common/ubsan/nonnull-1.c.jj 2014-06-26 18:19:27.655457620 +0200
+++ gcc/testsuite/c-c++-common/ubsan/nonnull-1.c 2014-06-26 18:27:22.038955861 +0200
@@ -0,0 +1,38 @@
+/* { dg-do run } */
+/* { dg-options "-fsanitize=nonnull,returns-nonnull" } */
+
+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 return value where non-null required\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*\.c:29:\[0-9]*:\[^\n\r]*null argument where non-null required .argument 1.\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*\.c:30:\[0-9]*:\[^\n\r]*null argument where non-null required .argument 3.\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*\.c:31:\[0-9]*:\[^\n\r]*null argument where non-null required .argument 1." } */
--- gcc/testsuite/c-c++-common/ubsan/nonnull-2.c.jj 2014-06-26 18:28:36.151565589 +0200
+++ gcc/testsuite/c-c++-common/ubsan/nonnull-2.c 2014-06-26 18:29:36.392248392 +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 return value where non-null required" } */
--- gcc/testsuite/c-c++-common/ubsan/nonnull-3.c.jj 2014-06-26 18:29:50.883172086 +0200
+++ gcc/testsuite/c-c++-common/ubsan/nonnull-3.c 2014-06-26 18:31:39.152601753 +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 argument where non-null required .argument 1." } */
--- gcc/testsuite/c-c++-common/ubsan/nonnull-4.c.jj 2014-06-26 18:31:49.118552359 +0200
+++ gcc/testsuite/c-c++-common/ubsan/nonnull-4.c 2014-06-26 18:32:13.526423944 +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/nonnull-5.c.jj 2014-06-26 18:32:23.284367076 +0200
+++ gcc/testsuite/c-c++-common/ubsan/nonnull-5.c 2014-06-26 18:33:42.154968619 +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;
+}
--- libsanitizer/ubsan/ubsan_handlers.cc.jj 2013-12-05 12:04:32.000000000 +0100
+++ libsanitizer/ubsan/ubsan_handlers.cc 2014-06-26 17:03:48.018251367 +0200
@@ -277,3 +277,31 @@ void __ubsan::__ubsan_handle_function_ty
__ubsan_handle_function_type_mismatch(Data, Function);
Die();
}
+
+void __ubsan::__ubsan_handle_nonnull_arg(NonNullArgData *Data, uptr ArgNo) {
+ SourceLocation Loc = Data->Loc.acquire();
+ if (Loc.isDisabled())
+ return;
+
+ Diag(Loc, DL_Error, "null argument where non-null required "
+ "(argument %0)") << ArgNo;
+}
+
+void __ubsan::__ubsan_handle_nonnull_arg_abort(NonNullArgData *Data,
+ uptr ArgNo) {
+ __ubsan::__ubsan_handle_nonnull_arg(Data, ArgNo);
+ Die();
+}
+
+void __ubsan::__ubsan_handle_nonnull_return(NonNullRetData *Data) {
+ SourceLocation Loc = Data->Loc.acquire();
+ if (Loc.isDisabled())
+ return;
+
+ Diag(Loc, DL_Error, "null return value where non-null required");
+}
+
+void __ubsan::__ubsan_handle_nonnull_return_abort(NonNullRetData *Data) {
+ __ubsan::__ubsan_handle_nonnull_return(Data);
+ Die();
+}
--- libsanitizer/ubsan/ubsan_handlers.h.jj 2013-12-05 12:04:32.000000000 +0100
+++ libsanitizer/ubsan/ubsan_handlers.h 2014-06-26 17:02:03.330803480 +0200
@@ -119,6 +119,20 @@ RECOVERABLE(function_type_mismatch,
FunctionTypeMismatchData *Data,
ValueHandle Val)
+struct NonNullArgData {
+ SourceLocation Loc;
+};
+
+/// \brief Handle passing null to function argument with nonnull attribute.
+RECOVERABLE(nonnull_arg, NonNullArgData *Data, uptr ArgNo)
+
+struct NonNullRetData {
+ SourceLocation Loc;
+};
+
+/// \brief Handle returning null from function with returns_nonnull attribute.
+RECOVERABLE(nonnull_return, NonNullRetData *Data)
+
}
#endif // UBSAN_HANDLERS_H
Jakub
More information about the Gcc-patches
mailing list