gold: Add -Bno-symbolic

Message ID 20210514204402.2100985-1-maskray@google.com
State New
Headers show
Series
  • gold: Add -Bno-symbolic
Related show

Commit Message

H.J. Lu via Binutils May 14, 2021, 8:44 p.m.
gold/
    PR 27834
    * options.h (General_options): Add -Bno-symbolic option, and make
    -Bsymbolic and -Bsymbolic-functions special.
    * options.cc (General_options): Handle these options.
---
 gold/options.cc | 21 +++++++++++++++++++++
 gold/options.h  | 34 ++++++++++++++++++++++++++++++----
 2 files changed, 51 insertions(+), 4 deletions(-)

-- 
2.31.1.751.gd2f1c929bd-goog

Comments

H.J. Lu via Binutils May 14, 2021, 10:03 p.m. | #1
> gold/

>     PR 27834

>     * options.h (General_options): Add -Bno-symbolic option, and make

>     -Bsymbolic and -Bsymbolic-functions special.

>     * options.cc (General_options): Handle these options.


Please mention the new member functions and enum in the ChangeLog.

> +  enum Bsymbolic_Kind

> +  {

> +    BSYMBOLIC_NONE,

> +    BSYMBOLIC_FUNCTIONS,

> +    BSYMBOLIC_ALL,

> +  };


Should be "Bsymbolic_kind".

OK with those changes. Thanks!

-cary
H.J. Lu via Binutils May 18, 2021, 6:27 a.m. | #2
On Fri, May 14, 2021 at 03:03:31PM -0700, Cary Coutant via Binutils wrote:
> > gold/

> >     PR 27834

> >     * options.h (General_options): Add -Bno-symbolic option, and make

> >     -Bsymbolic and -Bsymbolic-functions special.

> >     * options.cc (General_options): Handle these options.

> 


This patch broke a number of tests in the gold testsuite.  On x86_64
compiling with gcc 10.2.1 20210107 I see:

FAIL: icf_preemptible_functions_test.sh
FAIL: x86_64_mov_to_lea.sh
FAIL: two_file_shared_2_test
FAIL: two_file_shared_1_pic_2_test
FAIL: two_file_separate_shared_12_test
FAIL: plugin_test_2

Reverting the patch cures the fails for me.

-- 
Alan Modra
Australia Development Lab, IBM
H.J. Lu via Binutils May 18, 2021, 6:27 a.m. | #3
On Sat, May 15, 2021 at 01:06:16PM +0930, Alan Modra wrote:
> On Fri, May 14, 2021 at 03:03:31PM -0700, Cary Coutant via Binutils wrote:

> > > gold/

> > >     PR 27834

> > >     * options.h (General_options): Add -Bno-symbolic option, and make

> > >     -Bsymbolic and -Bsymbolic-functions special.

> > >     * options.cc (General_options): Handle these options.

> > 

> 

> This patch broke a number of tests in the gold testsuite.  On x86_64


Fixed as follows.  Committing as obvious.

	PR 27834
	* options.cc (General_options::General_options): Init bsymbolic_.

diff --git a/gold/options.cc b/gold/options.cc
index 1818e5dc345..5a55bd8ba6d 100644
--- a/gold/options.cc
+++ b/gold/options.cc
@@ -1008,7 +1008,8 @@ namespace gold
 {
 
 General_options::General_options()
-  : printed_version_(false),
+  : bsymbolic_(BSYMBOLIC_NONE),
+    printed_version_(false),
     execstack_status_(EXECSTACK_FROM_INPUT),
     icf_status_(ICF_NONE),
     static_(false),

-- 
Alan Modra
Australia Development Lab, IBM
H.J. Lu via Binutils May 18, 2021, 7:55 p.m. | #4
> > This patch broke a number of tests in the gold testsuite.  On x86_64

>

> Fixed as follows.  Committing as obvious.

>

>         PR 27834

>         * options.cc (General_options::General_options): Init bsymbolic_.


Thanks, Alan!

-cary
Fangrui Song May 19, 2021, 12:16 a.m. | #5
On 2021-05-18, Cary Coutant via Binutils wrote:
>> > This patch broke a number of tests in the gold testsuite.  On x86_64

>>

>> Fixed as follows.  Committing as obvious.

>>

>>         PR 27834

>>         * options.cc (General_options::General_options): Init bsymbolic_.

>

>Thanks, Alan!

>

>-cary


Thanks!

BTW, I think -Bsymbolic-non-weak-definitions will be a great default for
Linux distributions.
https://sourceware.org/bugzilla/show_bug.cgi?id=27871
How do you think? :)

Patch

diff --git a/gold/options.cc b/gold/options.cc
index fdf0a174797..1818e5dc345 100644
--- a/gold/options.cc
+++ b/gold/options.cc
@@ -340,6 +340,27 @@  General_options::parse_V(const char*, const char*, Command_line*)
     printf("   %s\n", *p);
 }
 
+void
+General_options::parse_Bno_symbolic(const char*, const char*,
+				    Command_line*)
+{
+  this->bsymbolic_ = BSYMBOLIC_NONE;
+}
+
+void
+General_options::parse_Bsymbolic_functions(const char*, const char*,
+					   Command_line*)
+{
+  this->bsymbolic_ = BSYMBOLIC_FUNCTIONS;
+}
+
+void
+General_options::parse_Bsymbolic(const char*, const char*,
+				 Command_line*)
+{
+  this->bsymbolic_ = BSYMBOLIC_ALL;
+}
+
 void
 General_options::parse_defsym(const char*, const char* arg,
 			      Command_line* cmdline)
diff --git a/gold/options.h b/gold/options.h
index 927e0734bba..4a65c8353c4 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -747,11 +747,20 @@  class General_options
   DEFINE_bool(Bshareable, options::ONE_DASH, '\0', false,
 	      N_("Generate shared library (alias for -G/-shared)"), NULL);
 
-  DEFINE_bool(Bsymbolic, options::ONE_DASH, '\0', false,
-	      N_("Bind defined symbols locally"), NULL);
+  DEFINE_special (
+      Bno_symbolic, options::ONE_DASH, '\0',
+      N_ ("Don't bind default visibility defined symbols locally for -shared"),
+      NULL);
 
-  DEFINE_bool(Bsymbolic_functions, options::ONE_DASH, '\0', false,
-	      N_("Bind defined function symbols locally"), NULL);
+  DEFINE_special (Bsymbolic_functions, options::ONE_DASH, '\0',
+		  N_ ("Bind default visibility defined function symbols "
+		      "locally for -shared"),
+		  NULL);
+
+  DEFINE_special (
+      Bsymbolic, options::ONE_DASH, '\0',
+      N_ ("Bind default visibility defined symbols locally for -shared"),
+      NULL);
 
   // c
 
@@ -1740,6 +1749,21 @@  class General_options
   endianness() const
   { return this->endianness_; }
 
+  enum Bsymbolic_Kind
+  {
+    BSYMBOLIC_NONE,
+    BSYMBOLIC_FUNCTIONS,
+    BSYMBOLIC_ALL,
+  };
+
+  bool
+  Bsymbolic() const
+  { return this->bsymbolic_ == BSYMBOLIC_ALL; }
+
+  bool
+  Bsymbolic_functions() const
+  { return this->bsymbolic_ == BSYMBOLIC_FUNCTIONS; }
+
   bool
   discard_all() const
   { return this->discard_locals_ == DISCARD_ALL; }
@@ -1873,6 +1897,8 @@  class General_options
   void
   copy_from_posdep_options(const Position_dependent_options&);
 
+  // Whether we bind default visibility defined symbols locally for -shared.
+  Bsymbolic_Kind bsymbolic_;
   // Whether we printed version information.
   bool printed_version_;
   // Whether to mark the stack as executable.