[Bug c++/108887] [13 Regression] ICE in process_function_and_variable_attributes since r13-3601
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108887 Jason Merrill changed: What|Removed |Added Resolution|--- |FIXED Status|ASSIGNED|RESOLVED --- Comment #10 from Jason Merrill --- Fixed.
[Bug c++/108887] [13 Regression] ICE in process_function_and_variable_attributes since r13-3601
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108887 --- Comment #9 from CVS Commits --- The trunk branch has been updated by Jason Merrill : https://gcc.gnu.org/g:a23b33a1bdeff7bc2289d9ebb7cb7b7ec0a605f5 commit r13-6944-ga23b33a1bdeff7bc2289d9ebb7cb7b7ec0a605f5 Author: Jason Merrill Date: Mon Mar 6 15:33:45 2023 -0500 c++: lambda mangling alias issues [PR107897] In 107897, by the time we are looking at the mangling clash, the alias has already been removed from the symbol table by analyze_functions, so we can't look at n->cpp_implicit_alias. So just assume that it's an alias if it's internal. In 108887 the problem is that removing the mangling alias from the symbol table confuses analyze_functions, because it ended up as first_analyzed somehow, so it becomes a dangling pointer. So instead we call reset() to neutralize the alias. To make this work for variables, I needed to move reset() from cgraph_node to symtab_node. PR c++/107897 PR c++/108887 gcc/ChangeLog: * cgraph.h: Move reset() from cgraph_node to symtab_node. * cgraphunit.cc (symtab_node::reset): Adjust. Also call remove_from_same_comdat_group. gcc/cp/ChangeLog: * decl2.cc (record_mangling): Use symtab_node::reset. gcc/testsuite/ChangeLog: * g++.dg/cpp2a/concepts-lambda3.C: Use -flto if supported. * g++.dg/cpp0x/lambda/lambda-mangle7.C: New test.
[Bug c++/108887] [13 Regression] ICE in process_function_and_variable_attributes since r13-3601
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108887 --- Comment #8 from Jan Hubicka --- > Also, reset() is only defined in cgraph_node, and I need it to work on both > functions and variables. Aha, this is a good point. I forgot that. I will make reset() working on symbols in general. I think it is cleanest way ahead. > > Clearing n->type seems to confuse things that expect all symbols to be either > function or variable. Yes, SYMBOL is not really used in symbol table except for temporary situations during the construction. So I would indeed expect things to be confused.
[Bug c++/108887] [13 Regression] ICE in process_function_and_variable_attributes since r13-3601
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108887 --- Comment #7 from Jason Merrill --- Also, reset() is only defined in cgraph_node, and I need it to work on both functions and variables. Clearing n->type seems to confuse things that expect all symbols to be either function or variable.
[Bug c++/108887] [13 Regression] ICE in process_function_and_variable_attributes since r13-3601
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108887 --- Comment #6 from Jason Merrill --- (In reply to Jan Hubicka from comment #3) > I think we may get around with only turning the node back to non-declaration > by calling n->reset()? It seems I also need to n->remove_from_same_comdat_group(). Should reset() do that? What about all the other things that remove() does before actually freeing the node? Would setting n->type to SYMTAB_SYMBOL be a suitable way of marking it as dead?
[Bug c++/108887] [13 Regression] ICE in process_function_and_variable_attributes since r13-3601
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108887 Jason Merrill changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |jason at gcc dot gnu.org
[Bug c++/108887] [13 Regression] ICE in process_function_and_variable_attributes since r13-3601
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108887 --- Comment #5 from Jan Hubicka --- > Perhaps, but shouldn't we also unlink_from_assembler_name_hash (node, false);? > I think the point of the current removal is that we've discovered the mangling > alias clashes with some other symbol. cgraph_nodes are associated to decls and we allow multiple nodes to share same name. So I think if you just reset it, things will work since new node will be created for the clashing decl and the alias will eventually be removed as unused (at least I hope so - this is a slipperly side-case)
[Bug c++/108887] [13 Regression] ICE in process_function_and_variable_attributes since r13-3601
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108887 --- Comment #4 from Jakub Jelinek --- Perhaps, but shouldn't we also unlink_from_assembler_name_hash (node, false);? I think the point of the current removal is that we've discovered the mangling alias clashes with some other symbol.
[Bug c++/108887] [13 Regression] ICE in process_function_and_variable_attributes since r13-3601
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108887 --- Comment #3 from Jan Hubicka --- We don't really have way to mark nodes for removal. I am not 100% sure I understand what the code does, but removing random nodes from cgraph in hook invoked from mangling seems dangerous, since we invoke DECL_ASSEMBLER_NODE quite randomly. I think we may get around with only turning the node back to non-delcaration by calling n->reset()? Honza
[Bug c++/108887] [13 Regression] ICE in process_function_and_variable_attributes since r13-3601
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108887 --- Comment #2 from Jakub Jelinek --- Honza, one possibility would be to add_cgraph_removal_hook for the duration of analyze_functions which would update first_analyzed and remove it at the end. Or do we have some way to mark a symtab node for deferred removal, instead of removing it right away?
[Bug c++/108887] [13 Regression] ICE in process_function_and_variable_attributes since r13-3601
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108887
--- Comment #1 from Jakub Jelinek ---
The ICE is actually in cgraph code, so it might as well be just some latent
cgraph bug triggered by the C++ changes.
What I see is that first_analyzed is set to a cgraph node for
_ZZN27LinkPaginationTimelineModel12fillTimelineEvENKUlP15AbstractAccountE0_clES1_
But that node is then removed in:
#0 0x77a4ec8f in __memset_evex_unaligned_erms () from /lib64/libc.so.6
#1 0x0094ec0b in ggc_free (p=0x7fffe9e93bb0) at
../../gcc/ggc-page.cc:1630
#2 0x00a24fb6 in symbol_table::release_symbol (this=0x7fffea13,
node=)
at ../../gcc/cgraph.h:2853
#3 0x00a1d498 in cgraph_node::remove (this=) at
../../gcc/cgraph.cc:1974
#4 0x00a08369 in symtab_node::remove (this=) at ../../gcc/symtab.cc:481
#5 0x005866e4 in record_mangling (decl=, need_warning=true) at ../../gcc/cp/decl2.cc:4751
#6 0x005f5d5d in mangle_decl (decl=) at ../../gcc/cp/mangle.cc:4196
#7 0x01456c70 in decl_assembler_name (decl=) at ../../gcc/tree.cc:743
#8 0x01457234 in assign_assembler_name_if_needed (t=) at ../../gcc/tree.cc:858
#9 0x00a2cf4d in cgraph_node::analyze (this=) at ../../gcc/cgraphunit.cc:669
#10 0x00a2effe in analyze_functions (first_time=true) at
../../gcc/cgraphunit.cc:1238
but nothing updates first_analyzed.
record_mangling has:
/* If this is already an alias, remove the alias, because the real
decl takes precedence. */
if (*slot && DECL_ARTIFICIAL (*slot) && DECL_IGNORED_P (*slot))
if (symtab_node *n = symtab_node::get (*slot))
if (n->cpp_implicit_alias)
{
n->remove ();
*slot = NULL_TREE;
}
So, to some extent this is related to PR107897.
The ICE is obviously gone with -fabi-compat-version=0 and so if we wouldn't
emit compatibility aliases for lambdas based on PR107897, this issue would be
latent again.
[Bug c++/108887] [13 Regression] ICE in process_function_and_variable_attributes since r13-3601
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108887 Marek Polacek changed: What|Removed |Added Last reconfirmed||2023-02-22 Status|UNCONFIRMED |NEW Ever confirmed|0 |1 Target Milestone|--- |13.0 Priority|P3 |P1 Keywords||ice-on-valid-code CC||mpolacek at gcc dot gnu.org
