This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[ubsan PATCH] -fsanitize=null fails to sanitize &s->i (PR sanitizer/80797)
- From: Marek Polacek <polacek at redhat dot com>
- To: GCC Patches <gcc-patches at gcc dot gnu dot org>, Jakub Jelinek <jakub at redhat dot com>
- Date: Wed, 17 May 2017 16:10:23 +0200
- Subject: [ubsan PATCH] -fsanitize=null fails to sanitize &s->i (PR sanitizer/80797)
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=polacek at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 59AE080C04
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 59AE080C04
We are failing to detect accessing a null pointer in &s->i because
v_3 = &s_2->i;
is not gimple_assign_load_p:
1997 if (flag_sanitize & (SANITIZE_NULL | SANITIZE_ALIGNMENT))
1998 {
1999 if (gimple_store_p (stmt))
2000 instrument_null (gsi, true);
2001 if (gimple_assign_load_p (stmt))
2002 instrument_null (gsi, false);
2003 }
So I think we can use gimple_assign_single_p instead of gimple_assign_load_p
and then strip the ADDR_EXPR in instrument_null before getting its base
address. (I've seen stripping ADDR_EXPR before get_base_address elsewhere in
the codebase, too.)
Bootstrapped-ubsan/regtested on x86_64-linux, ok for trunk?
2017-05-17 Marek Polacek <polacek@redhat.com>
PR sanitizer/80797
* ubsan.c (instrument_null): Unwrap ADDR_EXPRs.
(pass_ubsan::execute): Call gimple_assign_single_p instead of
gimple_assign_load_p.
* c-c++-common/ubsan/null-12.c: New test.
diff --git gcc/testsuite/c-c++-common/ubsan/null-12.c gcc/testsuite/c-c++-common/ubsan/null-12.c
index e69de29..3478dc1 100644
--- gcc/testsuite/c-c++-common/ubsan/null-12.c
+++ gcc/testsuite/c-c++-common/ubsan/null-12.c
@@ -0,0 +1,42 @@
+/* PR sanitizer/80797 */
+/* { dg-do run } */
+/* { dg-options "-fsanitize=undefined" } */
+
+struct S
+{
+ int i;
+};
+
+struct R
+{
+ struct T {
+ int i;
+ } *t;
+} r;
+
+int
+main ()
+{
+ struct S *s = 0;
+ struct S *s2[1] = { };
+
+ int *v1 = &s->i;
+ int *v2 = &(*s).i;
+ int *v3 = &s2[0]->i;
+ int *v4 = &s->i + 1;
+ int *v5 = &r.t->i;
+
+ asm ("" : : "r" (&v1) : "memory");
+ asm ("" : : "r" (&v2) : "memory");
+ asm ("" : : "r" (&v3) : "memory");
+ asm ("" : : "r" (&v4) : "memory");
+ asm ("" : : "r" (&v5) : "memory");
+
+ return 0;
+}
+
+/* { dg-output "member access within null pointer of type 'struct S'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*member access within null pointer of type 'struct S'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*member access within null pointer of type 'struct S'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*member access within null pointer of type 'struct S'\[^\n\r]*(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*member access within null pointer of type 'struct T'" } */
diff --git gcc/ubsan.c gcc/ubsan.c
index 4159cc5..a4808d2 100644
--- gcc/ubsan.c
+++ gcc/ubsan.c
@@ -1208,6 +1208,9 @@ instrument_null (gimple_stmt_iterator gsi, 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);
tree base = get_base_address (t);
const enum tree_code code = TREE_CODE (base);
if (code == MEM_REF
@@ -1998,7 +2001,7 @@ pass_ubsan::execute (function *fun)
{
if (gimple_store_p (stmt))
instrument_null (gsi, true);
- if (gimple_assign_load_p (stmt))
+ if (gimple_assign_single_p (stmt))
instrument_null (gsi, false);
}
Marek