[ELF] Add -Bno-symbolic

[MaskRay]

Summary

This option will be available in GNU ld 2.27 (https://sourceware.org/bugzilla/show_bug.cgi?id=27834).
This option can cancel previously specified -Bsymbolic and
-Bsymbolic-functions. This is useful for excluding some links when the
default uses -Bsymbolic-functions.

[MaskRay]

(I know our -Bsymbolic-functions tests are insufficient, e.g. its effect on data symbols and its interaction with -Bsymbolic is untested. I may add tests in a separate patch.)

[jhenderson]

lld/ELF/Driver.cpp

956–963

If I'm reading this change correctly, there is a change in behaviour for the case where both -Bsymbolic and -Bsymbolic-functions are specified. Previously, both config variables would be set and since -Bsymbolic-functions is a subset of -Bsymbolic, it would behave the same as -Bsymbolic. Now, if the arguments were ordered -Bsymbolic -Bsymbolic-functions, only the config->bsymbolicFunctions variable would be set, and consequently we'd see a behaviour change.

Is this intentional? It's fine, if that's what GNU does, but it probably deserves a release note.

[MaskRay]

add a release note

[MaskRay]

lld/ELF/Driver.cpp

956–963

Yes. Added

[MaskRay]

rebase on D102461

[MaskRay]

I hope that Linux distributions can make better use of -Bsymbolic-functions to fix a technical fallout for ELF dynamic linking. -fno-pic should be fixed to use GOT: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100593

Perhaps it needs 5 or even 10 years to shift to the better world but they will need -Bno-symbolic sooner or later:)

[jhenderson]

You said that -Bsymbolic-functions is undertested, so I assume the behaviour of "last one wins" for -Bsymbolic-functions + -Bsymbolic will be covered by that work? If so, LGTM. If not, I think you should expand the bsymbolic.s input such that -Bsymbolic and -Bsymbolic-functions output looks different somehow.

[jhenderson]

In D102383#2758982, @jhenderson wrote:

You said that -Bsymbolic-functions is undertested, so I assume the behaviour of "last one wins" for -Bsymbolic-functions + -Bsymbolic will be covered by that work? If so, LGTM. If not, I think you should expand the bsymbolic.s input such that -Bsymbolic and -Bsymbolic-functions output looks different somehow.

Oh, I missed D102461, combined with a misreading of the test coverage. LGTM!

[peter.smith]

LGTM too.

As an aside, is it worth attempting something like "-warn-backrefs", say "-warn-interposition" for executables. In that case we know the definitions of the executable and all the shared libraries as well as the DT_NEEDED order. We could do something like:
"Warning symbol S of type STT_FUNC is defined in executable A and shared objects B and C, using definition in A.". There would be limitations within LLD as we don't keep much more than the symbol table of shared objects, and it might not know all the shared objects being loaded. There is a possibility of optionally doing more such as loading the dynamic section to check if DF_SYMBOLIC is set, this would permit "Warning symbol S is defined in executable A and shared object B with DF_SYMBOLIC flag. A and B will use different definitions."

Nothing stopping this being an external tool although having it built into the linker makes it easier to use.

[MaskRay]

In D102383#2759061, @peter.smith wrote:

LGTM too.

As an aside, is it worth attempting something like "-warn-backrefs", say "-warn-interposition" for executables. In that case we know the definitions of the executable and all the shared libraries as well as the DT_NEEDED order. We could do something like:
"Warning symbol S of type STT_FUNC is defined in executable A and shared objects B and C, using definition in A.". There would be limitations within LLD as we don't keep much more than the symbol table of shared objects, and it might not know all the shared objects being loaded. There is a possibility of optionally doing more such as loading the dynamic section to check if DF_SYMBOLIC is set, this would permit "Warning symbol S is defined in executable A and shared object B with DF_SYMBOLIC flag. A and B will use different definitions."

Nothing stopping this being an external tool although having it built into the linker makes it easier to use.

This is a good idea. Similar to archive shadowing, if the linker command line contains ... a.so b.so ... and we know that not all b.so definitions are provided by previous DSOs or the executable, there is a potential interposition risk.

(As of DF_SYMBOLIC, it would be really difficult to eliminate copy relocations so -Bsymbolic might be impractical for a long time.)

lld/ELF/Config.h

llvm::MapVector<std::pair<const InputSectionBase *, const InputSectionBase *>,
                   uint64_t>
       callGraphProfile;
   bool allowMultipleDefinition;
   bool androidPackDynRelocs;
   bool armHasBlx = false;
   bool armHasMovtMovw = false;
   bool armJ1J2BranchEncoding = false;
   bool asNeeded = false;
-  bool bsymbolic;
-  bool bsymbolicFunctions;
+  bool bsymbolic = false;
+  bool bsymbolicFunctions = false;
   bool callGraphProfileSort;
   bool checkSections;
   bool compressDebugSections;
   bool cref;
   std::vector<std::pair<llvm::GlobPattern, uint64_t>> deadRelocInNonAlloc;
   bool defineCommon;
   bool demangle = true;
   bool dependentLibraries;

lld/ELF/Driver.cpp

static void readConfigs(opt::InputArgList &args) {
   errorHandler().vsDiagnostics =
       args.hasArg(OPT_visual_studio_diagnostics_format, false);
 
   config->allowMultipleDefinition =
       args.hasFlag(OPT_allow_multiple_definition,
                    OPT_no_allow_multiple_definition, false) ||
       hasZOption(args, "muldefs");
   config->auxiliaryList = args::getStrings(args, OPT_auxiliary);
-  config->bsymbolic = args.hasArg(OPT_Bsymbolic);
-  config->bsymbolicFunctions = args.hasArg(OPT_Bsymbolic_functions);
+  if (opt::Arg *arg = args.getLastArg(OPT_Bno_symbolic, OPT_Bsymbolic_functions,
+                                      OPT_Bsymbolic)) {
+    if (arg->getOption().matches(OPT_Bsymbolic_functions))
+      config->bsymbolicFunctions = true;
+    else if (arg->getOption().matches(OPT_Bsymbolic))
+      config->bsymbolic = true;
+  }
   config->checkSections =
       args.hasFlag(OPT_check_sections, OPT_no_check_sections, true);
   config->chroot = args.getLastArgValue(OPT_chroot);
   config->compressDebugSections = getCompressDebugSections(args);
   config->cref = args.hasFlag(OPT_cref, OPT_no_cref, false);
   config->defineCommon = args.hasFlag(OPT_define_common, OPT_no_define_common,
                                       !args.hasArg(OPT_relocatable));
   config->optimizeBBJumps =
       args.hasFlag(OPT_optimize_bb_jumps, OPT_no_optimize_bb_jumps, false);
if (Expected<GlobPattern> pat = GlobPattern::create(pattern))
       config->warnBackrefsExclude.push_back(std::move(*pat));
     else
       error(arg->getSpelling() + ": " + toString(pat.takeError()));
   }
 
   // When producing an executable, --dynamic-list specifies non-local defined
   // symbols which are required to be exported. When producing a shared object,
   // symbols not specified by --dynamic-list are non-preemptible.
-  config->symbolic =
-      args.hasArg(OPT_Bsymbolic) || args.hasArg(OPT_dynamic_list);
+  config->symbolic = config->bsymbolic || args.hasArg(OPT_dynamic_list);
   for (auto *arg : args.filtered(OPT_dynamic_list))
     if (Optional<MemoryBufferRef> buffer = readFile(arg->getValue()))
       readDynamicList(*buffer);
 
   // --export-dynamic-symbol specifies additional --dynamic-list symbols if any
   // other option expresses a symbolic intention: -no-pie, -pie, -Bsymbolic,
   // -Bsymbolic-functions (if STT_FUNC), --dynamic-list.
   for (auto *arg : args.filtered(OPT_export_dynamic_symbol))

lld/ELF/Options.td

 
 multiclass B<string name, string help1, string help2> {
   def NAME: Flag<["--", "-"], name>, HelpText<help1>;
   def no_ # NAME: Flag<["--", "-"], "no-" # name>, HelpText<help2>;
 }
 
 defm auxiliary: Eq<"auxiliary", "Set DT_AUXILIARY field to the specified name">;
 
-def Bsymbolic: F<"Bsymbolic">, HelpText<"Bind defined symbols locally">;
+def Bno_symbolic: F<"Bno-symbolic">, HelpText<"Don't bind default visibility defined symbols locally for -shared (default)">;
+
+def Bsymbolic: F<"Bsymbolic">, HelpText<"Bind default visibility defined symbols locally for -shared">;
 
 def Bsymbolic_functions: F<"Bsymbolic-functions">,
-  HelpText<"Bind defined function symbols locally">;
+  HelpText<"Bind default visibility defined function symbols locally for -shared">;
 
 def Bdynamic: F<"Bdynamic">, HelpText<"Link against shared libraries (default)">;
 
 def Bstatic: F<"Bstatic">, HelpText<"Do not link against shared libraries">;
 
 def build_id: F<"build-id">, HelpText<"Alias for --build-id=fast">;
 
 def build_id_eq: J<"build-id=">, HelpText<"Generate build ID note">,

lld/docs/ReleaseNotes.rst

 from the `LLVM releases web site <https://llvm.org/releases/>`_.
 
 Non-comprehensive list of changes in this release
 =================================================
 
 ELF Improvements
 ----------------
 
-* ...
+* ``-Bsymbolic -Bsymbolic-functions`` has been changed to behave the same as ``-Bsymbolic-functions``. This matches GNU ld.
+  (`D102461 <https://reviews.llvm.org/D102461>`_)
+* ``-Bno-symbolic`` has been added.
+  (`D102461 <https://reviews.llvm.org/D102461>`_)
 
 Breaking changes
 ----------------
 
 * ``--shuffle-sections=<seed>`` has been changed to ``--shuffle-sections=<section-glob>=<seed>``.
   Specify ``*`` as ``<section-glob>`` to get the previous behavior.
 
 COFF Improvements

lld/docs/ld.lld.1

 .It Fl -auxiliary Ns = Ns Ar value
 Set the
 .Dv DT_AUXILIARY
 field to the specified name.
 .It Fl -Bdynamic , Fl -dy
 Link against shared libraries.
 .It Fl -Bstatic , Fl -static , Fl -dn
 Do not link against shared libraries.
-.It Fl -Bsymbolic
-Bind defined symbols locally.
-.It Fl -Bsymbolic-functions
-Bind defined function symbols locally.
+.It Fl Bno-symbolic
+Don't bind default visibility defined symbols locally for
+.Fl shared
+(default).
+.It Fl Bsymbolic
+Bind default visibility defined symbols locally for
+.Fl shared.
+Also set the
+.Dv DF_SYMBOLIC
+flag.
+.It Fl Bsymbolic-functions
+Bind default visibility defined function symbols locally for
+.Fl shared.
 .It Fl -build-id Ns = Ns Ar value
 Generate a build ID note.
 .Ar value
 may be one of
 .Cm fast ,
 .Cm md5 ,
 .Cm sha1 ,
 .Cm tree ,

lld/test/ELF/bsymbolic.s

 ## -Bsymbolic makes all definitions non-preemptible.
 # RUN: ld.lld -shared -Bsymbolic %t/a.o %t/b.o -o %t2.so
 # RUN: llvm-readobj -r %t2.so | FileCheck %s --check-prefix=REL_ALL
 # RUN: llvm-objdump -d %t2.so | FileCheck %s --check-prefix=ASM_ALL
 
 # RUN: ld.lld -shared -Bsymbolic-functions -Bsymbolic %t/a.o %t/b.o -o %t.so
 # RUN: cmp %t.so %t2.so
 # RUN: ld.lld -shared -Bsymbolic -Bsymbolic-functions %t/a.o %t/b.o -o %t.so
+# RUN: cmp %t.so %t1.so
+# RUN: ld.lld -shared -Bno-symbolic -Bsymbolic %t/a.o %t/b.o -o %t.so
 # RUN: cmp %t.so %t2.so
 
+## -Bno-symbolic can cancel previously specified -Bsymbolic and -Bsymbolic-functions.
+# RUN: ld.lld -shared -Bsymbolic -Bno-symbolic %t/a.o %t/b.o -o %t.so
+# RUN: cmp %t.so %t0.so
+# RUN: ld.lld -shared -Bsymbolic-functions -Bno-symbolic %t/a.o %t/b.o -o %t.so
+# RUN: cmp %t.so %t0.so
+
 # REL_DEF:      .rela.dyn {
 # REL_DEF-NEXT:   R_X86_64_RELATIVE -
 # REL_DEF-NEXT:   R_X86_64_RELATIVE -
 # REL_DEF-NEXT:   R_X86_64_64 data_default
 # REL_DEF-NEXT: }
 # REL_DEF-NEXT: .rela.plt {
 # REL_DEF-NEXT:   R_X86_64_JUMP_SLOT default
 # REL_DEF-NEXT:   R_X86_64_JUMP_SLOT ext_default