On 1/28/21 1:31 AM, Richard Biener wrote:
On Thu, Jan 28, 2021 at 12:08 AM Martin Sebor via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
Attached is another attempt to fix the problem caused by allowing
front-end trees representing nontrivial VLA bound expressions to
stay in attribute access attached to functions. Since removing
these trees seems to be everyone's preference this patch does that
by extending the free_lang_data pass to look for and zero out these
trees.
Because free_lang_data only frees anything when LTO is enabled and
we want these trees cleared regardless to keep them from getting
clobbered during gimplification, this change also modifies the pass
to do the clearing even when the pass is otherwise inactive.
if (TREE_CODE (bound) == NOP_EXPR)
+ bound = TREE_OPERAND (bound, 0);
+
+ if (TREE_CODE (bound) == CONVERT_EXPR)
+ {
+ tree op0 = TREE_OPERAND (bound, 0);
+ tree bndtyp = TREE_TYPE (bound);
+ tree op0typ = TREE_TYPE (op0);
+ if (TYPE_PRECISION (bndtyp) == TYPE_PRECISION (op0typ))
+ bound = op0;
+ }
+
+ if (TREE_CODE (bound) == NON_LVALUE_EXPR)
+ bound = TREE_OPERAND (bound, 0);
all of the above can be just
STRIP_NOPS (bound);
which also handles nesting of the above in any order.
No, it can't be just STRIP_NOPS.
The goal is to strip the meaningless (to the user) cast to sizetype
from the array type. For example:
void f (int n, int[n]);
void f (int n, int[n + 1]);
I want the type in the warning to reflect the source:
warning: argument 2 of type ‘int[n + 1]’ declared with mismatched
bound ‘n + 1’ [-Wvla-parameter]
and not:
warning: ... ‘int[(sizetype)(n + 1)]’ ...
+ if (TREE_CODE (bound) == PLUS_EXPR
+ && integer_all_onesp (TREE_OPERAND (bound, 1)))
+ {
+ bound = TREE_OPERAND (bound, 0);
+ if (TREE_CODE (bound) == NOP_EXPR)
+ bound = TREE_OPERAND (bound, 0);
+ }
so it either does or does not strip a -1 but has no indication on
whether it did that? That looks fragile and broken.
Indication to what? The caller? The function is only used to recover
a meaningful VLA bound for warnings and its callers aren't interested
in whether the -1 was stripped or not.
Anyway, the split out of this function seems unrelated to the
original problem and should be submitted separately.
It was a remnant of the previous patch where it was used to format
the string representation of the VLA bounds and called from three
sites. I've removed the function from this revision (and restored
the one site in the pretty printer that open-codes the same thing).
+ for (vblist = TREE_VALUE (vblist); vblist; vblist = TREE_CHAIN (vblist))
+ {
+ tree *pvbnd = &TREE_VALUE (vblist);
+ if (!*pvbnd || DECL_P (*pvbnd))
+ continue;
so this doesn't let constant bounds prevail but only symbolical ones? Not
that I care but I'd have expected || CONSTANT_CLASS_P (*pvbnd)
There must be some confusion here. There are no constant VLA bounds.
The essential purpose of this patch is to remove expressions from
the attributes, such as PLUS_EXPR, that denote nontrivial VLA bounds.
The test above retains decls that might refer to function parameters
or global variables so that they can be mentioned in middle end
warnings.
Attached is yet another revision of this fix that moves the call
to attr_access:free_lang_data() to c_parse_final_cleanups() as
Jakub suggested.
Martin
PR middle-end/97172 - ICE: tree code 'ssa_name' is not supported in LTO streams
gcc/ChangeLog:
PR middle-end/97172
* attribs.c (attr_access::free_lang_data): Define new function.
* attribs.h (attr_access::free_lang_data): Declare new function.
gcc/c/ChangeLog:
PR middle-end/97172
* c-decl.c (free_attr_access_data): New function.
(c_parse_final_cleanups): Call free_attr_access_data.
gcc/testsuite/ChangeLog:
PR middle-end/97172
* gcc.dg/pr97172.c: New test.
diff --git a/gcc/attribs.c b/gcc/attribs.c
index 94991fbbeab..81322d40f1d 100644
--- a/gcc/attribs.c
+++ b/gcc/attribs.c
@@ -2238,6 +2238,38 @@ attr_access::vla_bounds (unsigned *nunspec) const
return list_length (size);
}
+/* Reset front end-specific attribute access data from ATTRS.
+ Called from the free_lang_data pass. */
+
+/* static */ void
+attr_access::free_lang_data (tree attrs)
+{
+ for (tree acs = attrs; (acs = lookup_attribute ("access", acs));
+ acs = TREE_CHAIN (acs))
+ {
+ tree vblist = TREE_VALUE (acs);
+ vblist = TREE_CHAIN (vblist);
+ if (!vblist)
+ continue;
+
+ vblist = TREE_VALUE (vblist);
+ if (!vblist)
+ continue;
+
+ for (vblist = TREE_VALUE (vblist); vblist; vblist = TREE_CHAIN (vblist))
+ {
+ tree *pvbnd = &TREE_VALUE (vblist);
+ if (!*pvbnd || DECL_P (*pvbnd))
+ continue;
+
+ /* VLA bounds that are expressions as opposed to DECLs are
+ only used in the front end. Reset them to keep front end
+ trees leaking into the middle end (see pr97172) and to
+ free up memory. */
+ *pvbnd = NULL_TREE;
+ }
+ }
+}
/* Defined in attr_access. */
constexpr char attr_access::mode_chars[];
diff --git a/gcc/attribs.h b/gcc/attribs.h
index 21d28a47f39..898e73db3e4 100644
--- a/gcc/attribs.h
+++ b/gcc/attribs.h
@@ -274,6 +274,9 @@ struct attr_access
/* Return the access mode corresponding to the character code. */
static access_mode from_mode_char (char);
+ /* Reset front end-specific attribute access data from attributes. */
+ static void free_lang_data (tree);
+
/* The character codes corresponding to all the access modes. */
static constexpr char mode_chars[5] = { '-', 'r', 'w', 'x', '^' };
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 4ba9477f5d1..be95643fcf9 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -12146,6 +12146,27 @@ collect_source_refs (void)
collect_source_ref (DECL_SOURCE_FILE (decl));
}
+/* Free attribute access data that are not needed by the middle end. */
+
+static void
+free_attr_access_data ()
+{
+ struct cgraph_node *n;
+
+ /* Iterate over all functions declared in the translation unit. */
+ FOR_EACH_FUNCTION (n)
+ {
+ tree fntype = TREE_TYPE (n->decl);
+ if (!fntype)
+ continue;
+ tree attrs = TYPE_ATTRIBUTES (fntype);
+ if (!attrs)
+ continue;
+
+ attr_access::free_lang_data (attrs);
+ }
+}
+
/* Perform any final parser cleanups and generate initial debugging
information. */
@@ -12190,6 +12211,9 @@ c_parse_final_cleanups (void)
c_write_global_declarations_1 (BLOCK_VARS (DECL_INITIAL (t)));
c_write_global_declarations_1 (BLOCK_VARS (ext_block));
+ if (!in_lto_p)
+ free_attr_access_data ();
+
timevar_stop (TV_PHASE_DEFERRED);
timevar_start (TV_PHASE_PARSING);
diff --git a/gcc/testsuite/gcc.dg/pr97172.c b/gcc/testsuite/gcc.dg/pr97172.c
new file mode 100644
index 00000000000..ab5b2e9e7e9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr97172.c
@@ -0,0 +1,50 @@
+/* PR middle-end/97172 - ICE: tree code ‘ssa_name’ is not supported in LTO
+ streams
+ { dg-do compile }
+ { dg-options "-Wall -flto" }
+ { dg-require-effective-target lto } */
+
+int n;
+
+void fn (int a[n]);
+void fnp1 (int a[n + 1]);
+
+void fx_n (int a[][n]);
+void fx_np1 (int a[][n + 1]);
+
+void f2_n (int a[2][n]);
+void f2_np1 (int a[2][n + 1]);
+
+void fn_3 (int a[n][3]);
+void fnp1_3 (int a[n + 1][3]);
+
+void fn_n (int a[n][n]);
+void fn_np1 (int a[n][n + 1]);
+void fnp1_np1 (int a[n + 1][n + 1]);
+
+void fn_n_n (int a[n][n][n]);
+void fn_n_np1 (int a[n][n][n + 1]);
+void fn_np1_np1 (int a[n][n + 1][n + 1]);
+void fnp1_np1_np1 (int a[n + 1][n + 1][n + 1]);
+
+
+void gn (int a[n]) { fn (a); }
+void gnp1 (int a[n + 1]) { fnp1 (a); }
+
+void gx_n (int a[][n]) { fx_n (a); }
+void gx_np1 (int a[][n + 1]) { fx_np1 (a); }
+
+void g2_n (int a[2][n]) { f2_n (a); }
+void g2_np1 (int a[2][n + 1]) { f2_np1 (a); }
+
+void gn_3 (int a[n][3]) { fn_3 (a); }
+void gnp1_3 (int a[n + 1][3]) { fnp1_3 (a); }
+
+void gn_n (int a[n][n]) { fn_n (a); }
+void gn_np1 (int a[n][n + 1]) { fn_np1 (a); }
+void gnp1_np1 (int a[n + 1][n + 1]) { fnp1_np1 (a); }
+
+void gn_n_n (int a[n][n][n]) { fn_n_n (a); }
+void gn_n_np1 (int a[n][n][n + 1]) { fn_n_np1 (a); }
+void gn_np1_np1 (int a[n][n + 1][n + 1]) { fn_np1_np1 (a); }
+void gnp1_np1_np1 (int a[n + 1][n + 1][n + 1]) { fnp1_np1_np1 (a); }
