[PATCH] ld: Lookup section in output with the same name

[H.J. Lu]

When there are more than one input sections with the same section name,
SECNAME, linker picks the first one to define __start_SECNAME and
__stop_SECNAME symbols. When the first input section is removed by
comdat group, we need to check if there is still an output section
with section name SECNAME.

OK for master?

H.J.
---
PR ld/23591
* ldlang.c (undef_start_stop): Lookup section in output with
the same name.
* testsuite/ld-elf/pr23591.d: New file.
* testsuite/ld-elf/pr23591a.s: Likewise.
* testsuite/ld-elf/pr23591b.s: Likewise.
* testsuite/ld-elf/pr23591c.s: Likewise.
---
ld/ldlang.c | 18 ++++++++++++++++++
ld/testsuite/ld-elf/pr23591.d | 10 ++++++++++
ld/testsuite/ld-elf/pr23591a.s | 16 ++++++++++++++++
ld/testsuite/ld-elf/pr23591b.s | 11 +++++++++++
ld/testsuite/ld-elf/pr23591c.s | 26 ++++++++++++++++++++++++++
5 files changed, 81 insertions(+)
create mode 100644 ld/testsuite/ld-elf/pr23591.d
create mode 100644 ld/testsuite/ld-elf/pr23591a.s
create mode 100644 ld/testsuite/ld-elf/pr23591b.s
create mode 100644 ld/testsuite/ld-elf/pr23591c.s

diff --git a/ld/ldlang.c b/ld/ldlang.c
index 8878ccdb63..0d6ade4c1e 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -6097,6 +6097,24 @@ undef_start_stop (struct bfd_link_hash_entry *h)
|| strcmp (h->u.def.section->name,
h->u.def.section->output_section->name) != 0)
{
+ asection *sec = bfd_get_section_by_name (link_info.output_bfd,
+ h->u.def.section->name);
+ if (sec != NULL)
+ {
+ /* When there are more than one input sections with the same
+ section name, SECNAME, linker picks the first one to define
+ __start_SECNAME and __stop_SECNAME symbols. When the first
+ input section is removed by comdat group, we need to check
+ if there is still an output section with section name
+ SECNAME. */
+ asection *i;
+ for (i = sec->map_head.s; i != NULL; i = i->map_head.s)
+ if (i->size != 0)
+ {
+ h->u.def.section = i;
+ return;
+ }
+ }
h->type = bfd_link_hash_undefined;
h->u.undef.abfd = NULL;
}
diff --git a/ld/testsuite/ld-elf/pr23591.d b/ld/testsuite/ld-elf/pr23591.d
new file mode 100644
index 0000000000..e1e90f51f7
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr23591.d
@@ -0,0 +1,10 @@
+#source: pr23591a.s
+#source: pr23591b.s
+#source: pr23591c.s
+#ld: -e _start
+#readelf: -sW
+#notarget: arm*-*-symbianelf*
+
+#...
+ +[0-9]+: +[a-f0-9]+ +0 +NOTYPE +GLOBAL +HIDDEN +[0-9]+ +___?start___sancov_cntrs
+#pass
diff --git a/ld/testsuite/ld-elf/pr23591a.s b/ld/testsuite/ld-elf/pr23591a.s
new file mode 100644
index 0000000000..f0db8a60ef
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr23591a.s
@@ -0,0 +1,16 @@
+ .ifdef UNDERSCORE
+ .hidden ___start___sancov_cntrs
+ .else
+ .hidden __start___sancov_cntrs
+ .endif
+ .text
+ .globl _start
+ .type _start, %function
+_start:
+ .byte 0
+ .data
+ .ifdef UNDERSCORE
+ .dc.a ___start___sancov_cntrs
+ .else
+ .dc.a __start___sancov_cntrs
+ .endif
diff --git a/ld/testsuite/ld-elf/pr23591b.s b/ld/testsuite/ld-elf/pr23591b.s
new file mode 100644
index 0000000000..b7453b8aab
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr23591b.s
@@ -0,0 +1,11 @@
+ .section .text,"axG",%progbits,foo1,comdat
+ .ifdef UNDERSCORE
+ .globl _foo1
+ .type _foo1, %function
+_foo1:
+ .else
+ .globl foo1
+ .type foo1, %function
+foo1:
+ .endif
+ .byte 0
diff --git a/ld/testsuite/ld-elf/pr23591c.s b/ld/testsuite/ld-elf/pr23591c.s
new file mode 100644
index 0000000000..cd0ada5dd6
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr23591c.s
@@ -0,0 +1,26 @@
+ .section __sancov_cntrs,"aG",%progbits,foo1,comdat
+ .long 0
+ .section .text,"axG",%progbits,foo1,comdat
+ .ifdef UNDERSCORE
+ .globl _foo1
+ .type _foo1, %function
+_foo1:
+ .else
+ .globl foo1
+ .type foo1, %function
+foo1:
+ .endif
+ .long 0
+ .section __sancov_cntrs,"aG",%progbits,foo2,comdat
+ .long 1
+ .section .text,"axG",%progbits,foo2,comdat
+ .ifdef UNDERSCORE
+ .globl _foo2
+ .type _foo2, %function
+_foo2:
+ .else
+ .globl foo2
+ .type foo2, %function
+foo2:
+ .endif
+ .long 1

[Alan Modra]

Post by H.J. Lu
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -6097,6 +6097,24 @@ undef_start_stop (struct bfd_link_hash_entry *h)
|| strcmp (h->u.def.section->name,
h->u.def.section->output_section->name) != 0)
{
+ asection *sec = bfd_get_section_by_name (link_info.output_bfd,
+ h->u.def.section->name);
+ if (sec != NULL)
+ {
+ /* When there are more than one input sections with the same
+ section name, SECNAME, linker picks the first one to define
+ __start_SECNAME and __stop_SECNAME symbols. When the first
+ input section is removed by comdat group, we need to check
+ if there is still an output section with section name
+ SECNAME. */
+ asection *i;
+ for (i = sec->map_head.s; i != NULL; i = i->map_head.s)
+ if (i->size != 0)

I think you should check that i->name matches h->u.def.section->name
here. At one stage we only defined start/stop symbols on orphans,
but now we do so whenever input section name matches output section
name and the name is alphanumeric. That means a linker script might
be involved in which case differently named input sections might map
to the output section.

Also, why do you check i->size? We define start/stop symbols on zero
sized sections!

Post by H.J. Lu
+ {
+ h->u.def.section = i;
+ return;
+ }
+ }
h->type = bfd_link_hash_undefined;
h->u.undef.abfd = NULL;
}

[H.J. Lu]

Post by Alan Modra

Post by H.J. Lu
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -6097,6 +6097,24 @@ undef_start_stop (struct bfd_link_hash_entry *h)
|| strcmp (h->u.def.section->name,
h->u.def.section->output_section->name) != 0)
{
+ asection *sec = bfd_get_section_by_name (link_info.output_bfd,
+ h->u.def.section->name);
+ if (sec != NULL)
+ {
+ /* When there are more than one input sections with the same
+ section name, SECNAME, linker picks the first one to define
+ __start_SECNAME and __stop_SECNAME symbols. When the first
+ input section is removed by comdat group, we need to check
+ if there is still an output section with section name
+ SECNAME. */
+ asection *i;
+ for (i = sec->map_head.s; i != NULL; i = i->map_head.s)
+ if (i->size != 0)

I think you should check that i->name matches h->u.def.section->name
here. At one stage we only defined start/stop symbols on orphans,
but now we do so whenever input section name matches output section
name and the name is alphanumeric. That means a linker script might
be involved in which case differently named input sections might map
to the output section.
Also, why do you check i->size? We define start/stop symbols on zero
sized sections!

Like this?

[Alan Modra]

Post by H.J. Lu
Like this?

Yes, looks good, thanks.

[Alan Modra]

Fixes pr23591 test failures on hppa64-hpux and score-elf, and xfails
frv-linux and lm32-linux.

PR ld/23591
* testsuite/ld-elf/pr23591a.s,
* testsuite/ld-elf/pr23591b.s,
* testsuite/ld-elf/pr23591c.s: Don't start directives in first column.
* testsuite/ld-elf/pr23591.d: xfail frv-linux and lm32-linux.
Allow __start___sancov_cntrs as a local symbol.

diff --git a/ld/testsuite/ld-elf/pr23591.d b/ld/testsuite/ld-elf/pr23591.d
index e002d73044..e5a7475324 100644
--- a/ld/testsuite/ld-elf/pr23591.d
+++ b/ld/testsuite/ld-elf/pr23591.d
@@ -3,7 +3,10 @@
#source: pr23591c.s
#ld: -e _start
#readelf: -sW
+#xfail: frv-*-linux* lm32-*-linux*
+# frv-linux and lm32-linux fail with complaints about emitting dynamic
+# relocations in read-only sections.

#...
- +[0-9]+: +[a-f0-9]+ +0 +NOTYPE +GLOBAL +HIDDEN +[0-9]+ +___?start___sancov_cntrs
+ +[0-9]+: +[a-f0-9]+ +0 +NOTYPE +(GLOBAL +HIDDEN|LOCAL +DEFAULT) +[0-9]+ +___?start___sancov_cntrs
#pass
diff --git a/ld/testsuite/ld-elf/pr23591a.s b/ld/testsuite/ld-elf/pr23591a.s
index ebdb7f8abb..7616b131cd 100644
--- a/ld/testsuite/ld-elf/pr23591a.s
+++ b/ld/testsuite/ld-elf/pr23591a.s
@@ -1,14 +1,14 @@
-.ifdef UNDERSCORE
+ .ifdef UNDERSCORE
.hidden ___start___sancov_cntrs
-.else
+ .else
.hidden __start___sancov_cntrs
-.endif
+ .endif
.text
.globl _start
.type _start, %function
_start:
-.ifdef UNDERSCORE
+ .ifdef UNDERSCORE
.dc.a ___start___sancov_cntrs
-.else
+ .else
.dc.a __start___sancov_cntrs
-.endif
+ .endif
diff --git a/ld/testsuite/ld-elf/pr23591b.s b/ld/testsuite/ld-elf/pr23591b.s
index 646e681cc5..4ab48fb4bb 100644
--- a/ld/testsuite/ld-elf/pr23591b.s
+++ b/ld/testsuite/ld-elf/pr23591b.s
@@ -1,11 +1,11 @@
.section .text,"axG",%progbits,foo1,comdat
-.ifdef UNDERSCORE
+ .ifdef UNDERSCORE
.globl _foo1
.type _foo1, %function
_foo1:
-.else
+ .else
.globl foo1
.type foo1, %function
foo1:
-.endif
+ .endif
.byte 0
diff --git a/ld/testsuite/ld-elf/pr23591c.s b/ld/testsuite/ld-elf/pr23591c.s
index 338671ceb7..695b218ad0 100644
--- a/ld/testsuite/ld-elf/pr23591c.s
+++ b/ld/testsuite/ld-elf/pr23591c.s
@@ -1,26 +1,26 @@
.section __sancov_cntrs,"aG",%progbits,foo1,comdat
.long 0
.section .text,"axG",%progbits,foo1,comdat
-.ifdef UNDERSCORE
+ .ifdef UNDERSCORE
.globl _foo1
.type _foo1, %function
_foo1:
-.else
+ .else
.globl foo1
.type foo1, %function
foo1:
-.endif
+ .endif
.long 0
.section __sancov_cntrs,"aG",%progbits,foo2,comdat
.long 1
.section .text,"axG",%progbits,foo2,comdat
-.ifdef UNDERSCORE
+ .ifdef UNDERSCORE
.globl _foo2
.type _foo2, %function
_foo2:
.long 1
-.else
+ .else
.globl foo2
.type foo2, %function
foo2:
-.endif
+ .endif