From patchwork Thu Mar 28 10:56:31 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 1917288 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=DPOhMyXZ; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; 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 [IPv6:2620:52:3:1:0:246e:9693:128c]) (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 4V50qR3xWYz1yYM for ; Thu, 28 Mar 2024 21:57:23 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 77B303858D32 for ; Thu, 28 Mar 2024 10:57:21 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 4387B3858D20 for ; Thu, 28 Mar 2024 10:56:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4387B3858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 4387B3858D20 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711623420; cv=none; b=tKKjKDWe4JN4+o+xgVCRzrNPm7LjmCN5m6nsFCRkdBuS6BZoeVpqSRf6v3JIPn3n81scovQAq+WR20pifWn+7IqSqzg8IzJ/Z46bE4oNUrctS/kVQjyyIcmICT+oe4DULLRp2QmO3tAp5xfzlzAV2KBJ0C39QDXoL0lxBKe1dSE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711623420; c=relaxed/simple; bh=nyZTBv6zNyP0KumhTbIaDPD1UOOXe+6mXEkmCRLFv74=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=eJNEmmKmLpOMx42j7mOnf3AAzPaN39ywpnYlPl5owiozhAXFiiy70g/8te6Gj3Otgk6/4c33LAB8I9xdrY78cckZSqeyQZxXLJ34IiqhVy/5jLH16fm+vnor6jadbHd7GV1jMrRJUq+nYPIGQg2mBvghmOa4Jp6qR50kIR4YRzA= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1711623417; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type; bh=QNUEJFHm9LgQPQeTN9Z5q9r9oqtsFCxFptiudS5Xwbo=; b=DPOhMyXZJF8OHo0GMX8Dcbfoz2DbRQB6pTAGkmuVeNY/m8y4S6PQ+PnQ/SWnn+IQGD/YRD bur7pH/qhyQsEXR/FAqpa/l0n6U7XcBCjvAN1TQB836DwZ0anjfXtxC7bidyk2ytaAbtl3 AWivQEouccg1EWSPQABome2qHCteLuo= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-97-J0UnVazdNBiJyn-NsUvrtQ-1; Thu, 28 Mar 2024 06:56:56 -0400 X-MC-Unique: J0UnVazdNBiJyn-NsUvrtQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 2D62C8007A7; Thu, 28 Mar 2024 10:56:56 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.57]) by smtp.corp.redhat.com (Postfix) with ESMTPS id E4C4240C6CB4; Thu, 28 Mar 2024 10:56:55 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 42SAuWXs743957 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Thu, 28 Mar 2024 11:56:33 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 42SAuVRe743956; Thu, 28 Mar 2024 11:56:31 +0100 Date: Thu, 28 Mar 2024 11:56:31 +0100 From: Jakub Jelinek To: Jan Hubicka , Richard Biener Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] profile-count: Avoid overflows into uninitialized [PR112303] Message-ID: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.2 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-3.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP 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: , Reply-To: Jakub Jelinek Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Hi! The testcase in the patch ICEs with Jakub --- gcc/tree-scalar-evolution.cc +++ gcc/tree-scalar-evolution.cc @@ -3881,7 +3881,7 @@ final_value_replacement_loop (class loop *loop) /* Propagate constants immediately, but leave an unused initialization around to avoid invalidating the SCEV cache. */ - if (CONSTANT_CLASS_P (def) && !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (rslt)) + if (0 && CONSTANT_CLASS_P (def) && !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (rslt)) replace_uses_by (rslt, def); /* Create the replacement statements. */ (the addition of the above made the ICE latent), because profile_count addition doesn't check for overflows and if unlucky, we can even overflow into the uninitialized value. Getting really huge profile counts is very easy even when not using recursive inlining in loops, e.g. __attribute__((noipa)) void bar (void) { __builtin_exit (0); } __attribute__((noipa)) void foo (void) { for (int i = 0; i < 1000; ++i) for (int j = 0; j < 1000; ++j) for (int k = 0; k < 1000; ++k) for (int l = 0; l < 1000; ++l) for (int m = 0; m < 1000; ++m) for (int n = 0; n < 1000; ++n) for (int o = 0; o < 1000; ++o) for (int p = 0; p < 1000; ++p) for (int q = 0; q < 1000; ++q) for (int r = 0; r < 1000; ++r) for (int s = 0; s < 1000; ++s) for (int t = 0; t < 1000; ++t) for (int u = 0; u < 1000; ++u) for (int v = 0; v < 1000; ++v) for (int w = 0; w < 1000; ++w) for (int x = 0; x < 1000; ++x) for (int y = 0; y < 1000; ++y) for (int z = 0; z < 1000; ++z) for (int a = 0; a < 1000; ++a) for (int b = 0; b < 1000; ++b) bar (); } int main () { foo (); } reaches the maximum count already on the 11th loop. Some other methods of profile_count like apply_scale already do use MIN (val, max_count) before assignment to m_val, this patch just extends that to other methods. Furthermore, one overload of apply_probability wasn't using safe_scale_64bit and so could very easily overflow as well - prob is required to be [0, 10000] and if m_val is near the max_count, it can overflow even with multiplications by 8. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? I wonder if we also shouldn't do if (safe_scale_64bit (..., &tmp)) tmp = max_count; in the existing as well as new spots, because if safe_scale_64bit returns true, it just wraps around I think. 2024-03-27 Jakub Jelinek PR tree-optimization/112303 * profile-count.h (profile_count::operator+): Perform addition in uint64_t variable and set m_val to MIN of that val and max_count. (profile_count::operator+=): Likewise. (profile_count::operator-=): Formatting fix. (profile_count::apply_probability): Use safe_scale_64bit even in the int overload. Set m_val to MIN of tmp and max_count. * gcc.c-torture/compile/pr112303.c: New test. --- gcc/profile-count.h.jj 2024-02-22 13:07:22.870344133 +0100 +++ gcc/profile-count.h 2024-03-27 15:16:16.461774110 +0100 @@ -910,7 +910,8 @@ public: profile_count ret; gcc_checking_assert (compatible_p (other)); - ret.m_val = m_val + other.m_val; + uint64_t ret_val = m_val + other.m_val; + ret.m_val = MIN (ret_val, max_count); ret.m_quality = MIN (m_quality, other.m_quality); return ret; } @@ -929,7 +930,8 @@ public: else { gcc_checking_assert (compatible_p (other)); - m_val += other.m_val; + uint64_t ret_val = m_val + other.m_val; + m_val = MIN (ret_val, max_count); m_quality = MIN (m_quality, other.m_quality); } return *this; @@ -957,7 +959,7 @@ public: else { gcc_checking_assert (compatible_p (other)); - m_val = m_val >= other.m_val ? m_val - other.m_val: 0; + m_val = m_val >= other.m_val ? m_val - other.m_val : 0; m_quality = MIN (m_quality, other.m_quality); } return *this; @@ -1127,7 +1129,9 @@ public: if (!initialized_p ()) return uninitialized (); profile_count ret; - ret.m_val = RDIV (m_val * prob, REG_BR_PROB_BASE); + uint64_t tmp; + safe_scale_64bit (m_val, prob, REG_BR_PROB_BASE, &tmp); + ret.m_val = MIN (tmp, max_count); ret.m_quality = MIN (m_quality, ADJUSTED); return ret; } @@ -1145,7 +1149,7 @@ public: uint64_t tmp; safe_scale_64bit (m_val, prob.m_val, profile_probability::max_probability, &tmp); - ret.m_val = tmp; + ret.m_val = MIN (tmp, max_count); ret.m_quality = MIN (m_quality, prob.m_quality); return ret; } --- gcc/testsuite/gcc.c-torture/compile/pr112303.c.jj 2024-03-27 15:16:57.873214557 +0100 +++ gcc/testsuite/gcc.c-torture/compile/pr112303.c 2024-03-26 12:04:18.741670482 +0100 @@ -0,0 +1,25 @@ +/* PR tree-optimization/112303 */ + +int a, b, d, e, f, **g, h; +char c; + +int * +foo (void) +{ + for (int i = 0; i < 3; i++) + { + for (h = 0; h < 2; h++) + ; + if (!b) + break; + } + while (f) + while (e) + { + c = 0; + while (d) + while (a) + *g = foo (); + } + return 0; +}