From patchwork Thu Jun 20 19:14:26 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Maciej W. Rozycki" X-Patchwork-Id: 1950457 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4W4qtg5Z0gz20X8 for ; Fri, 21 Jun 2024 05:14:50 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E6FAF3894C2C for ; Thu, 20 Jun 2024 19:14:47 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from angie.orcam.me.uk (angie.orcam.me.uk [IPv6:2001:4190:8020::34]) by sourceware.org (Postfix) with ESMTP id 041D3389247F for ; Thu, 20 Jun 2024 19:14:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 041D3389247F Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=orcam.me.uk Authentication-Results: sourceware.org; spf=none smtp.mailfrom=orcam.me.uk ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 041D3389247F Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2001:4190:8020::34 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718910870; cv=none; b=rD+iCDbfrD0cY308z0gKIxMZp8K6zivFJKK2XrKBoWkIjCRTwYH/NipQ3vGIMf416h8yKz6Z4RipKsD64xfm7uD08ksBkkXQAe8h9eXGAYEVaqMnrQiVDCMEOYGUC5vDhfYCeQiLiFQHbecE1WKBZQibk+s9l9nYa440Y+TqXwc= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718910870; c=relaxed/simple; bh=Lx1fC5cSqv/HGUkSMMOe5NWB3td0vdcfgP488Rq5fa0=; h=Date:From:To:Subject:Message-ID:MIME-Version; b=w5KhTKre8PAO2yIpiatdkLKyyzCajymrFGC5/GmTrm2hp+AE1hebQr09QiDzOHl4ocFupcqKNP1spRvHgR6RovIAplhp2tO6/DbA1DAGLmmd7dhYDojC0UDp+CvaWPedbCfWAgyPWZCYVQiAghc6bKY3vy+Rat7jWB1Q5WIq7Gw= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by angie.orcam.me.uk (Postfix, from userid 500) id 3CB5892009C; Thu, 20 Jun 2024 21:14:26 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by angie.orcam.me.uk (Postfix) with ESMTP id 39FB692009B for ; Thu, 20 Jun 2024 20:14:26 +0100 (BST) Date: Thu, 20 Jun 2024 20:14:26 +0100 (BST) From: "Maciej W. Rozycki" To: gcc-patches@gcc.gnu.org Subject: [PATCH][PR115565] cse: Don't use a valid regno for non-register in comparison_qty Message-ID: User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 X-Spam-Status: No, score=-3488.4 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, KAM_INFOUSMEBIZ, KAM_LAZY_DOMAIN_SECURITY, KAM_SHORT, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Use INT_MIN rather than -1 in `comparison_qty' where a comparison is not with a register, because the value of -1 is actually a valid reference to register 0 in the case where it has not been assigned a quantity. Using -1 makes `REG_QTY (REGNO (folded_arg1)) == ent->comparison_qty' comparison in `fold_rtx' to incorrectly trigger in rare circumstances and return true for a memory reference making CSE consider a comparison operation to evaluate to a constant expression and consequently make the resulting code incorrectly execute or fail to execute conditional blocks. This has caused a miscompilation of rwlock.c from LinuxThreads for the `alpha-linux-gnu' target, where `rwlock->__rw_writer != thread_self ()' expression (where `thread_self' returns the thread pointer via a PALcode call) has been decided to be always true (with `ent->comparison_qty' using -1 for a reference to to `rwlock->__rw_writer', while register 0 holding the thread pointer retrieved by `thread_self') and code for the false case has been optimized away where it mustn't have, causing program lockups. The issue has been observed as a regression from commit 08a692679fb8 ("Undefined cse.c behaviour causes 3.4 regression on HPUX"), , and up to commit 932ad4d9b550 ("Make CSE path following use the CFG"), , where CSE has been restructured sufficiently for the issue not to trigger with the original reproducer anymore. However the original bug remains and can trigger, because `comparison_qty' will still be assigned -1 for a memory reference and the `reg_qty' member of a `cse_reg_info_table' entry will still be assigned -1 for register 0 where the entry has not been assigned a quantity, e.g. at initialization. Use INT_MIN then as noted above, so that the value remains negative, for consistency with the REGNO_QTY_VALID_P macro (even though not used on `comparison_qty'), and then so that it should not ever match a valid negated register number, fixing the regression with commit 08a692679fb8. gcc/ PR rtl-optimization/115565 * cse.cc (record_jump_cond): Use INT_MIN rather than -1 for `comparison_qty' if !REG_P. --- Hi, Oh boy, this was hard to chase and debug! See the PR referred for details. Sadly I have no reproducer for GCC 15, this bug seems too elusive to make one easily. This has passed verification in native `powerpc64le-linux-gnu' and `x86_64-linux-gnu' regstraps, as well as with the `alpha-linux-gnu' target. OK to apply and backport to the release branches? Maciej --- gcc/cse.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) gcc-cse-comparison-qty.diff Index: gcc/gcc/cse.cc =================================================================== --- gcc.orig/gcc/cse.cc +++ gcc/gcc/cse.cc @@ -239,7 +239,7 @@ static int next_qty; the constant being compared against, or zero if the comparison is not against a constant. `comparison_qty' holds the quantity being compared against when the result is known. If the comparison - is not with a register, `comparison_qty' is -1. */ + is not with a register, `comparison_qty' is INT_MIN. */ struct qty_table_elem { @@ -4058,7 +4058,7 @@ record_jump_cond (enum rtx_code code, ma else { ent->comparison_const = op1; - ent->comparison_qty = -1; + ent->comparison_qty = INT_MIN; } return;