From patchwork Sun Aug 18 22:57:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Law X-Patchwork-Id: 1973625 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=LX2eIKE+; dkim-atps=neutral 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 4WnB2P5xfsz1yXf for ; Mon, 19 Aug 2024 08:57:33 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id EF261384F032 for ; Sun, 18 Aug 2024 22:57:31 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-oa1-x31.google.com (mail-oa1-x31.google.com [IPv6:2001:4860:4864:20::31]) by sourceware.org (Postfix) with ESMTPS id 1A2C03861835 for ; Sun, 18 Aug 2024 22:57:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1A2C03861835 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 1A2C03861835 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2001:4860:4864:20::31 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1724021834; cv=none; b=RpSgMh1mgqRqkpz/QAah2PrZ4gBqBWOrwagxPhfDvgwoP2nECqZ2SMtRFvp7utqlvBG+GVf7GBkNh0godL3HCwHGzeGmSmpZ81+3iTyS+K4zGsiSvhPOJ3weC+CWP+ap8jYRRsp9FKgX2R5hmtXRwedPo9DAamDXRmrllWYSUaE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1724021834; c=relaxed/simple; bh=4SKKWiNupz7KqpXpHg+RqiIiK1itDP0ShxvL9J5sER8=; h=DKIM-Signature:Message-ID:Date:MIME-Version:From:To:Subject; b=oCUHt18kcEkOAP3Md3coNf/yvU4UAZX1rkGHo7EMySkl8KI8aFt+AvhAlAb93qnwTIQgZYTgCZCd1ybByK2cq4NNk4aLuemFLEhgrjc/3INM324pKB2NboZaE5V/F3TcJvqfl7vxzKP2n49aZ7VyoyI27kieif7bl09FeUyqbQA= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-oa1-x31.google.com with SMTP id 586e51a60fabf-268eec6c7c1so2546813fac.3 for ; Sun, 18 Aug 2024 15:57:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1724021831; x=1724626631; darn=gcc.gnu.org; h=subject:to:from:content-language:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=6P1PotgCwjDDtygeqylFGivZ6Rtpy522crbS2T3Cz2M=; b=LX2eIKE+aeP0jRTh7OUeLtmzoSUSBrVBcSpMqv9rcoiQ+Pu0+qIbycQwXwKthkbkPJ e1oJtLuhHP1gj8cVwSh8l6Fm+GlpVQ+/g/TNdxV3fDx+uUGVjMrIKFhuN0Id0u3BtIHn i6U4BiivKqBlfzJbrBdNy+D/E3sFXGu+C6itopLq+bx17Q2EEN0atp/ijlr+SoIDj7kr Sh9EKYBuobmOEHx9xqeJ8VUI1nexxGfrovH4bVlFW1oEUzLU27qY4xRphl/ezJN1/yeC 1hn6A8SvsQEbElyIEo680NYS0iOGXFJx27gg5Dg028NQgblNxX55wOeEArze3SxC0rFr D2GQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724021831; x=1724626631; h=subject:to:from:content-language:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=6P1PotgCwjDDtygeqylFGivZ6Rtpy522crbS2T3Cz2M=; b=M9Ywekj2idKpbpYPLmjypf+8CEB5aMQb4u8t/7JGv8kJBN0YP/s3TX+iOvGCe8IBm5 4egGrtNCWx+jGyqj+zZ3UR0/Y/3J+WGHCSCbWr6q/ek6WElYEDKKYXYiQHHaechGRmRw n4xp+sfG7VCuwu1LQaMItq67W3M+ZMzfBz5+SJZFKGvGEho+IxMhBOcT44B9mPdKk4kd gRmw9J9EDDw7//ua5Dm4nzGwcA7pZQwILHC8hlyhsla0iXsypD6qLpECYm8DfYNsmL8X m/kLJM8Vuy9lHHpoR/hB1gV022QbqlrM9sqkgxFuRfRLFgCt8An+NVD//+9RIsGH8M54 xCyA== X-Gm-Message-State: AOJu0YwOWvSrvgbpCrUpp4zyeRQZHll3ZcBeeAFIigdZaPML4HJkNqjf KH+Zhh99WFM5GykjWXVxAisWHyvo6j3+LHlWzu8ZDZvjNZXzSKxhhHqypqQN X-Google-Smtp-Source: AGHT+IFyL61i19EKBIZxVGEvxcrcPLwtNbeUeAPQk/K2fQnkcQPfW6WiLThxgTRi3EjXVDofOa45Mw== X-Received: by 2002:a05:6871:24c4:b0:261:13c6:e843 with SMTP id 586e51a60fabf-2701c38c009mr9472454fac.15.1724021830881; Sun, 18 Aug 2024 15:57:10 -0700 (PDT) Received: from [172.31.0.109] ([136.36.72.243]) by smtp.gmail.com with ESMTPSA id 5614622812f47-3dd33deeabbsm1947313b6e.41.2024.08.18.15.57.09 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 18 Aug 2024 15:57:10 -0700 (PDT) Message-ID: <64ed6b0a-46e7-4e44-ab9d-1eba439f5189@gmail.com> Date: Sun, 18 Aug 2024 16:57:08 -0600 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Content-Language: en-US From: Jeff Law To: "gcc-patches@gcc.gnu.org" Subject: [committed][PR rtl-optimization/115876] Avoid ubsan in ext-dce.cc X-Spam-Status: No, score=-8.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, 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 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 This fixes two general ubsan issues in ext-dce, both related to use-side processsing of modes > DImode. In ext_dce_process_uses we can be presented with something like this as a use (subreg:SI (reg:TF) 12) That will result in an out of range shift for a HOST_WIDE_INT object. Where this happens is safe to just break from the SET context and process the subjects. This will ultimately result in seeing (reg:TF) and we'll mark all bit groups as live. In carry_backpropagate we can be presented with a TImode shift (for example) and the shift count can be > 63 for such a shift. This naturally trips ubsan as well as we're operating on 64 bit objects. We can just return mmask in this case noting that every bit group is live. The combination of these two fixes eliminates all the reported ubsan issues in ext-dce seen in a bootstrap and regression test on x86. While I was in there I went ahead and fixed the various hardcoded 63/64 values to be HOST_BITS_PER_WIDE_INT based. Bootstrapped and regression tested on x86 with no regressions. Also built with ubsan enabled and verified the build logs and testsuite logs don't call out any issues in ext-dce anymore. Pushing to the trunk. Jeff commit f10d2ee95356b9de6c44d701c4dfa8fb088714d2 Author: Jeff Law Date: Sun Aug 18 16:55:52 2024 -0600 [PR rtl-optimization/115876] Avoid ubsan in ext-dce.cc This fixes two general ubsan issues in ext-dce, both related to use-side processsing of modes > DImode. In ext_dce_process_uses we can be presented with something like this as a use (subreg:SI (reg:TF) 12) That will result in an out of range shift for a HOST_WIDE_INT object. Where this happens is safe to just break from the SET context and process the subjects. This will ultimately result in seeing (reg:TF) and we'll mark all bit groups as live. In carry_backpropagate we can be presented with a TImode shift (for example) and the shift count can be > 63 for such a shift. This naturally trips ubsan as well as we're operating on 64 bit objects. We can just return mmask in this case noting that every bit group is live. The combination of these two fixes eliminates all the reported ubsan issues in ext-dce seen in a bootstrap and regression test on x86. While I was in there I went ahead and fixed the various hardcoded 63/64 values to be HOST_BITS_PER_WIDE_INT based. Bootstrapped and regression tested on x86 with no regressions. Also built with ubsan enabled and verified the build logs and testsuite logs don't call out any issues in ext-dce anymore. Pushing to the trunk. PR rtl-optimization/115876 gcc * ext-dce.cc (ext_dce_process_sets): Replace hardcoded 63/64 instances with HOST_BITS_PER_WIDE_INT based values. (carry_backpropagate): Handle modes with more bits than HOST_BITS_PER_WIDE_INT gracefully, avoiding undefined behavior. (ext_dce_process_uses): Handle subreg offsets which would result in ubsan shifts gracefully, avoiding undefined behavior. diff --git a/gcc/ext-dce.cc b/gcc/ext-dce.cc index 017e2de000d..eee9208f0d6 100644 --- a/gcc/ext-dce.cc +++ b/gcc/ext-dce.cc @@ -207,7 +207,7 @@ ext_dce_process_sets (rtx_insn *insn, rtx obj, bitmap live_tmp) wider than DImode. */ scalar_int_mode outer_mode; if (!is_a (GET_MODE (x), &outer_mode) - || GET_MODE_BITSIZE (outer_mode) > 64) + || GET_MODE_BITSIZE (outer_mode) > HOST_BITS_PER_WIDE_INT) { /* Skip the subrtxs of this destination. There is little value in iterating into the subobjects, so @@ -239,7 +239,7 @@ ext_dce_process_sets (rtx_insn *insn, rtx obj, bitmap live_tmp) that case. Remember, we can not just continue to process the inner RTXs due to the STRICT_LOW_PART. */ if (!is_a (GET_MODE (SUBREG_REG (x)), &outer_mode) - || GET_MODE_BITSIZE (outer_mode) > 64) + || GET_MODE_BITSIZE (outer_mode) > HOST_BITS_PER_WIDE_INT) { /* Skip the subrtxs of the STRICT_LOW_PART. We can't process them because it'll set objects as no longer @@ -293,7 +293,7 @@ ext_dce_process_sets (rtx_insn *insn, rtx obj, bitmap live_tmp) the top of the loop which just complicates the flow even more. */ if (!is_a (GET_MODE (SUBREG_REG (x)), &outer_mode) - || GET_MODE_BITSIZE (outer_mode) > 64) + || GET_MODE_BITSIZE (outer_mode) > HOST_BITS_PER_WIDE_INT) { skipped_dest = true; iter.skip_subrtxes (); @@ -329,7 +329,7 @@ ext_dce_process_sets (rtx_insn *insn, rtx obj, bitmap live_tmp) } /* BIT >= 64 indicates something went horribly wrong. */ - gcc_assert (bit <= 63); + gcc_assert (bit <= HOST_BITS_PER_WIDE_INT - 1); /* Now handle the actual object that was changed. */ if (REG_P (x)) @@ -483,6 +483,17 @@ carry_backpropagate (unsigned HOST_WIDE_INT mask, enum rtx_code code, rtx x) enum machine_mode mode = GET_MODE_INNER (GET_MODE (x)); unsigned HOST_WIDE_INT mmask = GET_MODE_MASK (mode); + + /* While we don't try to optimize operations on types larger + than 64 bits, we do want to make sure not to invoke undefined + behavior when presented with such operations during use + processing. The safe thing to do is to just return mmask + for that scenario indicating every possible chunk is life. */ + scalar_int_mode smode; + if (!is_a (mode, &smode) + || GET_MODE_BITSIZE (smode) > HOST_BITS_PER_WIDE_INT) + return mmask; + switch (code) { case PLUS: @@ -733,8 +744,17 @@ ext_dce_process_uses (rtx_insn *insn, rtx obj, && SUBREG_PROMOTED_UNSIGNED_P (y))))) break; - /* The SUBREG's mode determine the live width. */ bit = subreg_lsb (y).to_constant (); + + /* If this is a wide object (more bits than we can fit + in a HOST_WIDE_INT), then just break from the SET + context. That will cause the iterator to walk down + into the subrtx and if we land on a REG we'll mark + the whole think live. */ + if (bit >= HOST_BITS_PER_WIDE_INT) + break; + + /* The SUBREG's mode determines the live width. */ if (dst_mask) { dst_mask <<= bit;