Ping Re: [PATCH RFC] c++: lambda mangling alias issues [PR107897]

[Jason Merrill <jason@redhat.com>]

On 3/8/23 11:54, Jason Merrill wrote:
> On 3/8/23 11:15, Jason Merrill wrote:
>> On 3/8/23 10:53, Jan Hubicka wrote:
>>>> Tested x86_64-pc-linux-gnu.  Does this look good, or do we want to 
>>>> factor the
>>>> flag clearing into a symtab_node counterpart to cgraph_node::reset?
>>>>
>>>> -- 8< --
>>>>
>>>> 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.  Fixed by clearing 
>>>> various flags
>>>> to neutralize the alias.
>>>>
>>>>     PR c++/107897
>>>>     PR c++/108887
>>>>
>>>> gcc/cp/ChangeLog:
>>>>
>>>>     * decl2.cc (record_mangling): Improve symbol table handling.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>>     * g++.dg/cpp2a/concepts-lambda3.C: Use -flto if supported.
>>>>     * g++.dg/cpp0x/lambda/lambda-mangle7.C: New test.
>>>> ---
>>>>   gcc/cp/decl2.cc                               | 25 +++++--
>>>>   .../g++.dg/cpp0x/lambda/lambda-mangle7.C      | 70 
>>>> +++++++++++++++++++
>>>>   gcc/testsuite/g++.dg/cpp2a/concepts-lambda3.C |  1 +
>>>>   3 files changed, 91 insertions(+), 5 deletions(-)
>>>>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-mangle7.C
>>>>
>>>> diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
>>>> index 387e24542cd..e6e58b08de4 100644
>>>> --- a/gcc/cp/decl2.cc
>>>> +++ b/gcc/cp/decl2.cc
>>>> @@ -4742,15 +4742,30 @@ record_mangling (tree decl, bool need_warning)
>>>>       = mangled_decls->find_slot_with_hash (id, 
>>>> IDENTIFIER_HASH_VALUE (id),
>>>>                         INSERT);
>>>> -  /* If this is already an alias, remove the alias, because the real
>>>> +  /* If this is already an alias, cancel 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)
>>>> +    {
>>>> +      if (symtab_node *n = symtab_node::get (*slot))
>>>>       {
>>>> -      n->remove ();
>>>> -      *slot = NULL_TREE;
>>>> +      if (n->cpp_implicit_alias)
>>>> +        {
>>>> +          /* Actually removing the node isn't safe if other code is 
>>>> already
>>>> +         holding a pointer to it, so just neutralize it.  */
>>>> +          n->remove_from_same_comdat_group ();
>>>> +          n->analyzed = false;
>>>> +          n->definition = false;
>>>> +          n->alias = false;
>>>> +          n->cpp_implicit_alias = false;
>>> We have n->reset () for that which is used in similar situation when
>>> frontends overwrites extern inline function by its different offline
>>> implementation.
>>
>> The problem there is that reset() is a member of cgraph_node, not 
>> symtab_node, and I need something that works for variables as well.
>>
>>> reset doesn't call remove_from_same_comdat_group probably because it was
>>> never used on anything in comdat group.  So I think it would make sense
>>> to call n->reset() here and add remove_from_same_comdat_group into that.
> 
> How about moving it to symtab_node and using dyn_cast for the cgraph 
> bits, like this:

Ping? (Patch in attachment in earlier message)