* [PATCH] Issue a linker error if TLS sections are not adjacent
@ 2014-01-22 19:30 H.J. Lu
2014-01-23 3:55 ` Alan Modra
0 siblings, 1 reply; 7+ messages in thread
From: H.J. Lu @ 2014-01-22 19:30 UTC (permalink / raw)
To: binutils
Hi,
Bad linker script may lead to TLS sections separated by non-TLS sections
in output. This patch changes linker assert to a linker error to
provide better linker diagnosis. OK for master?
Thanks.
H.J.
--
bfd/
PR ld/16498
* elf.c (_bfd_elf_map_sections_to_segments): Issue a linker error
if TLS sections are not adjacent.
ld/testsuite/
PR ld/16498
* ld-elf/pr16498.d: New file.
* ld-elf/pr16498.s: Likewise.
* ld-elf/pr16498.d: Likewise.
---
bfd/ChangeLog | 6 ++++++
bfd/elf.c | 8 +++++++-
ld/testsuite/ChangeLog | 7 +++++++
ld/testsuite/ld-elf/pr16498.d | 3 +++
ld/testsuite/ld-elf/pr16498.s | 23 +++++++++++++++++++++++
ld/testsuite/ld-elf/pr16498.t | 6 ++++++
6 files changed, 52 insertions(+), 1 deletion(-)
create mode 100644 ld/testsuite/ld-elf/pr16498.d
create mode 100644 ld/testsuite/ld-elf/pr16498.s
create mode 100644 ld/testsuite/ld-elf/pr16498.t
diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 3996dfc..4f74e4b 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,9 @@
+2014-01-22 H.J. Lu <hongjiu.lu@intel.com>
+
+ PR ld/16498
+ * elf.c (_bfd_elf_map_sections_to_segments): Issue a linker error
+ if TLS sections are not adjacent.
+
2014-01-22 Alan Modra <amodra@gmail.com>
* elflink.c (elf_link_add_object_symbols): Call minfo for --as-needed.
diff --git a/bfd/elf.c b/bfd/elf.c
index 3815e32..ece9038 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -4119,7 +4119,13 @@ _bfd_elf_map_sections_to_segments (bfd *abfd, struct bfd_link_info *info)
m->p_flags_valid = 1;
for (i = 0; i < (unsigned int) tls_count; ++i)
{
- BFD_ASSERT (first_tls->flags & SEC_THREAD_LOCAL);
+ if ((first_tls->flags & SEC_THREAD_LOCAL) == 0)
+ {
+ _bfd_error_handler
+ (_("%B: TLS sections are not adjacent"), abfd);
+ bfd_set_error (bfd_error_bad_value);
+ goto error_return;
+ }
m->sections[i] = first_tls;
first_tls = first_tls->next;
}
diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
index a269b07..c355dd5 100644
--- a/ld/testsuite/ChangeLog
+++ b/ld/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2014-01-22 H.J. Lu <hongjiu.lu@intel.com>
+
+ PR ld/16498
+ * ld-elf/pr16498.d: New file.
+ * ld-elf/pr16498.s: Likewise.
+ * ld-elf/pr16498.d: Likewise.
+
2014-01-22 Alan Modra <amodra@gmail.com>
* ld-scripts/pr14962-2.d: Correct target triple.
diff --git a/ld/testsuite/ld-elf/pr16498.d b/ld/testsuite/ld-elf/pr16498.d
new file mode 100644
index 0000000..f4ce024
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr16498.d
@@ -0,0 +1,3 @@
+#ld: -shared -T pr16498.t
+#error: .*: TLS sections are not adjacent
+#target: *-*-linux* *-*-gnu* *-*-nacl*
diff --git a/ld/testsuite/ld-elf/pr16498.s b/ld/testsuite/ld-elf/pr16498.s
new file mode 100644
index 0000000..77f80e6
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr16498.s
@@ -0,0 +1,23 @@
+ .globl data
+ .data
+ .align 32
+ .type data, %object
+ .size data, 120
+data:
+ .long 1
+ .zero 116
+ .globl foo
+ .section .tbss,"awT",%nobits
+ .align 4
+ .type foo, %object
+ .size foo, 4
+foo:
+ .zero 4
+ .globl bar
+ .section .tdata,"awT",%progbits
+ .align 16
+ .type bar, %object
+ .size bar, 80
+bar:
+ .long 1
+ .zero 76
diff --git a/ld/testsuite/ld-elf/pr16498.t b/ld/testsuite/ld-elf/pr16498.t
new file mode 100644
index 0000000..928724f
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr16498.t
@@ -0,0 +1,6 @@
+SECTIONS
+{
+ .tdata : { *(.tdata) }
+ .data : { *(.data)
+ }
+}
--
1.8.4.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Issue a linker error if TLS sections are not adjacent
2014-01-22 19:30 [PATCH] Issue a linker error if TLS sections are not adjacent H.J. Lu
@ 2014-01-23 3:55 ` Alan Modra
2014-01-23 4:19 ` H.J. Lu
0 siblings, 1 reply; 7+ messages in thread
From: Alan Modra @ 2014-01-23 3:55 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils
On Wed, Jan 22, 2014 at 11:30:13AM -0800, H.J. Lu wrote:
> PR ld/16498
> * elf.c (_bfd_elf_map_sections_to_segments): Issue a linker error
> if TLS sections are not adjacent.
This part is OK.
> +SECTIONS
> +{
> + .tdata : { *(.tdata) }
> + .data : { *(.data)
> + }
> +}
On the other hand, the testcase is really showing a fault in orphan
section handling. If ld was a little more clever, it would put .tbss
after .tdata and your testcase would no longer give an error.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Issue a linker error if TLS sections are not adjacent
2014-01-23 3:55 ` Alan Modra
@ 2014-01-23 4:19 ` H.J. Lu
2014-01-23 16:17 ` H.J. Lu
2014-01-24 12:54 ` TLS orphan section placement Alan Modra
0 siblings, 2 replies; 7+ messages in thread
From: H.J. Lu @ 2014-01-23 4:19 UTC (permalink / raw)
To: Binutils
On Wed, Jan 22, 2014 at 7:55 PM, Alan Modra <amodra@gmail.com> wrote:
> On Wed, Jan 22, 2014 at 11:30:13AM -0800, H.J. Lu wrote:
>> PR ld/16498
>> * elf.c (_bfd_elf_map_sections_to_segments): Issue a linker error
>> if TLS sections are not adjacent.
>
> This part is OK.
I'd like to augment my patch like
---
index 3815e32..c0303fc 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -4117,11 +4117,31 @@ _bfd_elf_map_sections_to_segments (bfd *abfd,
struct bfd_link_info *info)
/* Mandated PF_R. */
m->p_flags = PF_R;
m->p_flags_valid = 1;
+ s = first_tls;
for (i = 0; i < (unsigned int) tls_count; ++i)
{
- BFD_ASSERT (first_tls->flags & SEC_THREAD_LOCAL);
- m->sections[i] = first_tls;
- first_tls = first_tls->next;
+ if ((s->flags & SEC_THREAD_LOCAL) == 0)
+ {
+ _bfd_error_handler
+ (_("%B: TLS sections are not adjacent:"), abfd);
+ s = first_tls;
+ i = 0;
+ while (i < (unsigned int) tls_count)
+ {
+ if ((s->flags & SEC_THREAD_LOCAL) != 0)
+ {
+ _bfd_error_handler (_(" TLS: %A"), s);
+ i++;
+ }
+ else
+ _bfd_error_handler (_(" non-TLS: %A"), s);
+ s = s->next;
+ }
+ bfd_set_error (bfd_error_bad_value);
+ goto error_return;
+ }
+ m->sections[i] = s;
+ s = s->next;
}
*pm = m;
---
So that user has a clue what went wrong. This gives me:
./ld -shared -T tbss.t -o tls.so x.o
./ld: tls.so: TLS sections are not adjacent:
./ld: TLS: .tdata
./ld: non-TLS: .data
./ld: TLS: .tbss
./ld: map sections to segments failed: Bad value
>> +SECTIONS
>> +{
>> + .tdata : { *(.tdata) }
>> + .data : { *(.data)
>> + }
>> +}
>
> On the other hand, the testcase is really showing a fault in orphan
> section handling. If ld was a little more clever, it would put .tbss
> after .tdata and your testcase would no longer give an error.
>
I am enclosing a simple patch which does it. However, it
doesn't solve the second testcase in the bug report since
it has a linker script:
---
SECTIONS
{
tls_data_init : { *(.tdata .tdata.* .gnu.linkonce.td.*) }
.data :
{
*(.data .data.* .gnu.linkonce.d.*)
}
/DISCARD/ : { *(.note.GNU-stack) *(.gnu_debuglink) *(.gnu.lto_*) }
}
---
which is derived from the real application I got. Since normally
we won't see this problem, I don't think it is worth the effect even
though the change is quite small. I decided not to include it in
my fix since it doesn't solve my original problem. I can include
it as well as the second testcase.
Thanks.
--
H.J.
-
--- /tmp/elf32.em 2014-01-22 09:49:47.335790428 -0800
+++ elf32.em 2014-01-22 10:14:47.147161310 -0800
@@ -1651,6 +1651,9 @@ gldelf_x86_64_place_orphan (asection *s,
{ ".rodata",
SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_DATA,
0, 0, 0, 0 },
+ { ".tdata",
+ SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD | SEC_DATA | SEC_THREAD_LOCAL,
+ 0, 0, 0, 0 },
{ ".data",
SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD | SEC_DATA,
0, 0, 0, 0 },
@@ -1674,6 +1677,7 @@ gldelf_x86_64_place_orphan (asection *s,
{
orphan_text = 0,
orphan_rodata,
+ orphan_tdata,
orphan_data,
orphan_bss,
orphan_rel,
@@ -1801,6 +1805,8 @@ gldelf_x86_64_place_orphan (asection *s,
place = &hold[orphan_bss];
else if ((s->flags & SEC_SMALL_DATA) != 0)
place = &hold[orphan_sdata];
+ else if ((s->flags & SEC_THREAD_LOCAL) != 0)
+ place = &hold[orphan_tdata];
else if ((s->flags & SEC_READONLY) == 0)
place = &hold[orphan_data];
else if (((iself && (sh_type == SHT_RELA || sh_type == SHT_REL))
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Issue a linker error if TLS sections are not adjacent
2014-01-23 4:19 ` H.J. Lu
@ 2014-01-23 16:17 ` H.J. Lu
2014-01-24 12:54 ` TLS orphan section placement Alan Modra
1 sibling, 0 replies; 7+ messages in thread
From: H.J. Lu @ 2014-01-23 16:17 UTC (permalink / raw)
To: Binutils
On Wed, Jan 22, 2014 at 8:19 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Jan 22, 2014 at 7:55 PM, Alan Modra <amodra@gmail.com> wrote:
>> On Wed, Jan 22, 2014 at 11:30:13AM -0800, H.J. Lu wrote:
>>> PR ld/16498
>>> * elf.c (_bfd_elf_map_sections_to_segments): Issue a linker error
>>> if TLS sections are not adjacent.
>>
>> This part is OK.
>
> I'd like to augment my patch like
>
> ---
> index 3815e32..c0303fc 100644
> --- a/bfd/elf.c
> +++ b/bfd/elf.c
> @@ -4117,11 +4117,31 @@ _bfd_elf_map_sections_to_segments (bfd *abfd,
> struct bfd_link_info *info)
> /* Mandated PF_R. */
> m->p_flags = PF_R;
> m->p_flags_valid = 1;
> + s = first_tls;
> for (i = 0; i < (unsigned int) tls_count; ++i)
> {
> - BFD_ASSERT (first_tls->flags & SEC_THREAD_LOCAL);
> - m->sections[i] = first_tls;
> - first_tls = first_tls->next;
> + if ((s->flags & SEC_THREAD_LOCAL) == 0)
> + {
> + _bfd_error_handler
> + (_("%B: TLS sections are not adjacent:"), abfd);
> + s = first_tls;
> + i = 0;
> + while (i < (unsigned int) tls_count)
> + {
> + if ((s->flags & SEC_THREAD_LOCAL) != 0)
> + {
> + _bfd_error_handler (_(" TLS: %A"), s);
> + i++;
> + }
> + else
> + _bfd_error_handler (_(" non-TLS: %A"), s);
> + s = s->next;
> + }
> + bfd_set_error (bfd_error_bad_value);
> + goto error_return;
> + }
> + m->sections[i] = s;
> + s = s->next;
> }
>
> *pm = m;
> ---
>
> So that user has a clue what went wrong. This gives me:
I checked in this enhanced patch without the testcase.
Thanks.
--
H.J.
^ permalink raw reply [flat|nested] 7+ messages in thread
* TLS orphan section placement
2014-01-23 4:19 ` H.J. Lu
2014-01-23 16:17 ` H.J. Lu
@ 2014-01-24 12:54 ` Alan Modra
2014-01-24 17:07 ` H.J. Lu
1 sibling, 1 reply; 7+ messages in thread
From: Alan Modra @ 2014-01-24 12:54 UTC (permalink / raw)
To: H.J. Lu; +Cc: Binutils
On Wed, Jan 22, 2014 at 08:19:31PM -0800, H.J. Lu wrote:
> On Wed, Jan 22, 2014 at 7:55 PM, Alan Modra <amodra@gmail.com> wrote:
> > On Wed, Jan 22, 2014 at 11:30:13AM -0800, H.J. Lu wrote:
> >> PR ld/16498
> >> * elf.c (_bfd_elf_map_sections_to_segments): Issue a linker error
> >> if TLS sections are not adjacent.
> >
> > This part is OK.
>
> I'd like to augment my patch like
Yes, this is OK too.
> > On the other hand, the testcase is really showing a fault in orphan
> > section handling. If ld was a little more clever, it would put .tbss
> > after .tdata and your testcase would no longer give an error.
> >
>
> I am enclosing a simple patch which does it. However, it
> doesn't solve the second testcase in the bug report since
> it has a linker script:
>
> ---
> SECTIONS
> {
> tls_data_init : { *(.tdata .tdata.* .gnu.linkonce.td.*) }
> .data :
> {
> *(.data .data.* .gnu.linkonce.d.*)
> }
> /DISCARD/ : { *(.note.GNU-stack) *(.gnu_debuglink) *(.gnu.lto_*) }
> }
> ---
Besides the place_orphan change you posted, which is OK to commit,
you need something like the following.
Ensures TLS orphans are placed adjacent to existing TLS sections,
and fixes places where the output_section_statement flags (which might
not be set) were tested when bfd_section flags were available.
* ldlang.c (lang_output_section_find_by_flags): Be careful to
test look->bfd_section->flags if available rather than
look->flags. Separate SEC_THREAD_LOCAL handling from
SEC_READONLY loop, and rewrite.
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 1a0d48f..9903f70 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -1495,7 +1495,7 @@ lang_output_section_find_by_flags (const asection *sec,
lang_match_sec_type_func match_type)
{
lang_output_section_statement_type *first, *look, *found;
- flagword flags;
+ flagword look_flags, sec_flags, differ;
/* We know the first statement on this list is *ABS*. May as well
skip it. */
@@ -1503,21 +1503,22 @@ lang_output_section_find_by_flags (const asection *sec,
first = first->next;
/* First try for an exact match. */
+ sec_flags = sec->flags;
found = NULL;
for (look = first; look; look = look->next)
{
- flags = look->flags;
+ look_flags = look->flags;
if (look->bfd_section != NULL)
{
- flags = look->bfd_section->flags;
+ look_flags = look->bfd_section->flags;
if (match_type && !match_type (link_info.output_bfd,
look->bfd_section,
sec->owner, sec))
continue;
}
- flags ^= sec->flags;
- if (!(flags & (SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD | SEC_READONLY
- | SEC_CODE | SEC_SMALL_DATA | SEC_THREAD_LOCAL)))
+ differ = look_flags ^ sec_flags;
+ if (!(differ & (SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD | SEC_READONLY
+ | SEC_CODE | SEC_SMALL_DATA | SEC_THREAD_LOCAL)))
found = look;
}
if (found != NULL)
@@ -1527,115 +1528,144 @@ lang_output_section_find_by_flags (const asection *sec,
return found;
}
- if ((sec->flags & SEC_CODE) != 0
- && (sec->flags & SEC_ALLOC) != 0)
+ if ((sec_flags & SEC_CODE) != 0
+ && (sec_flags & SEC_ALLOC) != 0)
{
/* Try for a rw code section. */
for (look = first; look; look = look->next)
{
- flags = look->flags;
+ look_flags = look->flags;
if (look->bfd_section != NULL)
{
- flags = look->bfd_section->flags;
+ look_flags = look->bfd_section->flags;
if (match_type && !match_type (link_info.output_bfd,
look->bfd_section,
sec->owner, sec))
continue;
}
- flags ^= sec->flags;
- if (!(flags & (SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD
- | SEC_CODE | SEC_SMALL_DATA | SEC_THREAD_LOCAL)))
+ differ = look_flags ^ sec_flags;
+ if (!(differ & (SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD
+ | SEC_CODE | SEC_SMALL_DATA | SEC_THREAD_LOCAL)))
found = look;
}
}
- else if ((sec->flags & (SEC_READONLY | SEC_THREAD_LOCAL)) != 0
- && (sec->flags & SEC_ALLOC) != 0)
+ else if ((sec_flags & SEC_READONLY) != 0
+ && (sec_flags & SEC_ALLOC) != 0)
{
/* .rodata can go after .text, .sdata2 after .rodata. */
for (look = first; look; look = look->next)
{
- flags = look->flags;
+ look_flags = look->flags;
if (look->bfd_section != NULL)
{
- flags = look->bfd_section->flags;
+ look_flags = look->bfd_section->flags;
if (match_type && !match_type (link_info.output_bfd,
look->bfd_section,
sec->owner, sec))
continue;
}
- flags ^= sec->flags;
- if (!(flags & (SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD
- | SEC_READONLY | SEC_SMALL_DATA))
- || (!(flags & (SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD
- | SEC_READONLY))
- && !(look->flags & SEC_SMALL_DATA))
- || (!(flags & (SEC_THREAD_LOCAL | SEC_ALLOC))
- && (look->flags & SEC_THREAD_LOCAL)
- && (!(flags & SEC_LOAD)
- || (look->flags & SEC_LOAD))))
+ differ = look_flags ^ sec_flags;
+ if (!(differ & (SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD
+ | SEC_READONLY | SEC_SMALL_DATA))
+ || (!(differ & (SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD
+ | SEC_READONLY))
+ && !(look_flags & SEC_SMALL_DATA)))
found = look;
}
}
- else if ((sec->flags & SEC_SMALL_DATA) != 0
- && (sec->flags & SEC_ALLOC) != 0)
+ else if ((sec_flags & SEC_THREAD_LOCAL) != 0
+ && (sec_flags & SEC_ALLOC) != 0)
+ {
+ /* .tdata can go after .data, .tbss after .tdata. Treat .tbss
+ as if it were a loaded section, and don't use match_type. */
+ bfd_boolean seen_thread_local = FALSE;
+
+ match_type = NULL;
+ for (look = first; look; look = look->next)
+ {
+ look_flags = look->flags;
+ if (look->bfd_section != NULL)
+ look_flags = look->bfd_section->flags;
+
+ differ = look_flags ^ (sec_flags | SEC_LOAD | SEC_HAS_CONTENTS);
+ if (!(differ & (SEC_THREAD_LOCAL | SEC_ALLOC)))
+ {
+ /* .tdata and .tbss must be adjacent and in that order. */
+ if (!(look_flags & SEC_LOAD)
+ && (sec_flags & SEC_LOAD))
+ /* ..so if we're at a .tbss section and we're placing
+ a .tdata section stop looking and return the
+ previous section. */
+ break;
+ found = look;
+ seen_thread_local = TRUE;
+ }
+ else if (seen_thread_local)
+ break;
+ else if (!(differ & (SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD)))
+ found = look;
+ }
+ }
+ else if ((sec_flags & SEC_SMALL_DATA) != 0
+ && (sec_flags & SEC_ALLOC) != 0)
{
/* .sdata goes after .data, .sbss after .sdata. */
for (look = first; look; look = look->next)
{
- flags = look->flags;
+ look_flags = look->flags;
if (look->bfd_section != NULL)
{
- flags = look->bfd_section->flags;
+ look_flags = look->bfd_section->flags;
if (match_type && !match_type (link_info.output_bfd,
look->bfd_section,
sec->owner, sec))
continue;
}
- flags ^= sec->flags;
- if (!(flags & (SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD
- | SEC_THREAD_LOCAL))
- || ((look->flags & SEC_SMALL_DATA)
- && !(sec->flags & SEC_HAS_CONTENTS)))
+ differ = look_flags ^ sec_flags;
+ if (!(differ & (SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD
+ | SEC_THREAD_LOCAL))
+ || ((look_flags & SEC_SMALL_DATA)
+ && !(sec_flags & SEC_HAS_CONTENTS)))
found = look;
}
}
- else if ((sec->flags & SEC_HAS_CONTENTS) != 0
- && (sec->flags & SEC_ALLOC) != 0)
+ else if ((sec_flags & SEC_HAS_CONTENTS) != 0
+ && (sec_flags & SEC_ALLOC) != 0)
{
/* .data goes after .rodata. */
for (look = first; look; look = look->next)
{
- flags = look->flags;
+ look_flags = look->flags;
if (look->bfd_section != NULL)
{
- flags = look->bfd_section->flags;
+ look_flags = look->bfd_section->flags;
if (match_type && !match_type (link_info.output_bfd,
look->bfd_section,
sec->owner, sec))
continue;
}
- flags ^= sec->flags;
- if (!(flags & (SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD
- | SEC_SMALL_DATA | SEC_THREAD_LOCAL)))
+ differ = look_flags ^ sec_flags;
+ if (!(differ & (SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD
+ | SEC_SMALL_DATA | SEC_THREAD_LOCAL)))
found = look;
}
}
- else if ((sec->flags & SEC_ALLOC) != 0)
+ else if ((sec_flags & SEC_ALLOC) != 0)
{
/* .bss goes after any other alloc section. */
for (look = first; look; look = look->next)
{
- flags = look->flags;
+ look_flags = look->flags;
if (look->bfd_section != NULL)
{
- flags = look->bfd_section->flags;
+ look_flags = look->bfd_section->flags;
if (match_type && !match_type (link_info.output_bfd,
look->bfd_section,
sec->owner, sec))
continue;
}
- flags ^= sec->flags;
- if (!(flags & SEC_ALLOC))
+ differ = look_flags ^ sec_flags;
+ if (!(differ & SEC_ALLOC))
found = look;
}
}
@@ -1644,11 +1674,11 @@ lang_output_section_find_by_flags (const asection *sec,
/* non-alloc go last. */
for (look = first; look; look = look->next)
{
- flags = look->flags;
+ look_flags = look->flags;
if (look->bfd_section != NULL)
- flags = look->bfd_section->flags;
- flags ^= sec->flags;
- if (!(flags & SEC_DEBUGGING))
+ look_flags = look->bfd_section->flags;
+ differ = look_flags ^ sec_flags;
+ if (!(differ & SEC_DEBUGGING))
found = look;
}
return found;
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: TLS orphan section placement
2014-01-24 12:54 ` TLS orphan section placement Alan Modra
@ 2014-01-24 17:07 ` H.J. Lu
2014-01-24 18:04 ` H.J. Lu
0 siblings, 1 reply; 7+ messages in thread
From: H.J. Lu @ 2014-01-24 17:07 UTC (permalink / raw)
To: Binutils
[-- Attachment #1: Type: text/plain, Size: 1753 bytes --]
On Fri, Jan 24, 2014 at 4:53 AM, Alan Modra <amodra@gmail.com> wrote:
> On Wed, Jan 22, 2014 at 08:19:31PM -0800, H.J. Lu wrote:
>> On Wed, Jan 22, 2014 at 7:55 PM, Alan Modra <amodra@gmail.com> wrote:
>> > On Wed, Jan 22, 2014 at 11:30:13AM -0800, H.J. Lu wrote:
>> >> PR ld/16498
>> >> * elf.c (_bfd_elf_map_sections_to_segments): Issue a linker error
>> >> if TLS sections are not adjacent.
>> >
>> > This part is OK.
>>
>> I'd like to augment my patch like
>
> Yes, this is OK too.
>
>> > On the other hand, the testcase is really showing a fault in orphan
>> > section handling. If ld was a little more clever, it would put .tbss
>> > after .tdata and your testcase would no longer give an error.
>> >
>>
>> I am enclosing a simple patch which does it. However, it
>> doesn't solve the second testcase in the bug report since
>> it has a linker script:
>>
>> ---
>> SECTIONS
>> {
>> tls_data_init : { *(.tdata .tdata.* .gnu.linkonce.td.*) }
>> .data :
>> {
>> *(.data .data.* .gnu.linkonce.d.*)
>> }
>> /DISCARD/ : { *(.note.GNU-stack) *(.gnu_debuglink) *(.gnu.lto_*) }
>> }
>> ---
>
> Besides the place_orphan change you posted, which is OK to commit,
> you need something like the following.
>
> Ensures TLS orphans are placed adjacent to existing TLS sections,
> and fixes places where the output_section_statement flags (which might
> not be set) were tested when bfd_section flags were available.
>
> * ldlang.c (lang_output_section_find_by_flags): Be careful to
> test look->bfd_section->flags if available rather than
> look->flags. Separate SEC_THREAD_LOCAL handling from
> SEC_READONLY loop, and rewrite.
>
These are 2 patches I checked in.
Thanks.
--
H.J.
[-- Attachment #2: 0001-Improve-orphaned-TLS-section-handling.patch --]
[-- Type: text/x-patch, Size: 4013 bytes --]
From d85e71fec0aa4d9d8ca0d8c2401cd8ab69fe2edc Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Wed, 22 Jan 2014 11:24:12 -0800
Subject: [PATCH 1/2] Improve orphaned TLS section handling
ld/
PR ld/16498
* emultempl/elf32.em (gld${EMULATION_NAME}_place_orphan): Improve
orphaned TLS section handling.
ld/testsuite/
PR ld/16498
* ld-elf/pr16498a.d: New file.
* ld-elf/pr16498a.s: Likewise.
* ld-elf/pr16498a.t: Likewise.
---
ld/ChangeLog | 6 ++++++
ld/emultempl/elf32.em | 6 ++++++
ld/testsuite/ChangeLog | 7 +++++++
ld/testsuite/ld-elf/pr16498a.d | 9 +++++++++
ld/testsuite/ld-elf/pr16498a.s | 23 +++++++++++++++++++++++
ld/testsuite/ld-elf/pr16498a.t | 6 ++++++
6 files changed, 57 insertions(+)
create mode 100644 ld/testsuite/ld-elf/pr16498a.d
create mode 100644 ld/testsuite/ld-elf/pr16498a.s
create mode 100644 ld/testsuite/ld-elf/pr16498a.t
diff --git a/ld/ChangeLog b/ld/ChangeLog
index 9beee8c..dcf0b15 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,9 @@
+2014-01-24 H.J. Lu <hongjiu.lu@intel.com>
+
+ PR ld/16498
+ * emultempl/elf32.em (gld${EMULATION_NAME}_place_orphan): Improve
+ orphaned TLS section handling.
+
2014-01-24 Alan Modra <amodra@gmail.com>
* ldlang.c (lang_output_section_find_by_flags): Be careful to
diff --git a/ld/emultempl/elf32.em b/ld/emultempl/elf32.em
index a4f04f1..fda0e68 100644
--- a/ld/emultempl/elf32.em
+++ b/ld/emultempl/elf32.em
@@ -1812,6 +1812,9 @@ gld${EMULATION_NAME}_place_orphan (asection *s,
{ ".rodata",
SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD | SEC_READONLY | SEC_DATA,
0, 0, 0, 0 },
+ { ".tdata",
+ SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD | SEC_DATA | SEC_THREAD_LOCAL,
+ 0, 0, 0, 0 },
{ ".data",
SEC_HAS_CONTENTS | SEC_ALLOC | SEC_LOAD | SEC_DATA,
0, 0, 0, 0 },
@@ -1835,6 +1838,7 @@ gld${EMULATION_NAME}_place_orphan (asection *s,
{
orphan_text = 0,
orphan_rodata,
+ orphan_tdata,
orphan_data,
orphan_bss,
orphan_rel,
@@ -1962,6 +1966,8 @@ gld${EMULATION_NAME}_place_orphan (asection *s,
place = &hold[orphan_bss];
else if ((s->flags & SEC_SMALL_DATA) != 0)
place = &hold[orphan_sdata];
+ else if ((s->flags & SEC_THREAD_LOCAL) != 0)
+ place = &hold[orphan_tdata];
else if ((s->flags & SEC_READONLY) == 0)
place = &hold[orphan_data];
else if (((iself && (sh_type == SHT_RELA || sh_type == SHT_REL))
diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
index a269b07..4f0a75b 100644
--- a/ld/testsuite/ChangeLog
+++ b/ld/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2014-01-24 H.J. Lu <hongjiu.lu@intel.com>
+
+ PR ld/16498
+ * ld-elf/pr16498a.d: New file.
+ * ld-elf/pr16498a.s: Likewise.
+ * ld-elf/pr16498a.t: Likewise.
+
2014-01-22 Alan Modra <amodra@gmail.com>
* ld-scripts/pr14962-2.d: Correct target triple.
diff --git a/ld/testsuite/ld-elf/pr16498a.d b/ld/testsuite/ld-elf/pr16498a.d
new file mode 100644
index 0000000..436bf97
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr16498a.d
@@ -0,0 +1,9 @@
+#ld: -shared -T pr16498a.t
+#readelf: -l --wide
+#target: *-*-linux* *-*-gnu* *-*-nacl*
+
+#...
+ TLS .*
+#...
+[ ]+[0-9]+[ ]+.tdata .tbss[ ]*
+#pass
diff --git a/ld/testsuite/ld-elf/pr16498a.s b/ld/testsuite/ld-elf/pr16498a.s
new file mode 100644
index 0000000..77f80e6
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr16498a.s
@@ -0,0 +1,23 @@
+ .globl data
+ .data
+ .align 32
+ .type data, %object
+ .size data, 120
+data:
+ .long 1
+ .zero 116
+ .globl foo
+ .section .tbss,"awT",%nobits
+ .align 4
+ .type foo, %object
+ .size foo, 4
+foo:
+ .zero 4
+ .globl bar
+ .section .tdata,"awT",%progbits
+ .align 16
+ .type bar, %object
+ .size bar, 80
+bar:
+ .long 1
+ .zero 76
diff --git a/ld/testsuite/ld-elf/pr16498a.t b/ld/testsuite/ld-elf/pr16498a.t
new file mode 100644
index 0000000..928724f
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr16498a.t
@@ -0,0 +1,6 @@
+SECTIONS
+{
+ .tdata : { *(.tdata) }
+ .data : { *(.data)
+ }
+}
--
1.8.4.2
[-- Attachment #3: 0002-Add-another-testcase-for-PR-ld-16498.patch --]
[-- Type: text/x-patch, Size: 1664 bytes --]
From a78ad74bbfbe2bee6b2909e9574892d38082e4ea Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 24 Jan 2014 09:03:21 -0800
Subject: [PATCH 2/2] Add another testcase for PR ld/16498
PR ld/16498
* ld-elf/pr16498b.d: New file.
* ld-elf/pr16498b.t: Likewise.
---
ld/testsuite/ChangeLog | 6 ++++++
ld/testsuite/ld-elf/pr16498b.d | 10 ++++++++++
ld/testsuite/ld-elf/pr16498b.t | 6 ++++++
3 files changed, 22 insertions(+)
create mode 100644 ld/testsuite/ld-elf/pr16498b.d
create mode 100644 ld/testsuite/ld-elf/pr16498b.t
diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
index 4f0a75b..9e8553a 100644
--- a/ld/testsuite/ChangeLog
+++ b/ld/testsuite/ChangeLog
@@ -1,6 +1,12 @@
2014-01-24 H.J. Lu <hongjiu.lu@intel.com>
PR ld/16498
+ * ld-elf/pr16498b.d: New file.
+ * ld-elf/pr16498b.t: Likewise.
+
+2014-01-24 H.J. Lu <hongjiu.lu@intel.com>
+
+ PR ld/16498
* ld-elf/pr16498a.d: New file.
* ld-elf/pr16498a.s: Likewise.
* ld-elf/pr16498a.t: Likewise.
diff --git a/ld/testsuite/ld-elf/pr16498b.d b/ld/testsuite/ld-elf/pr16498b.d
new file mode 100644
index 0000000..c70c239
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr16498b.d
@@ -0,0 +1,10 @@
+#source: pr16498a.s
+#ld: -shared -T pr16498b.t
+#readelf: -l --wide
+#target: *-*-linux* *-*-gnu* *-*-nacl*
+
+#...
+ TLS .*
+#...
+[ ]+[0-9]+[ ]+tls_data_init .tbss[ ]*
+#pass
diff --git a/ld/testsuite/ld-elf/pr16498b.t b/ld/testsuite/ld-elf/pr16498b.t
new file mode 100644
index 0000000..b88f9b8
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr16498b.t
@@ -0,0 +1,6 @@
+SECTIONS
+{
+ tls_data_init : { *(.tdata) }
+ .data : { *(.data)
+ }
+}
--
1.8.4.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: TLS orphan section placement
2014-01-24 17:07 ` H.J. Lu
@ 2014-01-24 18:04 ` H.J. Lu
0 siblings, 0 replies; 7+ messages in thread
From: H.J. Lu @ 2014-01-24 18:04 UTC (permalink / raw)
To: Binutils
On Fri, Jan 24, 2014 at 9:07 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Jan 24, 2014 at 4:53 AM, Alan Modra <amodra@gmail.com> wrote:
>> On Wed, Jan 22, 2014 at 08:19:31PM -0800, H.J. Lu wrote:
>>> On Wed, Jan 22, 2014 at 7:55 PM, Alan Modra <amodra@gmail.com> wrote:
>>> > On Wed, Jan 22, 2014 at 11:30:13AM -0800, H.J. Lu wrote:
>>> >> PR ld/16498
>>> >> * elf.c (_bfd_elf_map_sections_to_segments): Issue a linker error
>>> >> if TLS sections are not adjacent.
>>> >
>>> > This part is OK.
>>>
>>> I'd like to augment my patch like
>>
>> Yes, this is OK too.
>>
>>> > On the other hand, the testcase is really showing a fault in orphan
>>> > section handling. If ld was a little more clever, it would put .tbss
>>> > after .tdata and your testcase would no longer give an error.
>>> >
>>>
>>> I am enclosing a simple patch which does it. However, it
>>> doesn't solve the second testcase in the bug report since
>>> it has a linker script:
>>>
>>> ---
>>> SECTIONS
>>> {
>>> tls_data_init : { *(.tdata .tdata.* .gnu.linkonce.td.*) }
>>> .data :
>>> {
>>> *(.data .data.* .gnu.linkonce.d.*)
>>> }
>>> /DISCARD/ : { *(.note.GNU-stack) *(.gnu_debuglink) *(.gnu.lto_*) }
>>> }
>>> ---
>>
>> Besides the place_orphan change you posted, which is OK to commit,
>> you need something like the following.
>>
>> Ensures TLS orphans are placed adjacent to existing TLS sections,
>> and fixes places where the output_section_statement flags (which might
>> not be set) were tested when bfd_section flags were available.
>>
>> * ldlang.c (lang_output_section_find_by_flags): Be careful to
>> test look->bfd_section->flags if available rather than
>> look->flags. Separate SEC_THREAD_LOCAL handling from
>> SEC_READONLY loop, and rewrite.
>>
>
> These are 2 patches I checked in.
>
I checked in this patch to replace .align with .p2align.
--
H.J.
--
diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
index 9e8553a..39cc0fb 100644
--- a/ld/testsuite/ChangeLog
+++ b/ld/testsuite/ChangeLog
@@ -1,5 +1,9 @@
2014-01-24 H.J. Lu <hongjiu.lu@intel.com>
+ * ld-elf/pr16498a.s: Replace .align with .p2align.
+
+2014-01-24 H.J. Lu <hongjiu.lu@intel.com>
+
PR ld/16498
* ld-elf/pr16498b.d: New file.
* ld-elf/pr16498b.t: Likewise.
diff --git a/ld/testsuite/ld-elf/pr16498a.s b/ld/testsuite/ld-elf/pr16498a.s
index 77f80e6..be503a2 100644
--- a/ld/testsuite/ld-elf/pr16498a.s
+++ b/ld/testsuite/ld-elf/pr16498a.s
@@ -1,6 +1,6 @@
.globl data
.data
- .align 32
+ .p2align 5
.type data, %object
.size data, 120
data:
@@ -8,14 +8,14 @@ data:
.zero 116
.globl foo
.section .tbss,"awT",%nobits
- .align 4
+ .p2align 2
.type foo, %object
.size foo, 4
foo:
.zero 4
.globl bar
.section .tdata,"awT",%progbits
- .align 16
+ .p2align 4
.type bar, %object
.size bar, 80
bar:
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-01-24 18:04 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-22 19:30 [PATCH] Issue a linker error if TLS sections are not adjacent H.J. Lu
2014-01-23 3:55 ` Alan Modra
2014-01-23 4:19 ` H.J. Lu
2014-01-23 16:17 ` H.J. Lu
2014-01-24 12:54 ` TLS orphan section placement Alan Modra
2014-01-24 17:07 ` H.J. Lu
2014-01-24 18:04 ` H.J. Lu
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).