public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [RFC] Make __bss_start point at actual start of .bss
@ 2023-06-23  6:37 Andreas Krebbel
  2023-06-23 10:15 ` Alan Modra
  0 siblings, 1 reply; 6+ messages in thread
From: Andreas Krebbel @ 2023-06-23  6:37 UTC (permalink / raw)
  To: binutils

Hi,

the way __bss_start is defined in our default linker script right now makes it point after the .data
section instead of the actual start of .bss. The symbol is defined outside of the section definition
and therefore is not moved together with the .bss section if ld applies any alignment to it.

This not only is very counterintuitive but also triggers a problem on zSystems. All our symbols need
to be 2 byte aligned. If .data has an odd size this symbol is misaligned and thereby not ABI conform
anymore. While we could add platform-dependent alignment for the symbol here I'm wondering whether
we perhaps should rather move __bss_start into the .bss section definition in order to have it point
at the actual start of the section. I've checked that even if there is no input .bss ld will create
an empty .bss output section if the symbol is referenced.

I've also checked with mold and lld. Both linkers put __bss_start at the actual beginning of .bss as
one would expect. Does anything speak against doing the same with ld?

Andreas

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC] Make __bss_start point at actual start of .bss
  2023-06-23  6:37 [RFC] Make __bss_start point at actual start of .bss Andreas Krebbel
@ 2023-06-23 10:15 ` Alan Modra
  2023-06-26  6:39   ` Andreas Krebbel
  0 siblings, 1 reply; 6+ messages in thread
From: Alan Modra @ 2023-06-23 10:15 UTC (permalink / raw)
  To: Andreas Krebbel; +Cc: binutils

On Fri, Jun 23, 2023 at 08:37:33AM +0200, Andreas Krebbel via Binutils wrote:
> While we could add platform-dependent alignment for the symbol here I'm wondering whether
> we perhaps should rather move __bss_start into the .bss section definition in order to have it point
> at the actual start of the section.

The question is, how is __bss_start used?  Is it the start of the .bss
section or the start of the bss segment?  I think it is mainly used as
the start of the segment, when loaders zero out all bss memory, so it
should stay before other bss-style sections like .sbss.

The comment in elf.sc about OTHER_BSS_SYMBOLS dates to commit
bff600cfa4e0, where despite the comment, __bss_start was left as the
start of the segment.

Aligning before __bss_start is likely the best thing to do.  If the
first two output sections in the bss segment were .sbss and .bss then
ALIGN (ALIGNOF(.sbss)!=0 ? ALIGNOF(.sbss) : ALIGNOF(.bss))
ought to work.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC] Make __bss_start point at actual start of .bss
  2023-06-23 10:15 ` Alan Modra
@ 2023-06-26  6:39   ` Andreas Krebbel
  2023-06-26 15:51     ` Michael Matz
  0 siblings, 1 reply; 6+ messages in thread
From: Andreas Krebbel @ 2023-06-26  6:39 UTC (permalink / raw)
  To: Alan Modra; +Cc: binutils

On 6/23/23 12:15, Alan Modra wrote:
> On Fri, Jun 23, 2023 at 08:37:33AM +0200, Andreas Krebbel via Binutils wrote:
>> While we could add platform-dependent alignment for the symbol here I'm wondering whether
>> we perhaps should rather move __bss_start into the .bss section definition in order to have it point
>> at the actual start of the section.
> 
> The question is, how is __bss_start used?  Is it the start of the .bss
> section or the start of the bss segment?  I think it is mainly used as
> the start of the segment, when loaders zero out all bss memory, so it
> should stay before other bss-style sections like .sbss.

The usage which triggered the problem came from qemu and the code there clears memory from
__bss_start through _end. This should work. It would fall short if somebody would actually just
clear __bss_start + sizeof(.bss).
Nick suggested to use the .bss section symbol instead of __bss_start. This looks like a good
workaround to me. We will miss clearing the padding bytes of the data section then, but this
shouldn't be a problem.
We nevertheless have to fix the issue with __bss_start for z.

> 
> The comment in elf.sc about OTHER_BSS_SYMBOLS dates to commit
> bff600cfa4e0, where despite the comment, __bss_start was left as the
> start of the segment.
> Aligning before __bss_start is likely the best thing to do.  If the
> first two output sections in the bss segment were .sbss and .bss then
> ALIGN (ALIGNOF(.sbss)!=0 ? ALIGNOF(.sbss) : ALIGNOF(.bss))
> ought to work.

For z it isn't really about aligning the symbol the same way as .bss. It rather is about applying an
alignment requirement mandated by the ABI. Would it perhaps make sense to introduce a variable
ABI_ALIGNMENT which could be used in the linker script template for aligning all free-floating symbols.

Bye,

Andreas

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC] Make __bss_start point at actual start of .bss
  2023-06-26  6:39   ` Andreas Krebbel
@ 2023-06-26 15:51     ` Michael Matz
  2023-06-26 16:10       ` Andreas Krebbel
  0 siblings, 1 reply; 6+ messages in thread
From: Michael Matz @ 2023-06-26 15:51 UTC (permalink / raw)
  To: Andreas Krebbel; +Cc: Alan Modra, binutils

Hello,

On Mon, 26 Jun 2023, Andreas Krebbel via Binutils wrote:

> > The question is, how is __bss_start used?  Is it the start of the .bss
> > section or the start of the bss segment?  I think it is mainly used as
> > the start of the segment, when loaders zero out all bss memory, so it
> > should stay before other bss-style sections like .sbss.
> 
> The usage which triggered the problem came from qemu and the code there 
> clears memory from __bss_start through _end. This should work.

But that's _exactly_ the case that Alan worried about: __bss_start being 
assumed to be the start of the bss segment, not merely of the section 
named .bss.  If bss-style sections come before .bss (like .sbss does on 
some targets) then __bss_start should be in front of those ones, otherwise 
the above clearing that qemu does would not work anymore (.sbss wouldn't 
be cleared).  See here (elf.sc):

  ${RELOCATING+${CREATE_SHLIB+PROVIDE (}${USER_LABEL_PREFIX}__bss_start = .${CREATE_SHLIB+)};}
  ${RELOCATING+${OTHER_BSS_SYMBOLS}}
  ${DATA_SDATA-${SBSS}}
  ${BSS_PLT+${PLT}}
  .${BSS_NAME}          ${RELOCATING-0} :
  {
     ... normal bss stuff ...

__bss_start needs to remain in front of the SBSS entry for the 
traditional usage to keep working (on targets where DATA_SDATA isn't set).


Ciao,
Michael.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC] Make __bss_start point at actual start of .bss
  2023-06-26 15:51     ` Michael Matz
@ 2023-06-26 16:10       ` Andreas Krebbel
  2023-06-26 16:27         ` Michael Matz
  0 siblings, 1 reply; 6+ messages in thread
From: Andreas Krebbel @ 2023-06-26 16:10 UTC (permalink / raw)
  To: Michael Matz; +Cc: Alan Modra, binutils

On 6/26/23 17:51, Michael Matz wrote:
> Hello,
> 
> On Mon, 26 Jun 2023, Andreas Krebbel via Binutils wrote:
> 
>>> The question is, how is __bss_start used?  Is it the start of the .bss
>>> section or the start of the bss segment?  I think it is mainly used as
>>> the start of the segment, when loaders zero out all bss memory, so it
>>> should stay before other bss-style sections like .sbss.
>>
>> The usage which triggered the problem came from qemu and the code there 
>> clears memory from __bss_start through _end. This should work.
> 
> But that's _exactly_ the case that Alan worried about: __bss_start being 
> assumed to be the start of the bss segment, not merely of the section 
> named .bss.  If bss-style sections come before .bss (like .sbss does on 
> some targets) then __bss_start should be in front of those ones, otherwise 
> the above clearing that qemu does would not work anymore (.sbss wouldn't 
> be cleared).  See here (elf.sc):
> 
>   ${RELOCATING+${CREATE_SHLIB+PROVIDE (}${USER_LABEL_PREFIX}__bss_start = .${CREATE_SHLIB+)};}
>   ${RELOCATING+${OTHER_BSS_SYMBOLS}}
>   ${DATA_SDATA-${SBSS}}
>   ${BSS_PLT+${PLT}}
>   .${BSS_NAME}          ${RELOCATING-0} :
>   {
>      ... normal bss stuff ...
> 
> __bss_start needs to remain in front of the SBSS entry for the 
> traditional usage to keep working (on targets where DATA_SDATA isn't set).

Agreed. I just wanted to state that in this case it isn't actually a problem that __bss_start
doesn't match the start of .bss because they use _end instead of the size of .bss to end address for
the memory clearing.

I've already posted a patch which follows Alan's recommendation.

Sorry for the confusion.

Bye,

Andreas

> 
> 
> Ciao,
> Michael.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC] Make __bss_start point at actual start of .bss
  2023-06-26 16:10       ` Andreas Krebbel
@ 2023-06-26 16:27         ` Michael Matz
  0 siblings, 0 replies; 6+ messages in thread
From: Michael Matz @ 2023-06-26 16:27 UTC (permalink / raw)
  To: Andreas Krebbel; +Cc: Alan Modra, binutils

Hey,

On Mon, 26 Jun 2023, Andreas Krebbel wrote:

> > __bss_start needs to remain in front of the SBSS entry for the 
> > traditional usage to keep working (on targets where DATA_SDATA isn't 
> > set).
> 
> Agreed. I just wanted to state that in this case it isn't actually a problem that __bss_start
> doesn't match the start of .bss because they use _end instead of the size of .bss to end address for
> the memory clearing.

Ah okay, I see.

> I've already posted a patch which follows Alan's recommendation.
> 
> Sorry for the confusion.

(And sorry for replying before reading _all_ incoming mails in the folder 
:) )


Ciao,
Michael.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-06-26 16:27 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-23  6:37 [RFC] Make __bss_start point at actual start of .bss Andreas Krebbel
2023-06-23 10:15 ` Alan Modra
2023-06-26  6:39   ` Andreas Krebbel
2023-06-26 15:51     ` Michael Matz
2023-06-26 16:10       ` Andreas Krebbel
2023-06-26 16:27         ` Michael Matz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).