From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 101468 invoked by alias); 7 Apr 2017 17:04:21 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 101380 invoked by uid 89); 7 Apr 2017 17:04:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= 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 ESMTP; Fri, 07 Apr 2017 17:04:17 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id ABFF83DBE5; Fri, 7 Apr 2017 17:04:17 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com ABFF83DBE5 Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=jakub@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com ABFF83DBE5 Received: from tucnak.zalov.cz (ovpn-117-111.ams2.redhat.com [10.36.117.111]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A579E5C88B; Fri, 7 Apr 2017 17:04:15 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id v37H4BpJ013314; Fri, 7 Apr 2017 19:04:11 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id v37H48xg013298; Fri, 7 Apr 2017 19:04:08 +0200 Date: Fri, 07 Apr 2017 17:04:00 -0000 From: Jakub Jelinek To: Martin =?utf-8?B?TGnFoWth?= Cc: GCC Patches , Marek Polacek Subject: Re: [PATCH] Evaluate a SAVE_EXPR before an UBSAN check (PR sanitizer/80350). Message-ID: <20170407170408.GF1914@tucnak> Reply-To: Jakub Jelinek References: <693e8de8-3fe7-ad42-6f9b-2fd749e7326f@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <693e8de8-3fe7-ad42-6f9b-2fd749e7326f@suse.cz> User-Agent: Mutt/1.7.1 (2016-10-04) X-IsSubscribed: yes X-SW-Source: 2017-04/txt/msg00392.txt.bz2 On Fri, Apr 07, 2017 at 04:26:50PM +0200, Martin Liška wrote: > Hello. > > Similar to what was done in Marek's r202113, when op1 is a SAVE_EXPR it must > be evaluated before condition, in order to be able to deliver the operand > to real shifting. And not just to a BB where ubsan report function is called. > > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > Apart from that make check RUNTESTFLAGS="ubsan.exp" works on x86_64-linux-gnu. > > Ready to be installed? > Martin > >From 2ff2e17d82ee85b09cb5f83afbee70f8b1a84f4f Mon Sep 17 00:00:00 2001 > From: marxin > Date: Fri, 7 Apr 2017 12:21:44 +0200 > Subject: [PATCH] Evaluate a SAVE_EXPR before an UBSAN check (PR > sanitizer/80350). > > gcc/c-family/ChangeLog: > > 2017-04-07 Martin Liska > > PR sanitizer/80350 > * c-ubsan.c (ubsan_instrument_shift): Evaluate RHS before > doing an UBSAN check. > > gcc/testsuite/ChangeLog: > > 2017-04-07 Martin Liska > > PR sanitizer/80350 > * c-c++-common/ubsan/pr80350.c: New test. > --- > gcc/c-family/c-ubsan.c | 4 +++- > gcc/testsuite/c-c++-common/ubsan/pr80350.c | 17 +++++++++++++++++ > 2 files changed, 20 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/c-c++-common/ubsan/pr80350.c > > diff --git a/gcc/c-family/c-ubsan.c b/gcc/c-family/c-ubsan.c > index 91bdef88320..ef45abdd19e 100644 > --- a/gcc/c-family/c-ubsan.c > +++ b/gcc/c-family/c-ubsan.c > @@ -171,7 +171,9 @@ ubsan_instrument_shift (location_t loc, enum tree_code code, > > /* In case we have a SAVE_EXPR in a conditional context, we need to > make sure it gets evaluated before the condition. */ > - t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), unshare_expr (op0), t); > + t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), > + fold_build2 (COMPOUND_EXPR, TREE_TYPE (op1), > + unshare_expr (op0), unshare_expr (op1)), t); For consistency with ubsan_instrument_division and better readability, can't you: /* In case we have a SAVE_EXPR in a conditional context, we need to make sure it gets evaluated before the condition. */ t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), unshare_expr (op0), t); t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t), unshare_expr (op1), t); Ok with that change. Jakub