This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: PATCH: PR ld/4701: binutils generates invalid klibc-based binary on Linux x86_64
On Thu, Jun 28, 2007 at 06:02:20AM -0700, H. J. Lu wrote:
> On Thu, Jun 28, 2007 at 03:05:15PM +0930, Alan Modra wrote:
> > On Wed, Jun 27, 2007 at 09:20:51PM -0700, H. J. Lu wrote:
> > > On Thu, Jun 28, 2007 at 12:30:27PM +0930, Alan Modra wrote:
> > > > On Wed, Jun 27, 2007 at 07:40:27PM -0700, H. J. Lu wrote:
> > > > > Linux ld.so isn't OK with your change. We haven't seen the problem
> > > > > because we won't get a PT_LOAD segment with .bss sections only
> > > > > using the default linker script. I uploaded a testcase to:
> > > > >
> > > > > http://sourceware.org/bugzilla/show_bug.cgi?id=4701
> > > >
> > > > Arggh! I was sure this worked! So we should go back to using my voff
> > > > code probably, or otherwise hack p_offset.
> > >
> > > Whatever we do, we should follow gABI.
> > > >
> > > > Meanwhile, please let me know exactly why klibc was unhappy, as I'm
> > > > concerned that whatever is causing the segv there might also be
> > > > affected by overlapping p_offset for bss segments.
> > >
> > > My testcase is derived from klibc. Executables have a one PT_LOAD
> > > segment with only a .bss section. The only difference is the failed
> > > ones in klibc are executables, which are loaded by kernel, and mine
> > > is a shared library, which is loaded by ld.so.
> >
> > This should comply with alignment requirement but not waste file
> > space. Can you try this against klibc executables?
> >
> > * elf.c (assign_file_positions_for_load_sections): Ensure bss
> > segments meet gABI alignment requirements.
> >
>
> It works on klibc. However, I got
>
> FAIL: overlay size
>
> on Linux/x86-64. You should be able to see it with a cross binutils.
>
>
This modified patch works on Linux/x86-64. I added
if ((ufile_ptr) off >= align)
{
}
so that we always pad if offset < align.
H.J.
-----
--- bfd/elf.c.bss 2007-06-27 19:07:44.000000000 -0700
+++ bfd/elf.c 2007-06-28 06:37:46.000000000 -0700
@@ -4429,6 +4429,7 @@ assign_file_positions_for_load_sections
m = m->next, p++, j++)
{
asection **secpp;
+ bfd_vma off_adjust;
/* If elf_segment_map is not from map_sections_to_segments, the
sections may not be correctly ordered. NOTE: sorting should
@@ -4484,11 +4485,11 @@ assign_file_positions_for_load_sections
else
p->p_align = 0;
+ off_adjust = 0;
if (p->p_type == PT_LOAD
&& m->count > 0)
{
bfd_size_type align;
- bfd_vma adjust;
unsigned int align_power = 0;
if (m->p_align_valid)
@@ -4515,27 +4516,38 @@ assign_file_positions_for_load_sections
set via struct bfd_elf_special_section. */
elf_section_type (m->sections[i]) = SHT_NOBITS;
- adjust = vma_page_aligned_bias (m->sections[0]->vma, off, align);
- if (adjust != 0)
+ off_adjust = vma_page_aligned_bias (m->sections[0]->vma, off, align);
+ if (off_adjust != 0)
{
/* If the first section isn't loadable, the same holds
- for any other sections. We don't need to align the
- segment on disk since the segment doesn't need file
- space. */
- i = 0;
- while (elf_section_type (m->sections[i]) == SHT_NOBITS)
+ for any other sections. We shouldn't need to align
+ the segment on disk since the segment doesn't need
+ file space, but the gABI arguably requires the
+ alignment and glibc ld.so checks it. So to comply
+ with the alignment requirement but not waste file
+ space, we adjust p_offset for just this segment.
+ This may put p_offset past the end of file, but
+ that shouldn't matter. */
+ bfd_boolean must_pad = TRUE;
+ if ((ufile_ptr) off >= align)
{
- /* If a segment starts with .tbss, we need to look
- at the next section to decide whether the segment
- has any loadable sections. */
- if ((elf_section_flags (m->sections[i]) & SHF_TLS) == 0
- || ++i >= m->count)
+ i = 0;
+ while (elf_section_type (m->sections[i]) == SHT_NOBITS)
{
- adjust = 0;
- break;
+ /* If a segment starts with .tbss, we need to look
+ at the next section to decide whether the segment
+ has any loadable sections. */
+ if (!(elf_section_flags (m->sections[i]) & SHF_TLS)
+ || ++i >= m->count)
+ {
+ must_pad = FALSE;
+ break;
+ }
}
}
- off += adjust;
+ off += off_adjust;
+ if (must_pad)
+ off_adjust = 0;
}
}
/* Make sure the .dynamic section is the first section in the
@@ -4732,6 +4744,7 @@ assign_file_positions_for_load_sections
p->p_flags |= PF_W;
}
}
+ off -= off_adjust;
/* Check that all sections are in a PT_LOAD segment.
Don't check funky gdb generated core files. */