Re: PATCH: PR ld/4701: binutils generates invalid klibc-based binary on Linux x86_64

["H. J. Lu" <hjl at lucon dot org>]

On Thu, Jun 28, 2007 at 09:31:40AM +0930, Alan Modra wrote:
> On Wed, Jun 27, 2007 at 11:44:10AM -0700, H. J. Lu wrote:
> > On Wed, Jun 27, 2007 at 11:09:03AM -0700, H. J. Lu wrote:
> > > I don't think we can skip segment alignment on disk when its offset
> > > < alignment. This patch fixes it. I will try to find a small testcase.
> 
> Why ever does file offset matter for a segment that isn't loaded from
> disk??

The segment in question is:

  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align

  LOAD           0x00000000000002b8 0x00000000006002b8 0x00000000006002b8
                 0x0000000000000000 0x0000000000000008  RW     200000

>From gABI:

p_offset
    This member gives the offset from the beginning of the file at
which the first byte of the segment resides.

p_vaddr
    This member gives the virtual address at which the first byte of
the segment resides in memory.

p_align
    As ``Program Loading'' describes in this chapter of the processor
supplement, loadable process segments must have congruent values for
p_vaddr and p_offset, modulo the page size. This member gives the value
to which the segments are aligned in memory and in the file. Values 0
and 1 mean no alignment is required. Otherwise, p_align should be a
positive, integral power of 2, and p_vaddr should equal p_offset,
modulo p_align.

Your change resulted in

  LOAD           0x00000000000002b2 0x00000000006002b8 0x00000000006002b8
                 0x0000000000000000 0x0000000000000008  RW     200000

I don't think it follows gABI.

> 
> > 	PR ld/4701
> > 	* elf.c (assign_file_positions_for_load_sections): Skip segment
> > 	alignment on disk only if its offset >= alignment.
> 
> And if it does matter, this patch can't be correct.  The old code used
> to always align.

The old code has

	  adjust = vma_page_aligned_bias (m->sections[0]->vma, off, align);
	  off += adjust;
	  if (adjust != 0
	      && !m->includes_filehdr
	      && !m->includes_phdrs
	      && (ufile_ptr) off >= align)
	    {
	      /* If the first section isn't loadable, the same holds for
		 any other sections.  Since the segment won't need file
		 space, we can make p_offset overlap some prior segment.
		 However, .tbss is special.  If a segment starts with
		 .tbss, we need to look at the next section to decide
		 whether the segment has any loadable sections.  */
	      i = 0;
	      while ((m->sections[i]->flags & SEC_LOAD) == 0
		     && (m->sections[i]->flags & SEC_HAS_CONTENTS) == 0)
		{
		  if ((m->sections[i]->flags & SEC_THREAD_LOCAL) == 0
		      || ++i >= m->count)
		    {
		      off -= adjust;
		      voff = adjust - align;
		      break;
		    }
		}
	    }

It won't align under certain conditions.


H.J.