Re: [PATCH] Add -fgnu-retain/-fno-gnu-retain
On 2/18/21 3:19 AM, Richard Biener via Binutils wrote: > On Wed, Feb 17, 2021 at 12:25 PM Jozef Lawrynowicz > wrote: >> On Tue, Feb 16, 2021 at 05:45:47PM +0100, Jakub Jelinek via Gcc-patches >> wrote: >>> On Mon, Feb 15, 2021 at 02:35:07PM -0800, H.J. Lu via Gcc-patches wrote: When building Linux kernel, ld in bninutils 2.36 with GCC 11 generates thousands of ld: warning: orphan section `.data.event_initcall_finish' from `init/main.o' being placed in section `.data.event_initcall_finish' ld: warning: orphan section `.data.event_initcall_start' from `init/main.o' being placed in section `.data.event_initcall_start' ld: warning: orphan section `.data.event_initcall_level' from `init/main.o' being placed in section `.data.event_initcall_level' Since these sections are marked with SHF_GNU_RETAIN, they are placed in separate sections. They become orphan sections since they aren't expected in the Linux kernel linker script. But orphan sections normally don't work well with the Linux kernel linker script and the resulting kernel crashed. Add -fgnu-retain to disable SHF_GNU_RETAIN for Linux kernel build with -fno-gnu-retain. >>> I'd say this shows that the changing of meaning of "used" attribute wasn't >>> really a good idea, the Linux kernel certainly won't be the only problem >>> and people use "used" attribute for many reasons and don't necessarily want >>> the different sections or different behavior of those sections if they use >>> it. >>> >>> So, can't we instead: >>> 1) restore the old behavior of "used" by default >>> 2) add "retain" attribute that implies the SHF_GNU_RETAIN stuff and fails >>>if the configured assembler/linker doesn't support those >>> 3) add a non-default option through which one could opt in for "used" >>>attribute to imply retain attribute too for projects that want that? >>> >> In previous discussions, it seemed to me that there was general support >> for the "used" attribute implying SHF_GNU_RETAIN and saving symbols from >> linker garbage collection[1]. Of course, the current implementation >> results in undesirable behavior - the thought that all linker scripts >> not supporting uniquely named sections would need to be updated is quite >> alarming. >> >> It's a shame that all this extra complication is required, just because >> we cannot have a ".retain ", directive. >> >> My preferred vision for this functionality was: >> - SHF_GNU_RETAIN section flag indicates a section should be saved >> from linker garbage collection. >> - ".retain " assembler directive applies SHF_GNU_RETAIN >> to the section containing . >> - GCC "used" attribute emits a ".retain " directive for >> the symbol declaration is is applied to. Applying the "used" >> attribute to a symbol declaration does not change the structure of >> the object file, beyond applying SHF_GNU_RETAIN to the section >> containing that symbol. >> >> H.J. vetoed ".retain "[2], since it fails the predicate: >> Assembler directive referring to a symbol must operate on the symbol >> table. >> >> So because of this veto, we have compromised on "code quality" so far, >> since any linker script not supporting named sections, used to link an >> application containing "used" symbols (now put into their own section) has >> potential undefined behavior. >> >> With the new proposed change to use a "retain" attribute, we now >> compromise on functionality, since the "used" attribute saving symbols >> from linker garbage collection is disabled by default. >> >> All because we cannot introduce an exception to the above predicate. >> >> I would like to re-open discussions on whether a ".retain >> directive is acceptable. This would enable "used" to imply >> SHF_GNU_RETAIN, without changing the structure of the object file, >> thereby allowing the new functionality to be used without linker script >> modifications. >> >> If a Binutils global maintainer could side one way or the other on >> ".retain " (an opinion was never given by the Binutils >> maintainers when we had the discussions on that mailing list, although >> Jeff did support .retain in the GCC discussions[3]), then it will be >> easier to proceed knowing definitively that ".retain" is rejected and we >> have no choice but to go this route of having a separate "retain" >> attribute. > So I then propose, for GCC 11, to rip out / disable any use of SHF_GNU_RETAIN, > effectively restoring GCC 10 beahvior and re-consider for GCC 12. FWIW concerns WRT SHF_GNU_RETAIN are the primary reason why we didn't include binutils-2.36 into Fedora 33. If we included binutils-2.36, then GCC would have started using SHF_GNU_RETAIN and we were concerned that there could easily be fallout. Or to put it another way, I'd fully support Richi's proposal -- I fear that folks using the combination of gcc-11 + binutils-2.36 are likely going to stumble over real problems in this
Re: [PATCH] Add -fgnu-retain/-fno-gnu-retain
On Thu, Feb 18, 2021 at 03:38:46PM +0100, Jakub Jelinek via Gcc-patches wrote: > On Thu, Feb 18, 2021 at 02:22:35PM +, Jozef Lawrynowicz wrote: > > I think it is a enhancement, and true to the spirit of the attribute, > > for "used" to save a symbol from linker garbage collection. > > > > Why should "used" mean: > > Save this symbol from compiler optimization, but allow the linker to > > remove it. > > > > I can't currently think of a use case where a "used" symbol should be > > kept by the compiler but removed by the linker. > > > > Often we see KEEP directives in linker scripts accompanying > > "used" in the source code. libgcc/crtstuff.c is a good example. GNU > > linker scripts need many specific KEEP directives to handle all the > > "used" symbols. > > > > If a developer marks a symbol as "used" in the source code, I think the > > intuitive thing is for that symbol to be present in the linked output > > file. > > There are many reasons why a symbol is marked "used", one of the very often > seen ones is e.g. that the symbol is referenced from inline asm. > You don't need to guard such symbols against GC. I suppose, for the cases where you cannot use extended ASM and therefore cannot specify the symbol as an operand to the asm statement. I wonder if that really could not be worked around though, since it would only be required for statics, and if it would even have a negative impact in practice. I am just conflicted because when I first proposed this new functionality, (as a separate "retain" attribute in fact), the feedback from a few different people was that it would be better to leverage "used" instead of creating a separate attribute. It seemed like a good idea to me. At this point, a separate "retain" attribute seems like an OK compromise. Thanks, Jozef
Re: [PATCH] Add -fgnu-retain/-fno-gnu-retain
On Thu, Feb 18, 2021 at 02:22:35PM +, Jozef Lawrynowicz wrote: > I think it is a enhancement, and true to the spirit of the attribute, > for "used" to save a symbol from linker garbage collection. > > Why should "used" mean: > Save this symbol from compiler optimization, but allow the linker to > remove it. > > I can't currently think of a use case where a "used" symbol should be > kept by the compiler but removed by the linker. > > Often we see KEEP directives in linker scripts accompanying > "used" in the source code. libgcc/crtstuff.c is a good example. GNU > linker scripts need many specific KEEP directives to handle all the > "used" symbols. > > If a developer marks a symbol as "used" in the source code, I think the > intuitive thing is for that symbol to be present in the linked output > file. There are many reasons why a symbol is marked "used", one of the very often seen ones is e.g. that the symbol is referenced from inline asm. You don't need to guard such symbols against GC. Jakub
Re: [PATCH] Add -fgnu-retain/-fno-gnu-retain
On Thu, Feb 18, 2021 at 01:08:54PM +0100, Jakub Jelinek via Gcc-patches wrote: > On Thu, Feb 18, 2021 at 12:00:59PM +, Jozef Lawrynowicz wrote: > > If we can add ".retain " to GAS, then I agree, current GCC > > SHF_GNU_RETAIN behavior should be removed. For GCC 12 we leverage > > .retain to implement the functionality where "used" saves symbols form > > linker garbage collection, without modifying section names or the > > structure of the object file. > > Even in that case I think it is a bad idea to change the "used" attribute > behavior, not everyone using "used" attribute needs or wants that behavior, > so it should be a functionality provided by a separate new attribute. I think it is a enhancement, and true to the spirit of the attribute, for "used" to save a symbol from linker garbage collection. Why should "used" mean: Save this symbol from compiler optimization, but allow the linker to remove it. I can't currently think of a use case where a "used" symbol should be kept by the compiler but removed by the linker. Often we see KEEP directives in linker scripts accompanying "used" in the source code. libgcc/crtstuff.c is a good example. GNU linker scripts need many specific KEEP directives to handle all the "used" symbols. If a developer marks a symbol as "used" in the source code, I think the intuitive thing is for that symbol to be present in the linked output file. Jozef
Re: [PATCH] Add -fgnu-retain/-fno-gnu-retain
On Thu, Feb 18, 2021 at 11:22:42PM +1030, Alan Modra via Binutils wrote: > On Wed, Feb 17, 2021 at 11:23:12AM +, Jozef Lawrynowicz wrote: > > In previous discussions, it seemed to me that there was general support > > for the "used" attribute implying SHF_GNU_RETAIN and saving symbols from > > linker garbage collection[1]. Of course, the current implementation > > results in undesirable behavior - the thought that all linker scripts > > not supporting uniquely named sections would need to be updated is quite > > alarming. > > > > It's a shame that all this extra complication is required, just because > > we cannot have a ".retain ", directive. > > Is that true? Isn't the problem really that retained sections are > named as if -ffunction-sections/-fdata-sections were in force? And > that is due to wanting separate sections so that garbage collection > works, since SHF_GNU_RETAIN is all about linker garbage collection. > > I don't see how having a ".retain " would help much. In the early GCC implementations, there were issues getting the default "unnamed" section names for symbols in GCC[1]. This prevented us from being able to simply emit a .section directive, replacing a .text/.data etc. directive. H.J. improved upon my initial attempt for doing this, but then the approach was changed to always put "used" symbols in uniquely named sections, and I don't know why. Hopefully H.J. can clarify why "used" symbols had to be put in sections with unique names. With a ".retain " directive, GCC doesn't need to work out the section associated with a symbol, alleviating the above issues, so we don't need to change the section a symbol is in to apply SHF_GNU_RETAIN to that section. The bugs we are seeing now (e.g. PR 99113) are because some linker scripts aren't set up to handle uniquely named sections. With ".retain ", the linker scripts will work without issues because we have not changed the layout of sections. > > > My preferred vision for this functionality was: > > - SHF_GNU_RETAIN section flag indicates a section should be saved > > from linker garbage collection. > > - ".retain " assembler directive applies SHF_GNU_RETAIN > > to the section containing . > > - GCC "used" attribute emits a ".retain " directive for > > the symbol declaration is is applied to. Applying the "used" > > attribute to a symbol declaration does not change the structure of > > the object file, beyond applying SHF_GNU_RETAIN to the section > > containing that symbol. > > That description seems to say that a ".retain foo" would mean > everything in foo's section is kept. If foo's section was the usual > .data, you've kept virtually everything from garbage collection. Yes, sort of. Everything from .data in the input object file - only the .data containing foo. Unused .data sections from other object files will be garbage collected, as required. The user has marked "foo" as used after all, so the linker is treating it as if it is linked to the entry point of the program in some way. Since garbage collection only works at the section level, we can't do any better - unless -ffunction/fdata-sections is used. > Surely you don't expect ".retain foo" to create a separate .data > section for foo? If you do, I'm strongly against that idea. No, I don't want ".retain" to change anything about the structure or contents of a section, beyond applying SHF_GNU_RETAIN to it. > > Note that gas indeed supports multiple sections named .data that can > serve the same purpose as -fdata-sections. See the gas doc for the > optional .section field "unique". That might be the best way to avoid > an under-the-hood -ffunction-sections/-fdata-sections. As mentioned above, there were issues getting the default "unnamed" section names for symbols in GCC. If we can reliably get the name of an unnamed section, then we could potentially do away with .retain. "unique" would allow finer granularity of garbage collection, but it is changing the structure of the object file, which I think we should try to avoid, as it leads to issues like we are seeing now. Thanks, Jozef [1] https://gcc.gnu.org/pipermail/gcc-patches/2020-November/557914.html > > -- > Alan Modra > Australia Development Lab, IBM
Re: [PATCH] Add -fgnu-retain/-fno-gnu-retain
On Wed, Feb 17, 2021 at 11:23:12AM +, Jozef Lawrynowicz wrote: > In previous discussions, it seemed to me that there was general support > for the "used" attribute implying SHF_GNU_RETAIN and saving symbols from > linker garbage collection[1]. Of course, the current implementation > results in undesirable behavior - the thought that all linker scripts > not supporting uniquely named sections would need to be updated is quite > alarming. > > It's a shame that all this extra complication is required, just because > we cannot have a ".retain ", directive. Is that true? Isn't the problem really that retained sections are named as if -ffunction-sections/-fdata-sections were in force? And that is due to wanting separate sections so that garbage collection works, since SHF_GNU_RETAIN is all about linker garbage collection. I don't see how having a ".retain " would help much. > My preferred vision for this functionality was: > - SHF_GNU_RETAIN section flag indicates a section should be saved > from linker garbage collection. > - ".retain " assembler directive applies SHF_GNU_RETAIN > to the section containing . > - GCC "used" attribute emits a ".retain " directive for > the symbol declaration is is applied to. Applying the "used" > attribute to a symbol declaration does not change the structure of > the object file, beyond applying SHF_GNU_RETAIN to the section > containing that symbol. That description seems to say that a ".retain foo" would mean everything in foo's section is kept. If foo's section was the usual .data, you've kept virtually everything from garbage collection. Surely you don't expect ".retain foo" to create a separate .data section for foo? If you do, I'm strongly against that idea. Note that gas indeed supports multiple sections named .data that can serve the same purpose as -fdata-sections. See the gas doc for the optional .section field "unique". That might be the best way to avoid an under-the-hood -ffunction-sections/-fdata-sections. -- Alan Modra Australia Development Lab, IBM
Re: [PATCH] Add -fgnu-retain/-fno-gnu-retain
On Thu, Feb 18, 2021 at 12:00:59PM +, Jozef Lawrynowicz wrote: > If we can add ".retain " to GAS, then I agree, current GCC > SHF_GNU_RETAIN behavior should be removed. For GCC 12 we leverage > .retain to implement the functionality where "used" saves symbols form > linker garbage collection, without modifying section names or the > structure of the object file. Even in that case I think it is a bad idea to change the "used" attribute behavior, not everyone using "used" attribute needs or wants that behavior, so it should be a functionality provided by a separate new attribute. Jakub
Re: [PATCH] Add -fgnu-retain/-fno-gnu-retain
On Thu, Feb 18, 2021 at 11:19:50AM +0100, Richard Biener via Binutils wrote: > On Wed, Feb 17, 2021 at 12:25 PM Jozef Lawrynowicz > wrote: > > > > On Tue, Feb 16, 2021 at 05:45:47PM +0100, Jakub Jelinek via Gcc-patches > > wrote: > > > On Mon, Feb 15, 2021 at 02:35:07PM -0800, H.J. Lu via Gcc-patches wrote: > > > > When building Linux kernel, ld in bninutils 2.36 with GCC 11 generates > > > > thousands of > > > > > > > > ld: warning: orphan section `.data.event_initcall_finish' from > > > > `init/main.o' being placed in section `.data.event_initcall_finish' > > > > ld: warning: orphan section `.data.event_initcall_start' from > > > > `init/main.o' being placed in section `.data.event_initcall_start' > > > > ld: warning: orphan section `.data.event_initcall_level' from > > > > `init/main.o' being placed in section `.data.event_initcall_level' > > > > > > > > Since these sections are marked with SHF_GNU_RETAIN, they are placed in > > > > separate sections. They become orphan sections since they aren't > > > > expected > > > > in the Linux kernel linker script. But orphan sections normally don't > > > > work > > > > well with the Linux kernel linker script and the resulting kernel > > > > crashed. > > > > > > > > Add -fgnu-retain to disable SHF_GNU_RETAIN for Linux kernel build with > > > > -fno-gnu-retain. > > > > > > I'd say this shows that the changing of meaning of "used" attribute wasn't > > > really a good idea, the Linux kernel certainly won't be the only problem > > > and people use "used" attribute for many reasons and don't necessarily > > > want > > > the different sections or different behavior of those sections if they use > > > it. > > > > > > So, can't we instead: > > > 1) restore the old behavior of "used" by default > > > 2) add "retain" attribute that implies the SHF_GNU_RETAIN stuff and fails > > >if the configured assembler/linker doesn't support those > > > 3) add a non-default option through which one could opt in for "used" > > >attribute to imply retain attribute too for projects that want that? > > > > > > > In previous discussions, it seemed to me that there was general support > > for the "used" attribute implying SHF_GNU_RETAIN and saving symbols from > > linker garbage collection[1]. Of course, the current implementation > > results in undesirable behavior - the thought that all linker scripts > > not supporting uniquely named sections would need to be updated is quite > > alarming. > > > > It's a shame that all this extra complication is required, just because > > we cannot have a ".retain ", directive. > > > > My preferred vision for this functionality was: > > - SHF_GNU_RETAIN section flag indicates a section should be saved > > from linker garbage collection. > > - ".retain " assembler directive applies SHF_GNU_RETAIN > > to the section containing . > > - GCC "used" attribute emits a ".retain " directive for > > the symbol declaration is is applied to. Applying the "used" > > attribute to a symbol declaration does not change the structure of > > the object file, beyond applying SHF_GNU_RETAIN to the section > > containing that symbol. > > > > H.J. vetoed ".retain "[2], since it fails the predicate: > > Assembler directive referring to a symbol must operate on the symbol > > table. > > > > So because of this veto, we have compromised on "code quality" so far, > > since any linker script not supporting named sections, used to link an > > application containing "used" symbols (now put into their own section) has > > potential undefined behavior. > > > > With the new proposed change to use a "retain" attribute, we now > > compromise on functionality, since the "used" attribute saving symbols > > from linker garbage collection is disabled by default. > > > > All because we cannot introduce an exception to the above predicate. > > > > I would like to re-open discussions on whether a ".retain > > directive is acceptable. This would enable "used" to imply > > SHF_GNU_RETAIN, without changing the structure of the object file, > > thereby allowing the new functionality to be used without linker script > > modifications. > > > > If a Binutils global maintainer could side one way or the other on > > ".retain " (an opinion was never given by the Binutils > > maintainers when we had the discussions on that mailing list, although > > Jeff did support .retain in the GCC discussions[3]), then it will be > > easier to proceed knowing definitively that ".retain" is rejected and we > > have no choice but to go this route of having a separate "retain" > > attribute. > > So I then propose, for GCC 11, to rip out / disable any use of SHF_GNU_RETAIN, > effectively restoring GCC 10 beahvior and re-consider for GCC 12. It depends if the Binutils maintainers would accept a patch implementing ".retain " in principle. If we can add ".retain " to GAS, then I agree, current GCC SHF_GNU_RETAIN behavior should be removed. For GCC 12 we leverage .retain
Re: [PATCH] Add -fgnu-retain/-fno-gnu-retain
On Wed, Feb 17, 2021 at 12:25 PM Jozef Lawrynowicz wrote: > > On Tue, Feb 16, 2021 at 05:45:47PM +0100, Jakub Jelinek via Gcc-patches wrote: > > On Mon, Feb 15, 2021 at 02:35:07PM -0800, H.J. Lu via Gcc-patches wrote: > > > When building Linux kernel, ld in bninutils 2.36 with GCC 11 generates > > > thousands of > > > > > > ld: warning: orphan section `.data.event_initcall_finish' from > > > `init/main.o' being placed in section `.data.event_initcall_finish' > > > ld: warning: orphan section `.data.event_initcall_start' from > > > `init/main.o' being placed in section `.data.event_initcall_start' > > > ld: warning: orphan section `.data.event_initcall_level' from > > > `init/main.o' being placed in section `.data.event_initcall_level' > > > > > > Since these sections are marked with SHF_GNU_RETAIN, they are placed in > > > separate sections. They become orphan sections since they aren't expected > > > in the Linux kernel linker script. But orphan sections normally don't work > > > well with the Linux kernel linker script and the resulting kernel crashed. > > > > > > Add -fgnu-retain to disable SHF_GNU_RETAIN for Linux kernel build with > > > -fno-gnu-retain. > > > > I'd say this shows that the changing of meaning of "used" attribute wasn't > > really a good idea, the Linux kernel certainly won't be the only problem > > and people use "used" attribute for many reasons and don't necessarily want > > the different sections or different behavior of those sections if they use > > it. > > > > So, can't we instead: > > 1) restore the old behavior of "used" by default > > 2) add "retain" attribute that implies the SHF_GNU_RETAIN stuff and fails > >if the configured assembler/linker doesn't support those > > 3) add a non-default option through which one could opt in for "used" > >attribute to imply retain attribute too for projects that want that? > > > > In previous discussions, it seemed to me that there was general support > for the "used" attribute implying SHF_GNU_RETAIN and saving symbols from > linker garbage collection[1]. Of course, the current implementation > results in undesirable behavior - the thought that all linker scripts > not supporting uniquely named sections would need to be updated is quite > alarming. > > It's a shame that all this extra complication is required, just because > we cannot have a ".retain ", directive. > > My preferred vision for this functionality was: > - SHF_GNU_RETAIN section flag indicates a section should be saved > from linker garbage collection. > - ".retain " assembler directive applies SHF_GNU_RETAIN > to the section containing . > - GCC "used" attribute emits a ".retain " directive for > the symbol declaration is is applied to. Applying the "used" > attribute to a symbol declaration does not change the structure of > the object file, beyond applying SHF_GNU_RETAIN to the section > containing that symbol. > > H.J. vetoed ".retain "[2], since it fails the predicate: > Assembler directive referring to a symbol must operate on the symbol > table. > > So because of this veto, we have compromised on "code quality" so far, > since any linker script not supporting named sections, used to link an > application containing "used" symbols (now put into their own section) has > potential undefined behavior. > > With the new proposed change to use a "retain" attribute, we now > compromise on functionality, since the "used" attribute saving symbols > from linker garbage collection is disabled by default. > > All because we cannot introduce an exception to the above predicate. > > I would like to re-open discussions on whether a ".retain > directive is acceptable. This would enable "used" to imply > SHF_GNU_RETAIN, without changing the structure of the object file, > thereby allowing the new functionality to be used without linker script > modifications. > > If a Binutils global maintainer could side one way or the other on > ".retain " (an opinion was never given by the Binutils > maintainers when we had the discussions on that mailing list, although > Jeff did support .retain in the GCC discussions[3]), then it will be > easier to proceed knowing definitively that ".retain" is rejected and we > have no choice but to go this route of having a separate "retain" > attribute. So I then propose, for GCC 11, to rip out / disable any use of SHF_GNU_RETAIN, effectively restoring GCC 10 beahvior and re-consider for GCC 12. Richard. > Thanks, > Jozef > > [1] https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557097.html > [2] https://gcc.gnu.org/pipermail/gcc-patches/2020-November/558123.html > [3] https://gcc.gnu.org/pipermail/gcc-patches/2020-November/558398.html
Re: [PATCH] Add -fgnu-retain/-fno-gnu-retain
On Tue, Feb 16, 2021 at 05:45:47PM +0100, Jakub Jelinek via Gcc-patches wrote: > On Mon, Feb 15, 2021 at 02:35:07PM -0800, H.J. Lu via Gcc-patches wrote: > > When building Linux kernel, ld in bninutils 2.36 with GCC 11 generates > > thousands of > > > > ld: warning: orphan section `.data.event_initcall_finish' from > > `init/main.o' being placed in section `.data.event_initcall_finish' > > ld: warning: orphan section `.data.event_initcall_start' from `init/main.o' > > being placed in section `.data.event_initcall_start' > > ld: warning: orphan section `.data.event_initcall_level' from `init/main.o' > > being placed in section `.data.event_initcall_level' > > > > Since these sections are marked with SHF_GNU_RETAIN, they are placed in > > separate sections. They become orphan sections since they aren't expected > > in the Linux kernel linker script. But orphan sections normally don't work > > well with the Linux kernel linker script and the resulting kernel crashed. > > > > Add -fgnu-retain to disable SHF_GNU_RETAIN for Linux kernel build with > > -fno-gnu-retain. > > I'd say this shows that the changing of meaning of "used" attribute wasn't > really a good idea, the Linux kernel certainly won't be the only problem > and people use "used" attribute for many reasons and don't necessarily want > the different sections or different behavior of those sections if they use > it. > > So, can't we instead: > 1) restore the old behavior of "used" by default > 2) add "retain" attribute that implies the SHF_GNU_RETAIN stuff and fails >if the configured assembler/linker doesn't support those > 3) add a non-default option through which one could opt in for "used" >attribute to imply retain attribute too for projects that want that? > In previous discussions, it seemed to me that there was general support for the "used" attribute implying SHF_GNU_RETAIN and saving symbols from linker garbage collection[1]. Of course, the current implementation results in undesirable behavior - the thought that all linker scripts not supporting uniquely named sections would need to be updated is quite alarming. It's a shame that all this extra complication is required, just because we cannot have a ".retain ", directive. My preferred vision for this functionality was: - SHF_GNU_RETAIN section flag indicates a section should be saved from linker garbage collection. - ".retain " assembler directive applies SHF_GNU_RETAIN to the section containing . - GCC "used" attribute emits a ".retain " directive for the symbol declaration is is applied to. Applying the "used" attribute to a symbol declaration does not change the structure of the object file, beyond applying SHF_GNU_RETAIN to the section containing that symbol. H.J. vetoed ".retain "[2], since it fails the predicate: Assembler directive referring to a symbol must operate on the symbol table. So because of this veto, we have compromised on "code quality" so far, since any linker script not supporting named sections, used to link an application containing "used" symbols (now put into their own section) has potential undefined behavior. With the new proposed change to use a "retain" attribute, we now compromise on functionality, since the "used" attribute saving symbols from linker garbage collection is disabled by default. All because we cannot introduce an exception to the above predicate. I would like to re-open discussions on whether a ".retain directive is acceptable. This would enable "used" to imply SHF_GNU_RETAIN, without changing the structure of the object file, thereby allowing the new functionality to be used without linker script modifications. If a Binutils global maintainer could side one way or the other on ".retain " (an opinion was never given by the Binutils maintainers when we had the discussions on that mailing list, although Jeff did support .retain in the GCC discussions[3]), then it will be easier to proceed knowing definitively that ".retain" is rejected and we have no choice but to go this route of having a separate "retain" attribute. Thanks, Jozef [1] https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557097.html [2] https://gcc.gnu.org/pipermail/gcc-patches/2020-November/558123.html [3] https://gcc.gnu.org/pipermail/gcc-patches/2020-November/558398.html
Re: [PATCH] Add -fgnu-retain/-fno-gnu-retain
On Mon, Feb 15, 2021 at 02:35:07PM -0800, H.J. Lu via Gcc-patches wrote: > When building Linux kernel, ld in bninutils 2.36 with GCC 11 generates > thousands of > > ld: warning: orphan section `.data.event_initcall_finish' from `init/main.o' > being placed in section `.data.event_initcall_finish' > ld: warning: orphan section `.data.event_initcall_start' from `init/main.o' > being placed in section `.data.event_initcall_start' > ld: warning: orphan section `.data.event_initcall_level' from `init/main.o' > being placed in section `.data.event_initcall_level' > > Since these sections are marked with SHF_GNU_RETAIN, they are placed in > separate sections. They become orphan sections since they aren't expected > in the Linux kernel linker script. But orphan sections normally don't work > well with the Linux kernel linker script and the resulting kernel crashed. > > Add -fgnu-retain to disable SHF_GNU_RETAIN for Linux kernel build with > -fno-gnu-retain. I'd say this shows that the changing of meaning of "used" attribute wasn't really a good idea, the Linux kernel certainly won't be the only problem and people use "used" attribute for many reasons and don't necessarily want the different sections or different behavior of those sections if they use it. So, can't we instead: 1) restore the old behavior of "used" by default 2) add "retain" attribute that implies the SHF_GNU_RETAIN stuff and fails if the configured assembler/linker doesn't support those 3) add a non-default option through which one could opt in for "used" attribute to imply retain attribute too for projects that want that? Jakub
[PATCH] Add -fgnu-retain/-fno-gnu-retain
When building Linux kernel, ld in bninutils 2.36 with GCC 11 generates
thousands of
ld: warning: orphan section `.data.event_initcall_finish' from `init/main.o'
being placed in section `.data.event_initcall_finish'
ld: warning: orphan section `.data.event_initcall_start' from `init/main.o'
being placed in section `.data.event_initcall_start'
ld: warning: orphan section `.data.event_initcall_level' from `init/main.o'
being placed in section `.data.event_initcall_level'
Since these sections are marked with SHF_GNU_RETAIN, they are placed in
separate sections. They become orphan sections since they aren't expected
in the Linux kernel linker script. But orphan sections normally don't work
well with the Linux kernel linker script and the resulting kernel crashed.
Add -fgnu-retain to disable SHF_GNU_RETAIN for Linux kernel build with
-fno-gnu-retain.
gcc/
PR target/99113
* common.opt: Add -fgnu-retain.
* toplev.c (process_options): Update flag_gnu_retain from
SUPPORTS_SHF_GNU_RETAIN.
* varasm.c (get_section): Replace SUPPORTS_SHF_GNU_RETAIN with
flag_gnu_retain.
(resolve_unique_section): Likewise.
(get_variable_section): Likewise.
(switch_to_section): Likewise.
* doc/invoke.texi: Document -fgnu-retain/-fno-gnu-retain.
gcc/testsuite/
* c-c++-common/pr99113.c: New test.
---
gcc/common.opt | 4
gcc/doc/invoke.texi | 8
gcc/testsuite/c-c++-common/pr99113.c | 7 +++
gcc/toplev.c | 9 +
gcc/varasm.c | 8
5 files changed, 32 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/pr99113.c
diff --git a/gcc/common.opt b/gcc/common.opt
index c75dd36843e..912e879e49d 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1666,6 +1666,10 @@ floop-unroll-and-jam
Common Var(flag_unroll_jam) Optimization
Perform unroll-and-jam on loops.
+fgnu-retain
+Common Var(flag_gnu_retain) Init(-1)
+Use SHF_GNU_RETAIN if supported by the assembler and the linker.
+
fgnu-tm
Common Var(flag_tm)
Enable support for GNU transactional memory.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index e8baa545eee..20962a8749e 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -16168,6 +16168,14 @@ DSOs; if your program relies on reinitialization of a
DSO via
@code{dlclose} and @code{dlopen}, you can use
@option{-fno-gnu-unique}.
+@item -fno-gnu-retain
+@opindex fno-gnu-retain
+@opindex fgnu-retain
+On systems with recent GNU assembler and linker, the compiler places
+preserved symbols in separate SHF_GNU_RETAIN sections. If your program,
+like Linux kernel, doesn't work with such symbol placement, you can use
+@option{-fno-gnu-retain} to disable SHF_GNU_RETAIN sections.
+
@item -fpcc-struct-return
@opindex fpcc-struct-return
Return ``short'' @code{struct} and @code{union} values in memory like
diff --git a/gcc/testsuite/c-c++-common/pr99113.c
b/gcc/testsuite/c-c++-common/pr99113.c
new file mode 100644
index 000..d3c830247a2
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr99113.c
@@ -0,0 +1,7 @@
+/* { dg-do compile } */
+/* { dg-options "-Wall -O2 -fno-gnu-retain " } */
+
+static int xyzzy __attribute__((__used__)) = 1;
+
+/* { dg-final { scan-assembler "xyzzy" } } */
+/* { dg-final { scan-assembler-not "\.data.*,\"awR\"" { target
R_flag_in_section } } } */
diff --git a/gcc/toplev.c b/gcc/toplev.c
index d8cc254adef..a1b18f5338c 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1761,6 +1761,15 @@ process_options (void)
if (flag_large_source_files)
line_table->default_range_bits = 0;
+ if (flag_gnu_retain == -1)
+flag_gnu_retain = SUPPORTS_SHF_GNU_RETAIN;
+ else if (flag_gnu_retain > 0 && !SUPPORTS_SHF_GNU_RETAIN)
+{
+ warning_at (UNKNOWN_LOCATION, 0, "%qs is not supported for this target",
+ "-fgnu-retain");
+ flag_gnu_retain = 0;
+}
+
/* Please don't change global_options after this point, those changes won't
be reflected in optimization_{default,current}_node. */
}
diff --git a/gcc/varasm.c b/gcc/varasm.c
index 29478ab0d8d..4e0e30abee5 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -297,7 +297,7 @@ get_section (const char *name, unsigned int flags, tree
decl,
slot = section_htab->find_slot_with_hash (name, htab_hash_string (name),
INSERT);
flags |= SECTION_NAMED;
- if (SUPPORTS_SHF_GNU_RETAIN
+ if (flag_gnu_retain
&& decl != nullptr
&& DECL_P (decl)
&& DECL_PRESERVE_P (decl))
@@ -487,7 +487,7 @@ resolve_unique_section (tree decl, int reloc
ATTRIBUTE_UNUSED,
if (DECL_SECTION_NAME (decl) == NULL
&& targetm_common.have_named_sections
&& (flag_function_or_data_sections
- || (SUPPORTS_SHF_GNU_RETAIN && DECL_PRESERVE_P (decl))
+ || (flag_gnu_retain && DECL_PRESERVE_P (decl))
|| DECL_COMDAT_GROUP
