This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] Remove the stripped group section from linker output
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Alan Modra <amodra at gmail dot com>
- Cc: Binutils <binutils at sourceware dot org>
- Date: Mon, 12 Feb 2018 20:06:29 -0800
- Subject: Re: [PATCH] Remove the stripped group section from linker output
- Authentication-results: sourceware.org; auth=none
- References: <20180212181340.GA22522@gmail.com> <20180213020152.GC30218@bubble.grove.modra.org>
On Mon, Feb 12, 2018 at 6:01 PM, Alan Modra <amodra@gmail.com> wrote:
> On Mon, Feb 12, 2018 at 10:13:40AM -0800, H.J. Lu wrote:
>> GCC 7 and above generates .debug_macro section in COMDAT group with
>> -g3. But ld fails to recognize that a group section shouldn't be in
>> output when all of its members are stripped. Update ld to remove the
>> stripped group section from linker output when all members are removed
>> by moving the stripped group section logic from objcopy.c to bfd.c so
>> that "ld -r -S" behaves the same as "strip -g".
>>
>> OK for master?
>
> I think that moving most of objcopy.c:is_strip_section into bfd is a
> bad idea. You've ended up with an ugly interface with two callback
> functions needed by objcopy, and a function with confusing parameter
> names.
>
>> +bfd_boolean
>> +bfd_stripped_group_section_p
>> + (bfd *abfd ATTRIBUTE_UNUSED, asection *sec,
>> + bfd_boolean relocatable_link,
>> + bfd_boolean (*strip_group_section_p) (bfd *, asection *),
>> + bfd_boolean (*strip_section_p) (bfd *, asection *))
>> +{
>> + if ((bfd_get_section_flags (abfd, sec) & SEC_GROUP) != 0)
>> + {
>> + asection *elt, *first;
>> +
>> + /* PR binutils/3181
>> + If we are going to strip the group signature symbol, then
>> + strip the group section too. */
>> + if (!relocatable_link && strip_group_section_p (abfd, sec))
>> + return TRUE;
>
> When I first looked at the patch I thought "won't that segfault during
> a final link?", because I'd seen that you pass NULL for the callback
> in bfd_elf_final_link but hadn't realized that relocatable_link was
> always false. So the name "relocatable_link" is a lie, but I think it
> would be better to leave objcopy.c alone and write a small function in
> elflink.c that simply iterates over the group elements.
>
> static bfd_boolean
> is_discarded_group (asection *sec)
> {
> asection *elt, *first;
>
> if ((sec->flags & SEC_GROUP) == 0)
> return FALSE;
>
> first = elt = elf_next_in_group (sec);
> while (elt != NULL)
> {
> if (!discarded_section (elt))
> return FALSE;
> elt = elf_next_in_group (elt);
> if (elt == first)
> break;
> }
> return TRUE;
> }
>
> I also think it would be a good idea to set SEC_EXCLUDE for the
> stripped section, just to be consistent with what happens with other
> stripped sections.
>
Like this?
--
H.J.
From 3c126842464a5923c0c56e19036681a15c630a32 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Mon, 12 Feb 2018 08:27:30 -0800
Subject: [PATCH] Remove the stripped group section from linker output
GCC 7 and above generates .debug_macro section in COMDAT group with
-g3. But ld fails to recognize that a group section shouldn't be in
output when all of its members are stripped. Update ld to remove the
stripped group section from linker output when all members are removed.
bfd/
PR ld/22836
* elflink.c (is_discarded_group): New function.
(bfd_elf_final_link): Remove the stripped group section from
linker output.
ld/
PR ld/22836
* testsuite/ld-elf/pr22836-1.s: New file.
* testsuite/ld-elf/pr22836-1a.d: Likewise.
* testsuite/ld-elf/pr22836-1b.d: Likewise.
---
bfd/elflink.c | 32 ++++++++++++++++++++++++++++++++
ld/testsuite/ld-elf/pr22836-1.s | 4 ++++
ld/testsuite/ld-elf/pr22836-1a.d | 15 +++++++++++++++
ld/testsuite/ld-elf/pr22836-1b.d | 15 +++++++++++++++
4 files changed, 66 insertions(+)
create mode 100644 ld/testsuite/ld-elf/pr22836-1.s
create mode 100644 ld/testsuite/ld-elf/pr22836-1a.d
create mode 100644 ld/testsuite/ld-elf/pr22836-1b.d
diff --git a/bfd/elflink.c b/bfd/elflink.c
index d1eb82020c..2fc26f529f 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -11512,6 +11512,31 @@ elf_final_link_free (bfd *obfd, struct elf_final_link_info *flinfo)
}
}
+/* Return TRUE if section SEC is a group section with all members
+ removed. */
+
+static bfd_boolean
+is_discarded_group (asection *sec)
+{
+ asection *elt, *first;
+
+ if ((sec->flags & SEC_GROUP) == 0)
+ return FALSE;
+
+ /* Remove the group section if all members are removed. */
+ first = elt = elf_next_in_group (sec);
+ while (elt != NULL)
+ {
+ if (!discarded_section (elt))
+ return FALSE;
+ elt = elf_next_in_group (elt);
+ if (elt == first)
+ break;
+ }
+
+ return TRUE;
+}
+
/* Do the final step of an ELF link. */
bfd_boolean
@@ -11618,6 +11643,13 @@ bfd_elf_final_link (bfd *abfd, struct bfd_link_info *info)
else
o->flags |= SEC_EXCLUDE;
}
+ else if (is_discarded_group (o))
+ {
+ /* Remove the stripped group section from linker output. */
+ o->flags |= SEC_EXCLUDE;
+ bfd_section_list_remove (abfd, o);
+ abfd->section_count--;
+ }
}
/* Count up the number of relocations we will output for each output
diff --git a/ld/testsuite/ld-elf/pr22836-1.s b/ld/testsuite/ld-elf/pr22836-1.s
new file mode 100644
index 0000000000..8be549ecca
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr22836-1.s
@@ -0,0 +1,4 @@
+ .section .debug_macro,"G",%progbits,foo,comdat
+ .long .LASF0
+.LASF0:
+ .string "__STDC__ 1"
diff --git a/ld/testsuite/ld-elf/pr22836-1a.d b/ld/testsuite/ld-elf/pr22836-1a.d
new file mode 100644
index 0000000000..5f8461f48e
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr22836-1a.d
@@ -0,0 +1,15 @@
+#source: pr22836-1.s
+#ld: -r -s
+#readelf: -S --wide
+#notarget: alpha-*-* cr16-*-* crx-*-* d30v-*-* dlx-*-* i960-*-* pj*-*-*
+# Disabled on alpha because alpha has a different .set directive.
+# cr16 and crx use non-standard scripts with memory regions, which don't
+# play well with comdat group sections under ld -r. Generic linker
+# targets don't support comdat group sections.
+
+#failif
+#...
+ \[[ 0-9]+\] \.debug_.* +.*
+#...
+COMDAT group section .*
+#...
diff --git a/ld/testsuite/ld-elf/pr22836-1b.d b/ld/testsuite/ld-elf/pr22836-1b.d
new file mode 100644
index 0000000000..20adc3a1f3
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr22836-1b.d
@@ -0,0 +1,15 @@
+#source: pr22836-1.s
+#ld: -r -S
+#readelf: -S --wide
+#notarget: alpha-*-* cr16-*-* crx-*-* d30v-*-* dlx-*-* i960-*-* pj*-*-*
+# Disabled on alpha because alpha has a different .set directive.
+# cr16 and crx use non-standard scripts with memory regions, which don't
+# play well with comdat group sections under ld -r. Generic linker
+# targets don't support comdat group sections.
+
+#failif
+#...
+ \[[ 0-9]+\] \.debug_.* +.*
+#...
+COMDAT group section .*
+#...
--
2.14.3