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

2020-11-05 Thread H.J. Lu via Gcc-patches
On Thu, Nov 5, 2020 at 3:37 AM Jozef Lawrynowicz
 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.

-- 
H.J.


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

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/5/20 4:00 AM, Jozef Lawrynowicz wrote:
> On Wed, Nov 04, 2020 at 03:58:56PM -0800, H.J. Lu wrote:
>> On Wed, Nov 4, 2020 at 3:00 PM Hans-Peter Nilsson  wrote:
>>> On Wed, 4 Nov 2020, H.J. Lu wrote:
 On Wed, Nov 4, 2020 at 1:56 PM Hans-Peter Nilsson  
 wrote:
> On Wed, 4 Nov 2020, H.J. Lu wrote:
>
>> On Wed, Nov 4, 2020 at 1:03 PM Hans-Peter Nilsson  
>> wrote:
>>> On Wed, 4 Nov 2020, H.J. Lu wrote:
 On Wed, Nov 4, 2020 at 10:09 AM Hans-Peter Nilsson  
 wrote:
> I'm not much more than a random voice, but an assembly directive
> that specifies the symbol (IIUC your .retain directive) to
 But .retain directive DOES NOT adjust symbol attribute.
> I see I missed to point out that I was speaking about the *gcc
> symbol* attribute "used".
 There is no such corresponding symbol attribute in ELF.
>>> I have not missed that, nor that SHF_GNU_RETAIN is so new that
>>> it's not in binutils master.  I have also not missed that gcc
>>> caters to other object formats too.  A common symbol-specific
>>> directive such as .retain, would be better than messing with
>>> section attributes, for gcc.
>> This is totally irrelevant to SHF_GNU_RETAIN.
>>
> It's cleaner to the compiler if it can pass on to the assembler
> the specific symbol that needs to be kept.
>
 SHF_GNU_RETAIN is for section and GCC should place the symbol,
 which should be kept, in the SHF_GNU_RETAIN section directly, not
 through .retain directive.
>>> This is where opinions differ.  Anyway, this is now repetition;
>>> I'm done.
>> .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]$
>>
>> A symbol directive should operate on the symbol table.
>> With 'R' flag, we got
>>
>> .file "x.c"
>> .text
>> .section .bss.xyzzy,"awR",@nobits
>> .align 4
>> .type xyzzy, @object
>> .size xyzzy, 4
>> xyzzy:
>> .zero 4
>> .ident "GCC: (GNU) 11.0.0 20201104 (experimental)"
>> .section .note.GNU-stack,"",@progbits
> I still think it is very wrong for the "used" attribute to place the
> symbol in a unique section. The structure of the sections in the object
> file should be no different whether the "used" attribute was applied to
> a symbol or not.

I tend to agree here.  Also note that someone could have a section
attribute in addition to the used attribute and that section attribute
might reference any arbitrary section.


> I will therefore have to make changes to GCC so that we can get the name
> of "unnamed" sections, and emit a .section directive with the "R" flag
> set on that section name, in order to avoid using a .retain directive.

ISTM that we could have the .retain  set the R flag in the bfd
section associated with 's definition.  That has other impacts
(namely that anything else in the same section is retained as well).


THe other alternative is to carry the attribute on the symbol into the
linker and teach the linker about the new symbol flag.


I don't have any fundamental objections to .retain.  I'm not sure why HJ
is so dead set against it.


jeff



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

2020-11-06 Thread Jeff Law via Gcc-patches


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
>  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.


jeff



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

2020-11-06 Thread H.J. Lu via Gcc-patches
On Fri, Nov 6, 2020 at 3:22 PM Jeff Law  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
> >  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.

-- 
H.J.


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

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/6/20 4:29 PM, H.J. Lu wrote:
> On Fri, Nov 6, 2020 at 3:22 PM Jeff Law  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
>>>  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.


jeff



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

2020-11-06 Thread H.J. Lu via Gcc-patches
On Fri, Nov 6, 2020 at 3:37 PM Jeff Law  wrote:
>
>
> On 11/6/20 4:29 PM, H.J. Lu wrote:
> > On Fri, Nov 6, 2020 at 3:22 PM Jeff Law  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
> >>>  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.

-- 
H.J.


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

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/6/20 4:45 PM, H.J. Lu wrote:
> On Fri, Nov 6, 2020 at 3:37 PM Jeff Law  wrote:
>>
>> On 11/6/20 4:29 PM, H.J. Lu wrote:
>>> On Fri, Nov 6, 2020 at 3:22 PM Jeff Law  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
>  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.


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
the object into a different section than it would normally live, then
that opens a whole new can of worms.


jeff



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

2020-11-06 Thread H.J. Lu via Gcc-patches
On Fri, Nov 6, 2020 at 4:01 PM Jeff Law  wrote:
>
>
> On 11/6/20 4:45 PM, H.J. Lu wrote:
> > On Fri, Nov 6, 2020 at 3:37 PM Jeff Law  wrote:
> >>
> >> On 11/6/20 4:29 PM, H.J. Lu wrote:
> >>> On Fri, Nov 6, 2020 at 3:22 PM Jeff Law  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
> >  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.

>
> 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.

> 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.

-- 
H.J.


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

2020-11-06 Thread Jeff Law via Gcc-patches


On 11/6/20 5:13 PM, H.J. Lu wrote:
> On Fri, Nov 6, 2020 at 4:01 PM Jeff Law  wrote:
>>
>> On 11/6/20 4:45 PM, H.J. Lu wrote:
>>> On Fri, Nov 6, 2020 at 3:37 PM Jeff Law  wrote:
 On 11/6/20 4:29 PM, H.J. Lu wrote:
> On Fri, Nov 6, 2020 at 3:22 PM Jeff Law  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
>>>  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

>



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

2020-11-06 Thread H.J. Lu via Gcc-patches
On Fri, Nov 6, 2020 at 4:17 PM Jeff Law  wrote:
>
>
> On 11/6/20 5:13 PM, H.J. Lu wrote:
> > On Fri, Nov 6, 2020 at 4:01 PM Jeff Law  wrote:
> >>
> >> On 11/6/20 4:45 PM, H.J. Lu wrote:
> >>> On Fri, Nov 6, 2020 at 3:37 PM Jeff Law  wrote:
>  On 11/6/20 4:29 PM, H.J. Lu wrote:
> > On Fri, Nov 6, 2020 at 3:22 PM Jeff Law  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
> >>>  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

But the .retain directive implementation being discussed here is different.
One problem with the .retain directive is we can have

.section .data
foo:
...
bar:

.retain bar
...
xxx:
...

What should assembler do with ".retain bar"?

> 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.

When GCC emits a symbol definition, it places the symbol in a section
with proper
attributes wh

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

2020-11-09 Thread Jozef Lawrynowicz
On Fri, Nov 06, 2020 at 04:39:33PM -0800, H.J. Lu via Gcc-patches wrote:
> On Fri, Nov 6, 2020 at 4:17 PM Jeff Law  wrote:
> >
> >
> > On 11/6/20 5:13 PM, H.J. Lu wrote:
> > > On Fri, Nov 6, 2020 at 4:01 PM Jeff Law  wrote:
> > >>
> > >> On 11/6/20 4:45 PM, H.J. Lu wrote:
> > >>> On Fri, Nov 6, 2020 at 3:37 PM Jeff Law  wrote:
> >  On 11/6/20 4:29 PM, H.J. Lu wrote:
> > > On Fri, Nov 6, 2020 at 3:22 PM Jeff Law  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
> > >>>  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
> 
> But the .retain directive implementation being discussed here is different.
> One problem with the .retain directive is we can have
> 
> .section .data
> foo:
> ...
> bar:
> 
> .retain bar
> ...
> xxx:
> ...
> 
> What should assembler do with ".retain bar"?
> 
> > what you've 

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

2020-11-09 Thread H.J. Lu via Gcc-patches
On Mon, Nov 9, 2020 at 9:41 AM Jozef Lawrynowicz
 wrote:
>
> On Fri, Nov 06, 2020 at 04:39:33PM -0800, H.J. Lu via Gcc-patches wrote:
> > On Fri, Nov 6, 2020 at 4:17 PM Jeff Law  wrote:
> > >
> > >
> > > On 11/6/20 5:13 PM, H.J. Lu wrote:
> > > > On Fri, Nov 6, 2020 at 4:01 PM Jeff Law  wrote:
> > > >>
> > > >> On 11/6/20 4:45 PM, H.J. Lu wrote:
> > > >>> On Fri, Nov 6, 2020 at 3:37 PM Jeff Law  wrote:
> > >  On 11/6/20 4:29 PM, H.J. Lu wrote:
> > > > On Fri, Nov 6, 2020 at 3:22 PM Jeff Law  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
> > > >>>  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 w

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

2020-11-09 Thread Jozef Lawrynowicz
On Mon, Nov 09, 2020 at 10:36:07AM -0800, H.J. Lu via Gcc-patches wrote:
> On Mon, Nov 9, 2020 at 9:41 AM Jozef Lawrynowicz
>  wrote:
> >
> > On Fri, Nov 06, 2020 at 04:39:33PM -0800, H.J. Lu via Gcc-patches wrote:
> > > On Fri, Nov 6, 2020 at 4:17 PM Jeff Law  wrote:
> > > >
> > > >
> > > > On 11/6/20 5:13 PM, H.J. Lu wrote:
> > > > > On Fri, Nov 6, 2020 at 4:01 PM Jeff Law  wrote:
> > > > >>
> > > > >> On 11/6/20 4:45 PM, H.J. Lu wrote:
> > > > >>> On Fri, Nov 6, 2020 at 3:37 PM Jeff Law  wrote:
> > > >  On 11/6/20 4:29 PM, H.J. Lu wrote:
> > > > > On Fri, Nov 6, 2020 at 3:22 PM Jeff Law  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
> > > > >>>  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 

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

2020-11-09 Thread H.J. Lu via Gcc-patches
On Mon, Nov 9, 2020 at 11:56 AM Jozef Lawrynowicz
 wrote:
>
> On Mon, Nov 09, 2020 at 10:36:07AM -0800, H.J. Lu via Gcc-patches wrote:
> > On Mon, Nov 9, 2020 at 9:41 AM Jozef Lawrynowicz
> >  wrote:
> > >
> > > On Fri, Nov 06, 2020 at 04:39:33PM -0800, H.J. Lu via Gcc-patches wrote:
> > > > On Fri, Nov 6, 2020 at 4:17 PM Jeff Law  wrote:
> > > > >
> > > > >
> > > > > On 11/6/20 5:13 PM, H.J. Lu wrote:
> > > > > > On Fri, Nov 6, 2020 at 4:01 PM Jeff Law  wrote:
> > > > > >>
> > > > > >> On 11/6/20 4:45 PM, H.J. Lu wrote:
> > > > > >>> On Fri, Nov 6, 2020 at 3:37 PM Jeff Law  wrote:
> > > > >  On 11/6/20 4:29 PM, H.J. Lu wrote:
> > > > > > On Fri, Nov 6, 2020 at 3:22 PM Jeff Law  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
> > > > > >>>  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.
> 

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

2020-11-10 Thread Jozef Lawrynowicz
On Mon, Nov 09, 2020 at 12:31:09PM -0800, H.J. Lu via Gcc-patches wrote:
> On Mon, Nov 9, 2020 at 11:56 AM Jozef Lawrynowicz
>  wrote:
> >
> > On Mon, Nov 09, 2020 at 10:36:07AM -0800, H.J. Lu via Gcc-patches wrote:
> > > On Mon, Nov 9, 2020 at 9:41 AM Jozef Lawrynowicz
> > >  wrote:
> > > >
> > > > On Fri, Nov 06, 2020 at 04:39:33PM -0800, H.J. Lu via Gcc-patches wrote:
> > > > > On Fri, Nov 6, 2020 at 4:17 PM Jeff Law  wrote:
> > > > > >
> > > > > >
> > > > > > On 11/6/20 5:13 PM, H.J. Lu wrote:
> > > > > > > On Fri, Nov 6, 2020 at 4:01 PM Jeff Law  wrote:
> > > > > > >>
> > > > > > >> On 11/6/20 4:45 PM, H.J. Lu wrote:
> > > > > > >>> On Fri, Nov 6, 2020 at 3:37 PM Jeff Law  wrote:
> > > > > >  On 11/6/20 4:29 PM, H.J. Lu wrote:
> > > > > > > On Fri, Nov 6, 2020 at 3:22 PM Jeff Law  
> > > > > > > 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
> > > > > > >>>  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 t

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

2020-11-03 Thread H.J. Lu via Gcc-patches
On Tue, Nov 3, 2020 at 9:41 AM Jozef Lawrynowicz
 wrote:
>
> The attached patch implements TARGET_ASM_MARK_DECL_PRESERVED for ELF GNU
> OSABI targets, so that declarations that have the "used" attribute
> applied will be saved from linker garbage collection.
>
> TARGET_ASM_MARK_DECL_PRESERVED will emit an assembler ".retain"

Can you use the "R" flag instead?

> directive for the decl, and the assembler will apply the SHF_GNU_RETAIN
> flag to the section containing the decl.
> The linker will not garbage collect sections marked with the
> SHF_GNU_RETAIN flag.
>
> SHF_GNU_RETAIN is a GNU OSABI ELF extension, and it was discussed on the
> GNU gABI mailing list here:
> https://sourceware.org/pipermail/gnu-gabi/2020q3/000429.html
>
> The Binutils patch to implement .retain and other SHF_GNU_RETAIN
> handling is posted here:
> https://sourceware.org/pipermail/binutils/2020-November/113993.html
>
> Successfully bootstrapped and regtested for x86_64-pc-linux-gnu, and
> regtested for arm-none-eabi.
>
> Ok for trunk?
>
> Thanks,
> Jozef



-- 
H.J.


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

2020-11-03 Thread Jozef Lawrynowicz
On Tue, Nov 03, 2020 at 09:57:58AM -0800, H.J. Lu via Gcc-patches wrote:
> On Tue, Nov 3, 2020 at 9:41 AM Jozef Lawrynowicz
>  wrote:
> >
> > The attached patch implements TARGET_ASM_MARK_DECL_PRESERVED for ELF GNU
> > OSABI targets, so that declarations that have the "used" attribute
> > applied will be saved from linker garbage collection.
> >
> > TARGET_ASM_MARK_DECL_PRESERVED will emit an assembler ".retain"
> 
> Can you use the "R" flag instead?
> 

For the benefit of this mailing list, I have copied my response from the
Binutils mailing list regarding this.
The "comm_section" example I gave is actually innacurate, but you can
see the examples of the variety of sections that would need to be
handled by doing

$ git grep -A2 "define.*SECTION_ASM_OP" gcc/ | grep "\".*\."

> ... snip ...
> Secondly, for seamless integration with the "used" attribute, we must be
> able to to mark the symbol with the used attribute applied as "retained"
> without changing its section name. For GCC "named" sections, this is
> straightforward, but for "unnamed" sections it is a giant mess.
>
> The section name for a GCC "unnamed" section is not readily available,
> instead a string which contains the full assembly code to switch to one
> of these text/data/bss/rodata/comm etc. sections is encoded in the
> structure.
>
> Backends define the assembly code to switch to these sections (some
> "*ASM_OP*" macro) in a variety of ways. For example, the unnamed section
> "comm_section", might correspond to a .bss section, or emit a .comm
> directive. I even looked at trying to parse them to extract what the
> name of a section will be, but it would be very messy and not robust.
>
> Meanwhile, having a .retain  directive is a very simmple
> solution, and keeps the GCC implementation really concise (patch
> attached). The assembler will know for sure what the section containing
> the symbol will be, and can apply the SHF_GNU_RETAIN flag directly.
>
> Finally, having a .retain directive means that we don't need to support
> multiple sections with the same name, but different states for the "R"
> flag. For example, and Fangrui raised this point in previous discussion,
> the following is undesirable, as it violates the rule we have about
> section flags set in .section directives being the same for sections of
> the same name:
>
>   .section .text,"ax",%progbits
>...
>   .section .text,"axR",%progbits
>   
>
>
> The above would be required if GCC can only mark decls are retained by
> explicitly placing them in a section with the SHF_GNU_RETAIN flag
> applied. The .retain  directive greatly simplifies the
> process for GCC.

> > directive for the decl, and the assembler will apply the SHF_GNU_RETAIN
> > flag to the section containing the decl.
> > The linker will not garbage collect sections marked with the
> > SHF_GNU_RETAIN flag.
> >
> > SHF_GNU_RETAIN is a GNU OSABI ELF extension, and it was discussed on the
> > GNU gABI mailing list here:
> > https://sourceware.org/pipermail/gnu-gabi/2020q3/000429.html
> >
> > The Binutils patch to implement .retain and other SHF_GNU_RETAIN
> > handling is posted here:
> > https://sourceware.org/pipermail/binutils/2020-November/113993.html
> >
> > Successfully bootstrapped and regtested for x86_64-pc-linux-gnu, and
> > regtested for arm-none-eabi.
> >
> > Ok for trunk?
> >
> > Thanks,
> > Jozef
> 
> 
> 
> -- 
> H.J.


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

2020-11-03 Thread H.J. Lu via Gcc-patches
On Tue, Nov 3, 2020 at 10:22 AM Jozef Lawrynowicz
 wrote:
>
> On Tue, Nov 03, 2020 at 09:57:58AM -0800, H.J. Lu via Gcc-patches wrote:
> > On Tue, Nov 3, 2020 at 9:41 AM Jozef Lawrynowicz
> >  wrote:
> > >
> > > The attached patch implements TARGET_ASM_MARK_DECL_PRESERVED for ELF GNU
> > > OSABI targets, so that declarations that have the "used" attribute
> > > applied will be saved from linker garbage collection.
> > >
> > > TARGET_ASM_MARK_DECL_PRESERVED will emit an assembler ".retain"
> >
> > Can you use the "R" flag instead?
> >
>
> For the benefit of this mailing list, I have copied my response from the
> Binutils mailing list regarding this.
> The "comm_section" example I gave is actually innacurate, but you can
> see the examples of the variety of sections that would need to be
> handled by doing
>
> $ git grep -A2 "define.*SECTION_ASM_OP" gcc/ | grep "\".*\."
>
> > ... snip ...
> > Secondly, for seamless integration with the "used" attribute, we must be
> > able to to mark the symbol with the used attribute applied as "retained"
> > without changing its section name. For GCC "named" sections, this is
> > straightforward, but for "unnamed" sections it is a giant mess.
> >
> > The section name for a GCC "unnamed" section is not readily available,
> > instead a string which contains the full assembly code to switch to one
> > of these text/data/bss/rodata/comm etc. sections is encoded in the
> > structure.
> >
> > Backends define the assembly code to switch to these sections (some
> > "*ASM_OP*" macro) in a variety of ways. For example, the unnamed section
> > "comm_section", might correspond to a .bss section, or emit a .comm
> > directive. I even looked at trying to parse them to extract what the
> > name of a section will be, but it would be very messy and not robust.
> >
> > Meanwhile, having a .retain  directive is a very simmple
> > solution, and keeps the GCC implementation really concise (patch
> > attached). The assembler will know for sure what the section containing
> > the symbol will be, and can apply the SHF_GNU_RETAIN flag directly.
> >

Please take a look at

https://gitlab.com/x86-gcc/gcc/-/commits/users/hjl/elf/shf_retain

which is built in top of

https://gcc.gnu.org/pipermail/gcc-patches/2020-February/539963.html

I think SECTION2_RETAIN matches SHF_GNU_RETAIN well.  If you
want, you extract my flags2 change and use it for SHF_GNU_RETAIN.

-- 
H.J.


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

2020-11-03 Thread Jozef Lawrynowicz
On Tue, Nov 03, 2020 at 11:58:04AM -0800, H.J. Lu via Gcc-patches wrote:
> On Tue, Nov 3, 2020 at 10:22 AM Jozef Lawrynowicz
>  wrote:
> >
> > On Tue, Nov 03, 2020 at 09:57:58AM -0800, H.J. Lu via Gcc-patches wrote:
> > > On Tue, Nov 3, 2020 at 9:41 AM Jozef Lawrynowicz
> > >  wrote:
> > > >
> > > > The attached patch implements TARGET_ASM_MARK_DECL_PRESERVED for ELF GNU
> > > > OSABI targets, so that declarations that have the "used" attribute
> > > > applied will be saved from linker garbage collection.
> > > >
> > > > TARGET_ASM_MARK_DECL_PRESERVED will emit an assembler ".retain"
> > >
> > > Can you use the "R" flag instead?
> > >
> >
> > For the benefit of this mailing list, I have copied my response from the
> > Binutils mailing list regarding this.
> > The "comm_section" example I gave is actually innacurate, but you can
> > see the examples of the variety of sections that would need to be
> > handled by doing
> >
> > $ git grep -A2 "define.*SECTION_ASM_OP" gcc/ | grep "\".*\."
> >
> > > ... snip ...
> > > Secondly, for seamless integration with the "used" attribute, we must be
> > > able to to mark the symbol with the used attribute applied as "retained"
> > > without changing its section name. For GCC "named" sections, this is
> > > straightforward, but for "unnamed" sections it is a giant mess.
> > >
> > > The section name for a GCC "unnamed" section is not readily available,
> > > instead a string which contains the full assembly code to switch to one
> > > of these text/data/bss/rodata/comm etc. sections is encoded in the
> > > structure.
> > >
> > > Backends define the assembly code to switch to these sections (some
> > > "*ASM_OP*" macro) in a variety of ways. For example, the unnamed section
> > > "comm_section", might correspond to a .bss section, or emit a .comm
> > > directive. I even looked at trying to parse them to extract what the
> > > name of a section will be, but it would be very messy and not robust.
> > >
> > > Meanwhile, having a .retain  directive is a very simmple
> > > solution, and keeps the GCC implementation really concise (patch
> > > attached). The assembler will know for sure what the section containing
> > > the symbol will be, and can apply the SHF_GNU_RETAIN flag directly.
> > >
> 
> Please take a look at
> 
> https://gitlab.com/x86-gcc/gcc/-/commits/users/hjl/elf/shf_retain
> 
> which is built in top of
> 
> https://gcc.gnu.org/pipermail/gcc-patches/2020-February/539963.html
> 
> I think SECTION2_RETAIN matches SHF_GNU_RETAIN well.  If you
> want, you extract my flags2 change and use it for SHF_GNU_RETAIN.

In your patch you have to make the assumption that data_section, always
corresponds to a section named .data. For just this example, c6x (which
supports the GNU ELF OSABI) does not fit the rule:

> c6x/elf-common.h:#define DATA_SECTION_ASM_OP "\t.section\t\".fardata\",\"aw\""

data_section for c6x corresponds to .fardata, not .data. So the use of
"used" on a data declaration would place it in a different section, that
if the "used" attribute was not applied.

For c6x and mips, readonly_data_section does not correspond to .rodata,
so that assumption cannot be made either:
> c6x/elf-common.h:#define READONLY_DATA_SECTION_ASM_OP 
> "\t.section\t\".const\",\"a\",@progbits"
> mips/mips.h:#define READONLY_DATA_SECTION_ASM_OP"\t.rdata"  /* 
> read-only data */

The same can be said for bss_section for c6x as well.

Furthermore, this is only considering the examples in
default_elf_select_section - the less standard unnamed section are used
in many backend's implementation of select_section, and we would need to
work out what section name they correspond to to properly support
SHF_GNU_RETAIN.

For every unnamed section, you either have to assume what the
corresponding section name is, or parse the associated assembly output
string for the section.

Given these edge cases which must be handled for GCC to robustly emit
the "R" flag for sections containing "used" symbols, surely it is
preferable to leverage the existing TARGET_ASM_MARK_DECL_PRESERVED and
emit a .retain  directive, which is extremely simple and
doesn't require any handling of these edge cases and non-standard
backend implementations.

The point about multiple section directives, some with the "R" flag some
without, still applies as a downside to trying to emit the .section
directives for the "used" attribute.

Thanks,
Jozef

> 
> -- 
> H.J.


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

2020-11-03 Thread H.J. Lu via Gcc-patches
On Tue, Nov 3, 2020 at 12:46 PM Jozef Lawrynowicz
 wrote:
>
> On Tue, Nov 03, 2020 at 11:58:04AM -0800, H.J. Lu via Gcc-patches wrote:
> > On Tue, Nov 3, 2020 at 10:22 AM Jozef Lawrynowicz
> >  wrote:
> > >
> > > On Tue, Nov 03, 2020 at 09:57:58AM -0800, H.J. Lu via Gcc-patches wrote:
> > > > On Tue, Nov 3, 2020 at 9:41 AM Jozef Lawrynowicz
> > > >  wrote:
> > > > >
> > > > > The attached patch implements TARGET_ASM_MARK_DECL_PRESERVED for ELF 
> > > > > GNU
> > > > > OSABI targets, so that declarations that have the "used" attribute
> > > > > applied will be saved from linker garbage collection.
> > > > >
> > > > > TARGET_ASM_MARK_DECL_PRESERVED will emit an assembler ".retain"
> > > >
> > > > Can you use the "R" flag instead?
> > > >
> > >
> > > For the benefit of this mailing list, I have copied my response from the
> > > Binutils mailing list regarding this.
> > > The "comm_section" example I gave is actually innacurate, but you can
> > > see the examples of the variety of sections that would need to be
> > > handled by doing
> > >
> > > $ git grep -A2 "define.*SECTION_ASM_OP" gcc/ | grep "\".*\."
> > >
> > > > ... snip ...
> > > > Secondly, for seamless integration with the "used" attribute, we must be
> > > > able to to mark the symbol with the used attribute applied as "retained"
> > > > without changing its section name. For GCC "named" sections, this is
> > > > straightforward, but for "unnamed" sections it is a giant mess.
> > > >
> > > > The section name for a GCC "unnamed" section is not readily available,
> > > > instead a string which contains the full assembly code to switch to one
> > > > of these text/data/bss/rodata/comm etc. sections is encoded in the
> > > > structure.
> > > >
> > > > Backends define the assembly code to switch to these sections (some
> > > > "*ASM_OP*" macro) in a variety of ways. For example, the unnamed section
> > > > "comm_section", might correspond to a .bss section, or emit a .comm
> > > > directive. I even looked at trying to parse them to extract what the
> > > > name of a section will be, but it would be very messy and not robust.
> > > >
> > > > Meanwhile, having a .retain  directive is a very simmple
> > > > solution, and keeps the GCC implementation really concise (patch
> > > > attached). The assembler will know for sure what the section containing
> > > > the symbol will be, and can apply the SHF_GNU_RETAIN flag directly.
> > > >
> >
> > Please take a look at
> >
> > https://gitlab.com/x86-gcc/gcc/-/commits/users/hjl/elf/shf_retain
> >
> > which is built in top of
> >
> > https://gcc.gnu.org/pipermail/gcc-patches/2020-February/539963.html
> >
> > I think SECTION2_RETAIN matches SHF_GNU_RETAIN well.  If you
> > want, you extract my flags2 change and use it for SHF_GNU_RETAIN.
>
> In your patch you have to make the assumption that data_section, always
> corresponds to a section named .data. For just this example, c6x (which
> supports the GNU ELF OSABI) does not fit the rule:
>
> > c6x/elf-common.h:#define DATA_SECTION_ASM_OP 
> > "\t.section\t\".fardata\",\"aw\""
>
> data_section for c6x corresponds to .fardata, not .data. So the use of
> "used" on a data declaration would place it in a different section, that
> if the "used" attribute was not applied.
>
> For c6x and mips, readonly_data_section does not correspond to .rodata,
> so that assumption cannot be made either:
> > c6x/elf-common.h:#define READONLY_DATA_SECTION_ASM_OP 
> > "\t.section\t\".const\",\"a\",@progbits"
> > mips/mips.h:#define READONLY_DATA_SECTION_ASM_OP"\t.rdata"  /* 
> > read-only data */
>
> The same can be said for bss_section for c6x as well.

Just add and use named_xxx_section.

> Furthermore, this is only considering the examples in
> default_elf_select_section - the less standard unnamed section are used
> in many backend's implementation of select_section, and we would need to
> work out what section name they correspond to to properly support
> SHF_GNU_RETAIN.
>
> For every unnamed section, you either have to assume what the
> corresponding section name is, or parse the associated assembly output
> string for the section.

My change is just  an example to show how it can be done, not a complete one.

> Given these edge cases which must be handled for GCC to robustly emit
> the "R" flag for sections containing "used" symbols, surely it is
> preferable to leverage the existing TARGET_ASM_MARK_DECL_PRESERVED and
> emit a .retain  directive, which is extremely simple and
> doesn't require any handling of these edge cases and non-standard
> backend implementations.

It is used to update the symbol table.  Other usage is abuse.

> The point about multiple section directives, some with the "R" flag some
> without, still applies as a downside to trying to emit the .section
> directives for the "used" attribute.
>

-- 
H.J.


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

2020-11-03 Thread H.J. Lu via Gcc-patches
On Tue, Nov 3, 2020 at 1:00 PM H.J. Lu  wrote:
>
> On Tue, Nov 3, 2020 at 12:46 PM Jozef Lawrynowicz
>  wrote:
> >
> > On Tue, Nov 03, 2020 at 11:58:04AM -0800, H.J. Lu via Gcc-patches wrote:
> > > On Tue, Nov 3, 2020 at 10:22 AM Jozef Lawrynowicz
> > >  wrote:
> > > >
> > > > On Tue, Nov 03, 2020 at 09:57:58AM -0800, H.J. Lu via Gcc-patches wrote:
> > > > > On Tue, Nov 3, 2020 at 9:41 AM Jozef Lawrynowicz
> > > > >  wrote:
> > > > > >
> > > > > > The attached patch implements TARGET_ASM_MARK_DECL_PRESERVED for 
> > > > > > ELF GNU
> > > > > > OSABI targets, so that declarations that have the "used" attribute
> > > > > > applied will be saved from linker garbage collection.
> > > > > >
> > > > > > TARGET_ASM_MARK_DECL_PRESERVED will emit an assembler ".retain"
> > > > >
> > > > > Can you use the "R" flag instead?
> > > > >
> > > >
> > > > For the benefit of this mailing list, I have copied my response from the
> > > > Binutils mailing list regarding this.
> > > > The "comm_section" example I gave is actually innacurate, but you can
> > > > see the examples of the variety of sections that would need to be
> > > > handled by doing
> > > >
> > > > $ git grep -A2 "define.*SECTION_ASM_OP" gcc/ | grep "\".*\."
> > > >
> > > > > ... snip ...
> > > > > Secondly, for seamless integration with the "used" attribute, we must 
> > > > > be
> > > > > able to to mark the symbol with the used attribute applied as 
> > > > > "retained"
> > > > > without changing its section name. For GCC "named" sections, this is
> > > > > straightforward, but for "unnamed" sections it is a giant mess.
> > > > >
> > > > > The section name for a GCC "unnamed" section is not readily available,
> > > > > instead a string which contains the full assembly code to switch to 
> > > > > one
> > > > > of these text/data/bss/rodata/comm etc. sections is encoded in the
> > > > > structure.
> > > > >
> > > > > Backends define the assembly code to switch to these sections (some
> > > > > "*ASM_OP*" macro) in a variety of ways. For example, the unnamed 
> > > > > section
> > > > > "comm_section", might correspond to a .bss section, or emit a .comm
> > > > > directive. I even looked at trying to parse them to extract what the
> > > > > name of a section will be, but it would be very messy and not robust.
> > > > >
> > > > > Meanwhile, having a .retain  directive is a very simmple
> > > > > solution, and keeps the GCC implementation really concise (patch
> > > > > attached). The assembler will know for sure what the section 
> > > > > containing
> > > > > the symbol will be, and can apply the SHF_GNU_RETAIN flag directly.
> > > > >
> > >
> > > Please take a look at
> > >
> > > https://gitlab.com/x86-gcc/gcc/-/commits/users/hjl/elf/shf_retain
> > >
> > > which is built in top of
> > >
> > > https://gcc.gnu.org/pipermail/gcc-patches/2020-February/539963.html
> > >
> > > I think SECTION2_RETAIN matches SHF_GNU_RETAIN well.  If you
> > > want, you extract my flags2 change and use it for SHF_GNU_RETAIN.
> >
> > In your patch you have to make the assumption that data_section, always
> > corresponds to a section named .data. For just this example, c6x (which
> > supports the GNU ELF OSABI) does not fit the rule:
> >
> > > c6x/elf-common.h:#define DATA_SECTION_ASM_OP 
> > > "\t.section\t\".fardata\",\"aw\""
> >
> > data_section for c6x corresponds to .fardata, not .data. So the use of
> > "used" on a data declaration would place it in a different section, that
> > if the "used" attribute was not applied.
> >
> > For c6x and mips, readonly_data_section does not correspond to .rodata,
> > so that assumption cannot be made either:
> > > c6x/elf-common.h:#define READONLY_DATA_SECTION_ASM_OP 
> > > "\t.section\t\".const\",\"a\",@progbits"
> > > mips/mips.h:#define READONLY_DATA_SECTION_ASM_OP"\t.rdata"  
> > > /* read-only data */
> >
> > The same can be said for bss_section for c6x as well.
>
> Just add and use named_xxx_section.
>
> > Furthermore, this is only considering the examples in
> > default_elf_select_section - the less standard unnamed section are used
> > in many backend's implementation of select_section, and we would need to
> > work out what section name they correspond to to properly support
> > SHF_GNU_RETAIN.
> >
> > For every unnamed section, you either have to assume what the
> > corresponding section name is, or parse the associated assembly output
> > string for the section.
>
> My change is just  an example to show how it can be done, not a complete one.
>
> > Given these edge cases which must be handled for GCC to robustly emit
> > the "R" flag for sections containing "used" symbols, surely it is
> > preferable to leverage the existing TARGET_ASM_MARK_DECL_PRESERVED and
> > emit a .retain  directive, which is extremely simple and
> > doesn't require any handling of these edge cases and non-standard
> > backend implementations.
>
> It is used to update the symbol table.  Other usage is abuse.
>

For

[hjl@gnu-cfl-2 gcc]$ cat /tmp/x.

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

2020-11-03 Thread Jozef Lawrynowicz
On Tue, Nov 03, 2020 at 01:09:43PM -0800, H.J. Lu via Gcc-patches wrote:
> On Tue, Nov 3, 2020 at 1:00 PM H.J. Lu  wrote:
> >
> > On Tue, Nov 3, 2020 at 12:46 PM Jozef Lawrynowicz
> >  wrote:
> > >
> > > On Tue, Nov 03, 2020 at 11:58:04AM -0800, H.J. Lu via Gcc-patches wrote:
> > > > On Tue, Nov 3, 2020 at 10:22 AM Jozef Lawrynowicz
> > > >  wrote:
> > > > >
> > > > > On Tue, Nov 03, 2020 at 09:57:58AM -0800, H.J. Lu via Gcc-patches 
> > > > > wrote:
> > > > > > On Tue, Nov 3, 2020 at 9:41 AM Jozef Lawrynowicz
> > > > > >  wrote:
> > > > > > >
> > > > > > > The attached patch implements TARGET_ASM_MARK_DECL_PRESERVED for 
> > > > > > > ELF GNU
> > > > > > > OSABI targets, so that declarations that have the "used" attribute
> > > > > > > applied will be saved from linker garbage collection.
> > > > > > >
> > > > > > > TARGET_ASM_MARK_DECL_PRESERVED will emit an assembler ".retain"
> > > > > >
> > > > > > Can you use the "R" flag instead?
> > > > > >
> > > > >
> > > > > For the benefit of this mailing list, I have copied my response from 
> > > > > the
> > > > > Binutils mailing list regarding this.
> > > > > The "comm_section" example I gave is actually innacurate, but you can
> > > > > see the examples of the variety of sections that would need to be
> > > > > handled by doing
> > > > >
> > > > > $ git grep -A2 "define.*SECTION_ASM_OP" gcc/ | grep "\".*\."
> > > > >
> > > > > > ... snip ...
> > > > > > Secondly, for seamless integration with the "used" attribute, we 
> > > > > > must be
> > > > > > able to to mark the symbol with the used attribute applied as 
> > > > > > "retained"
> > > > > > without changing its section name. For GCC "named" sections, this is
> > > > > > straightforward, but for "unnamed" sections it is a giant mess.
> > > > > >
> > > > > > The section name for a GCC "unnamed" section is not readily 
> > > > > > available,
> > > > > > instead a string which contains the full assembly code to switch to 
> > > > > > one
> > > > > > of these text/data/bss/rodata/comm etc. sections is encoded in the
> > > > > > structure.
> > > > > >
> > > > > > Backends define the assembly code to switch to these sections (some
> > > > > > "*ASM_OP*" macro) in a variety of ways. For example, the unnamed 
> > > > > > section
> > > > > > "comm_section", might correspond to a .bss section, or emit a .comm
> > > > > > directive. I even looked at trying to parse them to extract what the
> > > > > > name of a section will be, but it would be very messy and not 
> > > > > > robust.
> > > > > >
> > > > > > Meanwhile, having a .retain  directive is a very 
> > > > > > simmple
> > > > > > solution, and keeps the GCC implementation really concise (patch
> > > > > > attached). The assembler will know for sure what the section 
> > > > > > containing
> > > > > > the symbol will be, and can apply the SHF_GNU_RETAIN flag directly.
> > > > > >
> > > >
> > > > Please take a look at
> > > >
> > > > https://gitlab.com/x86-gcc/gcc/-/commits/users/hjl/elf/shf_retain
> > > >
> > > > which is built in top of
> > > >
> > > > https://gcc.gnu.org/pipermail/gcc-patches/2020-February/539963.html
> > > >
> > > > I think SECTION2_RETAIN matches SHF_GNU_RETAIN well.  If you
> > > > want, you extract my flags2 change and use it for SHF_GNU_RETAIN.
> > >
> > > In your patch you have to make the assumption that data_section, always
> > > corresponds to a section named .data. For just this example, c6x (which
> > > supports the GNU ELF OSABI) does not fit the rule:
> > >
> > > > c6x/elf-common.h:#define DATA_SECTION_ASM_OP 
> > > > "\t.section\t\".fardata\",\"aw\""
> > >
> > > data_section for c6x corresponds to .fardata, not .data. So the use of
> > > "used" on a data declaration would place it in a different section, that
> > > if the "used" attribute was not applied.
> > >
> > > For c6x and mips, readonly_data_section does not correspond to .rodata,
> > > so that assumption cannot be made either:
> > > > c6x/elf-common.h:#define READONLY_DATA_SECTION_ASM_OP 
> > > > "\t.section\t\".const\",\"a\",@progbits"
> > > > mips/mips.h:#define READONLY_DATA_SECTION_ASM_OP"\t.rdata"  
> > > > /* read-only data */
> > >
> > > The same can be said for bss_section for c6x as well.
> >
> > Just add and use named_xxx_section.
> >

I guess new macros for targets that use non-standard names in unnamed
sections could work.

  c6x/elf-common.h:
#define READONLY_DATA_SECTION_NAME ".const"
#define READONLY_DATA_SECTION_ASM_OP 
"\t.section\t\".const\",\"a\",@progbits"

> > > Furthermore, this is only considering the examples in
> > > default_elf_select_section - the less standard unnamed section are used
> > > in many backend's implementation of select_section, and we would need to
> > > work out what section name they correspond to to properly support
> > > SHF_GNU_RETAIN.
> > >
> > > For every unnamed section, you either have to assume what the
> > > corresponding section name is, or parse the associated assembly output
> > > string for t

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

2020-11-03 Thread H.J. Lu via Gcc-patches
On Tue, Nov 3, 2020 at 1:57 PM Jozef Lawrynowicz
 wrote:
>
> On Tue, Nov 03, 2020 at 01:09:43PM -0800, H.J. Lu via Gcc-patches wrote:
> > On Tue, Nov 3, 2020 at 1:00 PM H.J. Lu  wrote:
> > >
> > > On Tue, Nov 3, 2020 at 12:46 PM Jozef Lawrynowicz
> > >  wrote:
> > > >
> > > > On Tue, Nov 03, 2020 at 11:58:04AM -0800, H.J. Lu via Gcc-patches wrote:
> > > > > On Tue, Nov 3, 2020 at 10:22 AM Jozef Lawrynowicz
> > > > >  wrote:
> > > > > >
> > > > > > On Tue, Nov 03, 2020 at 09:57:58AM -0800, H.J. Lu via Gcc-patches 
> > > > > > wrote:
> > > > > > > On Tue, Nov 3, 2020 at 9:41 AM Jozef Lawrynowicz
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > The attached patch implements TARGET_ASM_MARK_DECL_PRESERVED 
> > > > > > > > for ELF GNU
> > > > > > > > OSABI targets, so that declarations that have the "used" 
> > > > > > > > attribute
> > > > > > > > applied will be saved from linker garbage collection.
> > > > > > > >
> > > > > > > > TARGET_ASM_MARK_DECL_PRESERVED will emit an assembler ".retain"
> > > > > > >
> > > > > > > Can you use the "R" flag instead?
> > > > > > >
> > > > > >
> > > > > > For the benefit of this mailing list, I have copied my response 
> > > > > > from the
> > > > > > Binutils mailing list regarding this.
> > > > > > The "comm_section" example I gave is actually innacurate, but you 
> > > > > > can
> > > > > > see the examples of the variety of sections that would need to be
> > > > > > handled by doing
> > > > > >
> > > > > > $ git grep -A2 "define.*SECTION_ASM_OP" gcc/ | grep "\".*\."
> > > > > >
> > > > > > > ... snip ...
> > > > > > > Secondly, for seamless integration with the "used" attribute, we 
> > > > > > > must be
> > > > > > > able to to mark the symbol with the used attribute applied as 
> > > > > > > "retained"
> > > > > > > without changing its section name. For GCC "named" sections, this 
> > > > > > > is
> > > > > > > straightforward, but for "unnamed" sections it is a giant mess.
> > > > > > >
> > > > > > > The section name for a GCC "unnamed" section is not readily 
> > > > > > > available,
> > > > > > > instead a string which contains the full assembly code to switch 
> > > > > > > to one
> > > > > > > of these text/data/bss/rodata/comm etc. sections is encoded in the
> > > > > > > structure.
> > > > > > >
> > > > > > > Backends define the assembly code to switch to these sections 
> > > > > > > (some
> > > > > > > "*ASM_OP*" macro) in a variety of ways. For example, the unnamed 
> > > > > > > section
> > > > > > > "comm_section", might correspond to a .bss section, or emit a 
> > > > > > > .comm
> > > > > > > directive. I even looked at trying to parse them to extract what 
> > > > > > > the
> > > > > > > name of a section will be, but it would be very messy and not 
> > > > > > > robust.
> > > > > > >
> > > > > > > Meanwhile, having a .retain  directive is a very 
> > > > > > > simmple
> > > > > > > solution, and keeps the GCC implementation really concise (patch
> > > > > > > attached). The assembler will know for sure what the section 
> > > > > > > containing
> > > > > > > the symbol will be, and can apply the SHF_GNU_RETAIN flag 
> > > > > > > directly.
> > > > > > >
> > > > >
> > > > > Please take a look at
> > > > >
> > > > > https://gitlab.com/x86-gcc/gcc/-/commits/users/hjl/elf/shf_retain
> > > > >
> > > > > which is built in top of
> > > > >
> > > > > https://gcc.gnu.org/pipermail/gcc-patches/2020-February/539963.html
> > > > >
> > > > > I think SECTION2_RETAIN matches SHF_GNU_RETAIN well.  If you
> > > > > want, you extract my flags2 change and use it for SHF_GNU_RETAIN.
> > > >
> > > > In your patch you have to make the assumption that data_section, always
> > > > corresponds to a section named .data. For just this example, c6x (which
> > > > supports the GNU ELF OSABI) does not fit the rule:
> > > >
> > > > > c6x/elf-common.h:#define DATA_SECTION_ASM_OP 
> > > > > "\t.section\t\".fardata\",\"aw\""
> > > >
> > > > data_section for c6x corresponds to .fardata, not .data. So the use of
> > > > "used" on a data declaration would place it in a different section, that
> > > > if the "used" attribute was not applied.
> > > >
> > > > For c6x and mips, readonly_data_section does not correspond to .rodata,
> > > > so that assumption cannot be made either:
> > > > > c6x/elf-common.h:#define READONLY_DATA_SECTION_ASM_OP 
> > > > > "\t.section\t\".const\",\"a\",@progbits"
> > > > > mips/mips.h:#define READONLY_DATA_SECTION_ASM_OP"\t.rdata"
> > > > >   /* read-only data */
> > > >
> > > > The same can be said for bss_section for c6x as well.
> > >
> > > Just add and use named_xxx_section.
> > >
>
> I guess new macros for targets that use non-standard names in unnamed
> sections could work.
>
>   c6x/elf-common.h:
> #define READONLY_DATA_SECTION_NAME ".const"
> #define READONLY_DATA_SECTION_ASM_OP 
> "\t.section\t\".const\",\"a\",@progbits"
>
> > > > Furthermore, this is only considering the examples in
> > > > default_elf_select_section - the le

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

2020-11-04 Thread H.J. Lu via Gcc-patches
On Tue, Nov 3, 2020 at 2:11 PM H.J. Lu  wrote:
>
> On Tue, Nov 3, 2020 at 1:57 PM Jozef Lawrynowicz
>  wrote:
> >
> > On Tue, Nov 03, 2020 at 01:09:43PM -0800, H.J. Lu via Gcc-patches wrote:
> > > On Tue, Nov 3, 2020 at 1:00 PM H.J. Lu  wrote:
> > > >
> > > > On Tue, Nov 3, 2020 at 12:46 PM Jozef Lawrynowicz
> > > >  wrote:
> > > > >
> > > > > On Tue, Nov 03, 2020 at 11:58:04AM -0800, H.J. Lu via Gcc-patches 
> > > > > wrote:
> > > > > > On Tue, Nov 3, 2020 at 10:22 AM Jozef Lawrynowicz
> > > > > >  wrote:
> > > > > > >
> > > > > > > On Tue, Nov 03, 2020 at 09:57:58AM -0800, H.J. Lu via Gcc-patches 
> > > > > > > wrote:
> > > > > > > > On Tue, Nov 3, 2020 at 9:41 AM Jozef Lawrynowicz
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > The attached patch implements TARGET_ASM_MARK_DECL_PRESERVED 
> > > > > > > > > for ELF GNU
> > > > > > > > > OSABI targets, so that declarations that have the "used" 
> > > > > > > > > attribute
> > > > > > > > > applied will be saved from linker garbage collection.
> > > > > > > > >
> > > > > > > > > TARGET_ASM_MARK_DECL_PRESERVED will emit an assembler 
> > > > > > > > > ".retain"
> > > > > > > >
> > > > > > > > Can you use the "R" flag instead?
> > > > > > > >
> > > > > > >
> > > > > > > For the benefit of this mailing list, I have copied my response 
> > > > > > > from the
> > > > > > > Binutils mailing list regarding this.
> > > > > > > The "comm_section" example I gave is actually innacurate, but you 
> > > > > > > can
> > > > > > > see the examples of the variety of sections that would need to be
> > > > > > > handled by doing
> > > > > > >
> > > > > > > $ git grep -A2 "define.*SECTION_ASM_OP" gcc/ | grep "\".*\."
> > > > > > >
> > > > > > > > ... snip ...
> > > > > > > > Secondly, for seamless integration with the "used" attribute, 
> > > > > > > > we must be
> > > > > > > > able to to mark the symbol with the used attribute applied as 
> > > > > > > > "retained"
> > > > > > > > without changing its section name. For GCC "named" sections, 
> > > > > > > > this is
> > > > > > > > straightforward, but for "unnamed" sections it is a giant mess.
> > > > > > > >
> > > > > > > > The section name for a GCC "unnamed" section is not readily 
> > > > > > > > available,
> > > > > > > > instead a string which contains the full assembly code to 
> > > > > > > > switch to one
> > > > > > > > of these text/data/bss/rodata/comm etc. sections is encoded in 
> > > > > > > > the
> > > > > > > > structure.
> > > > > > > >
> > > > > > > > Backends define the assembly code to switch to these sections 
> > > > > > > > (some
> > > > > > > > "*ASM_OP*" macro) in a variety of ways. For example, the 
> > > > > > > > unnamed section
> > > > > > > > "comm_section", might correspond to a .bss section, or emit a 
> > > > > > > > .comm
> > > > > > > > directive. I even looked at trying to parse them to extract 
> > > > > > > > what the
> > > > > > > > name of a section will be, but it would be very messy and not 
> > > > > > > > robust.
> > > > > > > >
> > > > > > > > Meanwhile, having a .retain  directive is a very 
> > > > > > > > simmple
> > > > > > > > solution, and keeps the GCC implementation really concise (patch
> > > > > > > > attached). The assembler will know for sure what the section 
> > > > > > > > containing
> > > > > > > > the symbol will be, and can apply the SHF_GNU_RETAIN flag 
> > > > > > > > directly.
> > > > > > > >
> > > > > >
> > > > > > Please take a look at
> > > > > >
> > > > > > https://gitlab.com/x86-gcc/gcc/-/commits/users/hjl/elf/shf_retain
> > > > > >
> > > > > > which is built in top of
> > > > > >
> > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2020-February/539963.html
> > > > > >
> > > > > > I think SECTION2_RETAIN matches SHF_GNU_RETAIN well.  If you
> > > > > > want, you extract my flags2 change and use it for SHF_GNU_RETAIN.
> > > > >
> > > > > In your patch you have to make the assumption that data_section, 
> > > > > always
> > > > > corresponds to a section named .data. For just this example, c6x 
> > > > > (which
> > > > > supports the GNU ELF OSABI) does not fit the rule:
> > > > >
> > > > > > c6x/elf-common.h:#define DATA_SECTION_ASM_OP 
> > > > > > "\t.section\t\".fardata\",\"aw\""
> > > > >
> > > > > data_section for c6x corresponds to .fardata, not .data. So the use of
> > > > > "used" on a data declaration would place it in a different section, 
> > > > > that
> > > > > if the "used" attribute was not applied.
> > > > >
> > > > > For c6x and mips, readonly_data_section does not correspond to 
> > > > > .rodata,
> > > > > so that assumption cannot be made either:
> > > > > > c6x/elf-common.h:#define READONLY_DATA_SECTION_ASM_OP 
> > > > > > "\t.section\t\".const\",\"a\",@progbits"
> > > > > > mips/mips.h:#define READONLY_DATA_SECTION_ASM_OP"\t.rdata"  
> > > > > > /* read-only data */
> > > > >
> > > > > The same can be said for bss_section for c6x as well.
> > > >
> > > > Just add and use named_xxx_section.
> > > >
> >
> > I

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

2020-11-04 Thread Jozef Lawrynowicz
On Wed, Nov 04, 2020 at 05:47:28AM -0800, H.J. Lu wrote:
> On Tue, Nov 3, 2020 at 2:11 PM H.J. Lu  wrote:
> >
> > On Tue, Nov 3, 2020 at 1:57 PM Jozef Lawrynowicz
> >  wrote:
> > >
> > > On Tue, Nov 03, 2020 at 01:09:43PM -0800, H.J. Lu via Gcc-patches wrote:
> > > > On Tue, Nov 3, 2020 at 1:00 PM H.J. Lu  wrote:
> > > > >
> > > > > On Tue, Nov 3, 2020 at 12:46 PM Jozef Lawrynowicz
> > > > >  wrote:
> > > > > >
> > > > > > On Tue, Nov 03, 2020 at 11:58:04AM -0800, H.J. Lu via Gcc-patches 
> > > > > > wrote:
> > > > > > > On Tue, Nov 3, 2020 at 10:22 AM Jozef Lawrynowicz
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > On Tue, Nov 03, 2020 at 09:57:58AM -0800, H.J. Lu via 
> > > > > > > > Gcc-patches wrote:
> > > > > > > > > On Tue, Nov 3, 2020 at 9:41 AM Jozef Lawrynowicz
> > > > > > > > >  wrote:
> > > > > > > > > >
> > > > > > > > > > The attached patch implements 
> > > > > > > > > > TARGET_ASM_MARK_DECL_PRESERVED for ELF GNU
> > > > > > > > > > OSABI targets, so that declarations that have the "used" 
> > > > > > > > > > attribute
> > > > > > > > > > applied will be saved from linker garbage collection.
> > > > > > > > > >
> > > > > > > > > > TARGET_ASM_MARK_DECL_PRESERVED will emit an assembler 
> > > > > > > > > > ".retain"
> > > > > > > > >
> > > > > > > > > Can you use the "R" flag instead?
> > > > > > > > >
> > > > > > > >
> > > > > > > > For the benefit of this mailing list, I have copied my response 
> > > > > > > > from the
> > > > > > > > Binutils mailing list regarding this.
> > > > > > > > The "comm_section" example I gave is actually innacurate, but 
> > > > > > > > you can
> > > > > > > > see the examples of the variety of sections that would need to 
> > > > > > > > be
> > > > > > > > handled by doing
> > > > > > > >
> > > > > > > > $ git grep -A2 "define.*SECTION_ASM_OP" gcc/ | grep "\".*\."
> > > > > > > >
> > > > > > > > > ... snip ...
> > > > > > > > > Secondly, for seamless integration with the "used" attribute, 
> > > > > > > > > we must be
> > > > > > > > > able to to mark the symbol with the used attribute applied as 
> > > > > > > > > "retained"
> > > > > > > > > without changing its section name. For GCC "named" sections, 
> > > > > > > > > this is
> > > > > > > > > straightforward, but for "unnamed" sections it is a giant 
> > > > > > > > > mess.
> > > > > > > > >
> > > > > > > > > The section name for a GCC "unnamed" section is not readily 
> > > > > > > > > available,
> > > > > > > > > instead a string which contains the full assembly code to 
> > > > > > > > > switch to one
> > > > > > > > > of these text/data/bss/rodata/comm etc. sections is encoded 
> > > > > > > > > in the
> > > > > > > > > structure.
> > > > > > > > >
> > > > > > > > > Backends define the assembly code to switch to these sections 
> > > > > > > > > (some
> > > > > > > > > "*ASM_OP*" macro) in a variety of ways. For example, the 
> > > > > > > > > unnamed section
> > > > > > > > > "comm_section", might correspond to a .bss section, or emit a 
> > > > > > > > > .comm
> > > > > > > > > directive. I even looked at trying to parse them to extract 
> > > > > > > > > what the
> > > > > > > > > name of a section will be, but it would be very messy and not 
> > > > > > > > > robust.
> > > > > > > > >
> > > > > > > > > Meanwhile, having a .retain  directive is a very 
> > > > > > > > > simmple
> > > > > > > > > solution, and keeps the GCC implementation really concise 
> > > > > > > > > (patch
> > > > > > > > > attached). The assembler will know for sure what the section 
> > > > > > > > > containing
> > > > > > > > > the symbol will be, and can apply the SHF_GNU_RETAIN flag 
> > > > > > > > > directly.
> > > > > > > > >
> > > > > > >
> > > > > > > Please take a look at
> > > > > > >
> > > > > > > https://gitlab.com/x86-gcc/gcc/-/commits/users/hjl/elf/shf_retain
> > > > > > >
> > > > > > > which is built in top of
> > > > > > >
> > > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2020-February/539963.html
> > > > > > >
> > > > > > > I think SECTION2_RETAIN matches SHF_GNU_RETAIN well.  If you
> > > > > > > want, you extract my flags2 change and use it for SHF_GNU_RETAIN.
> > > > > >
> > > > > > In your patch you have to make the assumption that data_section, 
> > > > > > always
> > > > > > corresponds to a section named .data. For just this example, c6x 
> > > > > > (which
> > > > > > supports the GNU ELF OSABI) does not fit the rule:
> > > > > >
> > > > > > > c6x/elf-common.h:#define DATA_SECTION_ASM_OP 
> > > > > > > "\t.section\t\".fardata\",\"aw\""
> > > > > >
> > > > > > data_section for c6x corresponds to .fardata, not .data. So the use 
> > > > > > of
> > > > > > "used" on a data declaration would place it in a different section, 
> > > > > > that
> > > > > > if the "used" attribute was not applied.
> > > > > >
> > > > > > For c6x and mips, readonly_data_section does not correspond to 
> > > > > > .rodata,
> > > > > > so that assumption cannot be made either:
> > > > > > > c6x/elf-common.h:#define R

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

2020-11-04 Thread H.J. Lu via Gcc-patches
On Wed, Nov 4, 2020 at 6:41 AM Jozef Lawrynowicz
 wrote:
>
> On Wed, Nov 04, 2020 at 05:47:28AM -0800, H.J. Lu wrote:
> > On Tue, Nov 3, 2020 at 2:11 PM H.J. Lu  wrote:
> > >
> > > On Tue, Nov 3, 2020 at 1:57 PM Jozef Lawrynowicz
> > >  wrote:
> > > >
> > > > On Tue, Nov 03, 2020 at 01:09:43PM -0800, H.J. Lu via Gcc-patches wrote:
> > > > > On Tue, Nov 3, 2020 at 1:00 PM H.J. Lu  wrote:
> > > > > >
> > > > > > On Tue, Nov 3, 2020 at 12:46 PM Jozef Lawrynowicz
> > > > > >  wrote:
> > > > > > >
> > > > > > > On Tue, Nov 03, 2020 at 11:58:04AM -0800, H.J. Lu via Gcc-patches 
> > > > > > > wrote:
> > > > > > > > On Tue, Nov 3, 2020 at 10:22 AM Jozef Lawrynowicz
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > On Tue, Nov 03, 2020 at 09:57:58AM -0800, H.J. Lu via 
> > > > > > > > > Gcc-patches wrote:
> > > > > > > > > > On Tue, Nov 3, 2020 at 9:41 AM Jozef Lawrynowicz
> > > > > > > > > >  wrote:
> > > > > > > > > > >
> > > > > > > > > > > The attached patch implements 
> > > > > > > > > > > TARGET_ASM_MARK_DECL_PRESERVED for ELF GNU
> > > > > > > > > > > OSABI targets, so that declarations that have the "used" 
> > > > > > > > > > > attribute
> > > > > > > > > > > applied will be saved from linker garbage collection.
> > > > > > > > > > >
> > > > > > > > > > > TARGET_ASM_MARK_DECL_PRESERVED will emit an assembler 
> > > > > > > > > > > ".retain"
> > > > > > > > > >
> > > > > > > > > > Can you use the "R" flag instead?
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > For the benefit of this mailing list, I have copied my 
> > > > > > > > > response from the
> > > > > > > > > Binutils mailing list regarding this.
> > > > > > > > > The "comm_section" example I gave is actually innacurate, but 
> > > > > > > > > you can
> > > > > > > > > see the examples of the variety of sections that would need 
> > > > > > > > > to be
> > > > > > > > > handled by doing
> > > > > > > > >
> > > > > > > > > $ git grep -A2 "define.*SECTION_ASM_OP" gcc/ | grep "\".*\."
> > > > > > > > >
> > > > > > > > > > ... snip ...
> > > > > > > > > > Secondly, for seamless integration with the "used" 
> > > > > > > > > > attribute, we must be
> > > > > > > > > > able to to mark the symbol with the used attribute applied 
> > > > > > > > > > as "retained"
> > > > > > > > > > without changing its section name. For GCC "named" 
> > > > > > > > > > sections, this is
> > > > > > > > > > straightforward, but for "unnamed" sections it is a giant 
> > > > > > > > > > mess.
> > > > > > > > > >
> > > > > > > > > > The section name for a GCC "unnamed" section is not readily 
> > > > > > > > > > available,
> > > > > > > > > > instead a string which contains the full assembly code to 
> > > > > > > > > > switch to one
> > > > > > > > > > of these text/data/bss/rodata/comm etc. sections is encoded 
> > > > > > > > > > in the
> > > > > > > > > > structure.
> > > > > > > > > >
> > > > > > > > > > Backends define the assembly code to switch to these 
> > > > > > > > > > sections (some
> > > > > > > > > > "*ASM_OP*" macro) in a variety of ways. For example, the 
> > > > > > > > > > unnamed section
> > > > > > > > > > "comm_section", might correspond to a .bss section, or emit 
> > > > > > > > > > a .comm
> > > > > > > > > > directive. I even looked at trying to parse them to extract 
> > > > > > > > > > what the
> > > > > > > > > > name of a section will be, but it would be very messy and 
> > > > > > > > > > not robust.
> > > > > > > > > >
> > > > > > > > > > Meanwhile, having a .retain  directive is a 
> > > > > > > > > > very simmple
> > > > > > > > > > solution, and keeps the GCC implementation really concise 
> > > > > > > > > > (patch
> > > > > > > > > > attached). The assembler will know for sure what the 
> > > > > > > > > > section containing
> > > > > > > > > > the symbol will be, and can apply the SHF_GNU_RETAIN flag 
> > > > > > > > > > directly.
> > > > > > > > > >
> > > > > > > >
> > > > > > > > Please take a look at
> > > > > > > >
> > > > > > > > https://gitlab.com/x86-gcc/gcc/-/commits/users/hjl/elf/shf_retain
> > > > > > > >
> > > > > > > > which is built in top of
> > > > > > > >
> > > > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2020-February/539963.html
> > > > > > > >
> > > > > > > > I think SECTION2_RETAIN matches SHF_GNU_RETAIN well.  If you
> > > > > > > > want, you extract my flags2 change and use it for 
> > > > > > > > SHF_GNU_RETAIN.
> > > > > > >
> > > > > > > In your patch you have to make the assumption that data_section, 
> > > > > > > always
> > > > > > > corresponds to a section named .data. For just this example, c6x 
> > > > > > > (which
> > > > > > > supports the GNU ELF OSABI) does not fit the rule:
> > > > > > >
> > > > > > > > c6x/elf-common.h:#define DATA_SECTION_ASM_OP 
> > > > > > > > "\t.section\t\".fardata\",\"aw\""
> > > > > > >
> > > > > > > data_section for c6x corresponds to .fardata, not .data. So the 
> > > > > > > use of
> > > > > > > "used" on a data declaration would place it in a dif

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

2020-11-04 Thread Hans-Peter Nilsson
On Wed, 4 Nov 2020, Jozef Lawrynowicz wrote:
> I personally do not see the problem with the .retain attribute, however
> if it is going to be a barrier to getting the functionality committed, I
> am happy to change it, since I really just want the functionality in
> upstream sources.
>
> If a global maintainer would comment on whether any of the proposed
> approaches are acceptable, then I will try to block out time from other
> deadlines so I can work on the fixups and submit a patch in time for the
> GCC 11 freeze.
>
> Thanks,
> Jozef

I'm not much more than a random voice, but an assembly directive
that specifies the symbol (IIUC your .retain directive) to
adjust a symbol attribute sounds cleaner to me, than requiring
gcc to know that this requires it to adjust what it knows about
section flags (again, IIUC).

brgds, H-P


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

2020-11-04 Thread H.J. Lu via Gcc-patches
On Wed, Nov 4, 2020 at 10:09 AM Hans-Peter Nilsson  wrote:
>
> On Wed, 4 Nov 2020, Jozef Lawrynowicz wrote:
> > I personally do not see the problem with the .retain attribute, however
> > if it is going to be a barrier to getting the functionality committed, I
> > am happy to change it, since I really just want the functionality in
> > upstream sources.
> >
> > If a global maintainer would comment on whether any of the proposed
> > approaches are acceptable, then I will try to block out time from other
> > deadlines so I can work on the fixups and submit a patch in time for the
> > GCC 11 freeze.
> >
> > Thanks,
> > Jozef
>
> I'm not much more than a random voice, but an assembly directive
> that specifies the symbol (IIUC your .retain directive) to

But .retain directive DOES NOT adjust symbol attribute.  Instead, it sets
the SHF_GNU_RETAIN bit on the section which contains the symbol
definition.  The same section can have many unrelated symbols.

> adjust a symbol attribute sounds cleaner to me, than requiring
> gcc to know that this requires it to adjust what it knows about
> section flags (again, IIUC).
>
> brgds, H-P



-- 
H.J.


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

2020-11-04 Thread Hans-Peter Nilsson
On Wed, 4 Nov 2020, H.J. Lu wrote:
> On Wed, Nov 4, 2020 at 10:09 AM Hans-Peter Nilsson  wrote:
> >
> > On Wed, 4 Nov 2020, Jozef Lawrynowicz wrote:
> > > I personally do not see the problem with the .retain attribute, however
> > > if it is going to be a barrier to getting the functionality committed, I
> > > am happy to change it, since I really just want the functionality in
> > > upstream sources.
> > >
> > > If a global maintainer would comment on whether any of the proposed
> > > approaches are acceptable, then I will try to block out time from other
> > > deadlines so I can work on the fixups and submit a patch in time for the
> > > GCC 11 freeze.
> > >
> > > Thanks,
> > > Jozef
> >
> > I'm not much more than a random voice, but an assembly directive
> > that specifies the symbol (IIUC your .retain directive) to
>
> But .retain directive DOES NOT adjust symbol attribute.  Instead, it sets
> the SHF_GNU_RETAIN bit on the section which contains the symbol
> definition.  The same section can have many unrelated symbols.

That's an implementation detail *left to the assembler and
linker*.  It's not something the compiler needs to know, and
teoretically it could even change.

> > adjust a symbol attribute sounds cleaner to me, than requiring
> > gcc to know that this requires it to adjust what it knows about
> > section flags (again, IIUC).
> >
> > brgds, H-P
>
>
>
> --
> H.J.
>


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

2020-11-04 Thread H.J. Lu via Gcc-patches
On Wed, Nov 4, 2020 at 1:03 PM Hans-Peter Nilsson  wrote:
>
> On Wed, 4 Nov 2020, H.J. Lu wrote:
> > On Wed, Nov 4, 2020 at 10:09 AM Hans-Peter Nilsson  
> > wrote:
> > >
> > > On Wed, 4 Nov 2020, Jozef Lawrynowicz wrote:
> > > > I personally do not see the problem with the .retain attribute, however
> > > > if it is going to be a barrier to getting the functionality committed, I
> > > > am happy to change it, since I really just want the functionality in
> > > > upstream sources.
> > > >
> > > > If a global maintainer would comment on whether any of the proposed
> > > > approaches are acceptable, then I will try to block out time from other
> > > > deadlines so I can work on the fixups and submit a patch in time for the
> > > > GCC 11 freeze.
> > > >
> > > > Thanks,
> > > > Jozef
> > >
> > > I'm not much more than a random voice, but an assembly directive
> > > that specifies the symbol (IIUC your .retain directive) to
> >
> > But .retain directive DOES NOT adjust symbol attribute.  Instead, it sets
> > the SHF_GNU_RETAIN bit on the section which contains the symbol
> > definition.  The same section can have many unrelated symbols.
>
> That's an implementation detail *left to the assembler and
> linker*.  It's not something the compiler needs to know, and
> teoretically it could even change.
>

The ELF extension is SHF_GNU_RETAIN.  .retain directive is a hack
which I strongly objected and showed that it wasn't needed to implement
SHF_GNU_RETAIN in binutils.


-- 
H.J.


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

2020-11-04 Thread Hans-Peter Nilsson



On Wed, 4 Nov 2020, H.J. Lu wrote:

> On Wed, Nov 4, 2020 at 1:03 PM Hans-Peter Nilsson  wrote:
> >
> > On Wed, 4 Nov 2020, H.J. Lu wrote:
> > > On Wed, Nov 4, 2020 at 10:09 AM Hans-Peter Nilsson  
> > > wrote:
> > > >
> > > > On Wed, 4 Nov 2020, Jozef Lawrynowicz wrote:
> > > > > I personally do not see the problem with the .retain attribute, 
> > > > > however
> > > > > if it is going to be a barrier to getting the functionality 
> > > > > committed, I
> > > > > am happy to change it, since I really just want the functionality in
> > > > > upstream sources.
> > > > >
> > > > > If a global maintainer would comment on whether any of the proposed
> > > > > approaches are acceptable, then I will try to block out time from 
> > > > > other
> > > > > deadlines so I can work on the fixups and submit a patch in time for 
> > > > > the
> > > > > GCC 11 freeze.
> > > > >
> > > > > Thanks,
> > > > > Jozef
> > > >
> > > > I'm not much more than a random voice, but an assembly directive
> > > > that specifies the symbol (IIUC your .retain directive) to
> > >
> > > But .retain directive DOES NOT adjust symbol attribute.

I see I missed to point out that I was speaking about the *gcc
symbol* attribute "used".

> > > Instead, it sets
> > > the SHF_GNU_RETAIN bit on the section which contains the symbol
> > > definition.  The same section can have many unrelated symbols.
> >
> > That's an implementation detail *left to the assembler and
> > linker*.  It's not something the compiler needs to know, and
> > teoretically it could even change.
> >
>
> The ELF extension is SHF_GNU_RETAIN.  .retain directive is a hack
> which I strongly objected and showed that it wasn't needed to implement
> SHF_GNU_RETAIN in binutils.

It's still an implementation detail better kept in the
assembler, that the mechanism used to retain a symbol for the
compiler, happens to map to a section attribute.  Some may call
*that* a hack.

It's cleaner to the compiler if it can pass on to the assembler
the specific symbol that needs to be kept.

brgds, H-P


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

2020-11-04 Thread H.J. Lu via Gcc-patches
On Wed, Nov 4, 2020 at 1:56 PM Hans-Peter Nilsson  wrote:
>
>
>
> On Wed, 4 Nov 2020, H.J. Lu wrote:
>
> > On Wed, Nov 4, 2020 at 1:03 PM Hans-Peter Nilsson  wrote:
> > >
> > > On Wed, 4 Nov 2020, H.J. Lu wrote:
> > > > On Wed, Nov 4, 2020 at 10:09 AM Hans-Peter Nilsson  
> > > > wrote:
> > > > >
> > > > > On Wed, 4 Nov 2020, Jozef Lawrynowicz wrote:
> > > > > > I personally do not see the problem with the .retain attribute, 
> > > > > > however
> > > > > > if it is going to be a barrier to getting the functionality 
> > > > > > committed, I
> > > > > > am happy to change it, since I really just want the functionality in
> > > > > > upstream sources.
> > > > > >
> > > > > > If a global maintainer would comment on whether any of the proposed
> > > > > > approaches are acceptable, then I will try to block out time from 
> > > > > > other
> > > > > > deadlines so I can work on the fixups and submit a patch in time 
> > > > > > for the
> > > > > > GCC 11 freeze.
> > > > > >
> > > > > > Thanks,
> > > > > > Jozef
> > > > >
> > > > > I'm not much more than a random voice, but an assembly directive
> > > > > that specifies the symbol (IIUC your .retain directive) to
> > > >
> > > > But .retain directive DOES NOT adjust symbol attribute.
>
> I see I missed to point out that I was speaking about the *gcc
> symbol* attribute "used".

There is no such corresponding symbol attribute in ELF.

> > > > Instead, it sets
> > > > the SHF_GNU_RETAIN bit on the section which contains the symbol
> > > > definition.  The same section can have many unrelated symbols.
> > >
> > > That's an implementation detail *left to the assembler and
> > > linker*.  It's not something the compiler needs to know, and
> > > teoretically it could even change.
> > >
> >
> > The ELF extension is SHF_GNU_RETAIN.  .retain directive is a hack
> > which I strongly objected and showed that it wasn't needed to implement
> > SHF_GNU_RETAIN in binutils.
>
> It's still an implementation detail better kept in the
> assembler, that the mechanism used to retain a symbol for the
> compiler, happens to map to a section attribute.  Some may call
> *that* a hack.
>
> It's cleaner to the compiler if it can pass on to the assembler
> the specific symbol that needs to be kept.
>

SHF_GNU_RETAIN is for section and GCC should place the symbol,
which should be kept, in the SHF_GNU_RETAIN section directly, not
through .retain directive.

-- 
H.J.


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

2020-11-04 Thread Hans-Peter Nilsson
On Wed, 4 Nov 2020, H.J. Lu wrote:
> On Wed, Nov 4, 2020 at 1:56 PM Hans-Peter Nilsson  wrote:
> > On Wed, 4 Nov 2020, H.J. Lu wrote:
> >
> > > On Wed, Nov 4, 2020 at 1:03 PM Hans-Peter Nilsson  
> > > wrote:
> > > >
> > > > On Wed, 4 Nov 2020, H.J. Lu wrote:
> > > > > On Wed, Nov 4, 2020 at 10:09 AM Hans-Peter Nilsson 
> > > > >  wrote:
> > > > > > I'm not much more than a random voice, but an assembly directive
> > > > > > that specifies the symbol (IIUC your .retain directive) to
> > > > >
> > > > > But .retain directive DOES NOT adjust symbol attribute.
> >
> > I see I missed to point out that I was speaking about the *gcc
> > symbol* attribute "used".
>
> There is no such corresponding symbol attribute in ELF.

I have not missed that, nor that SHF_GNU_RETAIN is so new that
it's not in binutils master.  I have also not missed that gcc
caters to other object formats too.  A common symbol-specific
directive such as .retain, would be better than messing with
section attributes, for gcc.

> > It's cleaner to the compiler if it can pass on to the assembler
> > the specific symbol that needs to be kept.
> >
>
> SHF_GNU_RETAIN is for section and GCC should place the symbol,
> which should be kept, in the SHF_GNU_RETAIN section directly, not
> through .retain directive.

This is where opinions differ.  Anyway, this is now repetition;
I'm done.

brgds, H-P


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

2020-11-04 Thread H.J. Lu via Gcc-patches
On Wed, Nov 4, 2020 at 3:00 PM Hans-Peter Nilsson  wrote:
>
> On Wed, 4 Nov 2020, H.J. Lu wrote:
> > On Wed, Nov 4, 2020 at 1:56 PM Hans-Peter Nilsson  wrote:
> > > On Wed, 4 Nov 2020, H.J. Lu wrote:
> > >
> > > > On Wed, Nov 4, 2020 at 1:03 PM Hans-Peter Nilsson  
> > > > wrote:
> > > > >
> > > > > On Wed, 4 Nov 2020, H.J. Lu wrote:
> > > > > > On Wed, Nov 4, 2020 at 10:09 AM Hans-Peter Nilsson 
> > > > > >  wrote:
> > > > > > > I'm not much more than a random voice, but an assembly directive
> > > > > > > that specifies the symbol (IIUC your .retain directive) to
> > > > > >
> > > > > > But .retain directive DOES NOT adjust symbol attribute.
> > >
> > > I see I missed to point out that I was speaking about the *gcc
> > > symbol* attribute "used".
> >
> > There is no such corresponding symbol attribute in ELF.
>
> I have not missed that, nor that SHF_GNU_RETAIN is so new that
> it's not in binutils master.  I have also not missed that gcc
> caters to other object formats too.  A common symbol-specific
> directive such as .retain, would be better than messing with
> section attributes, for gcc.

This is totally irrelevant to SHF_GNU_RETAIN.

> > > It's cleaner to the compiler if it can pass on to the assembler
> > > the specific symbol that needs to be kept.
> > >
> >
> > SHF_GNU_RETAIN is for section and GCC should place the symbol,
> > which should be kept, in the SHF_GNU_RETAIN section directly, not
> > through .retain directive.
>
> This is where opinions differ.  Anyway, this is now repetition;
> I'm done.

.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]$

A symbol directive should operate on the symbol table.
With 'R' flag, we got

.file "x.c"
.text
.section .bss.xyzzy,"awR",@nobits
.align 4
.type xyzzy, @object
.size xyzzy, 4
xyzzy:
.zero 4
.ident "GCC: (GNU) 11.0.0 20201104 (experimental)"
.section .note.GNU-stack,"",@progbits

-- 
H.J.


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

2020-11-05 Thread Jozef Lawrynowicz
On Wed, Nov 04, 2020 at 03:58:56PM -0800, H.J. Lu wrote:
> On Wed, Nov 4, 2020 at 3:00 PM Hans-Peter Nilsson  wrote:
> >
> > On Wed, 4 Nov 2020, H.J. Lu wrote:
> > > On Wed, Nov 4, 2020 at 1:56 PM Hans-Peter Nilsson  
> > > wrote:
> > > > On Wed, 4 Nov 2020, H.J. Lu wrote:
> > > >
> > > > > On Wed, Nov 4, 2020 at 1:03 PM Hans-Peter Nilsson  
> > > > > wrote:
> > > > > >
> > > > > > On Wed, 4 Nov 2020, H.J. Lu wrote:
> > > > > > > On Wed, Nov 4, 2020 at 10:09 AM Hans-Peter Nilsson 
> > > > > > >  wrote:
> > > > > > > > I'm not much more than a random voice, but an assembly directive
> > > > > > > > that specifies the symbol (IIUC your .retain directive) to
> > > > > > >
> > > > > > > But .retain directive DOES NOT adjust symbol attribute.
> > > >
> > > > I see I missed to point out that I was speaking about the *gcc
> > > > symbol* attribute "used".
> > >
> > > There is no such corresponding symbol attribute in ELF.
> >
> > I have not missed that, nor that SHF_GNU_RETAIN is so new that
> > it's not in binutils master.  I have also not missed that gcc
> > caters to other object formats too.  A common symbol-specific
> > directive such as .retain, would be better than messing with
> > section attributes, for gcc.
> 
> This is totally irrelevant to SHF_GNU_RETAIN.
> 
> > > > It's cleaner to the compiler if it can pass on to the assembler
> > > > the specific symbol that needs to be kept.
> > > >
> > >
> > > SHF_GNU_RETAIN is for section and GCC should place the symbol,
> > > which should be kept, in the SHF_GNU_RETAIN section directly, not
> > > through .retain directive.
> >
> > This is where opinions differ.  Anyway, this is now repetition;
> > I'm done.
> 
> .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]$
> 
> A symbol directive should operate on the symbol table.
> With 'R' flag, we got
> 
> .file "x.c"
> .text
> .section .bss.xyzzy,"awR",@nobits
> .align 4
> .type xyzzy, @object
> .size xyzzy, 4
> xyzzy:
> .zero 4
> .ident "GCC: (GNU) 11.0.0 20201104 (experimental)"
> .section .note.GNU-stack,"",@progbits

I still think it is very wrong for the "used" attribute to place the
symbol in a unique section. The structure of the sections in the object
file should be no different whether the "used" attribute was applied to
a symbol or not.

I will therefore have to make changes to GCC so that we can get the name
of "unnamed" sections, and emit a .section directive with the "R" flag
set on that section name, in order to avoid using a .retain directive.

"used" applied to a function
---
  Before:
TEXT_SECTION_ASM_OP 
func:

  After:
.section TEXT_SECTION_NAME,"axR",%progbits
func:

Where TEXT_SECTION_NAME is a new macro which defines the section name
corresponding to TEXT_SECTION_ASM_OP.
Similar new macros are required for all *SECTION_ASM_OP.

Since we can't use the .retain directive, this is the cludge that will
be required to robustly support all targets.

The alternative is to just infer that the mapping of unnamed sections to
section names is always the following:
  text_section-> .text,"ax",%progbits
  data_section-> .data,"aw"
  bss_section -> .bss,"aw",%nobits
  rodata_section  -> .rodata,"a",
  etc.

This section name assumption does not hold for a couple of ELF targets.

Also, many targets omit the specification of the flags, leaving that
choice to the assembler, instead the compiler will now have to infer
what the assembler will do, all because we can't have the .retain
directive.

.retain  makes life very easy for GCC, but I understand your
objection from a theoretical point of view.

You previously objected to .retain , to apply
SHF_GNU_RETAIN to . This does not violate your rule about
a directive applying flags to a different type of structure to what is
named in the directive.

If we can have .retain , then we don't have to make
assumptions about section flags in GCC, we can just name the section use
in the ASM_OP.

Do you still oppose .retain ?

Another alternative is to disallow "used" from applying SHF_GNU_RETAIN,
unless the symbol is in a named section. Obviously this is pretty gross,
but would mean we don't need to handle *SECTION_ASM_OP sections.

Thanks,
Jozef
> 
> -- 
> H.J.


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

2020-11-05 Thread Hans-Peter Nilsson
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.

brgds, H-P


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

2020-11-05 Thread Jozef Lawrynowicz
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.

Thanks,
Jozef


> 
> brgds, H-P