[PATCH] gold: Add -Bno-symbolic

[PATCH] gold: Add -Bno-symbolic

[Fangrui Song]

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(-)

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.
-- 
2.31.1.751.gd2f1c929bd-goog

Re: [PATCH] gold: Add -Bno-symbolic

[Cary Coutant]

> 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

Re: [PATCH] gold: Add -Bno-symbolic

[Alan Modra]

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

Re: [PATCH] gold: Add -Bno-symbolic

[Alan Modra]

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

Re: [PATCH] gold: Add -Bno-symbolic

[Cary Coutant]

> > 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

Re: [PATCH] gold: Add -Bno-symbolic

[Fangrui Song]

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? :)