Bug 11899 - segments not sorted by LMA
Summary: segments not sorted by LMA
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: gold (show other bugs)
Version: 2.21
: P3 normal
Target Milestone: ---
Assignee: Ian Lance Taylor
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-10 11:18 UTC by Nick Clifton
Modified: 2011-07-09 00:17 UTC (History)
1 user (show)

See Also:
Host: any
Target: any
Build: any
Last reconfirmed:


Attachments
Sort segments by their load address (324 bytes, patch)
2010-08-10 11:19 UTC, Nick Clifton
Details | Diff
Revised patch (354 bytes, patch)
2010-08-20 08:43 UTC, Nick Clifton
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clifton 2010-08-10 11:18:49 UTC
The code in Layout::set_segment_offsets() expects the segments to be sorted by
their load address, but currently they are sorted by the LMA of their first section.

To reproduce:
  % cat phdr.s
        .text
        .word 1

        .data
        .word 2

  % cat phdrs.ld
PHDRS
{
  headers PT_PHDR FILEHDR PHDRS;
  data    PT_LOAD AT (0xfff50000);
  text    PT_LOAD AT (0xfff40000);
}

SECTIONS
{
  .data 0 :
  {
    *(.data)
  } : data

  .text :
  {
    *(.text)
  } : text
}

  % gcc -c phdr.s
  % gold -T phdrs.ld phdr.o
  gold: internal error in set_segment_offsets, at gold/layout.cc:2641
Comment 1 Nick Clifton 2010-08-10 11:19:55 UTC
Created attachment 4922 [details]
Sort segments by their load address
Comment 2 Nick Clifton 2010-08-10 11:20:09 UTC
Proposed patch uploaded.
Comment 3 Ian Lance Taylor 2010-08-19 23:00:12 UTC
The patch is not correct in general, because the paddr_ field of segments is not 
set in general at the point where it is tested.  It will be set when a linker 
script is used, but not by default.  The paddr_ field is set when 
Layout::set_segment_offsets calls set_section_addresses, but not before.

Your patch would probably be correct if it tested something like
  seg1->are_addresses_set() ? seg1->paddr() : seg1->first_section_load_address()

Want to try that?
Comment 4 Nick Clifton 2010-08-20 08:43:27 UTC
Created attachment 4943 [details]
Revised patch
Comment 5 Nick Clifton 2010-08-20 08:45:31 UTC
Hi Ian,

  Thanks for catching that for me.  Your suggested improvement works just fine,
so I have uploaded a revised patch.

  OK to apply ?

Cheers
  Nick

gold/ChangeLog
2010-08-20  Nick Clifton  <nickc@redhat.com>

	PR 11899
	* layout.cc (segment_precedes): Sort segments by their physical
	addresses, if they have been set.

Comment 6 Ian Lance Taylor 2010-08-23 04:50:45 UTC
I would use parentheses and line up the ?: under the first part of the 
expression.

Patch is OK.

Thanks.
Comment 7 cvs-commit@gcc.gnu.org 2010-08-24 07:24:32 UTC
Subject: Bug 11899

CVSROOT:	/cvs/src
Module name:	src
Changes by:	nickc@sourceware.org	2010-08-24 07:24:10

Modified files:
	gold           : ChangeLog layout.cc 

Log message:
	PR 11899
	* layout.cc (segment_precedes): Sort segments by their physical
	addresses, if they have been set.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gold/ChangeLog.diff?cvsroot=src&r1=1.621&r2=1.622
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gold/layout.cc.diff?cvsroot=src&r1=1.176&r2=1.177

Comment 8 Nick Clifton 2010-08-24 07:25:01 UTC
Subject: Re:  segments not sorted by LMA

Hi Ian,

> I would use parentheses and line up the ?: under the first part of the
> expression.
>
> Patch is OK.

Thanks - patch committed with that change.

Cheers
   Nick

Comment 9 Ian Lance Taylor 2011-07-09 00:17:32 UTC
This seems to be fixed.