Bug 25029

Also, if I rename the .data section to .rdata it all works out fine.

(In reply to Florin Saftoiu from comment #0)
Hi Florin,

> This used to work like a charm with ld 2.30, but produces an invalid file
> with ld 2.32. Specifically, the entry point address seems wrong.

What seems to be wrong with the entry point ?

I tried to reproduce the problem locally, but to me the entry point looks correct:

  % objdump -f test_64.exe
  [...]
  start address 0x0000000100401000

  % objdump -d test_64.exe
  [...]
  Disassembly of section .text:

  0000000100401000 <__rt_psrelocs_end>:
     100401000:	48 83 ec 08          	sub    $0x8,%rsp
     100401004:	48 83 ec 20          	sub    $0x20,%rsp

  0000000100401008 <_main.DisplayMessageBox>:
     100401008:	b9 00 00 00 00       	mov    $0x0,%ecx
  [...]

Please note, I am not a PE format expert, so there could well be something
simple that is wrong and I am just not seeing it.

Cheers
  Nick

Hi Nick,

Thanks for your reply.

You're right about the entry point, it is correct. But Windows still wouldn't run my program so I took another look at the file.

The problem is in fact with the SizeOfImage and SizeOfHeaders fields.
If I call the data section .rdata, then I get a SizeOfImage of 0x4000 and a SizeOfHeaders of 0x200 which is correct.
But if I call the data section .data, then I get a SizeOfImage of 0x3000 and a SizeOfHeaders of 0x400.

Microsoft says (https://docs.microsoft.com/en-us/windows/win32/debug/pe-format?redirectedfrom=MSDN#optional-header-image-only) that SizeOfImage should be "The size (in bytes) of the image, including all headers, as the image is loaded in memory. It must be a multiple of SectionAlignment." and SizeOfHeaders should be "The combined size of an MS-DOS stub, PE header, and section headers rounded up to a multiple of FileAlignment."

It seems that ld considers the empty 512 bytes, from the discarded .rdata section, as part of the header. I've no hypothesis about why the SizeOfImage is messed up.

Hope this helps.

Best regards,
Florin

PS: I actually modified the .exe file to change SizeOfImage to 0x4000 and SizeOfHeaders to 0x200 and I can now execute it in Windows.

Hi Florin,

Just checking...

> The problem is in fact with the SizeOfImage and SizeOfHeaders fields.
> If I call the data section .rdata, then I get a SizeOfImage of 0x4000 and a
> SizeOfHeaders of 0x200 which is correct.
> But if I call the data section .data, then I get a SizeOfImage of 0x3000 and
> a SizeOfHeaders of 0x400.

I assume that you mean that the SizeOfImage is 0x4000 with .data, not 0x3000 ?
The actual size of the executable does not change...

> It seems that ld considers the empty 512 bytes, from the discarded .rdata
> section, as part of the header.

Agreed - and I am uploading a patch which I think will fix the problem.
Please could you give it a try and let me know if it actually works ?

Cheers
  Nick

Created attachment 12038 [details]
Proposed patch

Hi Nick,

SizeOfImage is actually 0x3000 with the section called .data and 0x4000 with the section called .rdata.

Here's the output from objdump.exe -p test_64_data.exe :
...
SizeOfImage             00003000
SizeOfHeaders           00000400
...

test_64_data.exe does NOT work.

Here's the output from objdump.exe -p test_64_rdata.exe :
...
SizeOfImage             00004000
SizeOfHeaders           00000200
...

test_64_rdata.exe does work.

I've manually edited test_64_data.exe and got the following results :
- SizeOfImage 0x3000, SizeOfHeaders 0x200 => does NOT work
- SizeOfImage 0x4000, SizeOfHeaders 0x400 => works
- SizeOfImage 0x4000, SizeOfHeaders 0x200 => works

So it seems that the bigger problem is the SizeOfImage and not the SizeOfHeaders.

Best regards,
Florin

Hi Florin,

(In reply to Florin Saftoiu from comment #7)
> SizeOfImage is actually 0x3000 with the section called .data and 0x4000 with
> the section called .rdata.

> So it seems that the bigger problem is the SizeOfImage and not the
> SizeOfHeaders.

Hmmm, there seems to be a problem here, because I am not seeing a
SizeOfImage value of 0x3000 even in the .data version of test_64:


  % objdump -p test_64.ok.exe test_64.bad.exe | grep SizeOfImage
  SizeOfImage		00004000
  SizeOfImage		00004000

So maybe this is related to the toolchain being used.  How did you
configure the binutils that you are using ?  I have been testing
using a toolchain configured as  --target=x86_64-pc-cygwin.

Cheers
  Nick

Hello,

I'm using GCC 9.2.0 and Binutils 2.32 from MSYS2. No Cygwin installed.

The target seems to be x86_64-w64-mingw32

PS ...>gcc -dumpmachine
x86_64-w64-mingw32

Are your results before or after applying your proposed patch ?

Hi Florin,

> The target seems to be x86_64-w64-mingw32

Darn - I get the same results (ie SizeOfImage == 0x4000 always) with this
configuration too.

> Are your results before or after applying your proposed patch ?

Both - I tried with and without the patch applied.

Maybe it is a host alignment issue.  Are you able to run the linker
under a debugger ?  If so, please could you put a breakpoint on 
_bfd_64i_swap_aouthdr_out() and step through to where the "isize"
variable is computed ?  (Near the end of the function).  It will
probably be computed three times, but it is the last one which
should be generating the 0x4000 value.  If you can find out why
0x3000 is being generated instead that would be really helpful.

Cheers
  Nick

Hi Nick,

Sorry it took so long, I finally got some time, so I compiled the msys2 binutils-2.32 package from source and ran it in GDB. Here's my GDB session :

$ gdb --args /c/work/projects/soft-to-you/osdev/gcc/src/MINGW-packages/mingw-w64-binutils/pkg/mingw-w64-x86_64-binutils/mingw64/bin/ld.exe -o test_64_broken.exe -L/mingw64/x86_64-w64-mingw32/lib test_64_broken.o -lkernel32 -luser32
GNU gdb (GDB) 8.3
Copyright (C) 2019 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-w64-mingw32".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from C:/work/projects/soft-to-you/osdev/gcc/src/MINGW-packages/mingw-w64-binutils/pkg/mingw-w64-x86_64-binutils/mingw64/bin/ld.exe...
(gdb) dir ../gcc/src/MINGW-packages/mingw-w64-binutils/src/build-x86_64-w64-mingw32/bfd
Source directories searched: C:\work\projects\soft-to-you\osdev\src/../gcc/src/MINGW-packages/mingw-w64-binutils/src/build-x86_64-w64-mingw32/bfd;$cdir;$cwd
(gdb) b _bfd_pex64i_swap_aouthdr_out
Breakpoint 1 at 0x471b39: file pex64igen.c, line 631.
(gdb) run
Starting program: C:\work\projects\soft-to-you\osdev\gcc\src\MINGW-packages\mingw-w64-binutils\pkg\mingw-w64-x86_64-binutils\mingw64\bin\ld.exe -o test_64_broken.exe -LC:/msys64/mingw64/x86_64-w64-mingw32/lib test_64_broken.o -lkernel32 -luser32
[New Thread 37688.0x8ab4]
[New Thread 37688.0x74e0]
[New Thread 37688.0xb8a0]

Thread 1 hit Breakpoint 1, _bfd_pex64i_swap_aouthdr_out (abfd=0x5ade490,
    in=0x1c8f130, out=0x5b50920) at pex64igen.c:631
warning: Source file is more recent than executable.
631       struct internal_aouthdr *aouthdr_in = (struct internal_aouthdr *) in;
(gdb) b 739
Breakpoint 2 at 0x471e73: file pex64igen.c, line 739.
(gdb) c
Continuing.

Thread 1 hit Breakpoint 2, _bfd_pex64i_swap_aouthdr_out (abfd=0x5ade490,
    in=0x1c8f130, out=0x5b50920) at pex64igen.c:739
739               isize = (sec->vma - extra->ImageBase
(gdb) print sec->name
$1 = 0x432bfa0 ".text"
(gdb) print isize
$2 = 0
(gdb) c
Continuing.

Thread 1 hit Breakpoint 2, _bfd_pex64i_swap_aouthdr_out (abfd=0x5ade490,
    in=0x1c8f130, out=0x5b50920) at pex64igen.c:739
739               isize = (sec->vma - extra->ImageBase
(gdb) print sec->name
$3 = 0x5ad34b0 ".data"
(gdb) print isize
$4 = 8192
(gdb) c
Continuing.

Thread 1 hit Breakpoint 2, _bfd_pex64i_swap_aouthdr_out (abfd=0x5ade490,
    in=0x1c8f130, out=0x5b50920) at pex64igen.c:739
739               isize = (sec->vma - extra->ImageBase
(gdb) print sec->name
$5 = 0x5ad41e0 ".idata"
(gdb) print isize
$6 = 12288
(gdb) c
Continuing.

Thread 1 hit Breakpoint 2, _bfd_pex64i_swap_aouthdr_out (abfd=0x5ade490,
    in=0x1c8f130, out=0x5b50920) at pex64igen.c:739
739               isize = (sec->vma - extra->ImageBase
(gdb) print sec->name
$7 = 0x5ad3dd0 ".rdata"
(gdb) print isize
$8 = 16384
(gdb) print sec->vma
$9 = 4206592
(gdb) print extra->ImageBase
$10 = 4194304
(gdb) next
740                        + SA (FA (pei_section_data (abfd, sec)->virt_size)));
(gdb) next
739               isize = (sec->vma - extra->ImageBase
(gdb) next
715         for (sec = abfd->sections; sec; sec = sec->next)
(gdb) print isize
$11 = 12288
(gdb) c
Continuing.
[Thread 37688.0xb8a0 exited with code 0]
[Thread 37688.0x74e0 exited with code 0]
[Thread 37688.0x8ab4 exited with code 0]
[Inferior 1 (process 37688) exited normally]

As far as I can tell the problem comes from the .rdata section which appears in the image before the .idata section but is here considered last.

Hope this helps.

Best regards,
Florin

Created attachment 12073 [details]
Proposed patch

Hi Florian,

  You are right - the sections are in a different order for me (.text, .data. .rdata, .idata).  I do not know why this is happening, but it has given me an idea for a potential patch.

  Please could you try out the patch attached to this update - along with the original patch - and see if this solves the problem.  Thanks.

Cheers
  Nick

Hi Nick,

I've tested with both patches applied and the executable is valid and works as expected. SizeOfHeaders is 0x200 and SizeOfImage is 0x4000 as expected. So I guess the patches are OK.

Weird about the order of the sections though, I'm guessing it could cause other problems, since it's the offset of the last section in the ld script that determines the size of the image.

Let me know if I can help further.

Best regards,
Florin

The master branch has been updated by Nick Clifton <nickc@sourceware.org>:

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

commit a23e9ba17f6ab8bef1f2cc02686e8567bdc728ca
Author: Nick Clifton <nickc@redhat.com>
Date:   Thu Dec 5 13:56:07 2019 +0000

    Fix a problem computing the size fields in the PE format header.
    
    	PR 25029
    	* peXXigen.c (_bfd_XXi_swap_aouthdr_out): Ignore empty sections
    	when computing the sizes stored in the headers.

Patch applied.