Bug 10400 - Gold produces incorrect debug and unwind info when incompatible COMDAT sections are present
Summary: Gold produces incorrect debug and unwind info when incompatible COMDAT sectio...
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: gold (show other bugs)
Version: 2.20
: P2 normal
Target Milestone: ---
Assignee: Ian Lance Taylor
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-15 20:38 UTC by Paul Pluzhnikov
Modified: 2009-07-17 01:28 UTC (History)
1 user (show)

See Also:
Host: x86_64-unknown-linux-gnu
Target: x86_64-unknown-linux-gnu
Build: x86_64-unknown-linux-gnu
Last reconfirmed:


Attachments
Patch (5.67 KB, patch)
2009-07-16 05:32 UTC, Ian Lance Taylor
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Pluzhnikov 2009-07-15 20:38:22 UTC
When multiple incompatible COMDAT sections are present, gold selects one,
but doesn't discard .debug_* and .eh_frame bits which refer to the other ones.

This then shows up as glitches in debugging the resulting executable under GDB.

Test:

--- cut --- test.h ---
#include <stdio.h>

inline int foo() {
  return 3;
}

extern char buf[4];
inline void HelloWorld() {
  // just some code to mess with stack layout
  printf("memcmp(): %d\n", __builtin_memcmp(buf, "abc", sizeof(buf)));
  printf("Hello %d\n", foo());
}
--- cut --- test.h ---

--- cut --- test1.cc ---
#include "test.h"

void Trampoline(void (*fn)() = HelloWorld);

char buf[4] = "abc";
int main() {
  HelloWorld();
  Trampoline();
  return 0;
}
--- cut --- test1.cc ---

--- cut --- test2.cc ---
#include "test.h"

void Trampoline(void (*fn)() = HelloWorld) {
  fn();
}

void NotCalled() {
  Trampoline();
}
--- cut --- test2.cc ---


./ld --version
GNU gold (GNU Binutils 2.19.51.20090714) 1.9
...

g++ -g -c test1.cc && g++ -g -c test2.cc -O2 && 
g++ -g test1.o test2.o -B. -o test-gold

First the unwind info:

nm test-gold | grep HelloWorld
00000000004002fe W _Z10HelloWorldv

readelf -wf test-gold    # contains inconsistent unwind descriptors:

# from test1.o, corresponds to code actually used
000000a8 00000024 0000004c FDE cie=00000060 pc=004002fe..0040033f
  DW_CFA_advance_loc4: 1 to 004002ff
  DW_CFA_def_cfa_offset: 16
  DW_CFA_offset: r6 at cfa-16
  DW_CFA_advance_loc4: 3 to 00400302
  DW_CFA_def_cfa_reg: r6
  DW_CFA_nop

# from test2.o, corresponds to discarded code
000000d8 0000001c 00000090 FDE cie=00000090 pc=004002fe..00400343
  DW_CFA_advance_loc4: 5 to 00400303
  DW_CFA_def_cfa_offset: 16
  DW_CFA_nop


This shows up as a glitch in GDB:

(gdb) b *&HelloWorld
Breakpoint 1 at 0x4002fe: file test.h, line 10.
(gdb) r

Breakpoint 1, HelloWorld () at test.h:10
10        printf("memcmp(): %d\n", __builtin_memcmp(buf, "abc", sizeof(buf)));
(gdb) bt
#0  HelloWorld () at test.h:10
#1  0x00000000004002e1 in main () at test1.cc:7
(gdb) si
HelloWorld () at test.h:8
8       inline void HelloWorld() {
(gdb) bt
#0  HelloWorld () at test.h:8
#1  0x00007fffffffdaa0 in ?? ()
#2  0x00000000004002e1 in main () at test1.cc:7
(gdb) si
0x0000000000400302      8       inline void HelloWorld() {
(gdb) bt
#0  0x0000000000400302 in HelloWorld () at test.h:8
#1  0x00007fffffffdaa0 in ?? ()
#2  0x00000000004002e1 in main () at test1.cc:7
(gdb) si
0x0000000000400307 in HelloWorld () at test.h:10
10        printf("memcmp(): %d\n", __builtin_memcmp(buf, "abc", sizeof(buf)));
(gdb) bt
#0  0x0000000000400307 in HelloWorld () at test.h:10
#1  0x00000000004002e1 in main () at test1.cc:7


Second, debug info differences:
readelf -w 

 <1><223>: Abbrev Number: 7 (DW_TAG_subprogram)
  <224>     DW_AT_external    : 1       
  <225>     DW_AT_name        : HelloWorld      
  <230>     DW_AT_decl_file   : 1       
  <231>     DW_AT_decl_line   : 8       
  <232>     DW_AT_MIPS_linkage_name: _Z10HelloWorldv    
  <242>     DW_AT_low_pc      : 0x4002fe        
  <24a>     DW_AT_high_pc     : 0x40033f        
  <252>     DW_AT_frame_base  : 0x4c    (location list)
...

 <1><36a>: Abbrev Number: 13 (DW_TAG_subprogram)
  <36b>     DW_AT_external    : 1       
  <36c>     DW_AT_name        : (indirect string, offset: 0x28): HelloWorld     
  <370>     DW_AT_decl_file   : 2       
  <371>     DW_AT_decl_line   : 8       
  <372>     DW_AT_MIPS_linkage_name: (indirect string, offset: 0xd5):
_Z10HelloWorldv   
  <376>     DW_AT_low_pc      : 0x4002fe        
  <37e>     DW_AT_high_pc     : 0x400343        
  <386>     DW_AT_frame_base  : 0x107   (location list)


Is HelloWorld spanning [0x4002fe,0x40033f] or [0x4002fe,0x400343] range?

Finally, GNU ld doesn't show the same problem: it zeros out debug* and
eh_frame entries corresponding to discarded sections:

000000d8 0000001c 00000090 FDE cie=00000090 pc=00000000..00000045
  DW_CFA_advance_loc4: 5 to 00000005
  DW_CFA_def_cfa_offset: 16
  DW_CFA_nop

And the debug info like this:

 <1><35d>: Abbrev Number: 12 (DW_TAG_subprogram)
  <35e>     DW_AT_external    : 1       
  <35f>     DW_AT_name        : foo     
  <363>     DW_AT_decl_file   : 2       
  <364>     DW_AT_decl_line   : 3       
  <365>     DW_AT_type        : <2dd>   
  <369>     DW_AT_inline      : 3       (declared as inline and inlined)
 <1><36a>: Abbrev Number: 13 (DW_TAG_subprogram)
  <36b>     DW_AT_external    : 1       
  <36c>     DW_AT_name        : (indirect string, offset: 0xb1): HelloWorld     
  <370>     DW_AT_decl_file   : 2       
  <371>     DW_AT_decl_line   : 8       
  <372>     DW_AT_MIPS_linkage_name: (indirect string, offset: 0x11d):
_Z10HelloWorldv  
  <376>     DW_AT_low_pc      : 0       
  <37e>     DW_AT_high_pc     : 0       
  <386>     DW_AT_frame_base  : 0x107   (location list)
Comment 1 Ian Lance Taylor 2009-07-16 00:08:25 UTC
I don't see any problem in the .eh_frame section.  It appears to me that
.eh_frame information for discarded sections is indeed being discarded.

I do see duplication in the .debug_frame section.  Note that readelf -wf dumps
both the .eh_frame section and the .debug_frame section.

Let me know if you do see a problem in the .eh_frame section.
Comment 2 Paul Pluzhnikov 2009-07-16 00:12:13 UTC
Sorry, my mistake.
Indeed it's the .debug_frame that is a problem (GDB uses both).
Comment 3 Ian Lance Taylor 2009-07-16 05:32:09 UTC
Created attachment 4055 [details]
Patch

The difference between gold and GNU ld seems to be that when GNU ld sees a
comdat section with a different size, it ignores it when doing relocation
processing.  This patch implements that in gold, along with some cleanups.  It
appears to fix the test case in the PR.  I haven't committed it yet--let me
know if you have a chance to try it on some other test cases.
Comment 4 Paul Pluzhnikov 2009-07-16 16:19:35 UTC
(In reply to comment #3)
> let me
> know if you have a chance to try it on some other test cases.

Thanks for the prompt fix.

I've linked two of my real executables (which both exhibited GDB glitches)
with patched gold, and the problem is fixed there as well :-)
Comment 5 Sourceware Commits 2009-07-17 01:07:45 UTC
Subject: Bug 10400

CVSROOT:	/cvs/src
Module name:	src
Changes by:	ian@sourceware.org	2009-07-17 01:07:34

Modified files:
	gold           : ChangeLog layout.cc layout.h object.cc object.h 
	                 plugin.cc 

Log message:
	PR 10400
	* layout.h: #include <map>.
	(class Kept_section): Change from struct to class.  Add accessors
	and setters.  Add section size to Comdat_group mapping.  Change
	Comdat_group to std::map.  Add is_comdat_ field.  Add
	linkonce_size field in union.
	(class Layout): Update declaration of find_or_add_kept_section.
	Don't declare find_kept_object.
	* layout.cc (Layout::find_or_add_kept_section): Remove candidate
	parameter.  Add object, shndx, is_comdat, and is_group_name
	parameters.  Change all callers.  Adjust for new Kept_section.
	(Layout::find_kept_object): Remove.
	* object.cc (Sized_relobj::include_section_group): Update use of
	Kept_section.  Rename secnum to shndx.  Only record
	Kept_comdat_section if sections are the same size.
	(Sized_relobj::include_linkonce_section): Update use of
	Kept_section.  Only record Kept_comdat_section if sections are the
	same size.  Set size of linkonce section.
	(Sized_relobj::map_to_kept_section): Update call to
	get_kept_comdat_section.
	* object.h (class Sized_relobj): Rename fields in
	Kept_comdat_section to drop trailing underscores; change object
	field to Relobj*.  Change Kept_comdat_section_table to store
	struct rather than pointer.
	(Sized_relobj::set_kept_comdat_section): Remove kept parameter.
	Add kept_object and kept_shndx parameters.  Change all callers.
	(Sized_relobj::get_kept_comdat_section): Change return type to
	bool.  Add kept_object and kept_shndx parameters.  Change all
	callers.
	* plugin.cc (Pluginobj::include_comdat_group): Update call to
	Layout::find_or_add_kept_section.

Patches:
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gold/ChangeLog.diff?cvsroot=src&r1=1.257&r2=1.258
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gold/layout.cc.diff?cvsroot=src&r1=1.131&r2=1.132
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gold/layout.h.diff?cvsroot=src&r1=1.67&r2=1.68
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gold/object.cc.diff?cvsroot=src&r1=1.95&r2=1.96
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gold/object.h.diff?cvsroot=src&r1=1.75&r2=1.76
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/gold/plugin.cc.diff?cvsroot=src&r1=1.15&r2=1.16

Comment 6 Ian Lance Taylor 2009-07-17 01:28:39 UTC
Patch committed.