* [PATCH] PR ld/20828: Move symbol version processing ahead of GC symbol sweep
@ 2017-02-01 19:31 Maciej W. Rozycki
2017-02-02 3:25 ` Alan Modra
0 siblings, 1 reply; 10+ messages in thread
From: Maciej W. Rozycki @ 2017-02-01 19:31 UTC (permalink / raw)
To: Alan Modra, Nick Clifton, James Cowgill; +Cc: binutils
Complement commit b531344c34b0 ("PR ld/20828: Reorder the symbol sweep
stage of section GC") and commit 81ff47b3a546 ("PR ld/20828: Fix linker
script symbols wrongly forced local with section GC") and move symbol
version processing ahead of the symbol sweep stage of section GC, all in
`bfd_elf_size_dynamic_sections', so that version symbols created stay in
the global scope and are not output as local symbols to the dynamic symbol
table in the presence of corresponding symbol definitions pulled from a
DSO involved in a link.
Consolidate the whole of symbol version processing into a single block
from all parts scattered across the function and rearranging the local
variables used as necessary, however leaving the setting of dynamic
entries associated with the DT_VERDEF, DT_VERDEFNUM, DT_VERNEED and
DT_VERNEEDNUM tags in the original places.
With the rearrangement of code blocks `Elf_Internal_Verneed *t' would
shadow the previous definition of `struct bfd_elf_version_tree *t', so
rename the former variable to `vn'.
bfd/
PR ld/20828
* elflink.c (bfd_elf_size_dynamic_sections): Move symbol version
processing ahead of the call to `elf_gc_sweep_symbol'.
ld/
PR ld/20828
* testsuite/ld-elf/pr20828-d.sd: New test.
* testsuite/ld-elf/pr20828-e.sd: New test.
* testsuite/ld-elf/pr20828-v.ver: New test version script.
* testsuite/ld-elf/pr20828.ld: Add `.gnu.version' and
`.gnu.version_d'.
* testsuite/ld-elf/shared.exp: Run the new tests.
---
I had to do a little bit of thinking to figure out how to verify across
all targets that this somewhat radical change does not cause any bad
interaction between version processing in `bfd_elf_size_dynamic_sections'
and the two backend methods (`->elf_backend_always_size_sections' and
`->elf_backend_size_dynamic_sections') called midway through. So I asked
myself what could actually cause a problem here and eventually came to a
conclusion that the only scenario I can think of is where one of the
methods adds an exported symbol which currently goes through the usual
version processing, but would not anymore after this change.
To check if that is the case I have extracted, with the aid of a
combination of usual *nix text processing tools, all the methods and
looked through them for symbol creation. And indeed a couple do, namely:
`elf32_arm_always_size_sections', `elfNN_aarch64_always_size_sections'
`elf_i386_always_size_sections', `elf_x86_64_always_size_sections', and
`elf_xtensa_always_size_sections', all of which add an _TLS_MODULE_BASE_
symbol, which however has a local scope and is made hidden right away, so
it does get any interesting changes applied through version script
processing.
And that was just it. So it appears to me we should be safe with my
proposed fix unless there's something particularly peculiar somewhere that
I have missed (which would not be unheard of, as for example it was with
the S+core target earlier on in the course of addressing this PR). I have
got good regression test results across my current set of 175 targets
though, which are reassuring in that at least this change does not break
something we have previously recognised.
In addition to that, prompted by the `cris-elf' ld-cris/libdso-12c.d and
ld-cris/libdso-13b.d tests I have cowardly decided to keep the order of
dynamic entries produced unchanged, in case a third-party piece of
software relies on it (even though it shouldn't), and therefore I have
moved the creation of the 4 types used with symbol versioning (VERDEF,
VERDEFNUM, VERNEED, VERNEEDNUM) away from the calculations of the
corresponding values and down towards the end of the function where they
are currently made. There's no harm from doing that IMO, even if only to
satisfy human readers who may have got used over the years to how the
entries have been ordered in dumps.
Unfortunately due to how `diff' decided to record the changes the patch
looks more complicated than it really is, as the parts which I *haven't*
moved are shown rather than those I did move. So to look what has changed
it may be easier to visually compare the unchanged vs updated version of
`bfd_elf_size_dynamic_sections' side by side.
I do hope I haven't missed anything then, but I will be happy to address
any concerns this proposal may raise. Otherwise, OK to apply to master?
Maciej
binutils-bfd-elf-size-dynamic-sections-version.diff
Index: binutils/bfd/elflink.c
===================================================================
--- binutils.orig/bfd/elflink.c 2017-01-30 22:51:09.000000000 +0000
+++ binutils/bfd/elflink.c 2017-01-31 04:04:19.446661678 +0000
@@ -5925,7 +5925,6 @@ bfd_elf_size_dynamic_sections (bfd *outp
size_t soname_indx;
bfd *dynobj;
const struct elf_backend_data *bed;
- struct elf_info_failed asvinfo;
*sinterpptr = NULL;
@@ -5934,188 +5933,17 @@ bfd_elf_size_dynamic_sections (bfd *outp
if (!is_elf_hash_table (info->hash))
return TRUE;
- bed = get_elf_backend_data (output_bfd);
-
- if (info->gc_sections && bed->can_gc_sections)
- {
- struct elf_gc_sweep_symbol_info sweep_info;
- unsigned long section_sym_count;
-
- /* Remove the symbols that were in the swept sections from the
- dynamic symbol table. GCFIXME: Anyone know how to get them
- out of the static symbol table as well? */
- sweep_info.info = info;
- sweep_info.hide_symbol = bed->elf_backend_hide_symbol;
- elf_link_hash_traverse (elf_hash_table (info), elf_gc_sweep_symbol,
- &sweep_info);
-
- _bfd_elf_link_renumber_dynsyms (output_bfd, info, §ion_sym_count);
- }
-
- /* Any syms created from now on start with -1 in
- got.refcount/offset and plt.refcount/offset. */
- elf_hash_table (info)->init_got_refcount
- = elf_hash_table (info)->init_got_offset;
- elf_hash_table (info)->init_plt_refcount
- = elf_hash_table (info)->init_plt_offset;
-
- if (bfd_link_relocatable (info)
- && !_bfd_elf_size_group_sections (info))
- return FALSE;
-
- /* The backend may have to create some sections regardless of whether
- we're dynamic or not. */
- if (bed->elf_backend_always_size_sections
- && ! (*bed->elf_backend_always_size_sections) (output_bfd, info))
- return FALSE;
-
- /* Determine any GNU_STACK segment requirements, after the backend
- has had a chance to set a default segment size. */
- if (info->execstack)
- elf_stack_flags (output_bfd) = PF_R | PF_W | PF_X;
- else if (info->noexecstack)
- elf_stack_flags (output_bfd) = PF_R | PF_W;
- else
- {
- bfd *inputobj;
- asection *notesec = NULL;
- int exec = 0;
-
- for (inputobj = info->input_bfds;
- inputobj;
- inputobj = inputobj->link.next)
- {
- asection *s;
-
- if (inputobj->flags
- & (DYNAMIC | EXEC_P | BFD_PLUGIN | BFD_LINKER_CREATED))
- continue;
- s = bfd_get_section_by_name (inputobj, ".note.GNU-stack");
- if (s)
- {
- if (s->flags & SEC_CODE)
- exec = PF_X;
- notesec = s;
- }
- else if (bed->default_execstack)
- exec = PF_X;
- }
- if (notesec || info->stacksize > 0)
- elf_stack_flags (output_bfd) = PF_R | PF_W | exec;
- if (notesec && exec && bfd_link_relocatable (info)
- && notesec->output_section != bfd_abs_section_ptr)
- notesec->output_section->flags |= SEC_CODE;
- }
-
dynobj = elf_hash_table (info)->dynobj;
if (dynobj != NULL && elf_hash_table (info)->dynamic_sections_created)
{
- struct elf_info_failed eif;
- struct elf_link_hash_entry *h;
- asection *dynstr;
+ struct bfd_elf_version_tree *verdefs;
+ unsigned long section_sym_count;
+ struct elf_info_failed asvinfo;
struct bfd_elf_version_tree *t;
struct bfd_elf_version_expr *d;
- asection *s;
bfd_boolean all_defined;
-
- *sinterpptr = bfd_get_linker_section (dynobj, ".interp");
- BFD_ASSERT (*sinterpptr != NULL || !bfd_link_executable (info) || info->nointerp);
-
- if (soname != NULL)
- {
- soname_indx = _bfd_elf_strtab_add (elf_hash_table (info)->dynstr,
- soname, TRUE);
- if (soname_indx == (size_t) -1
- || !_bfd_elf_add_dynamic_entry (info, DT_SONAME, soname_indx))
- return FALSE;
- }
-
- if (info->symbolic)
- {
- if (!_bfd_elf_add_dynamic_entry (info, DT_SYMBOLIC, 0))
- return FALSE;
- info->flags |= DF_SYMBOLIC;
- }
-
- if (rpath != NULL)
- {
- size_t indx;
- bfd_vma tag;
-
- indx = _bfd_elf_strtab_add (elf_hash_table (info)->dynstr, rpath,
- TRUE);
- if (indx == (size_t) -1)
- return FALSE;
-
- tag = info->new_dtags ? DT_RUNPATH : DT_RPATH;
- if (!_bfd_elf_add_dynamic_entry (info, tag, indx))
- return FALSE;
- }
-
- if (filter_shlib != NULL)
- {
- size_t indx;
-
- indx = _bfd_elf_strtab_add (elf_hash_table (info)->dynstr,
- filter_shlib, TRUE);
- if (indx == (size_t) -1
- || !_bfd_elf_add_dynamic_entry (info, DT_FILTER, indx))
- return FALSE;
- }
-
- if (auxiliary_filters != NULL)
- {
- const char * const *p;
-
- for (p = auxiliary_filters; *p != NULL; p++)
- {
- size_t indx;
-
- indx = _bfd_elf_strtab_add (elf_hash_table (info)->dynstr,
- *p, TRUE);
- if (indx == (size_t) -1
- || !_bfd_elf_add_dynamic_entry (info, DT_AUXILIARY, indx))
- return FALSE;
- }
- }
-
- if (audit != NULL)
- {
- size_t indx;
-
- indx = _bfd_elf_strtab_add (elf_hash_table (info)->dynstr, audit,
- TRUE);
- if (indx == (size_t) -1
- || !_bfd_elf_add_dynamic_entry (info, DT_AUDIT, indx))
- return FALSE;
- }
-
- if (depaudit != NULL)
- {
- size_t indx;
-
- indx = _bfd_elf_strtab_add (elf_hash_table (info)->dynstr, depaudit,
- TRUE);
- if (indx == (size_t) -1
- || !_bfd_elf_add_dynamic_entry (info, DT_DEPAUDIT, indx))
- return FALSE;
- }
-
- eif.info = info;
- eif.failed = FALSE;
-
- /* If we are supposed to export all symbols into the dynamic symbol
- table (this is not the normal case), then do so. */
- if (info->export_dynamic
- || (bfd_link_executable (info) && info->dynamic))
- {
- elf_link_hash_traverse (elf_hash_table (info),
- _bfd_elf_export_symbol,
- &eif);
- if (eif.failed)
- return FALSE;
- }
+ asection *s;
/* Make all global versions with definition. */
for (t = info->version_info; t != NULL; t = t->next)
@@ -6200,129 +6028,6 @@ bfd_elf_size_dynamic_sections (bfd *outp
}
}
- /* Find all symbols which were defined in a dynamic object and make
- the backend pick a reasonable value for them. */
- elf_link_hash_traverse (elf_hash_table (info),
- _bfd_elf_adjust_dynamic_symbol,
- &eif);
- if (eif.failed)
- return FALSE;
-
- /* Add some entries to the .dynamic section. We fill in some of the
- values later, in bfd_elf_final_link, but we must add the entries
- now so that we know the final size of the .dynamic section. */
-
- /* If there are initialization and/or finalization functions to
- call then add the corresponding DT_INIT/DT_FINI entries. */
- h = (info->init_function
- ? elf_link_hash_lookup (elf_hash_table (info),
- info->init_function, FALSE,
- FALSE, FALSE)
- : NULL);
- if (h != NULL
- && (h->ref_regular
- || h->def_regular))
- {
- if (!_bfd_elf_add_dynamic_entry (info, DT_INIT, 0))
- return FALSE;
- }
- h = (info->fini_function
- ? elf_link_hash_lookup (elf_hash_table (info),
- info->fini_function, FALSE,
- FALSE, FALSE)
- : NULL);
- if (h != NULL
- && (h->ref_regular
- || h->def_regular))
- {
- if (!_bfd_elf_add_dynamic_entry (info, DT_FINI, 0))
- return FALSE;
- }
-
- s = bfd_get_section_by_name (output_bfd, ".preinit_array");
- if (s != NULL && s->linker_has_input)
- {
- /* DT_PREINIT_ARRAY is not allowed in shared library. */
- if (! bfd_link_executable (info))
- {
- bfd *sub;
- asection *o;
-
- for (sub = info->input_bfds; sub != NULL;
- sub = sub->link.next)
- if (bfd_get_flavour (sub) == bfd_target_elf_flavour)
- for (o = sub->sections; o != NULL; o = o->next)
- if (elf_section_data (o)->this_hdr.sh_type
- == SHT_PREINIT_ARRAY)
- {
- _bfd_error_handler
- (_("%B: .preinit_array section is not allowed in DSO"),
- sub);
- break;
- }
-
- bfd_set_error (bfd_error_nonrepresentable_section);
- return FALSE;
- }
-
- if (!_bfd_elf_add_dynamic_entry (info, DT_PREINIT_ARRAY, 0)
- || !_bfd_elf_add_dynamic_entry (info, DT_PREINIT_ARRAYSZ, 0))
- return FALSE;
- }
- s = bfd_get_section_by_name (output_bfd, ".init_array");
- if (s != NULL && s->linker_has_input)
- {
- if (!_bfd_elf_add_dynamic_entry (info, DT_INIT_ARRAY, 0)
- || !_bfd_elf_add_dynamic_entry (info, DT_INIT_ARRAYSZ, 0))
- return FALSE;
- }
- s = bfd_get_section_by_name (output_bfd, ".fini_array");
- if (s != NULL && s->linker_has_input)
- {
- if (!_bfd_elf_add_dynamic_entry (info, DT_FINI_ARRAY, 0)
- || !_bfd_elf_add_dynamic_entry (info, DT_FINI_ARRAYSZ, 0))
- return FALSE;
- }
-
- dynstr = bfd_get_linker_section (dynobj, ".dynstr");
- /* If .dynstr is excluded from the link, we don't want any of
- these tags. Strictly, we should be checking each section
- individually; This quick check covers for the case where
- someone does a /DISCARD/ : { *(*) }. */
- if (dynstr != NULL && dynstr->output_section != bfd_abs_section_ptr)
- {
- bfd_size_type strsize;
-
- strsize = _bfd_elf_strtab_size (elf_hash_table (info)->dynstr);
- if ((info->emit_hash
- && !_bfd_elf_add_dynamic_entry (info, DT_HASH, 0))
- || (info->emit_gnu_hash
- && !_bfd_elf_add_dynamic_entry (info, DT_GNU_HASH, 0))
- || !_bfd_elf_add_dynamic_entry (info, DT_STRTAB, 0)
- || !_bfd_elf_add_dynamic_entry (info, DT_SYMTAB, 0)
- || !_bfd_elf_add_dynamic_entry (info, DT_STRSZ, strsize)
- || !_bfd_elf_add_dynamic_entry (info, DT_SYMENT,
- bed->s->sizeof_sym))
- return FALSE;
- }
- }
-
- if (! _bfd_elf_maybe_strip_eh_frame_hdr (info))
- return FALSE;
-
- /* The backend must work out the sizes of all the other dynamic
- sections. */
- if (dynobj != NULL
- && bed->elf_backend_size_dynamic_sections != NULL
- && ! (*bed->elf_backend_size_dynamic_sections) (output_bfd, info))
- return FALSE;
-
- if (dynobj != NULL && elf_hash_table (info)->dynamic_sections_created)
- {
- unsigned long section_sym_count;
- struct bfd_elf_version_tree *verdefs;
- asection *s;
-
/* Set up the version definition section. */
s = bfd_get_linker_section (dynobj, ".gnu.version_d");
BFD_ASSERT (s != NULL);
@@ -6341,7 +6046,6 @@ bfd_elf_size_dynamic_sections (bfd *outp
{
unsigned int cdefs;
bfd_size_type size;
- struct bfd_elf_version_tree *t;
bfd_byte *p;
Elf_Internal_Verdef def;
Elf_Internal_Verdaux defaux;
@@ -6557,34 +6261,9 @@ bfd_elf_size_dynamic_sections (bfd *outp
}
}
- if (!_bfd_elf_add_dynamic_entry (info, DT_VERDEF, 0)
- || !_bfd_elf_add_dynamic_entry (info, DT_VERDEFNUM, cdefs))
- return FALSE;
-
elf_tdata (output_bfd)->cverdefs = cdefs;
}
- if ((info->new_dtags && info->flags) || (info->flags & DF_STATIC_TLS))
- {
- if (!_bfd_elf_add_dynamic_entry (info, DT_FLAGS, info->flags))
- return FALSE;
- }
- else if (info->flags & DF_BIND_NOW)
- {
- if (!_bfd_elf_add_dynamic_entry (info, DT_BIND_NOW, 0))
- return FALSE;
- }
-
- if (info->flags_1)
- {
- if (bfd_link_executable (info))
- info->flags_1 &= ~ (DF_1_INITFIRST
- | DF_1_NODELETE
- | DF_1_NOOPEN);
- if (!_bfd_elf_add_dynamic_entry (info, DT_FLAGS_1, info->flags_1))
- return FALSE;
- }
-
/* Work out the size of the version reference section. */
s = bfd_get_linker_section (dynobj, ".gnu.version_r");
@@ -6608,7 +6287,7 @@ bfd_elf_size_dynamic_sections (bfd *outp
s->flags |= SEC_EXCLUDE;
else
{
- Elf_Internal_Verneed *t;
+ Elf_Internal_Verneed *vn;
unsigned int size;
unsigned int crefs;
bfd_byte *p;
@@ -6616,15 +6295,15 @@ bfd_elf_size_dynamic_sections (bfd *outp
/* Build the version dependency section. */
size = 0;
crefs = 0;
- for (t = elf_tdata (output_bfd)->verref;
- t != NULL;
- t = t->vn_nextref)
+ for (vn = elf_tdata (output_bfd)->verref;
+ vn != NULL;
+ vn = vn->vn_nextref)
{
Elf_Internal_Vernaux *a;
size += sizeof (Elf_External_Verneed);
++crefs;
- for (a = t->vn_auxptr; a != NULL; a = a->vna_nextptr)
+ for (a = vn->vn_auxptr; a != NULL; a = a->vna_nextptr)
size += sizeof (Elf_External_Vernaux);
}
@@ -6634,40 +6313,40 @@ bfd_elf_size_dynamic_sections (bfd *outp
return FALSE;
p = s->contents;
- for (t = elf_tdata (output_bfd)->verref;
- t != NULL;
- t = t->vn_nextref)
+ for (vn = elf_tdata (output_bfd)->verref;
+ vn != NULL;
+ vn = vn->vn_nextref)
{
unsigned int caux;
Elf_Internal_Vernaux *a;
size_t indx;
caux = 0;
- for (a = t->vn_auxptr; a != NULL; a = a->vna_nextptr)
+ for (a = vn->vn_auxptr; a != NULL; a = a->vna_nextptr)
++caux;
- t->vn_version = VER_NEED_CURRENT;
- t->vn_cnt = caux;
+ vn->vn_version = VER_NEED_CURRENT;
+ vn->vn_cnt = caux;
indx = _bfd_elf_strtab_add (elf_hash_table (info)->dynstr,
- elf_dt_name (t->vn_bfd) != NULL
- ? elf_dt_name (t->vn_bfd)
- : lbasename (t->vn_bfd->filename),
+ elf_dt_name (vn->vn_bfd) != NULL
+ ? elf_dt_name (vn->vn_bfd)
+ : lbasename (vn->vn_bfd->filename),
FALSE);
if (indx == (size_t) -1)
return FALSE;
- t->vn_file = indx;
- t->vn_aux = sizeof (Elf_External_Verneed);
- if (t->vn_nextref == NULL)
- t->vn_next = 0;
+ vn->vn_file = indx;
+ vn->vn_aux = sizeof (Elf_External_Verneed);
+ if (vn->vn_nextref == NULL)
+ vn->vn_next = 0;
else
- t->vn_next = (sizeof (Elf_External_Verneed)
+ vn->vn_next = (sizeof (Elf_External_Verneed)
+ caux * sizeof (Elf_External_Vernaux));
- _bfd_elf_swap_verneed_out (output_bfd, t,
+ _bfd_elf_swap_verneed_out (output_bfd, vn,
(Elf_External_Verneed *) p);
p += sizeof (Elf_External_Verneed);
- for (a = t->vn_auxptr; a != NULL; a = a->vna_nextptr)
+ for (a = vn->vn_auxptr; a != NULL; a = a->vna_nextptr)
{
a->vna_hash = bfd_elf_hash (a->vna_nodename);
indx = _bfd_elf_strtab_add (elf_hash_table (info)->dynstr,
@@ -6686,10 +6365,6 @@ bfd_elf_size_dynamic_sections (bfd *outp
}
}
- if (!_bfd_elf_add_dynamic_entry (info, DT_VERNEED, 0)
- || !_bfd_elf_add_dynamic_entry (info, DT_VERNEEDNUM, crefs))
- return FALSE;
-
elf_tdata (output_bfd)->cverrefs = crefs;
}
}
@@ -6703,6 +6378,343 @@ bfd_elf_size_dynamic_sections (bfd *outp
s->flags |= SEC_EXCLUDE;
}
}
+
+ bed = get_elf_backend_data (output_bfd);
+
+ if (info->gc_sections && bed->can_gc_sections)
+ {
+ struct elf_gc_sweep_symbol_info sweep_info;
+ unsigned long section_sym_count;
+
+ /* Remove the symbols that were in the swept sections from the
+ dynamic symbol table. GCFIXME: Anyone know how to get them
+ out of the static symbol table as well? */
+ sweep_info.info = info;
+ sweep_info.hide_symbol = bed->elf_backend_hide_symbol;
+ elf_link_hash_traverse (elf_hash_table (info), elf_gc_sweep_symbol,
+ &sweep_info);
+
+ _bfd_elf_link_renumber_dynsyms (output_bfd, info, §ion_sym_count);
+ }
+
+ /* Any syms created from now on start with -1 in
+ got.refcount/offset and plt.refcount/offset. */
+ elf_hash_table (info)->init_got_refcount
+ = elf_hash_table (info)->init_got_offset;
+ elf_hash_table (info)->init_plt_refcount
+ = elf_hash_table (info)->init_plt_offset;
+
+ if (bfd_link_relocatable (info)
+ && !_bfd_elf_size_group_sections (info))
+ return FALSE;
+
+ /* The backend may have to create some sections regardless of whether
+ we're dynamic or not. */
+ if (bed->elf_backend_always_size_sections
+ && ! (*bed->elf_backend_always_size_sections) (output_bfd, info))
+ return FALSE;
+
+ /* Determine any GNU_STACK segment requirements, after the backend
+ has had a chance to set a default segment size. */
+ if (info->execstack)
+ elf_stack_flags (output_bfd) = PF_R | PF_W | PF_X;
+ else if (info->noexecstack)
+ elf_stack_flags (output_bfd) = PF_R | PF_W;
+ else
+ {
+ bfd *inputobj;
+ asection *notesec = NULL;
+ int exec = 0;
+
+ for (inputobj = info->input_bfds;
+ inputobj;
+ inputobj = inputobj->link.next)
+ {
+ asection *s;
+
+ if (inputobj->flags
+ & (DYNAMIC | EXEC_P | BFD_PLUGIN | BFD_LINKER_CREATED))
+ continue;
+ s = bfd_get_section_by_name (inputobj, ".note.GNU-stack");
+ if (s)
+ {
+ if (s->flags & SEC_CODE)
+ exec = PF_X;
+ notesec = s;
+ }
+ else if (bed->default_execstack)
+ exec = PF_X;
+ }
+ if (notesec || info->stacksize > 0)
+ elf_stack_flags (output_bfd) = PF_R | PF_W | exec;
+ if (notesec && exec && bfd_link_relocatable (info)
+ && notesec->output_section != bfd_abs_section_ptr)
+ notesec->output_section->flags |= SEC_CODE;
+ }
+
+ if (dynobj != NULL && elf_hash_table (info)->dynamic_sections_created)
+ {
+ struct elf_info_failed eif;
+ struct elf_link_hash_entry *h;
+ asection *dynstr;
+ asection *s;
+
+ *sinterpptr = bfd_get_linker_section (dynobj, ".interp");
+ BFD_ASSERT (*sinterpptr != NULL || !bfd_link_executable (info) || info->nointerp);
+
+ if (soname != NULL)
+ {
+ soname_indx = _bfd_elf_strtab_add (elf_hash_table (info)->dynstr,
+ soname, TRUE);
+ if (soname_indx == (size_t) -1
+ || !_bfd_elf_add_dynamic_entry (info, DT_SONAME, soname_indx))
+ return FALSE;
+ }
+
+ if (info->symbolic)
+ {
+ if (!_bfd_elf_add_dynamic_entry (info, DT_SYMBOLIC, 0))
+ return FALSE;
+ info->flags |= DF_SYMBOLIC;
+ }
+
+ if (rpath != NULL)
+ {
+ size_t indx;
+ bfd_vma tag;
+
+ indx = _bfd_elf_strtab_add (elf_hash_table (info)->dynstr, rpath,
+ TRUE);
+ if (indx == (size_t) -1)
+ return FALSE;
+
+ tag = info->new_dtags ? DT_RUNPATH : DT_RPATH;
+ if (!_bfd_elf_add_dynamic_entry (info, tag, indx))
+ return FALSE;
+ }
+
+ if (filter_shlib != NULL)
+ {
+ size_t indx;
+
+ indx = _bfd_elf_strtab_add (elf_hash_table (info)->dynstr,
+ filter_shlib, TRUE);
+ if (indx == (size_t) -1
+ || !_bfd_elf_add_dynamic_entry (info, DT_FILTER, indx))
+ return FALSE;
+ }
+
+ if (auxiliary_filters != NULL)
+ {
+ const char * const *p;
+
+ for (p = auxiliary_filters; *p != NULL; p++)
+ {
+ size_t indx;
+
+ indx = _bfd_elf_strtab_add (elf_hash_table (info)->dynstr,
+ *p, TRUE);
+ if (indx == (size_t) -1
+ || !_bfd_elf_add_dynamic_entry (info, DT_AUXILIARY, indx))
+ return FALSE;
+ }
+ }
+
+ if (audit != NULL)
+ {
+ size_t indx;
+
+ indx = _bfd_elf_strtab_add (elf_hash_table (info)->dynstr, audit,
+ TRUE);
+ if (indx == (size_t) -1
+ || !_bfd_elf_add_dynamic_entry (info, DT_AUDIT, indx))
+ return FALSE;
+ }
+
+ if (depaudit != NULL)
+ {
+ size_t indx;
+
+ indx = _bfd_elf_strtab_add (elf_hash_table (info)->dynstr, depaudit,
+ TRUE);
+ if (indx == (size_t) -1
+ || !_bfd_elf_add_dynamic_entry (info, DT_DEPAUDIT, indx))
+ return FALSE;
+ }
+
+ eif.info = info;
+ eif.failed = FALSE;
+
+ /* If we are supposed to export all symbols into the dynamic symbol
+ table (this is not the normal case), then do so. */
+ if (info->export_dynamic
+ || (bfd_link_executable (info) && info->dynamic))
+ {
+ elf_link_hash_traverse (elf_hash_table (info),
+ _bfd_elf_export_symbol,
+ &eif);
+ if (eif.failed)
+ return FALSE;
+ }
+
+ /* Find all symbols which were defined in a dynamic object and make
+ the backend pick a reasonable value for them. */
+ elf_link_hash_traverse (elf_hash_table (info),
+ _bfd_elf_adjust_dynamic_symbol,
+ &eif);
+ if (eif.failed)
+ return FALSE;
+
+ /* Add some entries to the .dynamic section. We fill in some of the
+ values later, in bfd_elf_final_link, but we must add the entries
+ now so that we know the final size of the .dynamic section. */
+
+ /* If there are initialization and/or finalization functions to
+ call then add the corresponding DT_INIT/DT_FINI entries. */
+ h = (info->init_function
+ ? elf_link_hash_lookup (elf_hash_table (info),
+ info->init_function, FALSE,
+ FALSE, FALSE)
+ : NULL);
+ if (h != NULL
+ && (h->ref_regular
+ || h->def_regular))
+ {
+ if (!_bfd_elf_add_dynamic_entry (info, DT_INIT, 0))
+ return FALSE;
+ }
+ h = (info->fini_function
+ ? elf_link_hash_lookup (elf_hash_table (info),
+ info->fini_function, FALSE,
+ FALSE, FALSE)
+ : NULL);
+ if (h != NULL
+ && (h->ref_regular
+ || h->def_regular))
+ {
+ if (!_bfd_elf_add_dynamic_entry (info, DT_FINI, 0))
+ return FALSE;
+ }
+
+ s = bfd_get_section_by_name (output_bfd, ".preinit_array");
+ if (s != NULL && s->linker_has_input)
+ {
+ /* DT_PREINIT_ARRAY is not allowed in shared library. */
+ if (! bfd_link_executable (info))
+ {
+ bfd *sub;
+ asection *o;
+
+ for (sub = info->input_bfds; sub != NULL;
+ sub = sub->link.next)
+ if (bfd_get_flavour (sub) == bfd_target_elf_flavour)
+ for (o = sub->sections; o != NULL; o = o->next)
+ if (elf_section_data (o)->this_hdr.sh_type
+ == SHT_PREINIT_ARRAY)
+ {
+ _bfd_error_handler
+ (_("%B: .preinit_array section is not allowed in DSO"),
+ sub);
+ break;
+ }
+
+ bfd_set_error (bfd_error_nonrepresentable_section);
+ return FALSE;
+ }
+
+ if (!_bfd_elf_add_dynamic_entry (info, DT_PREINIT_ARRAY, 0)
+ || !_bfd_elf_add_dynamic_entry (info, DT_PREINIT_ARRAYSZ, 0))
+ return FALSE;
+ }
+ s = bfd_get_section_by_name (output_bfd, ".init_array");
+ if (s != NULL && s->linker_has_input)
+ {
+ if (!_bfd_elf_add_dynamic_entry (info, DT_INIT_ARRAY, 0)
+ || !_bfd_elf_add_dynamic_entry (info, DT_INIT_ARRAYSZ, 0))
+ return FALSE;
+ }
+ s = bfd_get_section_by_name (output_bfd, ".fini_array");
+ if (s != NULL && s->linker_has_input)
+ {
+ if (!_bfd_elf_add_dynamic_entry (info, DT_FINI_ARRAY, 0)
+ || !_bfd_elf_add_dynamic_entry (info, DT_FINI_ARRAYSZ, 0))
+ return FALSE;
+ }
+
+ dynstr = bfd_get_linker_section (dynobj, ".dynstr");
+ /* If .dynstr is excluded from the link, we don't want any of
+ these tags. Strictly, we should be checking each section
+ individually; This quick check covers for the case where
+ someone does a /DISCARD/ : { *(*) }. */
+ if (dynstr != NULL && dynstr->output_section != bfd_abs_section_ptr)
+ {
+ bfd_size_type strsize;
+
+ strsize = _bfd_elf_strtab_size (elf_hash_table (info)->dynstr);
+ if ((info->emit_hash
+ && !_bfd_elf_add_dynamic_entry (info, DT_HASH, 0))
+ || (info->emit_gnu_hash
+ && !_bfd_elf_add_dynamic_entry (info, DT_GNU_HASH, 0))
+ || !_bfd_elf_add_dynamic_entry (info, DT_STRTAB, 0)
+ || !_bfd_elf_add_dynamic_entry (info, DT_SYMTAB, 0)
+ || !_bfd_elf_add_dynamic_entry (info, DT_STRSZ, strsize)
+ || !_bfd_elf_add_dynamic_entry (info, DT_SYMENT,
+ bed->s->sizeof_sym))
+ return FALSE;
+ }
+ }
+
+ if (! _bfd_elf_maybe_strip_eh_frame_hdr (info))
+ return FALSE;
+
+ /* The backend must work out the sizes of all the other dynamic
+ sections. */
+ if (dynobj != NULL
+ && bed->elf_backend_size_dynamic_sections != NULL
+ && ! (*bed->elf_backend_size_dynamic_sections) (output_bfd, info))
+ return FALSE;
+
+ if (dynobj != NULL && elf_hash_table (info)->dynamic_sections_created)
+ {
+ if (elf_tdata (output_bfd)->cverdefs)
+ {
+ unsigned int crefs = elf_tdata (output_bfd)->cverdefs;
+
+ if (!_bfd_elf_add_dynamic_entry (info, DT_VERDEF, 0)
+ || !_bfd_elf_add_dynamic_entry (info, DT_VERDEFNUM, crefs))
+ return FALSE;
+ }
+
+ if ((info->new_dtags && info->flags) || (info->flags & DF_STATIC_TLS))
+ {
+ if (!_bfd_elf_add_dynamic_entry (info, DT_FLAGS, info->flags))
+ return FALSE;
+ }
+ else if (info->flags & DF_BIND_NOW)
+ {
+ if (!_bfd_elf_add_dynamic_entry (info, DT_BIND_NOW, 0))
+ return FALSE;
+ }
+
+ if (info->flags_1)
+ {
+ if (bfd_link_executable (info))
+ info->flags_1 &= ~ (DF_1_INITFIRST
+ | DF_1_NODELETE
+ | DF_1_NOOPEN);
+ if (!_bfd_elf_add_dynamic_entry (info, DT_FLAGS_1, info->flags_1))
+ return FALSE;
+ }
+
+ if (elf_tdata (output_bfd)->cverrefs)
+ {
+ unsigned int crefs = elf_tdata (output_bfd)->cverrefs;
+
+ if (!_bfd_elf_add_dynamic_entry (info, DT_VERNEED, 0)
+ || !_bfd_elf_add_dynamic_entry (info, DT_VERNEEDNUM, crefs))
+ return FALSE;
+ }
+ }
return TRUE;
}
Index: binutils/ld/testsuite/ld-elf/pr20828-d.sd
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ binutils/ld/testsuite/ld-elf/pr20828-d.sd 2017-01-31 14:46:30.926958068 +0000
@@ -0,0 +1,9 @@
+# Make sure `vdata' is global rather than local in the dynamic symbol table,
+# e.g.:
+# Num: Value Size Type Bind Vis Ndx Name
+# 1: 00000000 0 OBJECT GLOBAL DEFAULT ABS vdata
+# vs:
+# 1: 00000000 0 OBJECT LOCAL DEFAULT ABS vdata
+#...
+ *[0-9]+: +[0-9a-f]+ +0 +OBJECT +GLOBAL +DEFAULT +ABS +vdata
+#pass
Index: binutils/ld/testsuite/ld-elf/pr20828-e.sd
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ binutils/ld/testsuite/ld-elf/pr20828-e.sd 2017-01-31 15:16:59.078917392 +0000
@@ -0,0 +1,9 @@
+# Make sure `edata' is global rather than local in the dynamic symbol table,
+# e.g.:
+# Num: Value Size Type Bind Vis Ndx Name
+# 1: 00000000 0 NOTYPE GLOBAL DEFAULT 1 edata@@vdata
+# vs:
+# 1: 00000000 0 NOTYPE LOCAL DEFAULT 1 edata@@vdata
+#...
+ *[0-9]+: +[0-9a-f]+ +0 +NOTYPE +GLOBAL +DEFAULT +[0-9]+ +edata@@vdata
+#pass
Index: binutils/ld/testsuite/ld-elf/pr20828-v.ver
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ binutils/ld/testsuite/ld-elf/pr20828-v.ver 2017-01-31 14:46:31.242435021 +0000
@@ -0,0 +1 @@
+vdata { global: edata; local: *; };
Index: binutils/ld/testsuite/ld-elf/pr20828.ld
===================================================================
--- binutils.orig/ld/testsuite/ld-elf/pr20828.ld 2017-01-31 14:46:26.169112837 +0000
+++ binutils/ld/testsuite/ld-elf/pr20828.ld 2017-01-31 14:46:31.411858383 +0000
@@ -10,6 +10,8 @@ SECTIONS
.hash : { *(.hash) }
.dynsym : { *(.dynsym) }
.dynstr : { *(.dynstr) }
+ .gnu.version : { *(.gnu.version) }
+ .gnu.version_d : { *(.gnu.version_d) }
.shstrtab : { *(.shstrtab) }
.symtab : { *(.symtab) }
.strtab : { *(.strtab) }
Index: binutils/ld/testsuite/ld-elf/shared.exp
===================================================================
--- binutils.orig/ld/testsuite/ld-elf/shared.exp 2017-01-31 14:46:26.528721361 +0000
+++ binutils/ld/testsuite/ld-elf/shared.exp 2017-01-31 15:19:08.139588324 +0000
@@ -77,7 +77,30 @@ if { [check_gc_sections_available] } {
{pr20828.s} \
{{readelf --dyn-syms pr20828-b.sd} \
{readelf --dyn-syms pr20828-c.sd}} \
- "pr20828-2.so"]]
+ "pr20828-2.so"] \
+ [list \
+ "PR ld/20828 dynamic symbols with section GC\
+ (versioned shared library)" \
+ "$LFLAGS -shared --gc-sections -T pr20828.ld\
+ --version-script=pr20828-v.ver" \
+ "" "$AFLAGS_PIC" \
+ {pr20828.s} \
+ {{readelf --dyn-syms pr20828-c.sd} \
+ {readelf --dyn-syms pr20828-d.sd} \
+ {readelf --dyn-syms pr20828-e.sd}} \
+ "libpr20828-v.so"] \
+ [list \
+ "PR ld/20828 dynamic symbols with section GC (versioned)" \
+ "$LFLAGS -shared --gc-sections -T pr20828.ld\
+ --version-script=pr20828-v.ver" \
+ "tmpdir/libpr20828-v.so" \
+ "$AFLAGS_PIC" \
+ {pr20828.s} \
+ {{readelf --dyn-syms pr20828-c.sd} \
+ {readelf --dyn-syms pr20828-d.sd} \
+ {readelf --dyn-syms pr20828-e.sd}} \
+ "pr20828-v.so"] \
+ ]
}
# Check to see if the C compiler works
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] PR ld/20828: Move symbol version processing ahead of GC symbol sweep
2017-02-01 19:31 [PATCH] PR ld/20828: Move symbol version processing ahead of GC symbol sweep Maciej W. Rozycki
@ 2017-02-02 3:25 ` Alan Modra
2017-02-08 18:19 ` Maciej W. Rozycki
0 siblings, 1 reply; 10+ messages in thread
From: Alan Modra @ 2017-02-02 3:25 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Nick Clifton, James Cowgill, binutils
I think you may be in trouble here:
if ((elf_tdata (output_bfd)->cverrefs == 0
&& elf_tdata (output_bfd)->cverdefs == 0)
|| _bfd_elf_link_renumber_dynsyms (output_bfd, info,
§ion_sym_count) == 0)
{
s = bfd_get_linker_section (dynobj, ".gnu.version");
s->flags |= SEC_EXCLUDE;
}
The problem being that _bfd_elf_export_symbol and the backend
size_dynamic_sections might make symbols dynamic where there were
possibly none before. Garbage collection also might change whether
there are dynamic symbols.
Another problem related to dynamic symbols is that running
_bfd_elf_link_assign_sym_version early means the tests of h->dynindx
in that function are no longer valid.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] PR ld/20828: Move symbol version processing ahead of GC symbol sweep
2017-02-02 3:25 ` Alan Modra
@ 2017-02-08 18:19 ` Maciej W. Rozycki
2017-02-09 19:49 ` Maciej W. Rozycki
0 siblings, 1 reply; 10+ messages in thread
From: Maciej W. Rozycki @ 2017-02-08 18:19 UTC (permalink / raw)
To: Alan Modra; +Cc: Nick Clifton, James Cowgill, binutils
Hi Alan,
Thanks for your input!
> I think you may be in trouble here:
>
> if ((elf_tdata (output_bfd)->cverrefs == 0
> && elf_tdata (output_bfd)->cverdefs == 0)
> || _bfd_elf_link_renumber_dynsyms (output_bfd, info,
> §ion_sym_count) == 0)
> {
> s = bfd_get_linker_section (dynobj, ".gnu.version");
> s->flags |= SEC_EXCLUDE;
> }
>
> The problem being that _bfd_elf_export_symbol and the backend
> size_dynamic_sections might make symbols dynamic where there were
> possibly none before. Garbage collection also might change whether
> there are dynamic symbols.
Good point, and fixed easily, by leaving this piece of code in its
original place. A minor update is required because the declaration of `s'
has been moved out of the scope of this block, so another one is required
to keep it local (I've decided that setting `->flags' directly without the
use of an auxiliary variable would impact the readability of this piece of
code).
> Another problem related to dynamic symbols is that running
> _bfd_elf_link_assign_sym_version early means the tests of h->dynindx
> in that function are no longer valid.
Thanks. There are two such places AFAICT.
First:
/* See if there is anything to force this symbol to
local scope. */
if (d == NULL && t->locals.list != NULL)
{
d = (*t->match) (&t->locals, NULL, alc);
if (d != NULL
&& h->dynindx != -1
&& ! info->export_dynamic)
(*bed->elf_backend_hide_symbol) (info, h, TRUE);
}
I think actually is not a problem, because the symbol sweep will not give
a symbol a new dynsym table entry if it doesn't have one already. It can
reassign an entry via `_bfd_elf_link_renumber_dynsyms', but this is all
right because we do not care about the exact index value here. It can
also take an entry away, but this is not an issue either as in that case,
likewise, it'll call `->elf_backend_hide_symbol', so it doesn't really
matter which of the two happens first. So there is no need to change
anything here AFAICT.
Second:
/* If we are building an application, we need to create a
version node for this version. */
if (t == NULL && bfd_link_executable (info))
{
struct bfd_elf_version_tree **pp;
int version_index;
/* If we aren't going to export this symbol, we don't need
to worry about it. */
if (h->dynindx == -1)
return TRUE;
t = (struct bfd_elf_version_tree *) bfd_zalloc (info->output_bfd,
sizeof *t);
if (t == NULL)
{
sinfo->failed = TRUE;
return FALSE;
}
t->name = p;
t->name_indx = (unsigned int) -1;
t->used = TRUE;
version_index = 1;
/* Don't count anonymous version tag. */
if (sinfo->info->version_info != NULL
&& sinfo->info->version_info->vernum == 0)
version_index = 0;
for (pp = &sinfo->info->version_info;
*pp != NULL;
pp = &(*pp)->next)
++version_index;
t->vernum = version_index;
*pp = t;
h->verinfo.vertree = t;
}
is a bit more complex, because of the opposite condition. With code as it
stands a symbol may have already been swept and its dynsym table entry
assignment removed, while with the change I have proposed the assignment
will still be there, pending removal, and a version node will be created
while it shouldn't. Again the exact index value does not matter here.
So I propose that we check at this point if the symbol is going to be
eventually swept and do not create a version node in that case. This
could become a problem if `->mark' was set for a symbol after this point
and before the symbol sweep later on, but we have no code doing so
currently AFAICT and we can just declare it a no-no.
Any flaws in my reasoning?
FAOD here is an incremental update which implements all of this while
keeping regression test results the same. If you agree that this is the
right direction, then I'll post v2 of the whole change. I think I can
invent a test suite case for the latter case as well, so I'll bundle that
too.
Maciej
binutils-bfd-elf-size-dynamic-sections-version-update.diff
Index: binutils/bfd/elflink.c
===================================================================
--- binutils.orig/bfd/elflink.c 2017-02-08 09:45:02.000000000 +0000
+++ binutils/bfd/elflink.c 2017-02-08 14:41:46.036973024 +0000
@@ -2051,6 +2051,21 @@ _bfd_elf_export_symbol (struct elf_link_
return TRUE;
}
\f
+/* Return TRUE if the symbol identified by H qualifies for removal
+ in section garbage collection. */
+
+static bfd_boolean
+elf_gc_symbol_eligible_p (struct elf_link_hash_entry *h)
+{
+ return (!h->mark
+ && (((h->root.type == bfd_link_hash_defined
+ || h->root.type == bfd_link_hash_defweak)
+ && !((h->def_regular || ELF_COMMON_DEF_P (h))
+ && h->root.u.def.section->gc_mark))
+ || h->root.type == bfd_link_hash_undefined
+ || h->root.type == bfd_link_hash_undefweak));
+}
+\f
/* Look through the symbols which are defined in other shared
libraries and referenced here. Update the list of version
dependencies. This will be put into the .gnu.version_r section.
@@ -2233,7 +2248,10 @@ _bfd_elf_link_assign_sym_version (struct
/* If we aren't going to export this symbol, we don't need
to worry about it. */
- if (h->dynindx == -1)
+ if (h->dynindx == -1
+ || (info->gc_sections
+ && bed->can_gc_sections
+ && elf_gc_symbol_eligible_p (h)))
return TRUE;
t = (struct bfd_elf_version_tree *) bfd_zalloc (info->output_bfd,
@@ -5886,13 +5904,7 @@ struct elf_gc_sweep_symbol_info
static bfd_boolean
elf_gc_sweep_symbol (struct elf_link_hash_entry *h, void *data)
{
- if (!h->mark
- && (((h->root.type == bfd_link_hash_defined
- || h->root.type == bfd_link_hash_defweak)
- && !((h->def_regular || ELF_COMMON_DEF_P (h))
- && h->root.u.def.section->gc_mark))
- || h->root.type == bfd_link_hash_undefined
- || h->root.type == bfd_link_hash_undefweak))
+ if (elf_gc_symbol_eligible_p (h))
{
struct elf_gc_sweep_symbol_info *inf;
@@ -5938,7 +5950,6 @@ bfd_elf_size_dynamic_sections (bfd *outp
if (dynobj != NULL && elf_hash_table (info)->dynamic_sections_created)
{
struct bfd_elf_version_tree *verdefs;
- unsigned long section_sym_count;
struct elf_info_failed asvinfo;
struct bfd_elf_version_tree *t;
struct bfd_elf_version_expr *d;
@@ -6368,15 +6379,6 @@ bfd_elf_size_dynamic_sections (bfd *outp
elf_tdata (output_bfd)->cverrefs = crefs;
}
}
-
- if ((elf_tdata (output_bfd)->cverrefs == 0
- && elf_tdata (output_bfd)->cverdefs == 0)
- || _bfd_elf_link_renumber_dynsyms (output_bfd, info,
- §ion_sym_count) == 0)
- {
- s = bfd_get_linker_section (dynobj, ".gnu.version");
- s->flags |= SEC_EXCLUDE;
- }
}
bed = get_elf_backend_data (output_bfd);
@@ -6676,6 +6678,8 @@ bfd_elf_size_dynamic_sections (bfd *outp
if (dynobj != NULL && elf_hash_table (info)->dynamic_sections_created)
{
+ unsigned long section_sym_count;
+
if (elf_tdata (output_bfd)->cverdefs)
{
unsigned int crefs = elf_tdata (output_bfd)->cverdefs;
@@ -6714,6 +6718,17 @@ bfd_elf_size_dynamic_sections (bfd *outp
|| !_bfd_elf_add_dynamic_entry (info, DT_VERNEEDNUM, crefs))
return FALSE;
}
+
+ if ((elf_tdata (output_bfd)->cverrefs == 0
+ && elf_tdata (output_bfd)->cverdefs == 0)
+ || _bfd_elf_link_renumber_dynsyms (output_bfd, info,
+ §ion_sym_count) == 0)
+ {
+ asection *s;
+
+ s = bfd_get_linker_section (dynobj, ".gnu.version");
+ s->flags |= SEC_EXCLUDE;
+ }
}
return TRUE;
}
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] PR ld/20828: Move symbol version processing ahead of GC symbol sweep
2017-02-08 18:19 ` Maciej W. Rozycki
@ 2017-02-09 19:49 ` Maciej W. Rozycki
2017-02-15 1:55 ` Alan Modra
0 siblings, 1 reply; 10+ messages in thread
From: Maciej W. Rozycki @ 2017-02-09 19:49 UTC (permalink / raw)
To: Alan Modra; +Cc: Nick Clifton, James Cowgill, binutils
On Wed, 8 Feb 2017, Maciej W. Rozycki wrote:
> Second:
>
> /* If we are building an application, we need to create a
> version node for this version. */
> if (t == NULL && bfd_link_executable (info))
> {
> struct bfd_elf_version_tree **pp;
> int version_index;
>
> /* If we aren't going to export this symbol, we don't need
> to worry about it. */
> if (h->dynindx == -1)
> return TRUE;
>
> t = (struct bfd_elf_version_tree *) bfd_zalloc (info->output_bfd,
> sizeof *t);
> if (t == NULL)
> {
> sinfo->failed = TRUE;
> return FALSE;
> }
>
> t->name = p;
> t->name_indx = (unsigned int) -1;
> t->used = TRUE;
>
> version_index = 1;
> /* Don't count anonymous version tag. */
> if (sinfo->info->version_info != NULL
> && sinfo->info->version_info->vernum == 0)
> version_index = 0;
> for (pp = &sinfo->info->version_info;
> *pp != NULL;
> pp = &(*pp)->next)
> ++version_index;
> t->vernum = version_index;
>
> *pp = t;
>
> h->verinfo.vertree = t;
> }
>
> is a bit more complex, because of the opposite condition. With code as it
> stands a symbol may have already been swept and its dynsym table entry
> assignment removed, while with the change I have proposed the assignment
> will still be there, pending removal, and a version node will be created
> while it shouldn't. Again the exact index value does not matter here.
A quick update. I have been trying to write a test case for this
execution path and I seem to be unable to, making me suspect that the
interaction between `->dynindx' and the symbol sweep does not actually
matter here.
The thing is this conditional only ever goes through for double-at `@@'
versioned symbols, such as with this example source:
.globl _adata
.symver _adata, _idata@@_vdata
_adata:
This is because:
1. The symbol has to be versioned in the incoming object due to:
p = strchr (h->root.root.string, ELF_VER_CHR);
if (p != NULL && h->verinfo.vertree == NULL)
earlier on in `_bfd_elf_link_assign_sym_version', i.e. there has to be
an `@' in the name.
2. The symbol must have been defined locally, due to:
if (!h->def_regular)
return TRUE;
earlier on in `_bfd_elf_link_assign_sym_version'.
3. We must be linking an executable rather than a DSO due to:
if (t == NULL && bfd_link_executable (info))
at the outer condition, quoted above.
4. The symbol must not be hidden for `->dynindx' to be non-minus-one:
if (h->dynindx == -1)
return TRUE;
so `_bfd_elf_add_default_symbol' must have set `->versioned =
versioned' according to:
p = strchr (name, ELF_VER_CHR);
if (h->versioned == unknown)
{
if (p == NULL)
{
h->versioned = unversioned;
return TRUE;
}
else
{
if (p[1] != ELF_VER_CHR)
{
h->versioned = versioned_hidden;
return TRUE;
}
else
h->versioned = versioned;
}
}
i.e. there has to be `@@' there or `_bfd_elf_fix_symbol_flags' called
earlier on in `_bfd_elf_link_assign_sym_version' would have hidden the
symbol:
else if (bfd_link_executable (eif->info)
&& h->versioned == versioned_hidden
&& !eif->info->export_dynamic
&& !h->dynamic
&& !h->ref_dynamic
&& h->def_regular)
(*bed->elf_backend_hide_symbol) (eif->info, h, TRUE);
that sets `->dynindx = -1' unless the symbol has been forcifully
exported or is referenced by a DSO, neither in which case it can be
garbage-collected.
So it looks to me like for the order between this code and the symbol
sweep to matter these conditions all at once have to be met:
1. Executable is being linked.
2. Symbol has been defined locally.
3. Symbol is being dynamically exported.
4. Symbol will be garbage-collected.
however #4 cannot happen together with #1-#3 at once, because meeting
#1-#3 means the symbol cannot be garbage-collected. IOW, if we get to
here, then either `->dynindx == -1' and we don't care or `->dynindx != -1'
and this won't change. So it looks like most of my update can be dropped
and only the part below retained.
Have I missed anything here?
NB version nodes seem to be always propagated to output even if their
originating symbols have been removed, e.g. by being forced local with an
anonymous version script, regardless of whether section garbage collection
has been enabled or not. So in any case it looks to me like we would be
trying to be too smart here by making efforts not to create version nodes.
Perhaps letting them through to output can be considered a preexisting bug
however.
Maciej
binutils-bfd-elf-size-dynamic-sections-version-update-2.diff
Index: binutils/bfd/elflink.c
===================================================================
--- binutils.orig/bfd/elflink.c 2017-02-09 05:24:05.743124850 +0000
+++ binutils/bfd/elflink.c 2017-02-09 19:36:48.153651299 +0000
@@ -5938,7 +5938,6 @@ bfd_elf_size_dynamic_sections (bfd *outp
if (dynobj != NULL && elf_hash_table (info)->dynamic_sections_created)
{
struct bfd_elf_version_tree *verdefs;
- unsigned long section_sym_count;
struct elf_info_failed asvinfo;
struct bfd_elf_version_tree *t;
struct bfd_elf_version_expr *d;
@@ -6368,15 +6367,6 @@ bfd_elf_size_dynamic_sections (bfd *outp
elf_tdata (output_bfd)->cverrefs = crefs;
}
}
-
- if ((elf_tdata (output_bfd)->cverrefs == 0
- && elf_tdata (output_bfd)->cverdefs == 0)
- || _bfd_elf_link_renumber_dynsyms (output_bfd, info,
- §ion_sym_count) == 0)
- {
- s = bfd_get_linker_section (dynobj, ".gnu.version");
- s->flags |= SEC_EXCLUDE;
- }
}
bed = get_elf_backend_data (output_bfd);
@@ -6676,6 +6666,8 @@ bfd_elf_size_dynamic_sections (bfd *outp
if (dynobj != NULL && elf_hash_table (info)->dynamic_sections_created)
{
+ unsigned long section_sym_count;
+
if (elf_tdata (output_bfd)->cverdefs)
{
unsigned int crefs = elf_tdata (output_bfd)->cverdefs;
@@ -6714,6 +6706,17 @@ bfd_elf_size_dynamic_sections (bfd *outp
|| !_bfd_elf_add_dynamic_entry (info, DT_VERNEEDNUM, crefs))
return FALSE;
}
+
+ if ((elf_tdata (output_bfd)->cverrefs == 0
+ && elf_tdata (output_bfd)->cverdefs == 0)
+ || _bfd_elf_link_renumber_dynsyms (output_bfd, info,
+ §ion_sym_count) == 0)
+ {
+ asection *s;
+
+ s = bfd_get_linker_section (dynobj, ".gnu.version");
+ s->flags |= SEC_EXCLUDE;
+ }
}
return TRUE;
}
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] PR ld/20828: Move symbol version processing ahead of GC symbol sweep
2017-02-09 19:49 ` Maciej W. Rozycki
@ 2017-02-15 1:55 ` Alan Modra
2017-02-15 3:43 ` Alan Modra
0 siblings, 1 reply; 10+ messages in thread
From: Alan Modra @ 2017-02-15 1:55 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Nick Clifton, James Cowgill, binutils
On Thu, Feb 09, 2017 at 07:48:50PM +0000, Maciej W. Rozycki wrote:
> 1. Executable is being linked.
>
> 2. Symbol has been defined locally.
>
> 3. Symbol is being dynamically exported.
>
> 4. Symbol will be garbage-collected.
>
> however #4 cannot happen together with #1-#3 at once, because meeting
> #1-#3 means the symbol cannot be garbage-collected. IOW, if we get to
> here, then either `->dynindx == -1' and we don't care or `->dynindx != -1'
> and this won't change. So it looks like most of my update can be dropped
> and only the part below retained.
>
> Have I missed anything here?
I think you are correct, and this update makes the original patch OK
to apply. I'm willing to approve it.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] PR ld/20828: Move symbol version processing ahead of GC symbol sweep
2017-02-15 1:55 ` Alan Modra
@ 2017-02-15 3:43 ` Alan Modra
2017-02-15 16:39 ` Maciej W. Rozycki
0 siblings, 1 reply; 10+ messages in thread
From: Alan Modra @ 2017-02-15 3:43 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Nick Clifton, James Cowgill, binutils
On Wed, Feb 15, 2017 at 12:24:51PM +1030, Alan Modra wrote:
> On Thu, Feb 09, 2017 at 07:48:50PM +0000, Maciej W. Rozycki wrote:
> > 1. Executable is being linked.
> >
> > 2. Symbol has been defined locally.
> >
> > 3. Symbol is being dynamically exported.
> >
> > 4. Symbol will be garbage-collected.
> >
> > however #4 cannot happen together with #1-#3 at once, because meeting
> > #1-#3 means the symbol cannot be garbage-collected. IOW, if we get to
> > here, then either `->dynindx == -1' and we don't care or `->dynindx != -1'
> > and this won't change. So it looks like most of my update can be dropped
> > and only the part below retained.
> >
> > Have I missed anything here?
>
> I think you are correct, and this update makes the original patch OK
> to apply. I'm willing to approve it.
So I went and tested the patch, and found failures on x86_64 and other
architectures.
FAIL: Build rdynamic-1
FAIL: vers4a
FAIL: vers9
This says the patch is not OK.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] PR ld/20828: Move symbol version processing ahead of GC symbol sweep
2017-02-15 3:43 ` Alan Modra
@ 2017-02-15 16:39 ` Maciej W. Rozycki
2017-02-15 23:34 ` Alan Modra
0 siblings, 1 reply; 10+ messages in thread
From: Maciej W. Rozycki @ 2017-02-15 16:39 UTC (permalink / raw)
To: Alan Modra; +Cc: Nick Clifton, James Cowgill, binutils
On Wed, 15 Feb 2017, Alan Modra wrote:
> So I went and tested the patch, and found failures on x86_64 and other
> architectures.
Umm, I didn't think about native/compiled testing, sorry. I have got
myself set up for x86_64-linux native testing now and...
> FAIL: Build rdynamic-1
... I can't reproduce this...
> FAIL: vers4a
> FAIL: vers9
... but I can see these, so I'll start from here and will also work on
adding versions of these that do not require a compiler or system
libraries, so that we have coverage for configurations that do not usually
receive native/compiled testing.
Thanks for verifying my change.
Maciej
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] PR ld/20828: Move symbol version processing ahead of GC symbol sweep
2017-02-15 16:39 ` Maciej W. Rozycki
@ 2017-02-15 23:34 ` Alan Modra
2017-02-20 19:26 ` Maciej W. Rozycki
0 siblings, 1 reply; 10+ messages in thread
From: Alan Modra @ 2017-02-15 23:34 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: binutils
On Wed, Feb 15, 2017 at 04:38:46PM +0000, Maciej W. Rozycki wrote:
> ... I can't reproduce this...
valgrind reports uninitialized writes, so some luck is involved in
getting the test to fail.
If I put MALLOC_PERTURB_=1 in my environment then the test passes but
readelf -s --wide tmpdir/rdynamic-1 shows:
Symbol table '.dynsym' contains 18 entries:
Num: Value Size Type Bind Vis Ndx Name
0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND
1: 0000000000000000 0 FUNC GLOBAL DEFAULT UND __libc_start_main@GLIBC_2.2.5 (2)
2: 0000000000000000 0 NOTYPE WEAK DEFAULT UND __gmon_start__
3: 0000000000601024 0 NOTYPE GLOBAL DEFAULT 25 _edata
4: 0000000000601020 0 NOTYPE GLOBAL DEFAULT 25 __data_start
5: fefefefefefefefe 0xfefefefefefefefe <processor specific>: 14 <processor specific>: 15 HIDDEN [<other>: fc] bad section index[65278] <corrupt>
6: 0000000000601028 0 NOTYPE GLOBAL DEFAULT 26 _end
7: 0000000000601020 0 NOTYPE WEAK DEFAULT 25 data_start
8: 00000000004007e0 4 OBJECT GLOBAL DEFAULT 16 _IO_stdin_used
9: 0000000000400760 101 FUNC GLOBAL DEFAULT 14 __libc_csu_init
10: 0000000000400650 42 FUNC GLOBAL DEFAULT 14 _start
11: 0000000000601024 0 NOTYPE GLOBAL DEFAULT 26 __bss_start
12: 0000000000400640 3 FUNC GLOBAL DEFAULT 14 main
13: fefefefefefefefe 0xfefefefefefefefe <processor specific>: 14 <processor specific>: 15 HIDDEN [<other>: fc] bad section index[65278] <corrupt>
14: 00000000004005f0 0 FUNC GLOBAL DEFAULT 11 _init
15: 00000000004007d0 2 FUNC GLOBAL DEFAULT 14 __libc_csu_fini
16: 00000000004007d4 0 FUNC GLOBAL DEFAULT 15 _fini
17: 0000000000400750 2 FUNC GLOBAL DEFAULT 14 rdynamic
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] PR ld/20828: Move symbol version processing ahead of GC symbol sweep
2017-02-15 23:34 ` Alan Modra
@ 2017-02-20 19:26 ` Maciej W. Rozycki
2017-02-21 22:50 ` Alan Modra
0 siblings, 1 reply; 10+ messages in thread
From: Maciej W. Rozycki @ 2017-02-20 19:26 UTC (permalink / raw)
To: Alan Modra; +Cc: binutils
On Wed, 15 Feb 2017, Alan Modra wrote:
> > ... I can't reproduce this...
>
> valgrind reports uninitialized writes, so some luck is involved in
> getting the test to fail.
>
> If I put MALLOC_PERTURB_=1 in my environment then the test passes but
> readelf -s --wide tmpdir/rdynamic-1 shows:
>
> Symbol table '.dynsym' contains 18 entries:
> Num: Value Size Type Bind Vis Ndx Name
> 0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND
> 1: 0000000000000000 0 FUNC GLOBAL DEFAULT UND __libc_start_main@GLIBC_2.2.5 (2)
> 2: 0000000000000000 0 NOTYPE WEAK DEFAULT UND __gmon_start__
> 3: 0000000000601024 0 NOTYPE GLOBAL DEFAULT 25 _edata
> 4: 0000000000601020 0 NOTYPE GLOBAL DEFAULT 25 __data_start
> 5: fefefefefefefefe 0xfefefefefefefefe <processor specific>: 14 <processor specific>: 15 HIDDEN [<other>: fc] bad section index[65278] <corrupt>
> 6: 0000000000601028 0 NOTYPE GLOBAL DEFAULT 26 _end
> 7: 0000000000601020 0 NOTYPE WEAK DEFAULT 25 data_start
> 8: 00000000004007e0 4 OBJECT GLOBAL DEFAULT 16 _IO_stdin_used
> 9: 0000000000400760 101 FUNC GLOBAL DEFAULT 14 __libc_csu_init
> 10: 0000000000400650 42 FUNC GLOBAL DEFAULT 14 _start
> 11: 0000000000601024 0 NOTYPE GLOBAL DEFAULT 26 __bss_start
> 12: 0000000000400640 3 FUNC GLOBAL DEFAULT 14 main
> 13: fefefefefefefefe 0xfefefefefefefefe <processor specific>: 14 <processor specific>: 15 HIDDEN [<other>: fc] bad section index[65278] <corrupt>
> 14: 00000000004005f0 0 FUNC GLOBAL DEFAULT 11 _init
> 15: 00000000004007d0 2 FUNC GLOBAL DEFAULT 14 __libc_csu_fini
> 16: 00000000004007d4 0 FUNC GLOBAL DEFAULT 15 _fini
> 17: 0000000000400750 2 FUNC GLOBAL DEFAULT 14 rdynamic
This must be something specific to your environment (or mine), which is
an obvious shortcoming (or, depending on how you look at it, a benefit) of
compiled tests -- in that results will inevitably depend on external
components such as target libc included in the build of individual test
cases (the shortcoming is the lack of universal reproducibility, while the
benefit is the diversity of test environments giving a higher chance to
hit an issue if one is there).
Or to put it short I still cannot reproduce it either way, and the dynsym
table always looks the same here, regardless of whether my original patch
has been applied or not, and if so, then if any of the updates has, as
follows:
Symbol table '.dynsym' contains 16 entries:
Num: Value Size Type Bind Vis Ndx Name
0: 0000000000000000 0 NOTYPE LOCAL DEFAULT UND
1: 0000000000000000 0 FUNC GLOBAL DEFAULT UND __libc_start_main@GLIBC_2.2.5 (2)
2: 0000000000000000 0 NOTYPE WEAK DEFAULT UND __gmon_start__
3: 0000000000601024 0 NOTYPE GLOBAL DEFAULT 25 _edata
4: 0000000000601020 0 NOTYPE GLOBAL DEFAULT 25 __data_start
5: 0000000000601028 0 NOTYPE GLOBAL DEFAULT 26 _end
6: 0000000000601020 0 NOTYPE WEAK DEFAULT 25 data_start
7: 0000000000400750 4 OBJECT GLOBAL DEFAULT 16 _IO_stdin_used
8: 00000000004006d0 101 FUNC GLOBAL DEFAULT 14 __libc_csu_init
9: 00000000004005d3 0 FUNC GLOBAL DEFAULT 14 _start
10: 0000000000601024 0 NOTYPE GLOBAL DEFAULT 26 __bss_start
11: 00000000004005d0 3 FUNC GLOBAL DEFAULT 14 main
12: 0000000000400580 0 FUNC GLOBAL DEFAULT 11 _init
13: 0000000000400740 2 FUNC GLOBAL DEFAULT 14 __libc_csu_fini
14: 0000000000400744 0 FUNC GLOBAL DEFAULT 15 _fini
15: 00000000004006c0 2 FUNC GLOBAL DEFAULT 14 rdynamic
That said however all 3 regressions have the use of `-export-dynamic' in
common, so I have used the other two I could get to trigger here as
reference tests and tracked the issue down to a piece of code in
`bfd_elf_size_dynamic_sections' that I previously missed, which handles
the option and symbols that would otherwise remain local to the dynamic
symbol table. These symbols obviously have to be passed through version
processing, so they have to be exported before linker versioning is
applied.
By the same reasoning I have shown in the previous message these symbols
will not be garbage-collected, or they wouldn't have been candidates for
exporting in the first place -- as indicated in `_bfd_elf_export_symbol'
they need to be either locally defined or locally referenced. So it is
safe AFAICT to have the symbol sweep pass executed after the forciful
symbol export loop, and therefore the loop can be moved ahead of version
processing and hence also the symbol sweep pass, while still satisfying
the requirements of both.
So here's an updated update, which removes the:
FAIL: vers4a
FAIL: vers9
regressions for me, and hopefully also your:
FAIL: Build rdynamic-1
(please verify that for me).
For the purpose of further review and for the avoidance of doubt as to
what my actual proposed change is right now, I'll post v2 of the original
change with all the amendments applied. It'll include a further test case
reduced from `vers4a' too, which does not rely on the presence of a
compiler (and which I yet need to actually push through my usual testing).
Thanks for your input!
Maciej
binutils-bfd-elf-size-dynamic-sections-version-update-3.diff
Index: binutils/bfd/elflink.c
===================================================================
--- binutils.orig/bfd/elflink.c 2017-02-17 23:27:33.275425647 +0000
+++ binutils/bfd/elflink.c 2017-02-17 23:46:30.128370517 +0000
@@ -5938,13 +5938,28 @@ bfd_elf_size_dynamic_sections (bfd *outp
if (dynobj != NULL && elf_hash_table (info)->dynamic_sections_created)
{
struct bfd_elf_version_tree *verdefs;
- unsigned long section_sym_count;
struct elf_info_failed asvinfo;
struct bfd_elf_version_tree *t;
struct bfd_elf_version_expr *d;
+ struct elf_info_failed eif;
bfd_boolean all_defined;
asection *s;
+ eif.info = info;
+ eif.failed = FALSE;
+
+ /* If we are supposed to export all symbols into the dynamic symbol
+ table (this is not the normal case), then do so. */
+ if (info->export_dynamic
+ || (bfd_link_executable (info) && info->dynamic))
+ {
+ elf_link_hash_traverse (elf_hash_table (info),
+ _bfd_elf_export_symbol,
+ &eif);
+ if (eif.failed)
+ return FALSE;
+ }
+
/* Make all global versions with definition. */
for (t = info->version_info; t != NULL; t = t->next)
for (d = t->globals.list; d != NULL; d = d->next)
@@ -6368,15 +6383,6 @@ bfd_elf_size_dynamic_sections (bfd *outp
elf_tdata (output_bfd)->cverrefs = crefs;
}
}
-
- if ((elf_tdata (output_bfd)->cverrefs == 0
- && elf_tdata (output_bfd)->cverdefs == 0)
- || _bfd_elf_link_renumber_dynsyms (output_bfd, info,
- §ion_sym_count) == 0)
- {
- s = bfd_get_linker_section (dynobj, ".gnu.version");
- s->flags |= SEC_EXCLUDE;
- }
}
bed = get_elf_backend_data (output_bfd);
@@ -6545,18 +6551,6 @@ bfd_elf_size_dynamic_sections (bfd *outp
eif.info = info;
eif.failed = FALSE;
- /* If we are supposed to export all symbols into the dynamic symbol
- table (this is not the normal case), then do so. */
- if (info->export_dynamic
- || (bfd_link_executable (info) && info->dynamic))
- {
- elf_link_hash_traverse (elf_hash_table (info),
- _bfd_elf_export_symbol,
- &eif);
- if (eif.failed)
- return FALSE;
- }
-
/* Find all symbols which were defined in a dynamic object and make
the backend pick a reasonable value for them. */
elf_link_hash_traverse (elf_hash_table (info),
@@ -6676,6 +6670,8 @@ bfd_elf_size_dynamic_sections (bfd *outp
if (dynobj != NULL && elf_hash_table (info)->dynamic_sections_created)
{
+ unsigned long section_sym_count;
+
if (elf_tdata (output_bfd)->cverdefs)
{
unsigned int crefs = elf_tdata (output_bfd)->cverdefs;
@@ -6714,6 +6710,17 @@ bfd_elf_size_dynamic_sections (bfd *outp
|| !_bfd_elf_add_dynamic_entry (info, DT_VERNEEDNUM, crefs))
return FALSE;
}
+
+ if ((elf_tdata (output_bfd)->cverrefs == 0
+ && elf_tdata (output_bfd)->cverdefs == 0)
+ || _bfd_elf_link_renumber_dynsyms (output_bfd, info,
+ §ion_sym_count) == 0)
+ {
+ asection *s;
+
+ s = bfd_get_linker_section (dynobj, ".gnu.version");
+ s->flags |= SEC_EXCLUDE;
+ }
}
return TRUE;
}
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] PR ld/20828: Move symbol version processing ahead of GC symbol sweep
2017-02-20 19:26 ` Maciej W. Rozycki
@ 2017-02-21 22:50 ` Alan Modra
0 siblings, 0 replies; 10+ messages in thread
From: Alan Modra @ 2017-02-21 22:50 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: binutils
On Mon, Feb 20, 2017 at 07:26:07PM +0000, Maciej W. Rozycki wrote:
> So here's an updated update, which removes the:
>
> FAIL: vers4a
> FAIL: vers9
>
> regressions for me, and hopefully also your:
>
> FAIL: Build rdynamic-1
It did.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-02-21 22:50 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-01 19:31 [PATCH] PR ld/20828: Move symbol version processing ahead of GC symbol sweep Maciej W. Rozycki
2017-02-02 3:25 ` Alan Modra
2017-02-08 18:19 ` Maciej W. Rozycki
2017-02-09 19:49 ` Maciej W. Rozycki
2017-02-15 1:55 ` Alan Modra
2017-02-15 3:43 ` Alan Modra
2017-02-15 16:39 ` Maciej W. Rozycki
2017-02-15 23:34 ` Alan Modra
2017-02-20 19:26 ` Maciej W. Rozycki
2017-02-21 22:50 ` 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).