This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] PR ld/17550: Section groups (comdat/linkonce) create undefined symbols unnecessarily
- 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: Fri, 6 May 2016 09:49:09 -0700
- Subject: Re: [PATCH] PR ld/17550: Section groups (comdat/linkonce) create undefined symbols unnecessarily
- Authentication-results: sourceware.org; auth=none
- References: <20160505130054 dot GA19324 at intel dot com> <20160506012057 dot GB31342 at bubble dot grove dot modra dot org>
On Thu, May 5, 2016 at 6:20 PM, Alan Modra <amodra@gmail.com> wrote:
> On Thu, May 05, 2016 at 06:00:54AM -0700, H.J. Lu wrote:
>> When a global symbol is defined in COMDAT group, we shouldn't leave an
>> undefined symbol in symbol table when the symbol section is discarded
>> unless there is a reference to the symbol outside of COMDAT group.
>>
>> OK for master?
>
> OK if you've tested this with our big list of targets.
>
This is what I checked in.
--
H.J.
From ae9f0baf952618833005cf40cb3e85b7fd67e9d3 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 5 May 2016 05:35:01 -0700
Subject: [PATCH] Strip global symbol defined in discarded section
When a global symbol is defined in COMDAT group, we shouldn't leave an
undefined symbol in symbol table when the symbol section is discarded
unless there is a reference to the symbol outside of COMDAT group.
bfd/
PR ld/17550
* elf-bfd.h (elf_link_hash_entry): Update comments for indx,
documenting that indx == -3 if symbol is defined in a discarded
section.
* elflink.c (elf_link_add_object_symbols): Set indx to -3 if
symbol is defined in a discarded section.
(elf_link_output_extsym): Strip a global symbol defined in a
discarded section.
ld/
PR ld/17550
* testsuite/ld-elf/pr17550-1.s: New file.
* testsuite/ld-elf/pr17550-2.s: Likewise.
* testsuite/ld-elf/pr17550-3.s: Likewise.
* testsuite/ld-elf/pr17550-4.s: Likewise.
* testsuite/ld-elf/pr17550a.d: Likewise.
* testsuite/ld-elf/pr17550b.d: Likewise.
* testsuite/ld-elf/pr17550c.d: Likewise.
* testsuite/ld-elf/pr17550d.d: Likewise.
---
bfd/elf-bfd.h | 3 ++-
bfd/elflink.c | 12 ++++++++++++
ld/testsuite/ld-elf/pr17550-1.s | 9 +++++++++
ld/testsuite/ld-elf/pr17550-2.s | 6 ++++++
ld/testsuite/ld-elf/pr17550-3.s | 14 ++++++++++++++
ld/testsuite/ld-elf/pr17550-4.s | 15 +++++++++++++++
ld/testsuite/ld-elf/pr17550a.d | 14 ++++++++++++++
ld/testsuite/ld-elf/pr17550b.d | 14 ++++++++++++++
ld/testsuite/ld-elf/pr17550c.d | 9 +++++++++
ld/testsuite/ld-elf/pr17550d.d | 13 +++++++++++++
10 files changed, 108 insertions(+), 1 deletion(-)
create mode 100644 ld/testsuite/ld-elf/pr17550-1.s
create mode 100644 ld/testsuite/ld-elf/pr17550-2.s
create mode 100644 ld/testsuite/ld-elf/pr17550-3.s
create mode 100644 ld/testsuite/ld-elf/pr17550-4.s
create mode 100644 ld/testsuite/ld-elf/pr17550a.d
create mode 100644 ld/testsuite/ld-elf/pr17550b.d
create mode 100644 ld/testsuite/ld-elf/pr17550c.d
create mode 100644 ld/testsuite/ld-elf/pr17550d.d
diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 9067dd9..863fc39 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -124,7 +124,8 @@ struct elf_link_hash_entry
struct bfd_link_hash_entry root;
/* Symbol index in output file. This is initialized to -1. It is
- set to -2 if the symbol is used by a reloc. */
+ set to -2 if the symbol is used by a reloc. It is set to -3 if
+ this symbol is defined in a discarded section. */
long indx;
/* Symbol index as a dynamic symbol. Initialized to -1, and remains
diff --git a/bfd/elflink.c b/bfd/elflink.c
index b6ff6b6..6ccd5fc 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -4082,6 +4082,7 @@ error_free_dyn:
bfd_boolean old_weak;
bfd_boolean override;
bfd_boolean common;
+ bfd_boolean discarded;
unsigned int old_alignment;
bfd *old_bfd;
bfd_boolean matched;
@@ -4092,6 +4093,7 @@ error_free_dyn:
sec = NULL;
value = isym->st_value;
common = bed->common_definition (isym);
+ discarded = FALSE;
bind = ELF_ST_BIND (isym->st_info);
switch (bind)
@@ -4142,6 +4144,7 @@ error_free_dyn:
/* Symbols from discarded section are undefined. We keep
its visibility. */
sec = bfd_und_section_ptr;
+ discarded = TRUE;
isym->st_shndx = SHN_UNDEF;
}
else if ((abfd->flags & (EXEC_P | DYNAMIC)) != 0)
@@ -4385,6 +4388,11 @@ error_free_dyn:
|| h->root.type == bfd_link_hash_warning)
h = (struct elf_link_hash_entry *) h->root.u.i.link;
+ /* Setting the index to -3 tells elf_link_output_extsym that
+ this symbol is defined in a discarded section. */
+ if (discarded)
+ h->indx = -3;
+
*sym_hash = h;
new_weak = (flags & BSF_WEAK) != 0;
@@ -9201,6 +9209,10 @@ elf_link_output_extsym (struct bfd_hash_entry *bh, void *data)
return FALSE;
}
}
+
+ /* Strip a global symbol defined in a discarded section. */
+ if (h->indx == -3)
+ return TRUE;
}
/* We should also warn if a forced local symbol is referenced from
diff --git a/ld/testsuite/ld-elf/pr17550-1.s b/ld/testsuite/ld-elf/pr17550-1.s
new file mode 100644
index 0000000..3da73dc
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr17550-1.s
@@ -0,0 +1,9 @@
+ .section .data,"awG",%progbits,foo_group,comdat
+ .dc.a x_alias
+ .type x, %object
+ .p2align 2
+ .size x, 4
+x:
+ .zero 4
+ .globl x_alias
+ .set x_alias,x
diff --git a/ld/testsuite/ld-elf/pr17550-2.s b/ld/testsuite/ld-elf/pr17550-2.s
new file mode 100644
index 0000000..eb4120f
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr17550-2.s
@@ -0,0 +1,6 @@
+ .section .data,"awG",%progbits,foo_group,comdat
+ .type x, %object
+ .p2align 2
+ .size x, 4
+x:
+ .zero 4
diff --git a/ld/testsuite/ld-elf/pr17550-3.s b/ld/testsuite/ld-elf/pr17550-3.s
new file mode 100644
index 0000000..3d7c252
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr17550-3.s
@@ -0,0 +1,14 @@
+ .data
+ .dc.a y
+ .section .data,"awG",%progbits,foo_group,comdat
+ .type y, %object
+ .size y, 4
+y:
+ .zero 4
+ .globl x
+ .type x, %object
+ .size x, 4
+x:
+ .zero 4
+ .globl x_alias
+ .set x_alias,x
diff --git a/ld/testsuite/ld-elf/pr17550-4.s b/ld/testsuite/ld-elf/pr17550-4.s
new file mode 100644
index 0000000..d0442fd
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr17550-4.s
@@ -0,0 +1,15 @@
+ .data
+ .dc.a y
+ .section .data,"awG",%progbits,foo_group,comdat
+ .globl y
+ .type y, %object
+ .size y, 4
+y:
+ .zero 4
+ .globl x
+ .type x, %object
+ .size x, 4
+x:
+ .zero 4
+ .globl x_alias
+ .set x_alias,x
diff --git a/ld/testsuite/ld-elf/pr17550a.d b/ld/testsuite/ld-elf/pr17550a.d
new file mode 100644
index 0000000..c8f0424
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr17550a.d
@@ -0,0 +1,14 @@
+#source: pr17550-1.s
+#source: pr17550-2.s
+#ld: -r
+#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]+: +[0-9a-f]+ +0 +OBJECT +GLOBAL +DEFAULT +UND x_alias
+#...
diff --git a/ld/testsuite/ld-elf/pr17550b.d b/ld/testsuite/ld-elf/pr17550b.d
new file mode 100644
index 0000000..d189747
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr17550b.d
@@ -0,0 +1,14 @@
+#source: pr17550-2.s
+#source: pr17550-1.s
+#ld: -r
+#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]+: +[0-9a-f]+ +0 +OBJECT +GLOBAL +DEFAULT +UND x_alias
+#...
diff --git a/ld/testsuite/ld-elf/pr17550c.d b/ld/testsuite/ld-elf/pr17550c.d
new file mode 100644
index 0000000..23a83f1
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr17550c.d
@@ -0,0 +1,9 @@
+#source: pr17550-2.s
+#source: pr17550-3.s
+#ld: -r
+#error: .*: defined in discarded section `\.data\[foo_group\]'
+#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.
diff --git a/ld/testsuite/ld-elf/pr17550d.d b/ld/testsuite/ld-elf/pr17550d.d
new file mode 100644
index 0000000..e8fad96
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr17550d.d
@@ -0,0 +1,13 @@
+#source: pr17550-2.s
+#source: pr17550-4.s
+#ld: -r
+#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.
+
+#...
+ +[0-9]+: +[0-9a-f]+ +0 +OBJECT +GLOBAL +DEFAULT +UND y
+#pass
--
2.5.5