[Bug ipa/109303] [13 Regression] ICE in push_agg_values_from_plats, at ipa-cp.cc:1458 since r13-3358-ge0403e95689af7d5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109303 --- Comment #12 from Martin Liška --- *** Bug 109408 has been marked as a duplicate of this bug. *** --- Comment #13 from Martin Liška --- *** Bug 109408 has been marked as a duplicate of this bug. ***
[Bug ipa/109303] [13 Regression] ICE in push_agg_values_from_plats, at ipa-cp.cc:1458 since r13-3358-ge0403e95689af7d5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109303 --- Comment #12 from Martin Liška --- *** Bug 109408 has been marked as a duplicate of this bug. *** --- Comment #13 from Martin Liška --- *** Bug 109408 has been marked as a duplicate of this bug. ***
[Bug ipa/109303] [13 Regression] ICE in push_agg_values_from_plats, at ipa-cp.cc:1458 since r13-3358-ge0403e95689af7d5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109303 --- Comment #10 from CVS Commits --- The master branch has been updated by Martin Jambor : https://gcc.gnu.org/g:da3fd01757297c1d20cf3dcd76046488da737569 commit r13-6986-gda3fd01757297c1d20cf3dcd76046488da737569 Author: Martin Jambor Date: Mon Apr 3 15:53:36 2023 +0200 ipa: Avoid constructing aggregate jump functions with huge offsets (PR 109303) We are in the process of changing data structures holding information about constants passed by reference and in aggregates to use unsigned int offsets rather than HOST_WIDE_INT (which was selected simply because that is what fell out of get_ref_base_and_extent at that time) in order to conserve memory, especially at WPA time. PR 109303 testcase discovers that we do not properly check that we only create jump functions with offsets (plus sizes) that fit into the smaller type. This patch adds the necessary check. gcc/ChangeLog: 2023-03-30 Martin Jambor PR ipa/109303 * ipa-prop.cc (determine_known_aggregate_parts): Check that the offset + size will be representable in unsigned int. gcc/testsuite/ChangeLog: 2023-03-30 Jakub Jelinek Martin Jambor PR ipa/109303 * gcc.dg/pr109303.c: New test.
[Bug ipa/109303] [13 Regression] ICE in push_agg_values_from_plats, at ipa-cp.cc:1458 since r13-3358-ge0403e95689af7d5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109303 Martin Jambor changed: What|Removed |Added Resolution|--- |FIXED Status|ASSIGNED|RESOLVED --- Comment #11 from Martin Jambor --- Fixed.
[Bug ipa/109303] [13 Regression] ICE in push_agg_values_from_plats, at ipa-cp.cc:1458 since r13-3358-ge0403e95689af7d5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109303 --- Comment #9 from Martin Jambor --- I have proposed the fix on the mailing list: https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614943.html
[Bug ipa/109303] [13 Regression] ICE in push_agg_values_from_plats, at ipa-cp.cc:1458 since r13-3358-ge0403e95689af7d5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109303
Martin Jambor changed:
What|Removed |Added
Status|NEW |ASSIGNED
Assignee|unassigned at gcc dot gnu.org |jamborm at gcc dot
gnu.org
--- Comment #8 from Martin Jambor ---
Thanks a lot for the testcase. The divisions must always be exact.
After having one more look, I moved the check a bit earlier still and am
currently testing this:
--- a/gcc/ipa-prop.cc
+++ b/gcc/ipa-prop.cc
@@ -2086,7 +2086,9 @@ determine_known_aggregate_parts (struct
ipa_func_body_info *fbi,
whether its value is clobbered any other dominating one. */
if ((content->value.pass_through.formal_id >= 0
|| content->value.pass_through.operand)
- && !clobber_by_agg_contents_list_p (all_list, content))
+ && !clobber_by_agg_contents_list_p (all_list, content)
+ && (content->offset + content->size - arg_offset
+ <= (HOST_WIDE_INT) UINT_MAX * BITS_PER_UNIT))
{
struct ipa_known_agg_contents_list *copy
= XALLOCA (struct ipa_known_agg_contents_list);
[Bug ipa/109303] [13 Regression] ICE in push_agg_values_from_plats, at ipa-cp.cc:1458 since r13-3358-ge0403e95689af7d5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109303 --- Comment #7 from Jakub Jelinek --- BTW, I think I got that offset >= (HOST_WIDE_INT) UINT_MAX * BITS_PER_UNIT wrong last time, if offset is equal to that, it is still representable (as UINT_MAX). So perhaps either offset > (HOST_WIDE_INT) UINT_MAX * BITS_PER_UNIT or >= (UINT_MAX + HOST_WIDE_INT_1) * BITS_PER_UNIT, depending on whether we expect the divisions to be exact or truncating. On the other side, if it is just an optimization, who cares...
[Bug ipa/109303] [13 Regression] ICE in push_agg_values_from_plats, at ipa-cp.cc:1458 since r13-3358-ge0403e95689af7d5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109303
--- Comment #6 from Jakub Jelinek ---
Ok. It is yours then, for testcase perhaps:
--- gcc/testsuite/gcc.dg/pr109303.c.jj 2023-03-29 18:42:31.068144102 +0200
+++ gcc/testsuite/gcc.dg/pr109303.c 2023-03-29 18:42:18.328330526 +0200
@@ -0,0 +1,24 @@
+/* PR ipa/109303 */
+/* { dg-do compile { target lp64 } } */
+/* { dg-options "-O2" } */
+
+struct __attribute__((packed)) A { char c1; short a1[__INT_MAX__]; };
+struct __attribute__((packed)) B { char c2; short a2[100]; };
+struct S { struct A p1; struct B p2[4]; };
+void bar (short int);
+
+static void
+foo (struct S *q)
+{
+ for (int i = 0; i < q->p1.c1; i++)
+for (int j = 0; j < q->p2[i].c2; j++)
+ bar (q->p2[i].a2[j]);
+}
+
+int
+main ()
+{
+ struct S q = {};
+ q.p2[0].c2 = q.p2[1].c2 = 3;
+ foo ();
+}
(without lp64 it obviously fails on 32-bit targets).
[Bug ipa/109303] [13 Regression] ICE in push_agg_values_from_plats, at ipa-cp.cc:1458 since r13-3358-ge0403e95689af7d5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109303
--- Comment #5 from Martin Jambor ---
(In reply to Jakub Jelinek from comment #4)
> --- gcc/ipa-cp.cc.jj 2023-03-14 19:12:19.949553036 +0100
> +++ gcc/ipa-cp.cc 2023-03-29 18:32:34.14423 +0200
> @@ -3117,7 +3117,9 @@ propagate_aggs_across_jump_function (str
> {
> HOST_WIDE_INT val_size;
>
> - if (item->offset < 0 || item->jftype == IPA_JF_UNKNOWN)
> + if (item->offset < 0
> + || item->jftype == IPA_JF_UNKNOWN
> + || item->offset >= (HOST_WIDE_INT) UINT_MAX * BITS_PER_UNIT)
> continue;
> val_size = tree_to_shwi (TYPE_SIZE (item->type));
>
> fixes the ICE and is then similar to the PR108605 fix. Dunno if the code
> can overflow also offset + size computations or something protects against
> that.
> Anyway, I think it would be worth it to switch all those unsigned int byte
> offsets to
> unsigned HOST_WIDE_INTs for GCC 14.
Actually, I am in the process of doing the reverse in order to try and
keep the memory footprint of the structures small. (The reason why
the HOST_WIDE_BITs are signed is what get_ref_base_and_extent used to
return). Unfortunately what I wanted to do but forgot is the
following (only lightly tested so far, it has the benefit that
uselessly large offsets are not even streamed):
diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc
index de45dbccf16..edc1f469914 100644
--- a/gcc/ipa-prop.cc
+++ b/gcc/ipa-prop.cc
@@ -1735,6 +1735,8 @@ build_agg_jump_func_from_list (struct
ipa_known_agg_contents_list *list,
item.offset = list->offset - arg_offset;
gcc_assert ((item.offset % BITS_PER_UNIT) == 0);
+ if (item.offset + list->size >= (HOST_WIDE_INT) UINT_MAX *
BITS_PER_UNIT)
+ continue;
jfunc->agg.items->quick_push (item);
}
[Bug ipa/109303] [13 Regression] ICE in push_agg_values_from_plats, at ipa-cp.cc:1458 since r13-3358-ge0403e95689af7d5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109303
--- Comment #4 from Jakub Jelinek ---
--- gcc/ipa-cp.cc.jj2023-03-14 19:12:19.949553036 +0100
+++ gcc/ipa-cp.cc 2023-03-29 18:32:34.14423 +0200
@@ -3117,7 +3117,9 @@ propagate_aggs_across_jump_function (str
{
HOST_WIDE_INT val_size;
- if (item->offset < 0 || item->jftype == IPA_JF_UNKNOWN)
+ if (item->offset < 0
+ || item->jftype == IPA_JF_UNKNOWN
+ || item->offset >= (HOST_WIDE_INT) UINT_MAX * BITS_PER_UNIT)
continue;
val_size = tree_to_shwi (TYPE_SIZE (item->type));
fixes the ICE and is then similar to the PR108605 fix. Dunno if the code can
overflow also offset + size computations or something protects against that.
Anyway, I think it would be worth it to switch all those unsigned int byte
offsets to
unsigned HOST_WIDE_INTs for GCC 14.
[Bug ipa/109303] [13 Regression] ICE in push_agg_values_from_plats, at ipa-cp.cc:1458 since r13-3358-ge0403e95689af7d5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109303 --- Comment #3 from Jakub Jelinek --- And the problem is similar to PR108605, most of IPA uses unsigned int as type for byte offsets and while some spots check for offsets while bit offsets are typically using HOST_WIDE_INT. So, some larger bit offsets can't be represented in unsigned int byte offset.
[Bug ipa/109303] [13 Regression] ICE in push_agg_values_from_plats, at ipa-cp.cc:1458 since r13-3358-ge0403e95689af7d5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109303
Jakub Jelinek changed:
What|Removed |Added
CC||jakub at gcc dot gnu.org
--- Comment #2 from Jakub Jelinek ---
Slightly adjusted testcase:
struct __attribute__((packed)) A { char c1; short a1[__INT_MAX__]; };
struct __attribute__((packed)) B { char c2; short a2[100]; };
struct S { struct A p1; struct B p2[4]; };
void bar (short int);
static void
foo (struct S *q)
{
for (int i = 0; i < q->p1.c1; i++)
for (int j = 0; j < q->p2[i].c2; j++)
bar (q->p2[i].a2[j]);
}
int
main ()
{
struct S q = {};
q.p2[0].c2 = q.p2[1].c2 = 3;
foo ();
}
[Bug ipa/109303] [13 Regression] ICE in push_agg_values_from_plats, at ipa-cp.cc:1458 since r13-3358-ge0403e95689af7d5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109303 Richard Biener changed: What|Removed |Added Priority|P3 |P1
[Bug ipa/109303] [13 Regression] ICE in push_agg_values_from_plats, at ipa-cp.cc:1458 since r13-3358-ge0403e95689af7d5
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109303 Martin Liška changed: What|Removed |Added Ever confirmed|0 |1 Status|UNCONFIRMED |NEW Keywords|needs-bisection | Last reconfirmed||2023-03-27 CC||jamborm at gcc dot gnu.org Summary|[13 Regression] ICE in |[13 Regression] ICE in |push_agg_values_from_plats, |push_agg_values_from_plats, |at ipa-cp.cc:1458 |at ipa-cp.cc:1458 since ||r13-3358-ge0403e95689af7d5 --- Comment #1 from Martin Liška --- Started with r13-3358-ge0403e95689af7d5.
