From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x636.google.com (mail-pl1-x636.google.com [IPv6:2607:f8b0:4864:20::636]) by sourceware.org (Postfix) with ESMTPS id 714343858D28 for ; Wed, 21 Jun 2023 14:04:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 714343858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pl1-x636.google.com with SMTP id d9443c01a7336-1b525af07a6so28254975ad.1 for ; Wed, 21 Jun 2023 07:04:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1687356255; x=1689948255; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=v0VpjFDF8mHfWqVe2Ij7DhNwKsx1t0v5DoVStDf4PgU=; b=cqrj5O7mxxiUcCrWiD4SKSORHAgSdqI0bXrrdIagedFuJKUWyPAHGXMASwBo1hO53/ rNof5EOcm5Byhks0SnFdFyv443Bb9GB96IvA7cluUSyiyIfMIMk3WGzSQsj6QxmM5Qn8 uxsNpLagghOpq//S9S3cjYqE0way/b43UAniPBORgbBZsqOHSHk/lom6Tu6Snp8kbHF2 J6DFNrOT+OeGMq9k+FWKniPJnjoCJ3U5Eu4WXyl7uIQ+DNC5AnvHgi8by4idFt9bej9A tOV3QB09gX9Dvc9sTovYs8kRYgNDlvSu1XLRsftwSY0Fsw16SA1K2ZrvYs4JWTxV8ouJ gVuA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687356255; x=1689948255; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=v0VpjFDF8mHfWqVe2Ij7DhNwKsx1t0v5DoVStDf4PgU=; b=F87NoFzclFcgv4Q1/EydMk1o3h36D1dllsmgMmm7J4eQx3f0RBRI6G54ujnMoweKig sF5r+5fX+DhNBWcJ+OaLZfJsjrjSy8jDEHvfcRVcYn1HGrhudi3PTrnAVjGUTu6e3pbD 7szX5lP1GHIMqCWKu871q4IlH5a7pKAfQ9UIs5kfq0IT3BEB81+MQU8SxsYqfGL0OcPW RoaWFMlzoOirteWImX5ksAZuwe/QO/3M4Ax/X6tOuQ5WduON/xBQ6hTG79H7GYCuGKIV rUElVKwSUL21q3IWvhxc2w81mdgwmsq8Mms0JoMHFZ9rtAMTxz1dFm3k6PRRGpSeh3wY DPtg== X-Gm-Message-State: AC+VfDype7soZGvNLCOhFk3gpcIuX9gBeh8fAFl0bgn7TMmWOrEr604R yEkzq6XZrI5jhk/0WvDWFvY= X-Google-Smtp-Source: ACHHUZ7XDLMwmYsG/TgnonhY16VrN6AzrPYDlmzkQ8BijxKw4oiGwQ8H2CZKEha54FxACMQCP/pWCg== X-Received: by 2002:a17:902:d38d:b0:1b0:6c10:6836 with SMTP id e13-20020a170902d38d00b001b06c106836mr9445192pld.33.1687356255223; Wed, 21 Jun 2023 07:04:15 -0700 (PDT) Received: from [172.31.0.109] ([136.36.130.248]) by smtp.gmail.com with ESMTPSA id p2-20020a170902e74200b001b39e866324sm3524198plf.306.2023.06.21.07.04.13 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 21 Jun 2023 07:04:14 -0700 (PDT) Message-ID: Date: Wed, 21 Jun 2023 08:04:13 -0600 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [PATCH] Improve DSE to handle stores before __builtin_unreachable () Content-Language: en-US To: Richard Biener Cc: gcc-patches@gcc.gnu.org, Jan Hubicka References: <20230620070009.C11983858D1E@sourceware.org> <7bddf09a-1535-8bcb-94da-8ad18b51963f@gmail.com> From: Jeff Law In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-2.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 6/21/23 00:41, Richard Biener wrote: >> I thought during the introduction of erroneous path isolation that we >> concluded stores, calls and such had observable side effects that must be >> preserved, even when we hit a block that leads to __builtin_unreachable. > > Indeed, I remember we repeatedly hit this in the past. But > double-checking I see that we instrument > > if (x) > *(int *)0 = 0; > > as > > [local count: 1073741824]: > if (x_2(D) != 0) > goto ; [50.00%] > else > goto ; [50.00%] > > [local count: 536870913]: > MEM[(int *)0B] ={v} 0; > __builtin_trap (); > > path isolation doesn't seem to use __builtin_unreachable (). I did > not add __builtin_trap () as possible sink (but I did want to treat > __builtin_unreachable () and __builtin_unreachable_trap () the same > way). The pass also marks the offending store as volatile. Nuts. I mixed up trap vs unreachable in my own head. Though I think for the purposes of this issue they should be treated the same. The only difference is one actively halts the code, the other silently lets it keep going. > So yes, I think preserving the original trap kind (if there is any) > is important and it still seems to work. I don't remember whether > we have any test coverage for that though. I'll also note that > __builtin_trap () has virtual operands (def and use) while > __builtin_unreachable[_trap] () are 'const'. Honza correctly > says they should probably be novops instead of 'const' preserving > the fact that they have side-effects. If we have test coverage it's probably minimal -- a few things to validate proper behavior around builtin_trap plus whatever regressions came up. > I think it's desirable for assertions. Since we elide plain > __builtin_unreachable () and fall thru whereever it leads that > shouldn't be an issue. > > If I manually add a __builtin_unreachable () to the above case > I see the *(int *)0 = 0; store DSEd. Maybe we should avoid > removing stores that might trap here? POSIX wise such a trap > could be a way to jump out of the path leading to unreachable () > via siglongjmp ... Yea, removing the store seemswrong . As you note, someone could have a handler to catch the store, then longjump elsewhere to continue some kind of sensible execution. The erroneous path isolation bits have some code to try and clean up the bogus path a bit. Ideally leaving a single statement with undefined beahvior and the trap. I wonder if there's any code you could re-use from that effort. Essentially a mini pass that cleans up paths post-dominated by a builtin_unreachable or builtin_trap. jeff