[PATCH] "used" attribute saves decl from linker garbage collection

[Jeff Law]

On 11/6/20 5:13 PM, H.J. Lu wrote:
> On Fri, Nov 6, 2020 at 4:01 PM Jeff Law <law@redhat.com> wrote:
>>
>> On 11/6/20 4:45 PM, H.J. Lu wrote:
>>> On Fri, Nov 6, 2020 at 3:37 PM Jeff Law <law@redhat.com> wrote:
>>>> On 11/6/20 4:29 PM, H.J. Lu wrote:
>>>>> On Fri, Nov 6, 2020 at 3:22 PM Jeff Law <law@redhat.com> wrote:
>>>>>> On 11/5/20 7:34 AM, H.J. Lu via Gcc-patches wrote:
>>>>>>> On Thu, Nov 5, 2020 at 3:37 AM Jozef Lawrynowicz
>>>>>>> <jozef.l@mittosystems.com> wrote:
>>>>>>>> On Thu, Nov 05, 2020 at 06:21:21AM -0500, Hans-Peter Nilsson wrote:
>>>>>>>>> On Wed, 4 Nov 2020, H.J. Lu wrote:
>>>>>>>>>> .retain is ill-defined.   For example,
>>>>>>>>>>
>>>>>>>>>> [hjl@gnu-cfl-2 gcc]$ cat /tmp/x.c
>>>>>>>>>> static int xyzzy __attribute__((__used__));
>>>>>>>>>> [hjl@gnu-cfl-2 gcc]$ ./xgcc -B./ -S /tmp/x.c -fcommon
>>>>>>>>>> [hjl@gnu-cfl-2 gcc]$ cat x.s
>>>>>>>>>> .file "x.c"
>>>>>>>>>> .text
>>>>>>>>>> .retain xyzzy  <<<<<<<<< What does it do?
>>>>>>>>>> .local xyzzy
>>>>>>>>>> .comm xyzzy,4,4
>>>>>>>>>> .ident "GCC: (GNU) 11.0.0 20201103 (experimental)"
>>>>>>>>>> .section .note.GNU-stack,"",@progbits
>>>>>>>>>> [hjl@gnu-cfl-2 gcc]$
>>>>>>>>> To answer that question: it's up to the assembler, but for ELF
>>>>>>>>> and SHF_GNU_RETAIN, it seems obvious it'd tell the assembler to
>>>>>>>>> set SHF_GNU_RETAIN for the section where the symbol ends up.
>>>>>>>>> We both know this isn't rocket science with binutils.
>>>>>>>> Indeed, and my patch handles it trivially:
>>>>>>>> https://sourceware.org/pipermail/binutils/2020-November/113993.html
>>>>>>>>
>>>>>>>>   +void
>>>>>>>>   +obj_elf_retain (int arg ATTRIBUTE_UNUSED)
>>>>>>>>   .... snip ....
>>>>>>>>   +  sym = get_sym_from_input_line_and_check ();
>>>>>>>>   +  symbol_get_obj (sym)->retain = 1;
>>>>>>>>
>>>>>>>>   @@ -2624,6 +2704,9 @@ elf_frob_symbol (symbolS *symp, int *puntp)
>>>>>>>>     }
>>>>>>>>        }
>>>>>>>>
>>>>>>>>   +  if (symbol_get_obj (symp)->retain)
>>>>>>>>   +    elf_section_flags (S_GET_SEGMENT (symp)) |= SHF_GNU_RETAIN;
>>>>>>>>   +
>>>>>>>>      /* Double check weak symbols.  */
>>>>>>>>      if (S_IS_WEAK (symp))
>>>>>>>>        {
>>>>>>>>
>>>>>>>> We could check that the symbol named in the .retain directive has
>>>>>>>> already been defined, however this isn't compatible with GCC
>>>>>>>> mark_decl_preserved handling, since mark_decl_preserved is called
>>>>>>>> emitted before the local symbols are defined in the assembly output
>>>>>>>> file.
>>>>>>>>
>>>>>>>> GAS should at least validate that the symbol named in the .retain
>>>>>>>> directive does end up as a symbol though.
>>>>>>>>
>>>>>>> Don't add .retain.
>>>>>> Why?  I don't see why you find it so objectionable.
>>>>>>
>>>>> An ELF symbol directive should operate on symbol table:
>>>>>
>>>>> http://www.sco.com/developers/gabi/latest/ch4.symtab.html
>>>>>
>>>>> not the section flags where the symbol is defined.
>>>> I agree in general, but I think this is one of those cases where it's
>>>> not so clear.  And what you're talking about is an implementation detail.
>>> There is no need for such a hack.  The proper thing to do in ELF is
>>> to place such a symbol in a section with SHF_GNU_RETAIN flag.   This
>>> also avoids the question what to do with SHN_COMMON.
>> I'm not sure that's a good idea either.  Moving symbols into a section
>> other than they'd normally live doesn't seem all that wise.
> In ELF, a symbol must be defined in a section.  If we want to keep a symbol,
> we should place it in an SHF_GNU_RETAIN section.

Again, that's an implementation detail and it's not clear to me that one
approach is inherently better than the other.


>
>> Let's face it, there's not a great solution here.  If we mark its
>> existing section, then everything in that section gets kept.  If we put
> FWIW, this is what .retain direct does and is one reason why I object
> it.

We could make .retain work with either approach.    I don't see .retain
as a problem at all. 



>
>> the object into a different section than it would normally live, then
>> that opens a whole new can of worms.
> We should place it in a section which it normally lives in and mark the
> section with SHF_GNU_RETAIN.

And why not do that with .retain?  We define its semantics as precisely
what you've written above.  The referenced symbol goes into its usual
section and its section is marked with SHF_GNU_RETAIN.  That seems much
cleaner than having to track all this in the compiler so that it can
twiddle the section flags.


jeff

>