public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* PATCH: PR ld/12851: --gc-sections doesn't work on note sections
@ 2011-06-08 20:10 H.J. Lu
  2011-06-08 23:52 ` Alan Modra
  0 siblings, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2011-06-08 20:10 UTC (permalink / raw)
  To: binutils

Hi,

SHT_NOTE sections are special.  We check it in elf_gc_sweep.  But
we missed it in bfd_elf_gc_sections.  This patch fixes it. OK for
trunk?

Thanks.


H.J.
---
bfd/

2011-06-08  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12851
	* elflink.c (bfd_elf_gc_sections): Also check SHT_NOTE sections.

ld/testsuite/

2011-06-08  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12851
	* ld-elf/pr12851.d: New.
	* ld-elf/pr12851.s: Likewise.

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 562217d..958b9fb 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -12013,8 +12013,12 @@ bfd_elf_gc_sections (bfd *abfd, struct bfd_link_info *info)
       if (bfd_get_flavour (sub) != bfd_target_elf_flavour)
 	continue;
 
+      /* Also keep SHT_NOTE sections.  */
       for (o = sub->sections; o != NULL; o = o->next)
-	if ((o->flags & (SEC_EXCLUDE | SEC_KEEP)) == SEC_KEEP && !o->gc_mark)
+	if ((o->flags & SEC_EXCLUDE) == 0
+	    && ((o->flags & SEC_KEEP) != 0
+		|| elf_section_data (o)->this_hdr.sh_type == SHT_NOTE)
+	    && !o->gc_mark)
 	  if (!_bfd_elf_gc_mark (info, o, gc_mark_hook))
 	    return FALSE;
     }
diff --git a/ld/testsuite/ld-elf/pr12851.d b/ld/testsuite/ld-elf/pr12851.d
new file mode 100644
index 0000000..84aa59a
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr12851.d
@@ -0,0 +1,14 @@
+#source: pr12851.s
+#source: start.s
+#ld: --gc-sections
+#readelf: -s --wide
+#notarget: arc-*-* d30v-*-* dlx-*-* i960-*-* or32-*-* pj*-*-*
+#notarget: alpha-*-* hppa64-*-* i370-*-* i860-*-* ia64-*-* mep-*-* mn10200-*-*
+#xfail: cr16-*-* crx-*-*
+# generic linker targets don't support --gc-sections, nor do a bunch of others
+# cr16 and crx use non-standard scripts with memory regions, which don't play
+# well with unique group sections under ld -r.
+
+#...
+ +.* _.stapsdt.base
+#pass
diff --git a/ld/testsuite/ld-elf/pr12851.s b/ld/testsuite/ld-elf/pr12851.s
new file mode 100644
index 0000000..784b91f
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr12851.s
@@ -0,0 +1,5 @@
+	.section .note.stapsdt,"?","note"
+	.dc.a _.stapsdt.base
+	.section .stapsdt.base,"a","progbits"
+_.stapsdt.base: .space 1
+	.size _.stapsdt.base,1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH: PR ld/12851: --gc-sections doesn't work on note sections
  2011-06-08 20:10 PATCH: PR ld/12851: --gc-sections doesn't work on note sections H.J. Lu
@ 2011-06-08 23:52 ` Alan Modra
  2011-06-09  0:25   ` Alan Modra
  0 siblings, 1 reply; 6+ messages in thread
From: Alan Modra @ 2011-06-08 23:52 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On Wed, Jun 08, 2011 at 01:10:14PM -0700, H.J. Lu wrote:
> SHT_NOTE sections are special.  We check it in elf_gc_sweep.  But
> we missed it in bfd_elf_gc_sections.

I think your patch is going in the wrong directions.  If we missed
anything in bfd_elf_gc_sections, it's that we keep note sections in
files for which all other sections are discarded.  

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH: PR ld/12851: --gc-sections doesn't work on note sections
  2011-06-08 23:52 ` Alan Modra
@ 2011-06-09  0:25   ` Alan Modra
  2011-06-09  0:50     ` H.J. Lu
  0 siblings, 1 reply; 6+ messages in thread
From: Alan Modra @ 2011-06-09  0:25 UTC (permalink / raw)
  To: H.J. Lu, binutils

On Thu, Jun 09, 2011 at 09:22:26AM +0930, Alan Modra wrote:
> On Wed, Jun 08, 2011 at 01:10:14PM -0700, H.J. Lu wrote:
> > SHT_NOTE sections are special.  We check it in elf_gc_sweep.  But
> > we missed it in bfd_elf_gc_sections.
> 
> I think your patch is going in the wrong directions.  If we missed
> anything in bfd_elf_gc_sections, it's that we keep note sections in
> files for which all other sections are discarded.  

Actually, I think that

	  else if ((o->flags & (SEC_DEBUGGING | SEC_LINKER_CREATED)) != 0
		   || (o->flags & (SEC_ALLOC | SEC_LOAD | SEC_RELOC)) == 0
		   || elf_section_data (o)->this_hdr.sh_type == SHT_NOTE)
	    {
	      /* Keep debug, special and SHT_NOTE sections.  */
	      o->gc_mark = 1;
	    }

doesn't belong in elf_gc_sweep at all.  It would be better to put code
marking these sections in a default gc_mark_extra_sections.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH: PR ld/12851: --gc-sections doesn't work on note sections
  2011-06-09  0:25   ` Alan Modra
@ 2011-06-09  0:50     ` H.J. Lu
  2011-06-09  1:35       ` Alan Modra
  0 siblings, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2011-06-09  0:50 UTC (permalink / raw)
  To: H.J. Lu, binutils

On Wed, Jun 8, 2011 at 5:25 PM, Alan Modra <amodra@gmail.com> wrote:
> On Thu, Jun 09, 2011 at 09:22:26AM +0930, Alan Modra wrote:
>> On Wed, Jun 08, 2011 at 01:10:14PM -0700, H.J. Lu wrote:
>> > SHT_NOTE sections are special.  We check it in elf_gc_sweep.  But
>> > we missed it in bfd_elf_gc_sections.
>>
>> I think your patch is going in the wrong directions.  If we missed
>> anything in bfd_elf_gc_sections, it's that we keep note sections in
>> files for which all other sections are discarded.
>
> Actually, I think that
>
>          else if ((o->flags & (SEC_DEBUGGING | SEC_LINKER_CREATED)) != 0
>                   || (o->flags & (SEC_ALLOC | SEC_LOAD | SEC_RELOC)) == 0
>                   || elf_section_data (o)->this_hdr.sh_type == SHT_NOTE)
>            {
>              /* Keep debug, special and SHT_NOTE sections.  */
>              o->gc_mark = 1;
>            }
>
> doesn't belong in elf_gc_sweep at all.  It would be better to put code
> marking these sections in a default gc_mark_extra_sections.
>

To keep SHT_NOTE sections, we need to keep sections referenced
by SHT_NOTE sections. But

 /* Grovel through relocs to find out who stays ...  */
  gc_mark_hook = bed->gc_mark_hook;
  for (sub = info->input_bfds; sub != NULL; sub = sub->link_next)
    {
      asection *o;

      if (bfd_get_flavour (sub) != bfd_target_elf_flavour)
        continue;

      for (o = sub->sections; o != NULL; o = o->next)
        if ((o->flags & (SEC_EXCLUDE | SEC_KEEP)) == SEC_KEEP && !o->gc_mark)
          if (!_bfd_elf_gc_mark (info, o, gc_mark_hook))
            return FALSE;
    }

doesn't check relocations in SHT_NOTE sections.  I don't know
how default gc_mark_extra_sections can fix.  Also when I add
debug and special sections, I get linker regressions.  It seems
that we want to keep debug and special sections, but not sections
referenced by them.

-- 
H.J.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH: PR ld/12851: --gc-sections doesn't work on note sections
  2011-06-09  0:50     ` H.J. Lu
@ 2011-06-09  1:35       ` Alan Modra
  2011-06-14  2:45         ` Alan Modra
  0 siblings, 1 reply; 6+ messages in thread
From: Alan Modra @ 2011-06-09  1:35 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On Wed, Jun 08, 2011 at 05:50:11PM -0700, H.J. Lu wrote:
> doesn't check relocations in SHT_NOTE sections.

I see.

>  I don't know
> how default gc_mark_extra_sections can fix.  Also when I add
> debug and special sections, I get linker regressions.  It seems
> that we want to keep debug and special sections, but not sections
> referenced by them.

That's true, we don't want to keep sections referenced by debug
sections.  It's also the case that adding a default
gc_mark_extra_sections really only benefits us if we want to implement
dropping notes and other special sections when all other sections in a
file are garbage collected.  So for the time being I guess your patch
is OK if you remove the SHT_NOTE test in elf_gc_sweep.  Please also
verify that your testcase behaves on Nick's large set of targets
before committing.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH: PR ld/12851: --gc-sections doesn't work on note sections
  2011-06-09  1:35       ` Alan Modra
@ 2011-06-14  2:45         ` Alan Modra
  0 siblings, 0 replies; 6+ messages in thread
From: Alan Modra @ 2011-06-14  2:45 UTC (permalink / raw)
  To: binutils

I think it ought to be safe to discard debug info from a file that has
all its code and data discarded.  This patch does that, and tweaks
the recent change to garbage collection of note sections to not keep
notes in unreferenced groups, as we did before.

bfd/
	PR ld/12851
	* elflink.c (_bfd_elf_gc_mark_extra_sections): New function.
	(elf_gc_sweep): Don't treat debug and sections like .comment
	specially here.
	(bfd_elf_gc_sections): Treat note sections as gc roots only when
	not part of a group.  Always call gc_mark_extra_sections.
	* elf-bfd.h (_bfd_elf_gc_mark_extra_sections): Declare.
	* elfxx-target.h (elf_backend_gc_mark_extra_sections): Default to
	_bfd_elf_gc_mark_extra_sections.
	* elf32-arm.c (elf32_arm_gc_mark_extra_sections): Call
	_bfd_elf_gc_mark_extra_sections.
	* elf32-tic6x.c (elf32_tic6x_gc_mark_extra_sections): Likewise.
ld/testsuite/
	* ld-elf/pr12851.d: Correct target selection and comment.

Index: bfd/elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.410
diff -u -p -r1.410 elflink.c
--- bfd/elflink.c	13 Jun 2011 00:59:14 -0000	1.410
+++ bfd/elflink.c	13 Jun 2011 12:09:15 -0000
@@ -11633,6 +11633,49 @@ _bfd_elf_gc_mark (struct bfd_link_info *
   return ret;
 }
 
+/* Keep debug and special sections.  */
+
+bfd_boolean
+_bfd_elf_gc_mark_extra_sections (struct bfd_link_info *info,
+				 elf_gc_mark_hook_fn mark_hook ATTRIBUTE_UNUSED)
+{
+  bfd *ibfd;
+
+  for (ibfd = info->input_bfds; ibfd != NULL; ibfd = ibfd->link_next)
+    {
+      asection *isec;
+      bfd_boolean some_kept;
+
+      if (bfd_get_flavour (ibfd) != bfd_target_elf_flavour)
+	continue;
+
+      /* Ensure all linker created sections are kept, and see whether
+	 any other section is already marked.  */
+      some_kept = FALSE;
+      for (isec = ibfd->sections; isec != NULL; isec = isec->next)
+	{
+	  if ((isec->flags & SEC_LINKER_CREATED) != 0)
+	    isec->gc_mark = 1;
+	  else if (isec->gc_mark)
+	    some_kept = TRUE;
+	}
+
+      /* If no section in this file will be kept, then we can
+	 toss out debug sections.  */
+      if (!some_kept)
+	continue;
+
+      /* Keep debug and special sections like .comment when they are
+	 not part of a group.  */
+      for (isec = ibfd->sections; isec != NULL; isec = isec->next)
+	if (elf_next_in_group (isec) == NULL
+	    && ((isec->flags & SEC_DEBUGGING) != 0
+		|| (isec->flags & (SEC_ALLOC | SEC_LOAD | SEC_RELOC)) == 0))
+	  isec->gc_mark = 1;
+    }
+  return TRUE;
+}
+
 /* Sweep symbols in swept sections.  Called via elf_link_hash_traverse.  */
 
 struct elf_gc_sweep_symbol_info
@@ -11690,12 +11733,6 @@ elf_gc_sweep (bfd *abfd, struct bfd_link
 	      asection *first = elf_next_in_group (o);
 	      o->gc_mark = first->gc_mark;
 	    }
-	  else if ((o->flags & (SEC_DEBUGGING | SEC_LINKER_CREATED)) != 0
-		   || (o->flags & (SEC_ALLOC | SEC_LOAD | SEC_RELOC)) == 0)
-	    {
-	      /* Keep debug and special sections.  */
-	      o->gc_mark = 1;
-	    }
 
 	  if (o->gc_mark)
 	    continue;
@@ -11966,19 +12003,23 @@ bfd_elf_gc_sections (bfd *abfd, struct b
       if (bfd_get_flavour (sub) != bfd_target_elf_flavour)
 	continue;
 
-      /* Also keep SHT_NOTE sections.  */
+      /* Start at sections marked with SEC_KEEP (ref _bfd_elf_gc_keep).
+	 Also treat note sections as a root, if the section is not part
+	 of a group.  */
       for (o = sub->sections; o != NULL; o = o->next)
-	if ((o->flags & SEC_EXCLUDE) == 0
+	if (!o->gc_mark
+	    && (o->flags & SEC_EXCLUDE) == 0
 	    && ((o->flags & SEC_KEEP) != 0
-		|| elf_section_data (o)->this_hdr.sh_type == SHT_NOTE)
-	    && !o->gc_mark)
-	  if (!_bfd_elf_gc_mark (info, o, gc_mark_hook))
-	    return FALSE;
+		|| (elf_section_data (o)->this_hdr.sh_type == SHT_NOTE
+		    && elf_next_in_group (o) == NULL )))
+	  {
+	    if (!_bfd_elf_gc_mark (info, o, gc_mark_hook))
+	      return FALSE;
+	  }
     }
 
   /* Allow the backend to mark additional target specific sections.  */
-  if (bed->gc_mark_extra_sections)
-    bed->gc_mark_extra_sections (info, gc_mark_hook);
+  bed->gc_mark_extra_sections (info, gc_mark_hook);
 
   /* ... and mark SEC_EXCLUDE for those that go.  */
   return elf_gc_sweep (abfd, info);
Index: bfd/elf-bfd.h
===================================================================
RCS file: /cvs/src/src/bfd/elf-bfd.h,v
retrieving revision 1.320
diff -u -p -r1.320 elf-bfd.h
--- bfd/elf-bfd.h	26 May 2011 04:28:18 -0000	1.320
+++ bfd/elf-bfd.h	10 Jun 2011 05:43:49 -0000
@@ -2169,6 +2169,9 @@ extern bfd_boolean _bfd_elf_gc_mark_fdes
 extern bfd_boolean _bfd_elf_gc_mark
   (struct bfd_link_info *, asection *, elf_gc_mark_hook_fn);
 
+extern bfd_boolean _bfd_elf_gc_mark_extra_sections
+  (struct bfd_link_info *, elf_gc_mark_hook_fn);
+
 extern bfd_boolean bfd_elf_gc_common_finalize_got_offsets
   (bfd *, struct bfd_link_info *);
 
Index: bfd/elfxx-target.h
===================================================================
RCS file: /cvs/src/src/bfd/elfxx-target.h,v
retrieving revision 1.125
diff -u -p -r1.125 elfxx-target.h
--- bfd/elfxx-target.h	6 Jun 2011 01:26:01 -0000	1.125
+++ bfd/elfxx-target.h	10 Jun 2011 05:44:14 -0000
@@ -143,7 +143,7 @@
 #define elf_backend_gc_mark_hook	_bfd_elf_gc_mark_hook
 #endif
 #ifndef elf_backend_gc_mark_extra_sections
-#define elf_backend_gc_mark_extra_sections	NULL
+#define elf_backend_gc_mark_extra_sections _bfd_elf_gc_mark_extra_sections
 #endif
 #ifndef elf_backend_gc_sweep_hook
 #define elf_backend_gc_sweep_hook	NULL
Index: bfd/elf32-arm.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-arm.c,v
retrieving revision 1.268
diff -u -p -r1.268 elf32-arm.c
--- bfd/elf32-arm.c	2 Jun 2011 13:43:14 -0000	1.268
+++ bfd/elf32-arm.c	10 Jun 2011 05:43:55 -0000
@@ -12467,6 +12469,8 @@ elf32_arm_gc_mark_extra_sections (struct
   Elf_Internal_Shdr **elf_shdrp;
   bfd_boolean again;
 
+  _bfd_elf_gc_mark_extra_sections (info, gc_mark_hook);
+
   /* Marking EH data may cause additional code sections to be marked,
      requiring multiple passes.  */
   again = TRUE;
Index: bfd/elf32-tic6x.c
===================================================================
RCS file: /cvs/src/src/bfd/elf32-tic6x.c,v
retrieving revision 1.24
diff -u -p -r1.24 elf32-tic6x.c
--- bfd/elf32-tic6x.c	20 May 2011 10:09:57 -0000	1.24
+++ bfd/elf32-tic6x.c	10 Jun 2011 05:43:59 -0000
@@ -1921,6 +1921,8 @@ elf32_tic6x_gc_mark_extra_sections (stru
   Elf_Internal_Shdr **elf_shdrp;
   bfd_boolean again;
 
+  _bfd_elf_gc_mark_extra_sections (info, gc_mark_hook);
+
   /* Marking EH data may cause additional code sections to be marked,
      requiring multiple passes.  */
   again = TRUE;
Index: ld/testsuite/ld-elf/pr12851.d
===================================================================
RCS file: /cvs/src/src/ld/testsuite/ld-elf/pr12851.d,v
retrieving revision 1.1
diff -u -p -r1.1 pr12851.d
--- ld/testsuite/ld-elf/pr12851.d	9 Jun 2011 04:52:15 -0000	1.1
+++ ld/testsuite/ld-elf/pr12851.d	13 Jun 2011 01:26:59 -0000
@@ -3,11 +3,8 @@
 #ld: --gc-sections
 #readelf: -s --wide
 #notarget: arc-*-* d30v-*-* dlx-*-* i960-*-* or32-*-* pj*-*-*
-#notarget: alpha-*-* hppa64-*-* i370-*-* i860-*-* ia64-*-* mep-*-* mn10200-*-*
-#xfail: cr16-*-* crx-*-*
+#notarget: hppa64-*-* i370-*-* i860-*-* ia64-*-* mep-*-* mn10200-*-*
 # generic linker targets don't support --gc-sections, nor do a bunch of others
-# cr16 and crx use non-standard scripts with memory regions, which don't play
-# well with unique group sections under ld -r.
 
 #...
  +.* _.stapsdt.base

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-06-14  2:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-08 20:10 PATCH: PR ld/12851: --gc-sections doesn't work on note sections H.J. Lu
2011-06-08 23:52 ` Alan Modra
2011-06-09  0:25   ` Alan Modra
2011-06-09  0:50     ` H.J. Lu
2011-06-09  1:35       ` Alan Modra
2011-06-14  2:45         ` Alan Modra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).