[PATCH v2] elf: Properly set sh_offset for .tbss sections

[Alan Modra]

On Tue, Apr 29, 2025 at 09:36:39AM +0800, H.J. Lu wrote:
> On Tue, Apr 29, 2025 at 8:44 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Tue, Apr 29, 2025 at 8:05 AM Alan Modra <amodra@gmail.com> wrote:
> > >
> > > On Mon, Apr 28, 2025 at 08:08:56AM +0200, Jan Beulich wrote:
> > > > On 28.04.2025 04:47, H.J. Lu wrote:
> > > > > Set sh_offset for .tbss sections to their nominal offset after aligning.
> > > > > They are not loaded from disk so the value doesn't really matter, except
> > > > > when the .tbss section is the first one in a PT_TLS segment.  In that
> > > > > case, it sets the p_offset for the PT_TLS segment, which according to
> > > > > the ELF gABI ought to satisfy p_offset % p_align == p_vaddr % p_align.
> > > > >
> > > > > bfd/
> > > > >
> > > > > PR ld/32896
> > > > > * elf.c (assign_file_positions_for_load_sections): Properly set
> > > > > sh_offset for .tbss sections.
> > > >
> > > > Why does the order of checks (PT_LOAD then SHT_NOBITS) need changing here?
> > > > While pre-existing, I'm also having a hard time seeing why among SHT_NOBITS
> > > > sections SHF_TLS ones would need special casing here. While I agree that
> > > > the usual requirements have to be met by PT_TLS, the entire change feels
> > > > like it's papering over an issue elsewhere (presumably in the laying out of
> > > > load segments).
> > >
> > > The patch is OK.  Please apply.
> > >
> > > Jan, there is a lot of history to this code.  sh_offset for SHT_NOBITS
> > > is somewhat arbitrary since those sections are not loaded from the
> > > image.  p_offset for a program header with p_filesz zero is likewise
> > > arbitrary, except that the gABI makes no exception regarding alignment
> > > modulo p_align for such headers.
> > >
> > > The problem is that various other tools break (or have been broken in
> > > the past) when we make what seems like logical changes to our code,
> > > such as the one you point out, that all SHT_NOBITS sections ought to
> > > be treated the same here.  See pr12921 and pr25662.
> > >
> > > --
> > > Alan Modra
> >
> > I'd like to avoid putting sh_offset of .tbss sections beyond EOF.
> > I will see what I can do.
> >
> 
> I'd like to check in this v2 patch.   The difference is
> 
> -   this_hdr->sh_offset = sec->filepos = off + adjust;
> +   file_ptr aligned_off = off + adjust;
> +   /* Put meaningless sh_offset of .tbss sections
> +      without file contents somewhere within the first
> +      page, in an attempt to not point past EOF and
> +      avoid sh_offset of zero.  */
> +   if (aligned_off >= (file_ptr) maxpagesize)
> +     aligned_off = (aligned_off + align - 1) % align + 1;
> +   this_hdr->sh_offset = sec->filepos = aligned_off;
> 
> so that sh_offset of .tbss sections is  somewhere within the first page.
> 
> OK for master?

I'd rather you committed v1.  This one might make a TLS segment with
both .tdata and .tbss look a little wierd.

-- 
Alan Modra