Bug 16821 - x86_64 PE/COFF: ld truncates addresses of symbols from linker scripts to 32 bit
Summary: x86_64 PE/COFF: ld truncates addresses of symbols from linker scripts to 32 bit
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.25
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-08 14:10 UTC by Corinna Vinschen
Modified: 2022-08-03 10:53 UTC (History)
5 users (show)

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


Attachments
Partial patch (847 bytes, patch)
2014-04-09 15:15 UTC, Nick Clifton
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Corinna Vinschen 2014-04-08 14:10:39 UTC
I just encountered a serious bug while building the Cygwin DLL for
x86_64-pc-cygwin, which I never noticed before (*blush*):

The Cygwin DLL is built to be located at the address 0x1:80040000,
so it's not located in the lower 32 bits area, but just a tad bit
higher.  It's also using its own linker script.

When I tried to access __image_base__ from the DLL itself, I found
to my surprise that __image_base__ was not 0x1:80040000, but instead
0x0:8004000, so it's truncated to 32 bit.

The only time __image_base__ occurs in the script is to compute the
start of the .text segment:

  .text  __image_base__ + __section_alignment__  :
  { ... }

Further investigation showed that four more symbols were affected:

  .rdata ALIGN(__section_alignment__) :
  {
    *(.rdata)
    *(SORT(.rdata$*))
    *(.rdata_cygwin_nocopy)
    __rt_psrelocs_start = .;
    *(.rdata_runtime_pseudo_reloc)
    __rt_psrelocs_end = .;

  }
  __rt_psrelocs_size = __rt_psrelocs_end - __rt_psrelocs_start;
  ___RUNTIME_PSEUDO_RELOC_LIST_END__ = .;
  __RUNTIME_PSEUDO_RELOC_LIST_END__ = .;
  ___RUNTIME_PSEUDO_RELOC_LIST__ = . - __rt_psrelocs_size;
  __RUNTIME_PSEUDO_RELOC_LIST__ = . - __rt_psrelocs_size;

In this piece of the script, the symbols ___RUNTIME_PSEUDO_RELOC_LIST__
and __RUNTIME_PSEUDO_RELOC_LIST__ have an address which is truncated
to 32 bit.  If I change this code to

  .rdata ALIGN(__section_alignment__) :
  {
    *(.rdata)
    *(SORT(.rdata$*))
    *(.rdata_cygwin_nocopy)
    ___RUNTIME_PSEUDO_RELOC_LIST__ = .;
    __RUNTIME_PSEUDO_RELOC_LIST__ = .;
    *(.rdata_runtime_pseudo_reloc)
    ___RUNTIME_PSEUDO_RELOC_LIST_END__ = .;
    __RUNTIME_PSEUDO_RELOC_LIST_END__ = .;
  }

the address of the two symbols is correct.

Additionally there are two symbols which are defined with the ABSOLUTE
macro:

    _SYM (_cygheap_start) = ABSOLUTE(.);
    [...]
    _SYM (_cygheap_end) = ABSOLUTE(.);

Both symbols have the correct address, but again truncated to 32 bit.
If I change the script to not use ABSOLUTE

    _SYM (_cygheap_start) = .;
    [...]
    _SYM (_cygheap_end) = .;

the 64 bit addresses are correct.

So it appears that during certain computations in ld, the addresses of
symbols are truncated to 32 bit values.

Given that x86_64 Cygwin executables are located at 0x1:00040000 by
default, this means that *ALL* Cygwin executables are affected by this bug.

Any chance to fix this ASAP?


Thanks,
Corinna
Comment 1 Nick Clifton 2014-04-09 15:15:51 UTC
Created attachment 7543 [details]
Partial patch
Comment 2 Nick Clifton 2014-04-09 15:24:30 UTC
Hi Corinna,

  DJ has found the cause of the problem: the PE and PE32+ file formats store the value of symbols in a 32-bit field.  For section relative symbols this amount is usually enough, as section addresses can be 64-bit.  But for absolute symbols any value needing more than 32-bits is silently truncated by the linker.

  I have uploaded a patch which is a partial fix for the problem.  It detects out-of-range absolute values and tries to convert them into section relative values.  This works for most cases, but it fails for the __image_base__ and __ImageBase__ symbols, and possibly some others that I have not yet encountered.  The problem is that these symbols have a value which is less than the lowest addressed section, but higher than 1^32.  (Note the value of ImageBase in the PE header is not affected by this problem.  It is only the *symbols* __image_base__ and __ImageBase__ that are affected).

  One thing that patch does not do at the moment is issue an error message when it knows that the truncation is taking place and it has not found a way around it.  I omitted the warning because I know that it will be triggered for every x86_64 cygwin binary that gets built, and most, if not all of them, do not care about the value of __image_base__.

  Any suggestions as to how to handle __image_base__ will be greatfully received.

Cheers
  Nick
Comment 3 Corinna Vinschen 2014-04-09 19:06:36 UTC
Hi Nick,

thanks for the patch.

(In reply to Nick Clifton from comment #2)
>   I have uploaded a patch which is a partial fix for the problem.  It
> detects out-of-range absolute values and tries to convert them into section
> relative values.  This works for most cases, but it fails for the
> __image_base__ and __ImageBase__ symbols, and possibly some others that I
> have not yet encountered.  The problem is that these symbols have a value
> which is less than the lowest addressed section, but higher than 1^32. 
> (Note the value of ImageBase in the PE header is not affected by this
> problem.  It is only the *symbols* __image_base__ and __ImageBase__ that are
> affected).

Well, I'm wondering if ld couldn't utilize the fact that executables
are never bigger than 2 Gigs.  I'm not entirely sure, but afaik the
relocation information is signed.  Couldn't __image_base__ be defined
with a negative offset relative to the first section?

>   One thing that patch does not do at the moment is issue an error message
> when it knows that the truncation is taking place and it has not found a way
> around it.  I omitted the warning because I know that it will be triggered
> for every x86_64 cygwin binary that gets built, and most, if not all of
> them, do not care about the value of __image_base__.

That sonds right to me, at least as long as there's no solution for the
__image_base__ problem.


Thanks,
Corinna
Comment 4 Nick Clifton 2014-04-11 13:13:20 UTC
Hi Corinna,

> Well, I'm wondering if ld couldn't utilize the fact that executables
> are never bigger than 2 Gigs.  I'm not entirely sure, but afaik the
> relocation information is signed.  Couldn't __image_base__ be defined
> with a negative offset relative to the first section?

Well, at the moment, __image_base__ is not a relocated value.  It is an absolute symbol.  I suppose that it might be possible to use a base relocation to adjust the value at run-time although that seems like a horrible hack too.

I did try another version of my original patch which created a new section at ImageBase.  That way all absolute values could be converted to section-relative values based upon this section.  Unfortunately that does not work because section addresses are stored in the PE header as offsets from ImageBase, but an offset of 0 is special.  It means that the section address is exacly 0, not ImageBase+0.  You cannot put this new section before ImageBase as the section addresses are all stored as positive offsets from ImageBase.  Plus if you put the new section above ImageBase then you have no way of converting symbols whose value is exacly ImageBase.  (eg __image_base__).

*sigh*

Cheers
  Nick
Comment 5 Corinna Vinschen 2014-04-11 13:34:15 UTC
(In reply to Nick Clifton from comment #4)
> Hi Corinna,
> 
> > Well, I'm wondering if ld couldn't utilize the fact that executables
> > are never bigger than 2 Gigs.  I'm not entirely sure, but afaik the
> > relocation information is signed.  Couldn't __image_base__ be defined
> > with a negative offset relative to the first section?
> 
> Well, at the moment, __image_base__ is not a relocated value.  It is an
> absolute symbol.  I suppose that it might be possible to use a base
> relocation to adjust the value at run-time although that seems like a
> horrible hack too.
> 
> I did try another version of my original patch which created a new section
> at ImageBase.  That way all absolute values could be converted to
> section-relative values based upon this section.  Unfortunately that does
> not work because section addresses are stored in the PE header as offsets
> from ImageBase, but an offset of 0 is special.  It means that the section
> address is exacly 0, not ImageBase+0.  You cannot put this new section
> before ImageBase as the section addresses are all stored as positive offsets
> from ImageBase.  Plus if you put the new section above ImageBase then you
> have no way of converting symbols whose value is exacly ImageBase.  (eg
> __image_base__).
> 
> *sigh*

Yeah, that sounds bad.  Also, you have to be quite careful with the section
layout because the Windows loader is pretty dumb and needs a rather standarized layout, otherwise it refuses to load the executable.  There isn't much wiggle room :(

I'm CCing ktietz, maybe he has some idea how to fix the __image_base__ value.


Corinna
Comment 6 Sourceware Commits 2014-04-11 15:07:40 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "gdb and binutils".

The branch, master has been updated
       via  32ae0d80cd430150ad9536aa160f34f504e129bc (commit)
      from  58a84dcf29b735ee776536b4c51ba90b51612b71 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=32ae0d80cd430150ad9536aa160f34f504e129bc

commit 32ae0d80cd430150ad9536aa160f34f504e129bc
Author: Nick Clifton <nickc@redhat.com>
Date:   Fri Apr 11 16:02:52 2014 +0100

    PE32+ binaries that use addresses > 1^32 have a problem in that the linker
    converts some address expressions into absolute values, but the PE format
    only stores absolutes as 32-bits.  This is a partial solution which attempts
    to convert such absolute values back to section relative ones instead.  It
    fails for symbols like __image_base and ImageBase__, but it is unclear as to
    whether these values are ever actually used by applications.
    
    	PR ld/16821
    	* peXXigen.c (abs_finder): New function.
    	(_bfd_XXi_swap_sym_out): For absolute symbols with values larger
    	than 1^32 try to convert them into section relative values
    	instead.

-----------------------------------------------------------------------

Summary of changes:
 bfd/ChangeLog  |    8 ++++++++
 bfd/peXXigen.c |   31 +++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 0 deletions(-)
Comment 7 Eric Botcazou 2014-04-12 17:28:52 UTC
This doesn't compile on 32-bit hosts:

peigen.c: In function ‘abs_finder’:
peigen.c:215:3: error: left shift count >= width of type [-Werror]
Comment 8 asmwarrior 2014-04-15 01:21:07 UTC
(In reply to Eric Botcazou from comment #7)
> This doesn't compile on 32-bit hosts:
> 
> peigen.c: In function ‘abs_finder’:
> peigen.c:215:3: error: left shift count >= width of type [-Werror]

I got the same build error on MinGW 32-bit hosts, the patch below fixes the build failure.

Yuanhui Zhang


 bfd/peXXigen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c
index 36d90cc..6d80f96 100644
--- a/bfd/peXXigen.c
+++ b/bfd/peXXigen.c
@@ -212,7 +212,7 @@ abs_finder (bfd * abfd ATTRIBUTE_UNUSED, asection * sec, void * data)
 {
   bfd_vma abs_val = * (bfd_vma *) data;
 
-  return (sec->vma <= abs_val) && ((sec->vma + (1L << 32)) > abs_val);
+  return (sec->vma <= abs_val) && ((sec->vma + (1LL << 32)) > abs_val);
 }
 
 unsigned int
Comment 9 Sourceware Commits 2014-04-22 10:02:07 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "gdb and binutils".

The branch, master has been updated
       via  285fc9d8f8ed30b8a9d680fbf37e8f1843b95bc0 (commit)
      from  5d3b02f0036dbf39863fd24414e28f28a53ea1fd (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=285fc9d8f8ed30b8a9d680fbf37e8f1843b95bc0

commit 285fc9d8f8ed30b8a9d680fbf37e8f1843b95bc0
Author: Yuanhui Zhang <asmwarrior@gmail.com>
Date:   Tue Apr 22 11:00:39 2014 +0100

    Fix build problem on 32-bit hosts with the recent patch for PR 16821.
    
    	PR ld/16821
    	* peXXigen.c (abs_finder): Fix for 32-bit host builds.

-----------------------------------------------------------------------

Summary of changes:
 bfd/ChangeLog  |    5 +++++
 bfd/peXXigen.c |   10 +++++++++-
 2 files changed, 14 insertions(+), 1 deletions(-)
Comment 10 Nick Clifton 2014-04-22 10:04:00 UTC
Thanks Yuanhui - patch applied.
Comment 11 Sourceware Commits 2014-04-22 15:59:04 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "gdb and binutils".

The branch, master has been updated
       via  d5f59c10fc37e325d3fbad4ae7970c7cf0857b46 (commit)
      from  73589c9dbddc7906fa6a150f2a2a0ff6b746e8ba (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d5f59c10fc37e325d3fbad4ae7970c7cf0857b46

commit d5f59c10fc37e325d3fbad4ae7970c7cf0857b46
Author: Nick Clifton <nickc@redhat.com>
Date:   Tue Apr 22 16:57:34 2014 +0100

    Another fix for building on a 32-bit host.
    
    	PR ld/16821
    	* peXXigen.c (_bfd_XXi_swap_sym_out): Fix for 32-bit hosts.

-----------------------------------------------------------------------

Summary of changes:
 bfd/ChangeLog  |    5 +++++
 bfd/peXXigen.c |    2 +-
 2 files changed, 6 insertions(+), 1 deletions(-)
Comment 12 Sourceware Commits 2014-04-28 08:35:55 UTC
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "gdb and binutils".

The branch, master has been updated
       via  40af4a3636504a0e7e0223b34ed1e7b15c4fa5da (commit)
      from  e3e163dbb0c50aa94af5416aca86d9ef9c225205 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=40af4a3636504a0e7e0223b34ed1e7b15c4fa5da

commit 40af4a3636504a0e7e0223b34ed1e7b15c4fa5da
Author: Nick Clifton <nickc@redhat.com>
Date:   Mon Apr 28 09:34:02 2014 +0100

    This patch reworks the fix to avoid a compile time warning so that it will work
    with later versions of gcc.
    
    	PR ld/16821
    	* peXXigen.c (_bfd_XXi_swap_sym_out): Rework fix to avoid compile
    	time warning.

-----------------------------------------------------------------------

Summary of changes:
 bfd/ChangeLog  |    6 ++++++
 bfd/peXXigen.c |   15 ++++++---------
 2 files changed, 12 insertions(+), 9 deletions(-)
Comment 13 Alan Modra 2022-08-03 10:53:48 UTC
Looks to be fixed.