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

[H.J. Lu]

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?

Thanks.

-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v2-0001-elf-Properly-set-sh_offset-for-.tbss-sections.patch
Type: text/x-patch
Size: 5654 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20250429/ab063f3e/attachment-0001.bin>