From patchwork Wed Nov 5 21:37:46 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 407179 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 8A68114007D for ; Thu, 6 Nov 2014 08:46:13 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:subject:message-id:mime-version:content-type; q=dns; s= default; b=b8PYGoEBTjlUXOOQwLOViTlAZQHrVflICi8eQ53gBCbybSCyF09i4 wEwvA8TEhCV+r0IvMER6xfwl5wQS7eq4FKJmWd23Pj/mA7zjhU/AWRagF+yqxDJS oECIZmct9YmVPY3PDbMa2gtQMFWnmMvsL9Imi9ukZip7sqG44GtP1g= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:subject:message-id:mime-version:content-type; s= default; bh=42eekRyg0fwe9dmya4wJoWIgz3g=; b=w8/EjnPnwRz08r7QGezk SPCJn+qZLv8Yv8GaiBkD9XLGDlfL7imZnH03qM+cDoq/CC3zrkFmsWkTq8q9JKQY gf4zaGnm0Ih+4ma7Fqn4X5kfG6KFosoLI/hTifMc6tEIWllM7+Xw2PILPOGyWZbB q1+jfEi52PZSUqVimONaHUw= Received: (qmail 25307 invoked by alias); 5 Nov 2014 21:46:03 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 25216 invoked by uid 89); 5 Nov 2014 21:46:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.8 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 05 Nov 2014 21:45:58 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sA5LjufX027361 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Wed, 5 Nov 2014 16:45:57 -0500 Received: from redhat.com (ovpn-116-30.ams2.redhat.com [10.36.116.30]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sA5LbkfB015473 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Wed, 5 Nov 2014 16:37:48 -0500 Date: Wed, 5 Nov 2014 22:37:46 +0100 From: Marek Polacek To: GCC Patches , Jakub Jelinek Subject: [PATCH] Limit removal of UBSAN_NULL checks Message-ID: <20141105213745.GZ20462@redhat.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.23 (2014-03-12) This patch limits optimizing UBSAN_NULL checks a bit, so that we don't lose diagnostics. If we know we're on a path where segv would have already occured, we can drop further checks (e.g. dereferencing a NULL pointer - we can't recover from that). Also if we're not recovering or using -fsanitize-undefined-trap-on-error we can clear checks that are dominated. Furthermore, if we have more checks for the same location, there's no point in keeping all of them. Bootstrap-ubsan/regtest passed on x86_64-linux, ok for trunk? 2014-11-05 Marek Polacek * sanopt.c (sanopt_optimize_walker): Limit removal of the checks. testsuite/ * c-c++-common/ubsan/align-2.c: Add dg-output. * c-c++-common/ubsan/align-4.c: Likewise. * c-c++-common/ubsan/align-6.c: New test. * c-c++-common/ubsan/align-7.c: New test. * c-c++-common/ubsan/align-8.c: New test. * g++.dg/ubsan/null-1.C: Add dg-output. * g++.dg/ubsan/null-2.C: Likewise. * g++.dg/ubsan/null-3.C: New test. * g++.dg/ubsan/null-4.C: New test. * g++.dg/ubsan/null-5.C: New test. Marek diff --git gcc/sanopt.c gcc/sanopt.c index 4483ff7..cb172d3 100644 --- gcc/sanopt.c +++ gcc/sanopt.c @@ -130,7 +130,27 @@ sanopt_optimize_walker (basic_block bb, struct sanopt_ctx *ctx) /* At this point we shouldn't have any statements that aren't dominating the current BB. */ tree align = gimple_call_arg (g, 2); - remove = tree_int_cst_le (cur_align, align); + int kind = tree_to_shwi (gimple_call_arg (g, 1)); + /* If this is a NULL pointer check where we had segv + anyway, we can remove it. */ + if (integer_zerop (align) + && (kind == UBSAN_LOAD_OF + || kind == UBSAN_STORE_OF + || kind == UBSAN_MEMBER_ACCESS)) + remove = true; + /* Otherwise remove the check in non-recovering + mode, or if the stmts have same location. */ + else if (integer_zerop (align)) + remove = !(flag_sanitize_recover & SANITIZE_NULL) + || flag_sanitize_undefined_trap_on_error + || gimple_location (g) + == gimple_location (stmt); + else if (tree_int_cst_le (cur_align, align)) + remove = !(flag_sanitize_recover + & SANITIZE_ALIGNMENT) + || flag_sanitize_undefined_trap_on_error + || gimple_location (g) + == gimple_location (stmt); break; } } diff --git gcc/testsuite/c-c++-common/ubsan/align-2.c gcc/testsuite/c-c++-common/ubsan/align-2.c index 02a26e2..071de8c 100644 --- gcc/testsuite/c-c++-common/ubsan/align-2.c +++ gcc/testsuite/c-c++-common/ubsan/align-2.c @@ -51,4 +51,6 @@ main () /* { dg-output "\.c:(13|16):\[0-9]*: \[^\n\r]*store to misaligned address 0x\[0-9a-fA-F]* for type 'int', which requires 4 byte alignment.*" } */ /* { dg-output "\.c:23:\[0-9]*: \[^\n\r]*member access within misaligned address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte alignment.*" } */ /* { dg-output "\.c:(29|30):\[0-9]*: \[^\n\r]*member access within misaligned address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte alignment.*" } */ +/* { dg-output "\.c:30:\[0-9]*: \[^\n\r]*member access within misaligned address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte alignment.*" } */ +/* { dg-output "\.c:31:\[0-9]*: \[^\n\r]*member access within misaligned address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte alignment.*" } */ /* { dg-output "\.c:37:\[0-9]*: \[^\n\r]*load of misaligned address 0x\[0-9a-fA-F]* for type 'long long int', which requires \[48] byte alignment" } */ diff --git gcc/testsuite/c-c++-common/ubsan/align-4.c gcc/testsuite/c-c++-common/ubsan/align-4.c index f010589..3252595 100644 --- gcc/testsuite/c-c++-common/ubsan/align-4.c +++ gcc/testsuite/c-c++-common/ubsan/align-4.c @@ -9,4 +9,6 @@ /* { dg-output "\[^\n\r]*\.c:(13|16):\[0-9]*: \[^\n\r]*store to misaligned address 0x\[0-9a-fA-F]* for type 'int', which requires 4 byte alignment.*" } */ /* { dg-output "\[^\n\r]*\.c:23:\[0-9]*: \[^\n\r]*member access within misaligned address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte alignment.*" } */ /* { dg-output "\[^\n\r]*\.c:(29|30):\[0-9]*: \[^\n\r]*member access within misaligned address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte alignment.*" } */ +/* { dg-output "\[^\n\r]*\.c:30:\[0-9]*: \[^\n\r]*member access within misaligned address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte alignment.*" } */ +/* { dg-output "\[^\n\r]*\.c:31:\[0-9]*: \[^\n\r]*member access within misaligned address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte alignment.*" } */ /* { dg-output "\[^\n\r]*\.c:37:\[0-9]*: \[^\n\r]*load of misaligned address 0x\[0-9a-fA-F]* for type 'long long int', which requires \[48] byte alignment" } */ diff --git gcc/testsuite/c-c++-common/ubsan/align-6.c gcc/testsuite/c-c++-common/ubsan/align-6.c index e69de29..5521292 100644 --- gcc/testsuite/c-c++-common/ubsan/align-6.c +++ gcc/testsuite/c-c++-common/ubsan/align-6.c @@ -0,0 +1,33 @@ +/* Limit this to known non-strict alignment targets. */ +/* { dg-do run { target { i?86-*-linux* x86_64-*-linux* } } } */ +/* { dg-options "-O -fsanitize=alignment -fsanitize-recover=alignment" } */ + +struct S { int a; char b; long long c; short d[10]; }; +struct T { char a; long long b; }; +struct U { char a; int b; int c; long long d; struct S e; struct T f; } __attribute__((packed)); +struct V { long long a; struct S b; struct T c; struct U u; } v; + +__attribute__((noinline, noclone)) int +foo (struct S *p) +{ + volatile int i; + i = p->a; + i = p->a; + i = p->a; + i = p->a; + return p->a; +} + +int +main () +{ + if (foo (&v.u.e)) + __builtin_abort (); + return 0; +} + +/* { dg-output "\.c:14:\[0-9]*: \[^\n\r]*member access within misaligned address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte alignment.*" } */ +/* { dg-output "\.c:15:\[0-9]*: \[^\n\r]*member access within misaligned address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte alignment.*" } */ +/* { dg-output "\.c:16:\[0-9]*: \[^\n\r]*member access within misaligned address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte alignment.*" } */ +/* { dg-output "\.c:17:\[0-9]*: \[^\n\r]*member access within misaligned address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte alignment.*" } */ +/* { dg-output "\.c:18:\[0-9]*: \[^\n\r]*member access within misaligned address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte alignment.*" } */ diff --git gcc/testsuite/c-c++-common/ubsan/align-7.c gcc/testsuite/c-c++-common/ubsan/align-7.c index e69de29..4a18d8d 100644 --- gcc/testsuite/c-c++-common/ubsan/align-7.c +++ gcc/testsuite/c-c++-common/ubsan/align-7.c @@ -0,0 +1,32 @@ +/* Limit this to known non-strict alignment targets. */ +/* { dg-do run { target { i?86-*-linux* x86_64-*-linux* } } } */ +/* { dg-options "-O -fsanitize=alignment -fno-sanitize-recover=alignment -fdump-tree-sanopt-details" } */ +/* { dg-shouldfail "ubsan" } */ + +struct S { int a; char b; long long c; short d[10]; }; +struct T { char a; long long b; }; +struct U { char a; int b; int c; long long d; struct S e; struct T f; } __attribute__((packed)); +struct V { long long a; struct S b; struct T c; struct U u; } v; + +__attribute__((noinline, noclone)) int +foo (struct S *p) +{ + volatile int i; + i = p->a; + i = p->a; + i = p->a; + i = p->a; + return p->a; +} + +int +main () +{ + if (foo (&v.u.e)) + __builtin_abort (); + return 0; +} + +/* { dg-output "\.c:15:\[0-9]*: \[^\n\r]*member access within misaligned address 0x\[0-9a-fA-F]* for type 'struct S', which requires \[48] byte alignment.*" } */ +/* { dg-final { scan-tree-dump-times "Optimizing" 4 "sanopt"} } */ +/* { dg-final { cleanup-tree-dump "sanopt" } } */ diff --git gcc/testsuite/c-c++-common/ubsan/align-8.c gcc/testsuite/c-c++-common/ubsan/align-8.c index e69de29..b930162 100644 --- gcc/testsuite/c-c++-common/ubsan/align-8.c +++ gcc/testsuite/c-c++-common/ubsan/align-8.c @@ -0,0 +1,31 @@ +/* Limit this to known non-strict alignment targets. */ +/* { dg-do run { target { i?86-*-linux* x86_64-*-linux* } } } */ +/* { dg-options "-O -fsanitize=alignment -fsanitize-undefined-trap-on-error -fdump-tree-sanopt-details" } */ +/* { dg-shouldfail "ubsan" } */ + +struct S { int a; char b; long long c; short d[10]; }; +struct T { char a; long long b; }; +struct U { char a; int b; int c; long long d; struct S e; struct T f; } __attribute__((packed)); +struct V { long long a; struct S b; struct T c; struct U u; } v; + +__attribute__((noinline, noclone)) int +foo (struct S *p) +{ + volatile int i; + i = p->a; + i = p->a; + i = p->a; + i = p->a; + return p->a; +} + +int +main () +{ + if (foo (&v.u.e)) + __builtin_abort (); + return 0; +} + +/* { dg-final { scan-tree-dump-times "Optimizing" 4 "sanopt"} } */ +/* { dg-final { cleanup-tree-dump "sanopt" } } */ diff --git gcc/testsuite/g++.dg/ubsan/null-1.C gcc/testsuite/g++.dg/ubsan/null-1.C index 83b3033..e1524b1 100644 --- gcc/testsuite/g++.dg/ubsan/null-1.C +++ gcc/testsuite/g++.dg/ubsan/null-1.C @@ -25,4 +25,6 @@ main (void) } // { dg-output "reference binding to null pointer of type 'int'(\n|\r\n|\r)" } +// { dg-output "\[^\n\r]*reference binding to null pointer of type 'int'(\n|\r\n|\r)" } // { dg-output "\[^\n\r]*reference binding to null pointer of type 'const L'(\n|\r\n|\r)" } +// { dg-output "\[^\n\r]*reference binding to null pointer of type 'int'(\n|\r\n|\r)" } diff --git gcc/testsuite/g++.dg/ubsan/null-2.C gcc/testsuite/g++.dg/ubsan/null-2.C index 0230c7c..88f387e 100644 --- gcc/testsuite/g++.dg/ubsan/null-2.C +++ gcc/testsuite/g++.dg/ubsan/null-2.C @@ -35,3 +35,5 @@ main (void) // { dg-output "\.C:26:\[0-9]*:\[\^\n\r]*member call on null pointer of type 'struct U'.*" } // { dg-output "\.C:29:\[0-9]*:\[\^\n\r]*member call on null pointer of type 'struct V'.*" } +// { dg-output "\.C:31:\[0-9]*:\[\^\n\r]*member call on null pointer of type 'struct V'.*" } +// { dg-output "\.C:33:\[0-9]*:\[\^\n\r]*member call on null pointer of type 'struct V'" } diff --git gcc/testsuite/g++.dg/ubsan/null-3.C gcc/testsuite/g++.dg/ubsan/null-3.C index e69de29..cd3716b 100644 --- gcc/testsuite/g++.dg/ubsan/null-3.C +++ gcc/testsuite/g++.dg/ubsan/null-3.C @@ -0,0 +1,20 @@ +// { dg-do run } +// { dg-options "-fsanitize=null" } + +int +main (void) +{ + int *p = 0; + + int &r1 = *p; + int &r2 = *p; + int &r3 = *p; + int &r4 = *p; + int &r5 = *p; +} + +// { dg-output "reference binding to null pointer of type 'int'(\n|\r\n|\r)" } +// { dg-output "\[^\n\r]*reference binding to null pointer of type 'int'(\n|\r\n|\r)" } +// { dg-output "\[^\n\r]*reference binding to null pointer of type 'int'(\n|\r\n|\r)" } +// { dg-output "\[^\n\r]*reference binding to null pointer of type 'int'(\n|\r\n|\r)" } +// { dg-output "\[^\n\r]*reference binding to null pointer of type 'int'(\n|\r\n|\r)" } diff --git gcc/testsuite/g++.dg/ubsan/null-4.C gcc/testsuite/g++.dg/ubsan/null-4.C index e69de29..9cb04ef 100644 --- gcc/testsuite/g++.dg/ubsan/null-4.C +++ gcc/testsuite/g++.dg/ubsan/null-4.C @@ -0,0 +1,19 @@ +// { dg-do run } +// { dg-options "-O -fsanitize=null -fno-sanitize-recover=null -fdump-tree-sanopt-details" } +// { dg-shouldfail "ubsan" } + +int +main (void) +{ + int *p = 0; + + int &r1 = *p; + int &r2 = *p; + int &r3 = *p; + int &r4 = *p; + int &r5 = *p; +} + +// { dg-output "reference binding to null pointer of type 'int'(\n|\r\n|\r)" } +// { dg-final { scan-tree-dump-times "Optimizing" 4 "sanopt"} } +// { dg-final { cleanup-tree-dump "sanopt" } } diff --git gcc/testsuite/g++.dg/ubsan/null-5.C gcc/testsuite/g++.dg/ubsan/null-5.C index e69de29..d8e4a68 100644 --- gcc/testsuite/g++.dg/ubsan/null-5.C +++ gcc/testsuite/g++.dg/ubsan/null-5.C @@ -0,0 +1,18 @@ +// { dg-do run } +// { dg-options "-O -fsanitize=null -fsanitize-undefined-trap-on-error -fdump-tree-sanopt-details" } +// { dg-shouldfail "ubsan" } + +int +main (void) +{ + int *p = 0; + + int &r1 = *p; + int &r2 = *p; + int &r3 = *p; + int &r4 = *p; + int &r5 = *p; +} + +// { dg-final { scan-tree-dump-times "Optimizing" 4 "sanopt"} } +// { dg-final { cleanup-tree-dump "sanopt" } }