From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com [IPv6:2a00:1450:4864:20::42e]) by sourceware.org (Postfix) with ESMTPS id B9A5F3858409 for ; Mon, 13 Dec 2021 14:45:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B9A5F3858409 Received: by mail-wr1-x42e.google.com with SMTP id u17so27468959wrt.3 for ; Mon, 13 Dec 2021 06:45:46 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:user-agent:in-reply-to :references:message-id:mime-version:content-transfer-encoding; bh=rHQnHnJQ+jk0JD96jIRkBerwUqO0EJedWVvwux4knns=; b=bboufvI0UAlCPd0I3PpAUCzNNweJ0RpwOm57pOHNXW6AQMEjcQBZf3kfuoV1EE/j1J JjQDTebW9k632+9D8cwVHZU5DFPxf0mjJxFZFM0EuRXSd1TcxNBPJ1dAbYxDIBg9BQgN bY7DW/zmEB2dTMayTevyoXCk77iQVScRBfvXTS00iOj9tivf1Z7bl2C5fiqWyy6Co2+f cMKlQqATvCZoULL4clHk964E2DJTW3MBM4eJYSSjSUyyfp1EvgPtWEeo/5uGSegJoWhc kAPsTP1sUPuPwu3cjGGI/J5gvatWEAYHwCMTHNNqxCFIpgDFJy+TgPLk3UogIEVkbbWG wY1Q== X-Gm-Message-State: AOAM532Izh0TIX4dGmkJImQuT4slUOsipi40T7z2Ui83kvVPG7qYreuf xMtGrmB8k+26rFH7jBZuFOgWO1HMx0E= X-Google-Smtp-Source: ABdhPJzEDJ3LvEwYOolFBT3zVbJlFT8/x8TlUpeJbuhzEiVtaUbYBLKSFMfAOwS2pHaV2wyK5k6jiA== X-Received: by 2002:adf:cd02:: with SMTP id w2mr32394839wrm.269.1639406745634; Mon, 13 Dec 2021 06:45:45 -0800 (PST) Received: from [127.0.0.1] (dynamic-002-247-253-179.2.247.pool.telefonica.de. [2.247.253.179]) by smtp.gmail.com with ESMTPSA id q24sm7155303wmj.21.2021.12.13.06.45.44 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 13 Dec 2021 06:45:45 -0800 (PST) Date: Mon, 13 Dec 2021 15:45:43 +0100 From: Richard Biener To: Alexander Monakov , gcc-patches@gcc.gnu.org CC: Alexey Nurmukhametov Subject: Re: [RFC PATCH] tree-ssa-sink: do not sink to in front of setjmp User-Agent: K-9 Mail for Android In-Reply-To: References: Message-ID: <4493B84A-DB6D-468B-86BF-DA5383D8CFE4@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 13 Dec 2021 14:45:48 -0000 On December 13, 2021 3:25:47 PM GMT+01:00, Alexander Monakov wrote: >Greetings! > >While testing our patch that reimplements -Wclobbered on GIMPLE we found >a case where tree-ssa-sink moves a statement to a basic block in front >of a setjmp call=2E > >I am confident that this is unintended and should be considered invalid >GIMPLE=2E=20 Does CFG validation not catch this? That is, doesn't setjmp force the star= t of a new BB? One of the edges incoming to a setjmp BB will be an edge from >the ABNORMAL_DISPATCHER block, corresponding to transfer of control flow >from a longjmp-like function and resulting in a "second return" from >setjmp=2E When that happens, it is not possible for GIMPLE statements in >front of setjmp to be somehow re-executed after longjmp=2E > >I am unsure how this could be prevented "once and for all" so the >following patch just attacks one place (out of three) in >tree-ssa-sink by checking 'bb_has_abnormal_pred (sinkbb)'=2E Alexey >(Cc'ed) bootstrapped and regtested the patch on trunk=2E I think sinking relies on dominance and post dominance here but post domin= ance may be too fragile with the abnormal cycles which are likely not backw= ards reachable from exit=2E=20 That said, checking for abnormal preds is OK, I just want to make sure we = detect the invalid CFG - do we?=20 >The testcase is just > >struct __jmp_buf_tag { }; >typedef struct __jmp_buf_tag jmp_buf[1]; >struct globals { jmp_buf listingbuf; }; >extern struct globals *const ptr_to_globals; >void foo() >{ > =C2=A0if ( _setjmp ( ((*ptr_to_globals)=2Elistingbuf ))) > =C2=A0=C2=A0 ; >} > >Before tree-ssa-sink we have > >void foo () >{ > =C2=A0 struct globals * ptr_to_globals=2E0_1; > =C2=A0 struct __jmp_buf_tag[1] * _2; > > =C2=A0 : > =C2=A0 ptr_to_globals=2E0_1 =3D ptr_to_globals; > =C2=A0 _2 =3D &ptr_to_globals=2E0_1->listingbuf; > > =C2=A0 : > =C2=A0 _setjmp (_2); > =C2=A0 goto ; [INV] > > =C2=A0 : > =C2=A0 =2EABNORMAL_DISPATCHER (0); > > =C2=A0 : > =C2=A0 return; > >} > >And tree-ssa-sink yields (see BB 3) > >Sinking _2 =3D &ptr_to_globals=2E0_1->listingbuf; > =C2=A0from bb 2 to bb 3 >void foo () >{ > =C2=A0 struct globals * ptr_to_globals=2E0_1; > =C2=A0 struct __jmp_buf_tag[1] * _2; > > =C2=A0 : > =C2=A0 ptr_to_globals=2E0_1 =3D ptr_to_globals; > > =C2=A0 : > =C2=A0 _2 =3D &ptr_to_globals=2E0_1->listingbuf; > =C2=A0 _setjmp (_2); > =C2=A0 goto ; [INV] > > =C2=A0 : > =C2=A0 =2EABNORMAL_DISPATCHER (0); > > =C2=A0 : > =C2=A0 return; > >} > >The patch: > >diff --git a/gcc/tree-ssa-sink=2Ec b/gcc/tree-ssa-sink=2Ec >index c5d67840be3=2E=2E317e2563114 100644 >--- a/gcc/tree-ssa-sink=2Ec >+++ b/gcc/tree-ssa-sink=2Ec >@@ -461,6 +461,9 @@ statement_sink_location (gimple *stmt, basic_block fr= ombb, > else > *togsi =3D gsi_after_labels (sinkbb); >=20 >+ if (bb_has_abnormal_pred (sinkbb)) >+ return false; >+ > return true; > } > }