* [gold commit] Fix problems with --dynamic-list option.
@ 2014-02-06 2:04 Cary Coutant
2014-02-06 22:30 ` Alan Modra
0 siblings, 1 reply; 3+ messages in thread
From: Cary Coutant @ 2014-02-06 2:04 UTC (permalink / raw)
To: binutils
Fix problems with the --dynamic-list option.
PR gold/13577 complains that even though symbols listed in
the --dynamic-list script are exported, they are still bound symbolically
if -Bsymbolic is also used. There are two underlying problems here.
First, -Bsymbolic should be overridden by --dynamic-list, since the
dynamic list provides an explicit list of symbols that are not bound
within the library, and if we go ahead and set DT_SYMBOLIC, then the
dynamic loader will bind it within the library anyway. Second, gold
did not properly identify the symbols listed in the file as preemptible.
PR gold/16530 complains that symbols listed in the --dynamic-list script
can still be garbage collected. I've fixed this by checking the symbols
as they're added to the symbol table. (Unlike the --export-dynamic-symbol
option, we can't iterate over the list, because the --dynamic-list script
can have wildcards in it.)
Tested on x86_64 and committed.
-cary
gold/
2014-02-05 Cary Coutant <ccoutant@google.com>
PR gold/13577
* options.cc (General_options::parse_dynamic_list):
Set have_dynamic_list_.
(General_options::General_options): Initialize have_dynamic_list_.
(General_options::finalize): Turn off -Bsymbolic and
-Bsymbolic-functions if --dynamic-list provided.
* options.h (General_options::have_dynamic_list): New function.
(General_options::have_dynamic_list_): New data member.
* symtab.h (Symbol::is_preemptible): Handle --dynamic-list
correctly.
PR gold/16530
* symtab.cc (Symbol_table::add_from_relobj): If symbol is named
in --dynamic-list, mark it.
* testsuite/Makefile.am (gc_dynamic_list_test.sh): New test case.
(dynamic_list_2): New test case.
* testsuite/Makefile.in: Regenerate.
* testsuite/dynamic_list_2.cc: New file.
* testsuite/dynamic_list_2.t: New file.
* testsuite/dynamic_list_lib1.cc: New file.
* testsuite/dynamic_list_lib2.cc: New file.
* testsuite/gc_dynamic_list_test.c: New file.
* testsuite/gc_dynamic_list_test.sh: New file.
* testsuite/gc_dynamic_list_test.t: New file.
diff --git a/gold/options.cc b/gold/options.cc
index 000e6d0..6b49459 100644
--- a/gold/options.cc
+++ b/gold/options.cc
@@ -549,6 +549,7 @@ General_options::parse_dynamic_list(const char*, const char* arg,
{
if (!read_dynamic_list(arg, cmdline, &this->dynamic_list_))
gold::gold_fatal(_("unable to parse dynamic-list script file %s"), arg);
+ this->have_dynamic_list_ = true;
}
void
@@ -918,6 +919,7 @@ General_options::General_options()
do_demangle_(false),
plugins_(NULL),
dynamic_list_(),
+ have_dynamic_list_(false),
incremental_mode_(INCREMENTAL_OFF),
incremental_disposition_(INCREMENTAL_STARTUP),
incremental_startup_disposition_(INCREMENTAL_CHECK),
@@ -1199,6 +1201,13 @@ General_options::finalize()
// in the path, as appropriate.
this->add_sysroot();
+ // --dynamic-list overrides -Bsymbolic and -Bsymbolic-functions.
+ if (this->have_dynamic_list())
+ {
+ this->set_Bsymbolic(false);
+ this->set_Bsymbolic_functions(false);
+ }
+
// Now that we've normalized the options, check for contradictory ones.
if (this->shared() && this->is_static())
gold_fatal(_("-shared and -static are incompatible"));
diff --git a/gold/options.h b/gold/options.h
index a2f5a88..da1ade9 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -1438,6 +1438,11 @@ class General_options
in_dynamic_list(const char* symbol) const
{ return this->dynamic_list_.version_script_info()->symbol_is_local(symbol); }
+ // True if a --dynamic-list script was provided.
+ bool
+ have_dynamic_list() const
+ { return this->have_dynamic_list_; }
+
// Finalize the dynamic list.
void
finalize_dynamic_list()
@@ -1591,6 +1596,8 @@ class General_options
// script.cc, we store this as a Script_options object, even though
// we only use a single Version_tree from it.
Script_options dynamic_list_;
+ // Whether a --dynamic-list file was provided.
+ bool have_dynamic_list_;
// The incremental linking mode.
Incremental_mode incremental_mode_;
// The disposition given by the --incremental-changed,
diff --git a/gold/symtab.cc b/gold/symtab.cc
index 2e17529..225856a 100644
--- a/gold/symtab.cc
+++ b/gold/symtab.cc
@@ -1258,7 +1258,8 @@ Symbol_table::add_from_relobj(
&& res->is_externally_visible()
&& !res->is_from_dynobj()
&& (parameters->options().shared()
- || parameters->options().export_dynamic()))
+ || parameters->options().export_dynamic()
+ || parameters->options().in_dynamic_list(res->name())))
this->gc_mark_symbol(res);
if (is_defined_in_discarded_section)
diff --git a/gold/symtab.h b/gold/symtab.h
index 1232c97..9aff274c 100644
--- a/gold/symtab.h
+++ b/gold/symtab.h
@@ -576,8 +576,14 @@ class Symbol
if (!parameters->options().shared())
return false;
- // If the user used -Bsymbolic, then nothing is preemptible.
- if (parameters->options().Bsymbolic())
+ // If the symbol was named in a --dynamic-list script, it is preemptible.
+ if (parameters->options().in_dynamic_list(this->name()))
+ return true;
+
+ // If the user used -Bsymbolic or provided a --dynamic-list script,
+ // then nothing (else) is preemptible.
+ if (parameters->options().Bsymbolic()
+ || parameters->options().have_dynamic_list())
return false;
// If the user used -Bsymbolic-functions, then functions are not
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index aca9df8..0b22c13 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -203,6 +203,16 @@ pr14265: pr14265.o
pr14265.stdout: pr14265
$(TEST_NM) --format=bsd --numeric-sort $< > $@
+check_SCRIPTS += gc_dynamic_list_test.sh
+check_DATA += gc_dynamic_list_test.stdout
+MOSTLYCLEANFILES += gc_dynamic_list_test
+gc_dynamic_list_test.o: gc_dynamic_list_test.c
+ $(COMPILE) -c -ffunction-sections -o $@ $<
+gc_dynamic_list_test: gc_dynamic_list_test.o gcctestdir/ld $(srcdir)/gc_dynamic_list_test.t
+ $(LINK) -Bgcctestdir/ -Wl,--gc-sections -Wl,--dynamic-list,$(srcdir)/gc_dynamic_list_test.t gc_dynamic_list_test.o
+gc_dynamic_list_test.stdout: gc_dynamic_list_test
+ $(TEST_NM) gc_dynamic_list_test > $@
+
check_SCRIPTS += icf_test.sh
check_DATA += icf_test.map
MOSTLYCLEANFILES += icf_test icf_test.map
@@ -1465,6 +1475,22 @@ dynamic_list: basic_test.o gcctestdir/ld $(srcdir)/dynamic_list.t
dynamic_list.stdout: dynamic_list
$(TEST_READELF) -W --dyn-syms dynamic_list > dynamic_list.stdout
+check_PROGRAMS += dynamic_list_2
+dynamic_list_2_SOURCES = dynamic_list_2.cc
+dynamic_list_2_DEPENDENCIES = gcctestdir/ld dynamic_list_lib1.so dynamic_list_lib2.so
+dynamic_list_2_LDFLAGS = -Bgcctestdir/ -L. -Wl,-R,. -Wl,--no-as-needed
+dynamic_list_2_LDADD = dynamic_list_lib1.so dynamic_list_lib2.so
+
+dynamic_list_lib1.so: gcctestdir/ld dynamic_list_lib1.o
+ $(CXXLINK) -Bgcctestdir/ -shared dynamic_list_lib1.o
+dynamic_list_lib1.o: dynamic_list_lib1.cc
+ $(CXXCOMPILE) -c -fpic -o $@ $<
+
+dynamic_list_lib2.so: gcctestdir/ld dynamic_list_lib2.o $(srcdir)/dynamic_list_2.t
+ $(CXXLINK) -Bgcctestdir/ -shared -Wl,--dynamic-list,$(srcdir)/dynamic_list_2.t dynamic_list_lib2.o
+dynamic_list_lib2.o: dynamic_list_lib2.cc
+ $(CXXCOMPILE) -c -fpic -o $@ $<
+
check_PROGRAMS += thin_archive_test_1
MOSTLYCLEANFILES += libthin1.a libthin3.a libthinall.a \
alt/thin_archive_test_2.o alt/thin_archive_test_4.o \
diff --git a/gold/testsuite/dynamic_list_2.cc b/gold/testsuite/dynamic_list_2.cc
new file mode 100644
index 0000000..34194b8
--- /dev/null
+++ b/gold/testsuite/dynamic_list_2.cc
@@ -0,0 +1,40 @@
+// dynamic_list_test_2.cc -- Test --dynamic-list with shared libraries.
+
+// Copyright 2014 Free Software Foundation, Inc.
+// Written by Cary Coutant <ccoutant@google.com>.
+
+// This file is part of gold.
+
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 3 of the License, or
+// (at your option) any later version.
+
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+// MA 02110-1301, USA.
+
+// The goal of this program is to verify that the --dynamic-list option
+// allows overrides for symbols listed in the file, and does symbolic
+// binding for symbols not listed.
+
+#include <cstring>
+
+extern const char* test_foo();
+extern const char* test_bar();
+
+int
+main(void)
+{
+ if (strcmp(test_foo(), "override") != 0)
+ return 1;
+ if (strcmp(test_bar(), "original") != 0)
+ return 2;
+ return 0;
+}
diff --git a/gold/testsuite/dynamic_list_2.t b/gold/testsuite/dynamic_list_2.t
new file mode 100644
index 0000000..a4e4381
--- /dev/null
+++ b/gold/testsuite/dynamic_list_2.t
@@ -0,0 +1,27 @@
+/* dynamic_list_2.t -- script file for building dynamic_list_lib2.so.
+
+ Copyright 2014 Free Software Foundation, Inc.
+ Written by Cary Coutant <ccoutant@google.com>.
+
+ This file is part of gold.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+ MA 02110-1301, USA. */
+
+{
+ extern "C" {
+ "foo";
+ };
+};
diff --git a/gold/testsuite/dynamic_list_lib1.cc b/gold/testsuite/dynamic_list_lib1.cc
new file mode 100644
index 0000000..9bc0717
--- /dev/null
+++ b/gold/testsuite/dynamic_list_lib1.cc
@@ -0,0 +1,37 @@
+// dynamic_list_test_lib1.cc -- Test --dynamic-list with shared libraries.
+
+// Copyright 2014 Free Software Foundation, Inc.
+// Written by Cary Coutant <ccoutant@google.com>.
+
+// This file is part of gold.
+
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 3 of the License, or
+// (at your option) any later version.
+
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+// MA 02110-1301, USA.
+
+// The goal of this program is to verify that the --dynamic-list option
+// allows overrides for symbols listed in the file, and does symbolic
+// binding for symbols not listed.
+
+extern "C" const char*
+foo()
+{
+ return "override";
+}
+
+extern "C" const char*
+bar()
+{
+ return "override";
+}
diff --git a/gold/testsuite/dynamic_list_lib2.cc b/gold/testsuite/dynamic_list_lib2.cc
new file mode 100644
index 0000000..2532a42
--- /dev/null
+++ b/gold/testsuite/dynamic_list_lib2.cc
@@ -0,0 +1,49 @@
+// dynamic_list_test_lib2.cc -- Test --dynamic-list with shared libraries.
+
+// Copyright 2014 Free Software Foundation, Inc.
+// Written by Cary Coutant <ccoutant@google.com>.
+
+// This file is part of gold.
+
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 3 of the License, or
+// (at your option) any later version.
+
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+// MA 02110-1301, USA.
+
+// The goal of this program is to verify that the --dynamic-list option
+// allows overrides for symbols listed in the file, and does symbolic
+// binding for symbols not listed.
+
+extern "C" const char*
+foo()
+{
+ return "original";
+}
+
+const char*
+test_foo()
+{
+ return foo();
+}
+
+extern "C" const char*
+bar()
+{
+ return "original";
+}
+
+const char*
+test_bar()
+{
+ return bar();
+}
diff --git a/gold/testsuite/gc_dynamic_list_test.c b/gold/testsuite/gc_dynamic_list_test.c
new file mode 100644
index 0000000..3b84e98
--- /dev/null
+++ b/gold/testsuite/gc_dynamic_list_test.c
@@ -0,0 +1,34 @@
+// gc_dynamic_list_test.cc -- Check that --gc-sections honors --dynamic-list.
+
+// Copyright 2014 Free Software Foundation, Inc.
+// Written by Cary Coutant <ccoutant@google.com>.
+
+// This file is part of gold.
+
+// This program is free software; you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation; either version 3 of the License, or
+// (at your option) any later version.
+
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with this program; if not, write to the Free Software
+// Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+// MA 02110-1301, USA.
+
+// The goal of this program is to verify that the symbol "keep" is not
+// garbage-collected when it is named in a --dynamic-list script.
+
+extern void keep(void);
+
+void
+keep(void)
+{}
+
+int
+main(void)
+{ return 0; }
diff --git a/gold/testsuite/gc_dynamic_list_test.sh b/gold/testsuite/gc_dynamic_list_test.sh
new file mode 100755
index 0000000..b1723dd
--- /dev/null
+++ b/gold/testsuite/gc_dynamic_list_test.sh
@@ -0,0 +1,39 @@
+#!/bin/sh
+
+# gc_comdat_test.sh -- test --gc-sections
+
+# Copyright 2014 Free Software Foundation, Inc.
+# Written by Cary Coutant <ccoutant@google.com>.
+
+# This file is part of gold.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+# MA 02110-1301, USA.
+
+# The goal of this program is to verify if comdat's and garbage
+# collection work together. Files gc_comdat_test_1.cc and
+# gc_comdat_test_2.cc are used in this test. This program checks
+# if the kept comdat section is garbage collected.
+
+check()
+{
+ if ! grep -q "$2" "$1"
+ then
+ echo "Garbage collection should not have collected '$2'"
+ exit 1
+ fi
+}
+
+check gc_dynamic_list_test.stdout "keep"
diff --git a/gold/testsuite/gc_dynamic_list_test.t b/gold/testsuite/gc_dynamic_list_test.t
new file mode 100644
index 0000000..8905541
--- /dev/null
+++ b/gold/testsuite/gc_dynamic_list_test.t
@@ -0,0 +1,25 @@
+/* gc_dynamic_list_test.t -- script file for gc_dynamic_list_test.cc
+
+ Copyright 2014 Free Software Foundation, Inc.
+ Written by Cary Coutant <ccoutant@google.com>.
+
+ This file is part of gold.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program; if not, write to the Free Software
+ Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
+ MA 02110-1301, USA. */
+
+{
+ keep;
+};
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [gold commit] Fix problems with --dynamic-list option.
2014-02-06 2:04 [gold commit] Fix problems with --dynamic-list option Cary Coutant
@ 2014-02-06 22:30 ` Alan Modra
2014-02-06 23:04 ` Cary Coutant
0 siblings, 1 reply; 3+ messages in thread
From: Alan Modra @ 2014-02-06 22:30 UTC (permalink / raw)
To: Cary Coutant; +Cc: binutils
On Wed, Feb 05, 2014 at 06:04:33PM -0800, Cary Coutant wrote:
> PR gold/16530 complains that symbols listed in the --dynamic-list script
> can still be garbage collected. I've fixed this by checking the symbols
> as they're added to the symbol table. (Unlike the --export-dynamic-symbol
> option, we can't iterate over the list, because the --dynamic-list script
> can have wildcards in it.)
Should --dynamic-list affect garbage collection? If so, what about
--dynamic-list-cpp-new, --dynamic-list-cpp-new, and --dynamic-list-data?
I'm thinking the next thing you'll see is a complaint that
--dynamic-list now unexpectedly prevents garbage collection, for
someone using a wildcard in their dynamic-list file.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [gold commit] Fix problems with --dynamic-list option.
2014-02-06 22:30 ` Alan Modra
@ 2014-02-06 23:04 ` Cary Coutant
0 siblings, 0 replies; 3+ messages in thread
From: Cary Coutant @ 2014-02-06 23:04 UTC (permalink / raw)
To: Cary Coutant, Binutils
>> PR gold/16530 complains that symbols listed in the --dynamic-list script
>> can still be garbage collected. I've fixed this by checking the symbols
>> as they're added to the symbol table. (Unlike the --export-dynamic-symbol
>> option, we can't iterate over the list, because the --dynamic-list script
>> can have wildcards in it.)
>
> Should --dynamic-list affect garbage collection? If so, what about
> --dynamic-list-cpp-new, --dynamic-list-cpp-new, and --dynamic-list-data?
>
> I'm thinking the next thing you'll see is a complaint that
> --dynamic-list now unexpectedly prevents garbage collection, for
> someone using a wildcard in their dynamic-list file.
I thought about this, and concluded that --dynamic-list should in fact
prevent garbage collection, because the symbols in that list have been
explicitly called out (even if via wildcard). The other forms of
--dynamic-list-xxx, on the other hand, call for a class of symbol, so
I reasoned that they should only affect what's left after garbage
collection. I could be wrong.
I'm more worried about gold users that have been expecting
--dynamic-list to do the wrong thing. After a bit of archeology, I
found that the initial implementation in gold was done so it could be
used as a file-based version of --export-dynamic-symbol -- i.e., for
executables rather than shared libraries. As a result, its
implementation w.r.t shared libraries was incomplete, and it was
actually possible to use it along with -Bsymbolic, which means that
even though the symbols are all exported and called via the PLT, the
DT_SYMBOLIC flag is set in the dynamic table, and the dynamic loader
is still going to resolve them all within the library. So the old gold
behavior is that --dynamic-list with -Bsymbolic preserved -Bsymbolic
behavior, while simply forcing each symbol in the list to be exported.
I suspect, however, that gold users (at least those within Google)
have been using --dynamic-list on executables, and that anyone using
it while building a shared library would have noticed the difference
between Gnu ld and gold (as PR 13577 did).
-cary
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-02-06 23:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-06 2:04 [gold commit] Fix problems with --dynamic-list option Cary Coutant
2014-02-06 22:30 ` Alan Modra
2014-02-06 23:04 ` Cary Coutant
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).