Bug 14265 - -gc-sections ignores KEEP annotations
Summary: -gc-sections ignores KEEP annotations
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: gold (show other bugs)
Version: 2.22
: P2 normal
Target Milestone: ---
Assignee: Nick Clifton
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-19 08:30 UTC by Enrico Scholz
Modified: 2012-08-14 08:33 UTC (History)
4 users (show)

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


Attachments
Proposed fix (2.15 KB, patch)
2012-08-10 14:57 UTC, Nick Clifton
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Enrico Scholz 2012-06-19 08:30:57 UTC
sections which are marked with KEEP() in a linker file will be removed
when -gc-sections is specified.  Old bfd linker kept them as expected.

In the sample below, it is expected that:

* __foo0_start == __foo0_end   (can be optimized way)

* __foo1_start < __foo1_end

* __foo2_end - __foo2_start == __foo1_end - __foo1_start


The last two expectations hold with bfd linker but not with gold.



--- x2.ld ---
SECTIONS
{
	.text : { *(.text) }

	__foo0_start = .;
	.foo0 : { *(.foo0.*) }
	__foo0_end = .;

	__foo1_start = .;
	.foo1 : { KEEP(*(.foo1.*)) }
	__foo1_end = .;

	.foo2 : {
		__foo2_start = .;
		KEEP(*(.foo2.*))
		__foo2_end = .;
	}
}


--- x2.c ---
int foo0 __attribute__((used,section(".foo0.0")));
int foo1 __attribute__((used,section(".foo1.0")));
int foo2 __attribute__((used,section(".foo2.0")));

extern unsigned long	__foo0_start;
extern unsigned long	__foo0_end;

extern unsigned long	__foo1_start;
extern unsigned long	__foo1_end;

extern unsigned long	__foo2_start;
extern unsigned long	__foo2_end;

int main() {
	return ((__foo0_end - __foo0_start) -
		(__foo1_end - __foo1_start) -
		(__foo2_end - __foo2_start));
}


-----

$ gcc -c x2.c
$ ld.gold -gc-sections -T x2.ld x2.o -o /tmp/x2-gold
$ ld.bfd  -gc-sections -T x2.ld x2.o -o /tmp/x2-bfd

$ readelf -s /tmp/x2-bfd /tmp/x2-gold

File: /tmp/x2-bfd

Symbol table '.symtab' contains 13 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND 
     1: 0000000000000000     0 SECTION LOCAL  DEFAULT    1 
     2: 0000000000000004     0 SECTION LOCAL  DEFAULT    2 
     3: 0000000000000000     0 SECTION LOCAL  DEFAULT    3 
     4: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS x2.c
     5: 0000000000000004     0 NOTYPE  LOCAL  DEFAULT    2 __foo2_start
     6: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  ABS __foo1_start
     7: 0000000000000008     0 NOTYPE  LOCAL  DEFAULT    2 __foo2_end
     8: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  ABS __foo0_start
     9: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  ABS __foo0_end
    10: 0000000000000004     0 NOTYPE  LOCAL  DEFAULT  ABS __foo1_end
    11: 0000000000000004     4 OBJECT  GLOBAL DEFAULT    2 foo2
    12: 0000000000000000     4 OBJECT  GLOBAL DEFAULT    1 foo1

File: /tmp/x2-gold

Symbol table '.symtab' contains 9 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND 
     1: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS x2.c
     2: 0000000000000000    76 FUNC    GLOBAL DEFAULT    1 main
     3: 0000000000000090     0 NOTYPE  GLOBAL DEFAULT  ABS __foo0_end
     4: 0000000000000090     0 NOTYPE  GLOBAL DEFAULT  ABS __foo0_start
     5: 0000000000000090     0 NOTYPE  GLOBAL DEFAULT  ABS __foo1_start
     6: 0000000000000090     0 NOTYPE  GLOBAL DEFAULT  ABS __foo1_end
     7: 0000000000000090     0 NOTYPE  GLOBAL DEFAULT  ABS __foo2_start
     8: 00000000000000dc     0 NOTYPE  GLOBAL DEFAULT  ABS __foo2_end


$ ld.gold --version
GNU gold (version 2.22.52.0.1-10.fc17 20120131) 1.11

$ ld.bfd --version
GNU ld version 2.22.52.0.1-10.fc17 20120131
Comment 1 Nick Clifton 2012-08-09 07:39:49 UTC
Hi Ian,

  I have been looking at this PR, trying to find a way to solve it, but I am hopelessly lost.  So I am asking for some guidance...

  It seems to me that gold parses the KEEP directives and stores them in Output_section_element_input class, but then it just ignores them.  Then when Sized_relobj_file runs garbage collection there appears to be no way to connect the Output_section pointer to the input sections that contributed to it.  So I guess my question is - when garbage collection occurs, how do I find out if a particular input section should be KEPT ?

Cheers
  Nick
Comment 2 Ian Lance Taylor 2012-08-09 15:00:36 UTC
The key point connecting the Output_section_element_input and the Layout code is the call to Output_section_element_input::match_name.  From the Layout side, it's the call to ss->output_section_name in Layout::chose_output_section.  So I would suggest having Output_section_element_input::match_name return whether the section is kept (e.g., in a new bool* parameter).  Then the Layout code can pass that information back from Layout::layout, and Sized_relobj_file::do_layout can add the input section to the worklist.

Thanks for looking at this.
Comment 3 Nick Clifton 2012-08-10 14:57:53 UTC
Created attachment 6571 [details]
Proposed fix
Comment 4 Nick Clifton 2012-08-10 15:01:07 UTC
Hi Ian,

  Please could you look at the proposed patch I have just uploaded and let me know if you are happy with it, or if it needs more work.  (I admit that I have not tested the patch extensively yet, just with the test case attached to the PR).

  I am going to create a linker test case as well, once the patch is OK.

Cheers
  Nick
Comment 5 Ian Lance Taylor 2012-08-11 03:51:52 UTC
Patch looks good, thanks.
Comment 6 cvs-commit@gcc.gnu.org 2012-08-14 08:32:00 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	nickc@sourceware.org	2012-08-14 08:31:57

Modified files:
	ld/testsuite   : ChangeLog 
	ld/testsuite/ld-gc: gc.exp 
	gold           : ChangeLog layout.cc layout.h object.cc 
	                 script-sections.cc script-sections.h 
	gold/testsuite : Makefile.am Makefile.in 
Added files:
	ld/testsuite/ld-gc: pr14265.c pr14265.d pr14265.t 
	gold/testsuite : pr14265.c pr14265.sh pr14265.t 

Log message:
	PR ld/14265
	* script-sections.cc (Sections_element::output_section_name): Add
	keep return parameter.
	(Output_section_element::match_name): Add keep return parameter.
	Return the value of the keep_ member.
	* script-sections.h (class Output_section): Update
	output_section_name prototype.
	* layout.cc (Layout::keep_input_section): New public member
	function.
	(Layout::choose_output_section): Pass keep parameter to
	output_section_name.
	* layout.h (class Layout): Add keep_input_section.
	* object.cc (Sized_relobj_file::do_layout): Check for kept input
	sections.
	* testsuite/Makefile.am: Add a test.
	* testsuite/Makefile.in: Regenerate.
	* testsuite/pr14265.c: Source file for the test.
	* testsuite/pr14265.t: Linker script for the test.
	* testsuite/pr14265.sh: Shell script for the test.
	
	* ld-gc/gc.exp: Add a new test.
	* ld-gc/pr14265.c: Source file for the new test.
	* ld-gc/pr14265.t: Linker script for the new test.
	* ld-gc/pr14265.d: Expected symbol dump.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ChangeLog.diff?cvsroot=src&r1=1.1593&r2=1.1594
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-gc/pr14265.c.diff?cvsroot=src&r1=NONE&r2=1.1
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-gc/pr14265.d.diff?cvsroot=src&r1=NONE&r2=1.1
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-gc/pr14265.t.diff?cvsroot=src&r1=NONE&r2=1.1
http://sourceware.org/cgi-bin/cvsweb.cgi/src/ld/testsuite/ld-gc/gc.exp.diff?cvsroot=src&r1=1.17&r2=1.18
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gold/ChangeLog.diff?cvsroot=src&r1=1.936&r2=1.937
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gold/layout.cc.diff?cvsroot=src&r1=1.233&r2=1.234
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gold/layout.h.diff?cvsroot=src&r1=1.103&r2=1.104
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gold/object.cc.diff?cvsroot=src&r1=1.156&r2=1.157
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gold/script-sections.cc.diff?cvsroot=src&r1=1.55&r2=1.56
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gold/script-sections.h.diff?cvsroot=src&r1=1.15&r2=1.16
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gold/testsuite/pr14265.c.diff?cvsroot=src&r1=NONE&r2=1.1
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gold/testsuite/pr14265.sh.diff?cvsroot=src&r1=NONE&r2=1.1
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gold/testsuite/pr14265.t.diff?cvsroot=src&r1=NONE&r2=1.1
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gold/testsuite/Makefile.am.diff?cvsroot=src&r1=1.194&r2=1.195
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gold/testsuite/Makefile.in.diff?cvsroot=src&r1=1.204&r2=1.205
Comment 7 Nick Clifton 2012-08-14 08:33:09 UTC
Patch and testsuite addition checked in.