From patchwork Wed Aug 7 14:58:03 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 265521 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 E545C2C029C for ; Thu, 8 Aug 2013 00:58:25 +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=JnjC6IH4YTwsjuBHs vem8zCSUFYYybX2sTK0D2IZEB+dx5wLdJYu33LPQy4M2HyalgeHU/4Lm7WPUWrYF WAPCV7K6MIXSOT+rW2Up86yoJfFuaGMW+0H1qttQgNRkPvHkjHLQInOyRa93a27p 4SSPmM+jYbLG0MDTMtT513jv6E= 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=FklQlQbyRkPYr4FphwDCY3p iBFw=; b=kVFmoUL84MNqGUMx0mVFvvigGVaqF2HpCrPTVqWtIRvDf0fAGMzi45F SpsozIy2A2SfNF3zmfxzhsq6gAu2vlLYHePNEO1LwQUzm+4VAY6LwlGt2iK+BiJl DuWFYAN2ybP1exhmcisSykDGyDdDURjy2eDey3AyW0sNFhSh+fZM= Received: (qmail 27672 invoked by alias); 7 Aug 2013 14:58:18 -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 27642 invoked by uid 89); 7 Aug 2013 14:58:18 -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; Wed, 07 Aug 2013 14:58:17 +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 r77Ew8EP016438 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 7 Aug 2013 10:58:08 -0400 Received: from redhat.com (ovpn-116-26.ams2.redhat.com [10.36.116.26]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r77Ew3aG007310 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Wed, 7 Aug 2013 10:58:06 -0400 Date: Wed, 7 Aug 2013 16:58:03 +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: <20130807145803.GE17022@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> <20130805112452.GX17022@redhat.com> <52016978.8010807@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <52016978.8010807@redhat.com> User-Agent: Mutt/1.5.20 (2009-06-14) On Tue, Aug 06, 2013 at 05:24:08PM -0400, Jason Merrill wrote: > On 08/05/2013 07:24 AM, Marek Polacek wrote: > >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); > > Yes, but why do we need to wrap op0 in a save_expr? That's only > necessary if we're going to be evaluating it twice on the same path, > but here it's only evaluated once on each path. True, in this particular case wrapping x in the SAVE_EXPR isn't needed, thus the following should work equally: (It would merit a comment, for sure.) I actually don't know what I prefer more, but if you like this version more, I'll go with it. Maybe this is better because we don't have to create SAVE_EXPR and also we avoid one fold_build call. Thanks, Marek --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -10493,9 +10493,12 @@ build_binary_op (location_t location, enum tree_code code, && (doing_div_or_mod || doing_shift)) { /* OP0 and/or OP1 might have side-effects. */ - op0 = c_save_expr (op0); + if (!doing_shift || flag_isoc99) + { + op0 = c_save_expr (op0); + op0 = c_fully_fold (op0, false, NULL); + } op1 = c_save_expr (op1); - op0 = c_fully_fold (op0, false, NULL); op1 = c_fully_fold (op1, false, NULL); if (doing_div_or_mod) instrument_expr = ubsan_instrument_division (location, op0, op1);