From patchwork Wed Sep 18 13:10:42 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 275712 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 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 886102C00EE for ; Wed, 18 Sep 2013 23:10:58 +1000 (EST) 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:cc:subject:message-id:mime-version:content-type; q=dns; s=default; b=nnncdTX3CIDREwCgLT85VKMR4JWovIo72V456z2vWLEsL//7iT ALjmmuBUI6gt1H5M5Um0M9lmPIlNOHN2ymNNq4NTFD3DJmhtHaH01TUyk9zy4h/k KlLgwt3FU1Po2wnrmJXjtM4OD6hjUDUMfXuQQakO7cduPFCuIGNJAxB4A= 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:cc:subject:message-id:mime-version:content-type; s= default; bh=xDze9/KYUC3QLH/oV3IS9P2+QJ8=; b=KgFB3VexX6awZp+k8xhX ohUL3rB+OqHNd1m5YvBDmHOI4PoaQF7B8KO2zADK852LetP0I8cQ5ejc3akynFkf SAqH28SZcO/1sbjMBoPqMCBN3JSl55TyVtclHn7/Qx8q2WbtxaoXVQ0iTOW3RIrd KajcAPqqSDEyf0F3Mb0/1mA= Received: (qmail 10805 invoked by alias); 18 Sep 2013 13:10:51 -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 10789 invoked by uid 89); 18 Sep 2013 13:10:50 -0000 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 18 Sep 2013 13:10:50 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.9 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r8IDAlVg030911 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 18 Sep 2013 09:10:47 -0400 Received: from redhat.com (ovpn-116-32.ams2.redhat.com [10.36.116.32]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r8IDAgIY016508 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Wed, 18 Sep 2013 09:10:45 -0400 Date: Wed, 18 Sep 2013 15:10:42 +0200 From: Marek Polacek To: GCC Patches Cc: Jakub Jelinek , Jason Merrill , "Joseph S. Myers" Subject: [PATCH] Properly honor -fsanitize options (PR sanitizer/58443) Message-ID: <20130918131042.GF15960@redhat.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-06-14) As mentioned in the PR, we weren't properly using the SANITIZE_* flags, which resulted in sanitizing shifts even though we only should sanitize divisions and vice versa. It also removed one unnecessary check; it's sufficient to check that instrument_expr != NULL. I also found a mistake in a test, it assumed that -fsanitize=shift also turns sanitizing of divisions. Thus fixed. Ran ubsan testsuite/bootstrap-ubsan on x86_64-linux, ok for trunk? 2013-09-18 Marek Polacek PR sanitize/58443 cp/ * typeck.c (cp_build_binary_op): Properly honor -fsanitize options. Remove unnecessary check. c/ * c-typeck.c (build_binary_op): Properly honor -fsanitize options. Remove unnecessary check. testsuite/ * g++.dg/ubsan/div-by-zero-1.C: Use the integer-divide-by-zero option instead of the shift option. * c-c++-common/ubsan/pr58443-1.c: New test. * c-c++-common/ubsan/pr58443-3.c: New test. * c-c++-common/ubsan/pr58443-2.c: New test. Marek --- gcc/cp/typeck.c.mp 2013-09-18 14:00:14.303869196 +0200 +++ gcc/cp/typeck.c 2013-09-18 14:08:21.287770112 +0200 @@ -4884,7 +4884,7 @@ cp_build_binary_op (location_t location, if (build_type == NULL_TREE) build_type = result_type; - if ((flag_sanitize & SANITIZE_UNDEFINED) + if ((flag_sanitize & SANITIZE_SHIFT || flag_sanitize & SANITIZE_DIVIDE) && !processing_template_decl && current_function_decl != 0 && !lookup_attribute ("no_sanitize_undefined", @@ -4898,7 +4898,7 @@ cp_build_binary_op (location_t location, tf_none)); op1 = maybe_constant_value (fold_non_dependent_expr_sfinae (op1, tf_none)); - if (doing_div_or_mod) + if (doing_div_or_mod && flag_sanitize & SANITIZE_DIVIDE) { /* For diagnostics we want to use the promoted types without shorten_binary_op. So convert the arguments to the @@ -4912,7 +4912,7 @@ cp_build_binary_op (location_t location, } instrument_expr = ubsan_instrument_division (location, cop0, cop1); } - else if (doing_shift) + else if (doing_shift && flag_sanitize & SANITIZE_SHIFT) instrument_expr = ubsan_instrument_shift (location, code, op0, op1); } @@ -4926,7 +4926,7 @@ cp_build_binary_op (location_t location, && !TREE_OVERFLOW_P (op1)) overflow_warning (location, result); - if ((flag_sanitize & SANITIZE_UNDEFINED) && instrument_expr != NULL) + if (instrument_expr != NULL) result = fold_build2 (COMPOUND_EXPR, TREE_TYPE (result), instrument_expr, result); --- gcc/c/c-typeck.c.mp 2013-09-18 14:00:06.704837584 +0200 +++ gcc/c/c-typeck.c 2013-09-18 14:04:57.372968880 +0200 @@ -10496,7 +10496,7 @@ build_binary_op (location_t location, en return error_mark_node; } - if (flag_sanitize & SANITIZE_UNDEFINED + if ((flag_sanitize & SANITIZE_SHIFT || flag_sanitize & SANITIZE_DIVIDE) && current_function_decl != 0 && !lookup_attribute ("no_sanitize_undefined", DECL_ATTRIBUTES (current_function_decl)) @@ -10507,9 +10507,9 @@ build_binary_op (location_t location, en op1 = c_save_expr (op1); op0 = c_fully_fold (op0, false, NULL); op1 = c_fully_fold (op1, false, NULL); - if (doing_div_or_mod) + if (doing_div_or_mod && flag_sanitize & SANITIZE_DIVIDE) instrument_expr = ubsan_instrument_division (location, op0, op1); - else if (doing_shift) + else if (doing_shift && flag_sanitize & SANITIZE_SHIFT) instrument_expr = ubsan_instrument_shift (location, code, op0, op1); } @@ -10537,7 +10537,7 @@ build_binary_op (location_t location, en ret = build1 (EXCESS_PRECISION_EXPR, semantic_result_type, ret); protected_set_expr_location (ret, location); - if ((flag_sanitize & SANITIZE_UNDEFINED) && instrument_expr != NULL) + if (instrument_expr != NULL) ret = fold_build2 (COMPOUND_EXPR, TREE_TYPE (ret), instrument_expr, ret); --- gcc/testsuite/g++.dg/ubsan/div-by-zero-1.C.mp 2013-09-18 14:24:42.735312422 +0200 +++ gcc/testsuite/g++.dg/ubsan/div-by-zero-1.C 2013-09-18 14:25:28.965491706 +0200 @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-fsanitize=shift -w" } */ +/* { dg-options "-fsanitize=integer-divide-by-zero -w" } */ void foo (int i) --- gcc/testsuite/c-c++-common/ubsan/pr58443-1.c.mp 2013-09-18 14:20:18.633348043 +0200 +++ gcc/testsuite/c-c++-common/ubsan/pr58443-1.c 2013-09-18 14:30:45.403701302 +0200 @@ -0,0 +1,11 @@ +/* PR sanitizer/58443 */ +/* { dg-do compile } */ +/* { dg-options "-fsanitize=shift,unreachable -w" } */ + +int +foo (int u, int o) +{ + return u / o; +} + +/* { dg-final { scan-assembler-not "__ubsan_handle_divrem_overflow" } } */ --- gcc/testsuite/c-c++-common/ubsan/pr58443-3.c.mp 2013-09-18 14:20:31.724398731 +0200 +++ gcc/testsuite/c-c++-common/ubsan/pr58443-3.c 2013-09-18 14:30:23.959623394 +0200 @@ -0,0 +1,18 @@ +/* PR sanitizer/58443 */ +/* { dg-do compile } */ +/* { dg-options "-fsanitize=undefined -w" } */ + +int +foo (int u, int o) +{ + return u >> o; +} + +int +bar (int u, int o) +{ + return u / o; +} + +/* { dg-final { scan-assembler "__ubsan_handle_divrem_overflow" } } */ +/* { dg-final { scan-assembler "__ubsan_handle_shift_out_of_bounds" } } */ --- gcc/testsuite/c-c++-common/ubsan/pr58443-2.c.mp 2013-09-18 14:20:27.692384073 +0200 +++ gcc/testsuite/c-c++-common/ubsan/pr58443-2.c 2013-09-18 14:28:17.272155763 +0200 @@ -0,0 +1,11 @@ +/* PR sanitizer/58443 */ +/* { dg-do compile } */ +/* { dg-options "-fsanitize=unreachable,integer-divide-by-zero -w" } */ + +int +foo (int u, int o) +{ + return u >> o; +} + +/* { dg-final { scan-assembler-not "__ubsan_handle_shift_out_of_bounds" } } */