Bug 13909 - PR ld/12570 causes eh_frame_hdr section to be sometimes too large
Summary: PR ld/12570 causes eh_frame_hdr section to be sometimes too large
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.22
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-03-27 21:47 UTC by Christoph Schulz
Modified: 2012-05-25 01:14 UTC (History)
2 users (show)

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


Attachments
corrects size of .eh_frame_hdr section when there are no entries under certain circumstances (262 bytes, patch)
2012-03-27 21:47 UTC, Christoph Schulz
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Christoph Schulz 2012-03-27 21:47:54 UTC
Created attachment 6304 [details]
corrects size of .eh_frame_hdr section when there are no entries under certain circumstances

Hello,

PR ld/12570 added unwind information for PLT. However, there is a small problem when the automatically generated .eh_frame is the only one: There is no binary_search table generated in the .eh_frame_hdr section, at least when targetting elf32-i386. Check the following example

kristov@peacock ~/tmp/eh $ gcc -o test.so -shared -lc test.c -nostartfiles -nodefaultlibs

with test.c being empty (and binutils-2.22 or HEAD). The result is:

kristov@peacock ~/tmp/eh $ objdump -s -j .eh_frame_hdr -j .eh_frame test.so 

test.so:     file format elf32-i386

Contents of section .eh_frame_hdr:
 015c 011bffff 08000000 00000000           ............    
Contents of section .eh_frame:
 0168 14000000 00000000 017a5200 017c0801  .........zR..|..
 0178 1b0c0404 88010000 24000000 1c000000  ........$.......
 0188 00000000 00000000 000e0846 0e0c4a0f  ...........F..J.
 0198 0b740478 003f1a3b 2a322422 00000000  .t.x.?.;*2$"....

We have a eh_frame_hdr with no binary_search table. Additionally, the section is too long (8 bytes would suffice); the last four bytes in the section are essentially undefined and can differ from build to build.

I tried to analyse this but was not able to completely understand the code. What I found out is the following:

BEFORE ALLOCATION: bfd_elf_size_dynamic_sections(elflink.c)
===========================================================
- When _bfd_elf_maybe_strip_eh_frame_hdr(elf-eh-frame.c) gets called, it finds that there is a .eh_frame section in libc.so with a CIE/FDE > 8 bytes and sets hdr_info->table to TRUE.

AFTER ALLOCATION: bfd_elf_discard_info(elflink.c)
=================================================
- The autogenerated section (created by elf_i386_create_dynamic_sections(elf32-i386.c)) is neither parsed by _bfd_elf_parse_eh_frame(elf-eh-frame.c) (called by bfd_elf_discard_info(elflink.c)), as it is marked as DYNAMIC (belonging to dynamic libc.so). Thus, sec_info_type is not set to ELF_INFO_TYPE_EH_FRAME. Furthermore, _bfd_elf_discard_section_eh_frame(elf-eh-frame.c) is never called on this section, such that hdr_info->fde_count remains zero. Even if it was called, it would return immediately because sec_info_type != ELF_INFO_TYPE_EH_FRAME.

- _bfd_elf_discard_section_eh_frame_hdr(elf-eh.frame.c) is always called at the end. Because hdr_info->table == TRUE, the size is computed to EH_FRAME_HDR_SIZE + 4 + hdr_info->fde_count == 12, as hdr_info->fde_count == 0.

FINAL LINK
==========
- As the autogenerated section has not been built by bfd_elf_write_section_eh_frame(elf-eh-frame.c), hdr_info->array remains empty (and hdr_info->array_count remains zero).

- Finally, when _bfd_elf_write_section_eh_frame_hdr(elf-eh-frame.c) is called by bld_elf_final_link(elflink.c), we have hdr_info->table == TRUE, hdr_info->array == NULL, hdr_info->array_count == 0 and hdr_info->fde_count == 0. This results in a section of size EH_FRAME_HDR_SIZE (8 bytes) with contents[2] == contents[3] == DW_EH_PE_omit ALTHOUGH the section size previously set by _bfd_elf_discard_section_eh_frame_hdr is four bytes larger. That means that the last four bytes are possibly random, resulting in irreproducible binaries.

WORKAROUND
==========
I "solved" this problem by the patch attached. Basically, in _bfd_elf_discard_section_eh_frame_hdr(elf-eh.frame.c), I changed the line

if (hdr_info->table)

to

if (hdr_info->table && hdr_info->fde_count > 0)

in order to account for the situation described above. This does not put a binary_search table into the .eh_frame_hdr section (which is not needed, I think, because there is only one CIE/FDE in .eh_frame anyway), but it does reduce the section size and avoids random bytes in the section. The patched linker produces the following result

kristov@peacock ~/tmp/eh $ objdump -s -j .eh_frame_hdr -j .eh_frame test.so 
test.so:     file format elf32-i386

Contents of section .eh_frame_hdr:
 015c 011bffff 04000000                    ........        
Contents of section .eh_frame:
 0164 14000000 00000000 017a5200 017c0801  .........zR..|..
 0174 1b0c0404 88010000 24000000 1c000000  ........$.......
 0184 00000000 00000000 000e0846 0e0c4a0f  ...........F..J.
 0194 0b740478 003f1a3b 2a322422 00000000  .t.x.?.;*2$"....

which is IMHO better.


Regards,

Christoph Schulz
Comment 1 H.J. Lu 2012-03-29 16:36:10 UTC
Please provide test.c.
Comment 2 Christoph Schulz 2012-03-30 05:46:16 UTC
(In reply to comment #1)
> Please provide test.c.

As stated in my comment, I used an *empty* test.c:

$ > test.c
$ gcc -o test.so -shared -lc test.c -nostartfiles -nodefaultlibs

Regards,

Christoph Schulz
Comment 3 H.J. Lu 2012-05-13 21:56:52 UTC
Please try

http://sourceware.org/ml/binutils/2012-05/msg00173.html

plus:

diff --git a/bfd/elf32-i386.c b/bfd/elf32-i386.c
index 31b3c57..49387e7 100644
--- a/bfd/elf32-i386.c
+++ b/bfd/elf32-i386.c
@@ -1016,7 +1016,8 @@ elf_i386_create_dynamic_sections (bfd *dynobj, struct bfd_link_info *info)
 
   if (!info->no_ld_generated_unwind_info
       && htab->plt_eh_frame == NULL
-      && htab->elf.splt != NULL)
+      && htab->elf.splt != NULL
+      && bfd_get_section_by_name (dynobj, ".eh_frame") != NULL)
     {
       flagword flags = get_elf_backend_data (dynobj)->dynamic_sec_flags;
       htab->plt_eh_frame
diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
index aafe60d..399cf8c 100644
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -980,7 +980,8 @@ elf_x86_64_create_dynamic_sections (bfd *dynobj,
 
   if (!info->no_ld_generated_unwind_info
       && htab->plt_eh_frame == NULL
-      && htab->elf.splt != NULL)
+      && htab->elf.splt != NULL
+      && bfd_get_section_by_name (dynobj, ".eh_frame") != NULL)
     {
       const struct elf_x86_64_backend_data *const abed
 	= get_elf_x86_64_backend_data (dynobj);
Comment 4 Christoph Schulz 2012-05-14 06:07:46 UTC
(In reply to comment #3)
> Please try
> 
> http://sourceware.org/ml/binutils/2012-05/msg00173.html
> 
> plus:
> 
> diff --git a/bfd/elf32-i386.c b/bfd/elf32-i386.c
> [...]

I had to adapt the patch in msg00173.html slightly as it did not cleanly apply to binutils 2.22 (I had to remove the "const struct elf_x86_64_backend_data *const abed = get_elf_x86_64_backend_data (dynobj);" part in bfd/elf64-x86-64.c). After this change and after applying the patches to the 2.22 sources, the linker produces neither an .eh_frame_hdr nor an .eh_frame section, which seems to be sensible. So both patches solve the problem as far as I can tell.


Thank you,

Christoph Schulz
Comment 5 H.J. Lu 2012-05-14 13:57:18 UTC
Fixes aren't in trunk yet.
Comment 6 cvs-commit@gcc.gnu.org 2012-05-22 14:46:55 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	hjl@sourceware.org	2012-05-22 14:46:45

Modified files:
	bfd            : ChangeLog elf32-i386.c elf64-x86-64.c 
	ld/testsuite   : ChangeLog 
	ld/testsuite/ld-i386: i386.exp 
	ld/testsuite/ld-x86-64: x86-64.exp 
Added files:
	ld/testsuite/ld-i386: dummy.s pr13909.d 
	ld/testsuite/ld-x86-64: pr13909.d 

Log message:
	Create PLT eh_frame section if there is .eh_frame section
	
	bfd/
	
	PR ld/13909
	* elf32-i386.c (elf_i386_create_dynamic_sections): Create PLT
	eh_frame section if there is an input .eh_frame section.
	* elf64-x86-64.c (elf_x86_64_create_dynamic_sections): Likewise.
	
	ld/testsuite/
	
	PR ld/13909
	* ld-i386/i386.exp: Run pr13909.
	* ld-x86-64/x86-64.exp: Likewise.
	
	* ld-i386/dummy.s: New file.
	* ld-i386/pr13909.d: Likewise.
	* ld-x86-64/pr13909.d: Likewise.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/ChangeLog.diff?cvsroot=src&r1=1.5706&r2=1.5707
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf32-i386.c.diff?cvsroot=src&r1=1.274&r2=1.275
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf64-x86-64.c.diff?cvsroot=src&r1=1.264&r2=1.265
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ChangeLog.diff?cvsroot=src&r1=1.1543&r2=1.1544
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-i386/dummy.s.diff?cvsroot=src&r1=NONE&r2=1.1
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-i386/pr13909.d.diff?cvsroot=src&r1=NONE&r2=1.1
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-i386/i386.exp.diff?cvsroot=src&r1=1.44&r2=1.45
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-x86-64/pr13909.d.diff?cvsroot=src&r1=NONE&r2=1.1
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-x86-64/x86-64.exp.diff?cvsroot=src&r1=1.44&r2=1.45
Comment 7 cvs-commit@gcc.gnu.org 2012-05-22 15:55:13 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	hjl@sourceware.org	2012-05-22 15:55:02

Modified files:
	bfd            : ChangeLog elf32-i386.c elf64-x86-64.c 
	ld/testsuite   : ChangeLog 
	ld/testsuite/ld-i386: i386.exp 
	ld/testsuite/ld-x86-64: x86-64.exp 
Removed files:
	ld/testsuite/ld-i386: dummy.s pr13909.d 
	ld/testsuite/ld-x86-64: pr13909.d 

Log message:
	Revert the change for PR ld/r13909
	
	bfd/
	
	PR ld/13909
	* elf32-i386.c (elf_i386_create_dynamic_sections): Revert the
	last change.
	* elf64-x86-64.c (elf_x86_64_create_dynamic_sections): Likewise.
	
	ld/testsuite/
	
	2012-05-22  H.J. Lu  <hongjiu.lu@intel.com>
	
	PR ld/13909
	* ld-i386/i386.exp: Revert the last change.
	* ld-x86-64/x86-64.exp: Likewise.
	
	* ld-i386/dummy.s: Removed.
	* ld-i386/pr13909.d: Likewise.
	* ld-x86-64/pr13909.d: Likewise.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/ChangeLog.diff?cvsroot=src&r1=1.5708&r2=1.5709
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf32-i386.c.diff?cvsroot=src&r1=1.275&r2=1.276
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf64-x86-64.c.diff?cvsroot=src&r1=1.265&r2=1.266
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ChangeLog.diff?cvsroot=src&r1=1.1544&r2=1.1545
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-i386/i386.exp.diff?cvsroot=src&r1=1.45&r2=1.46
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-i386/dummy.s.diff?cvsroot=src&r1=1.1&r2=NONE
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-i386/pr13909.d.diff?cvsroot=src&r1=1.1&r2=NONE
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-x86-64/x86-64.exp.diff?cvsroot=src&r1=1.45&r2=1.46
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-x86-64/pr13909.d.diff?cvsroot=src&r1=1.1&r2=NONE
Comment 8 cvs-commit@gcc.gnu.org 2012-05-23 04:35:37 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	amodra@sourceware.org	2012-05-23 04:35:31

Modified files:
	bfd            : ChangeLog elflink.c 

Log message:
	PR ld/13909
	* elflink.c (bfd_elf_discard_info): Don't ignore dynamic BFDs.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/ChangeLog.diff?cvsroot=src&r1=1.5709&r2=1.5710
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elflink.c.diff?cvsroot=src&r1=1.443&r2=1.444
Comment 9 Alan Modra 2012-05-23 04:39:18 UTC
Fixed
Comment 10 cvs-commit@gcc.gnu.org 2012-05-25 01:12:26 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	amodra@sourceware.org	2012-05-25 01:12:20

Modified files:
	bfd            : ChangeLog elf-eh-frame.c elf-bfd.h elflink.c 
	                 elf32-ppc.c elf64-ppc.c elf32-i386.c 
	                 elf64-x86-64.c 

Log message:
	PR ld/13909
	* elf-eh-frame.c (_bfd_elf_eh_frame_present): New function.
	(_bfd_elf_maybe_strip_eh_frame_hdr): Use it here.
	* elf-bfd.h (_bfd_elf_eh_frame_present): Declare.
	* elflink.c (bfd_elf_size_dynamic_sections): Let the backend
	size dynamic sections before stripping eh_frame_hdr.
	(bfd_elf_gc_sections): Handle multiple .eh_frame sections.
	* elf32-ppc.c (ppc_elf_size_dynamic_sections): Drop glink_eh_frame
	if no other .eh_frame sections exist.
	* elf64-ppc.c (ppc64_elf_size_stubs): Likewise.
	* elf32-i386.c (elf_i386_create_dynamic_sections): Don't size
	or alloc plt_eh_frame here..
	(elf_i386_size_dynamic_sections): ..do it here instead.  Don't
	specially keep sgotplt, iplt, tgotplt, sdynbss for symbols.
	(elf_i386_finish_dynamic_sections): Check plt_eh_frame->contents
	before writing plt offset.
	* elf64-x86-64.c (elf_x86_64_create_dynamic_sections): Don't size
	or alloc plt_eh_frame here..
	(elf_x86_64_size_dynamic_sections): ..do it here instead.
	(elf_x86_64_finish_dynamic_sections): Check plt_eh_frame->contents
	before writing plt offset.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/ChangeLog.diff?cvsroot=src&r1=1.5712&r2=1.5713
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf-eh-frame.c.diff?cvsroot=src&r1=1.89&r2=1.90
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf-bfd.h.diff?cvsroot=src&r1=1.338&r2=1.339
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elflink.c.diff?cvsroot=src&r1=1.444&r2=1.445
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf32-ppc.c.diff?cvsroot=src&r1=1.316&r2=1.317
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf64-ppc.c.diff?cvsroot=src&r1=1.384&r2=1.385
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf32-i386.c.diff?cvsroot=src&r1=1.276&r2=1.277
http://sourceware.org/cgi-bin/cvsweb.cgi/src/bfd/elf64-x86-64.c.diff?cvsroot=src&r1=1.266&r2=1.267
Comment 11 cvs-commit@gcc.gnu.org 2012-05-25 01:14:02 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	amodra@sourceware.org	2012-05-25 01:13:59

Modified files:
	ld             : ChangeLog 
	ld/emultempl   : elf32.em 

Log message:
	PR ld/13909
	* emultempl/elf32.em (gld${EMULATION_NAME}_after_open): Handle
	multiple .eh_frame sections attached to bfd.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/ChangeLog.diff?cvsroot=src&r1=1.2446&r2=1.2447
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/emultempl/elf32.em.diff?cvsroot=src&r1=1.227&r2=1.228