From patchwork Mon Aug 5 11:24:52 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 264649 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 CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 117472C0092 for ; Mon, 5 Aug 2013 21:25:21 +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:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=YVDWjtF5jEzRXyTCP 1w7Cjw6cE1GMPtm/g/g1crxH10CkfbpE/RjdcBXpoJStmIpeIpNIJOeygb6w0oe9 8d+7EwdZj4+Sz9PyZeXlYVwWk9hFIFYzEsvkkj7YDhkGkQhGjUjjJ8q7STN959qW ZoBx8gcFeKqF5RqhOB6H/dKuD0= 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:references:mime-version :content-type:in-reply-to; s=default; bh=qw+FPsTIlIcGo4vLIZUmIIr Ybm4=; b=h49an1nsfW9VUmNVKuLiUGrviQqJrt5md+Brk/pk5JdbCQcQ7AG96S5 s25oKj1U8penL6Ljj8o+Tlgk/t8xTlYoC9HevmN6d7bGc3fAanFwe2j4l/ijXsku xvIXnHK8KtlA+8z/5U2DVE0ykG0ZZw6W+hAzzfGT3j1ghNPmjjtQ= Received: (qmail 18624 invoked by alias); 5 Aug 2013 11:25:06 -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 18561 invoked by uid 89); 5 Aug 2013 11:25:06 -0000 X-Spam-SWARE-Status: No, score=-5.3 required=5.0 tests=AWL, BAYES_00, RCVD_IN_HOSTKARMA_W, RCVD_IN_HOSTKARMA_WL, RDNS_NONE, SPF_HELO_PASS, SPF_PASS autolearn=no version=3.3.1 Received: from Unknown (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Mon, 05 Aug 2013 11:25:05 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r75BOukm013259 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 5 Aug 2013 07:24:57 -0400 Received: from redhat.com (ovpn-116-37.ams2.redhat.com [10.36.116.37]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r75BOq8C007714 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Mon, 5 Aug 2013 07:24:55 -0400 Date: Mon, 5 Aug 2013 13:24:52 +0200 From: Marek Polacek To: Jason Merrill Cc: GCC Patches , Jakub Jelinek , Jeff Law , "Joseph S. Myers" Subject: Re: Request to merge Undefined Behavior Sanitizer in (take 2) Message-ID: <20130805112452.GX17022@redhat.com> References: <20130725153227.GC32538@redhat.com> <51F186E9.9090404@redhat.com> <20130731173321.GT17022@redhat.com> <51F95CF7.8020605@redhat.com> <20130801180615.GU17022@redhat.com> <51FD2EC0.4060607@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <51FD2EC0.4060607@redhat.com> User-Agent: Mutt/1.5.20 (2009-06-14) On Sat, Aug 03, 2013 at 12:24:32PM -0400, Jason Merrill wrote: > Where are the SAVE_EXPRs coming from? It doesn't seem to me that x > needs to be wrapped in a SAVE_EXPR at all in this case. For cases > where the SAVE_EXPR is needed and not used in the test, you could > add the SAVE_EXPR before the condition with a COMPOUND_EXPR. Those SAVE_EXPRs are coming from c-typeck.c in this case: if (flag_sanitize & SANITIZE_UNDEFINED && current_function_decl != 0 && (doing_div_or_mod || doing_shift)) { /* OP0 and/or OP1 might have side-effects. */ op0 = c_save_expr (op0); op1 = c_save_expr (op1); I've resorted to creating (SAVE_EXPR , ...) in the condition in case we're in C89, i.e. creating COMPOUND_EXPR in the condition itself, it seems to work pretty well. Firstly, I added explicit (void) cast via NOP_EXPR (as 'if ((void) x, y)' doesn't trigger -Wunused-value warning), but that didn't seem to be necessary... Does that look up to snuff to you? Thanks, 2013-08-05 Marek Polacek * c-ubsan.c (ubsan_instrument_shift): Properly evaluate SAVE_EXPR even in the C89 mode. * gcc.dg/ubsan/save-expr-1.c: New test. Marek --- gcc/c-family/c-ubsan.c.mp 2013-08-05 13:13:49.245693141 +0200 +++ gcc/c-family/c-ubsan.c 2013-08-05 13:13:53.170709181 +0200 @@ -131,6 +131,12 @@ ubsan_instrument_shift (location_t loc, build_int_cst (type0, 0)); tt = fold_build2 (TRUTH_OR_EXPR, boolean_type_node, x, tt); } + + /* In case we have a SAVE_EXPR in a conditional context, we need to + make sure it gets evaluated before the condition. */ + if (!c_dialect_cxx () && !flag_isoc99) + t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), op0, t); + tree data = ubsan_create_data ("__ubsan_shift_data", loc, ubsan_type_descriptor (type0), ubsan_type_descriptor (type1), NULL_TREE); --- gcc/testsuite/gcc.dg/ubsan/save-expr-1.c.mp 2013-08-05 13:14:51.057960067 +0200 +++ gcc/testsuite/gcc.dg/ubsan/save-expr-1.c 2013-08-05 13:17:44.582675494 +0200 @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-fsanitize=shift -std=c89 -Wall -Werror -O" } */ + +static int x; +int +main (void) +{ + int o = 1; + int y = x << o; + return y; +}