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 > <joze...@mittosystems.com> 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 > > > <joze...@mittosystems.com> 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 <l...@redhat.com> wrote: > > > > > > > > > > > > > > > > > > On 11/6/20 5:13 PM, H.J. Lu wrote: > > > > > > > On Fri, Nov 6, 2020 at 4:01 PM Jeff Law <l...@redhat.com> wrote: > > > > > > >> > > > > > > >> On 11/6/20 4:45 PM, H.J. Lu wrote: > > > > > > >>> On Fri, Nov 6, 2020 at 3:37 PM Jeff Law <l...@redhat.com> wrote: > > > > > > >>>> On 11/6/20 4:29 PM, H.J. Lu wrote: > > > > > > >>>>> On Fri, Nov 6, 2020 at 3:22 PM Jeff Law <l...@redhat.com> > > > > > > >>>>> wrote: > > > > > > >>>>>> On 11/5/20 7:34 AM, H.J. Lu via Gcc-patches wrote: > > > > > > >>>>>>> On Thu, Nov 5, 2020 at 3:37 AM Jozef Lawrynowicz > > > > > > >>>>>>> <joze...@mittosystems.com> wrote: > > > > > > >>>>>>>> On Thu, Nov 05, 2020 at 06:21:21AM -0500, Hans-Peter > > > > > > >>>>>>>> Nilsson wrote: > > > > > > >>>>>>>>> On Wed, 4 Nov 2020, H.J. Lu wrote: > > > > > > >>>>>>>>>> .retain is ill-defined. For example, > > > > > > >>>>>>>>>> > > > > > > >>>>>>>>>> [hjl@gnu-cfl-2 gcc]$ cat /tmp/x.c > > > > > > >>>>>>>>>> static int xyzzy __attribute__((__used__)); > > > > > > >>>>>>>>>> [hjl@gnu-cfl-2 gcc]$ ./xgcc -B./ -S /tmp/x.c -fcommon > > > > > > >>>>>>>>>> [hjl@gnu-cfl-2 gcc]$ cat x.s > > > > > > >>>>>>>>>> .file "x.c" > > > > > > >>>>>>>>>> .text > > > > > > >>>>>>>>>> .retain xyzzy <<<<<<<<< What does it do? > > > > > > >>>>>>>>>> .local xyzzy > > > > > > >>>>>>>>>> .comm xyzzy,4,4 > > > > > > >>>>>>>>>> .ident "GCC: (GNU) 11.0.0 20201103 (experimental)" > > > > > > >>>>>>>>>> .section .note.GNU-stack,"",@progbits > > > > > > >>>>>>>>>> [hjl@gnu-cfl-2 gcc]$ > > > > > > >>>>>>>>> To answer that question: it's up to the assembler, but > > > > > > >>>>>>>>> for ELF > > > > > > >>>>>>>>> and SHF_GNU_RETAIN, it seems obvious it'd tell the > > > > > > >>>>>>>>> assembler to > > > > > > >>>>>>>>> set SHF_GNU_RETAIN for the section where the symbol ends > > > > > > >>>>>>>>> up. > > > > > > >>>>>>>>> We both know this isn't rocket science with binutils. > > > > > > >>>>>>>> Indeed, and my patch handles it trivially: > > > > > > >>>>>>>> https://sourceware.org/pipermail/binutils/2020-November/113993.html > > > > > > >>>>>>>> > > > > > > >>>>>>>> +void > > > > > > >>>>>>>> +obj_elf_retain (int arg ATTRIBUTE_UNUSED) > > > > > > >>>>>>>> .... snip .... > > > > > > >>>>>>>> + sym = get_sym_from_input_line_and_check (); > > > > > > >>>>>>>> + symbol_get_obj (sym)->retain = 1; > > > > > > >>>>>>>> > > > > > > >>>>>>>> @@ -2624,6 +2704,9 @@ elf_frob_symbol (symbolS *symp, > > > > > > >>>>>>>> int *puntp) > > > > > > >>>>>>>> } > > > > > > >>>>>>>> } > > > > > > >>>>>>>> > > > > > > >>>>>>>> + if (symbol_get_obj (symp)->retain) > > > > > > >>>>>>>> + elf_section_flags (S_GET_SEGMENT (symp)) |= > > > > > > >>>>>>>> SHF_GNU_RETAIN; > > > > > > >>>>>>>> + > > > > > > >>>>>>>> /* Double check weak symbols. */ > > > > > > >>>>>>>> if (S_IS_WEAK (symp)) > > > > > > >>>>>>>> { > > > > > > >>>>>>>> > > > > > > >>>>>>>> We could check that the symbol named in the .retain > > > > > > >>>>>>>> directive has > > > > > > >>>>>>>> already been defined, however this isn't compatible with > > > > > > >>>>>>>> GCC > > > > > > >>>>>>>> mark_decl_preserved handling, since mark_decl_preserved is > > > > > > >>>>>>>> called > > > > > > >>>>>>>> emitted before the local symbols are defined in the > > > > > > >>>>>>>> assembly output > > > > > > >>>>>>>> file. > > > > > > >>>>>>>> > > > > > > >>>>>>>> GAS should at least validate that the symbol named in the > > > > > > >>>>>>>> .retain > > > > > > >>>>>>>> directive does end up as a symbol though. > > > > > > >>>>>>>> > > > > > > >>>>>>> Don't add .retain. > > > > > > >>>>>> Why? I don't see why you find it so objectionable. > > > > > > >>>>>> > > > > > > >>>>> An ELF symbol directive should operate on symbol table: > > > > > > >>>>> > > > > > > >>>>> http://www.sco.com/developers/gabi/latest/ch4.symtab.html > > > > > > >>>>> > > > > > > >>>>> not the section flags where the symbol is defined. > > > > > > >>>> I agree in general, but I think this is one of those cases > > > > > > >>>> where it's > > > > > > >>>> not so clear. And what you're talking about is an > > > > > > >>>> implementation detail. > > > > > > >>> There is no need for such a hack. The proper thing to do in > > > > > > >>> ELF is > > > > > > >>> to place such a symbol in a section with SHF_GNU_RETAIN flag. > > > > > > >>> This > > > > > > >>> also avoids the question what to do with SHN_COMMON. > > > > > > >> I'm not sure that's a good idea either. Moving symbols into a > > > > > > >> section > > > > > > >> other than they'd normally live doesn't seem all that wise. > > > > > > > In ELF, a symbol must be defined in a section. If we want to > > > > > > > keep a symbol, > > > > > > > we should place it in an SHF_GNU_RETAIN section. > > > > > > > > > > > > Again, that's an implementation detail and it's not clear to me > > > > > > that one > > > > > > approach is inherently better than the other. > > > > > > > > > > > > > > > > > > > > > > > > > >> Let's face it, there's not a great solution here. If we mark its > > > > > > >> existing section, then everything in that section gets kept. If > > > > > > >> we put > > > > > > > FWIW, this is what .retain direct does and is one reason why I > > > > > > > object > > > > > > > it. > > > > > > > > > > > > We could make .retain work with either approach. I don't see > > > > > > .retain > > > > > > as a problem at all. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >> the object into a different section than it would normally live, > > > > > > >> then > > > > > > >> that opens a whole new can of worms. > > > > > > > We should place it in a section which it normally lives in and > > > > > > > mark the > > > > > > > section with SHF_GNU_RETAIN. > > > > > > > > > > > > And why not do that with .retain? We define its semantics as > > > > > > precisely > > > > > > > > > > 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 which GCC tracks for each symbol. It can be extended to > > > > > track > > > > > SHF_GNU_RETAIN. > > > > > > > > The attached patch is rough around the edges but shows my approach for > > > > marking unnamed sections as retained, by converting them to named > > > > sections. > > > > > > > > I figure we don't have to wrap every usage of SECTION_RETAIN in > > > > HAVE_GAS_SECTION_RETAIN as long as any set of SECTION_RETAIN in the > > > > flags field is wrapped in the macro. > > > > > > > > I think a flag to turn off the behavior (in the same way the behavior is > > > > disabled if !defined(HAVE_GAS_SECTION_RETAIN)) would be beneficial, > > > > I haven't added that yet. > > > > > > > > Decls that would go in comm_section, tls_comm_section and sometimes > > > > lcomm_section do not get retained as we can't apply the retain section > > > > flag to these symbols. Given these go in one big common block, and > > > > contain uninitialized data, I don't think there is a valid use case for > > > > which these types of symbols need to be retained, but are not referenced > > > > by the program. So I've avoided converting them to .bss or anything like > > > > that. > > > > > > > > Some targets alias lcomm_section for bss_section, so we can retain > > > > sections for that case. > > > > > > > > So far bootstrap and light testing on x86_64-pc-linux-gnu and > > > > arm-none-eabi has shown no problems. > > > > > > > > > > Binutils change looks OK. But GCC changes look too intrusive. > > > Also SHF_GNU_RETAIN can't be used on Solaris since ld on > > > Solaris may not support it. > > > > I see Solaris ELF OSABI doesn't get set in bfd/elf32-i386.c because: > > > > /* Restore default: we cannot use ELFOSABI_SOLARIS, otherwise > > ELFOSABI_NONE > > objects won't be recognized. */ > > #undef ELF_OSABI > > > > Since GNU OSABI support is implied for ELFOSABI_NONE, I guess there > > needs to be an exception for the "R" section flag support where we check > > if the target OS is Solaris. > > GNU ld on Solaris should support SHF_GNU_RETAIN. > > > > > > > Can you improve > > > > > > https://gitlab.com/x86-gcc/gcc/-/tree/users/hjl/elf/shf_retain > > > > > > to do what you need? > > > > I don't want "used" to implicitly create named sections for decls which > > would otherwise just be placed in the standard .text/.data/.rodata/.bss > > sections. > > > > Unless we just ignore the requirement to "retain" for unnamed sections, > > something like my patch is needed where we transform assembler output > > from > > .text -> .section .text,"axR" > > or > > .bss -> .section .bss,"awR" > > etc. > > > > Please checkout > > https://gitlab.com/x86-gcc/gcc/-/tree/users/hjl/elf/shf_retain
Ah yes, using the prefix the section would have if it were named is a much simpler solution. For arm-none-eabi we are losing the "R" flag on data/bss/rodata at opt levels above -O0 though, this is because SECTION2_RETAIN is not being set on object_block sections. Can you add me to that gitlab project so I can submit a merge request for the fix for this please? Also in the merge request are new tests and fixes to existing tests, my new tests go in gcc.c-torture so we catch any other issues from building at different optimization levels. I'm going to resubmit my latest Binutils patch once I've put it through the Binutils tests for ELF targets again. Thanks, Jozef > > -- > H.J.
