[gold commit] Fix problems with --dynamic-list option.

[ccoutant at google dot com (Cary Coutant)]

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;
+};