Alternative fix for PR33097

[grimar]

ELF/LTO.cpp

149

Don't we need to allow STV_PROTECTED ?
Also you can probably omit calling computeBinding() as it always return just Binding for relocatable output anyways.
And ExternalReloc sound similar to "External relocation", a bit confusing naming IMO. May be better to not define any variable at all, and just:

if (Config->Relocatable)
  R.VisibleToRegularObj =
      Sym->Binding != STB_LOCAL &&
      (Sym->Visibility == STV_DEFAULT || Sym->Visibility == STV_PROTECTED);
else
  R.VisibleToRegularObj = Sym->IsUsedInRegularObj ||
                          (R.Prevailing && Sym->includeInDynsym()) ||
                          UsedStartStop.count(ObjSym.getSectionName());

[davide]

ELF/LTO.cpp

149

Yes, I'm under the impression we need to handle also PROTECTED here. Peter?

[pcc]

ELF/LTO.cpp

149

I don't think we can drop any global symbols here regardless of visibility. Hidden symbols may only be dropped during a final (exec or DSO) link.

[davide]

ELF/LTO.cpp

149

I assume we can drop symbols marked with STB_LOCAL binding though?

[davide]

Also, FWIW, we should consider making the same change to the gold plugin.

[pcc]

ELF/LTO.cpp

149

I don't think we should ever see STB_LOCAL symbols here during a relocatable link because:

• we never add local symbols to the global symbol table nor are local symbols exposed by the lto api
• output symbols have the same binding as input symbols during relocatable links.

[davide]

Oh, fair enough, my bad.
I guess George's patch should go in instead. Let me abandon this.

ELF/LTO.cpp

for (const lto::InputFile::Symbol &ObjSym : Obj.symbols()) {
 
     // Ideally we shouldn't check for SF_Undefined but currently IRObjectFile
     // reports two symbols for module ASM defined. Without this check, lld
     // flags an undefined in IR with a definition in ASM as prevailing.
     // Once IRObjectFile is fixed to report only one symbol this hack can
     // be removed.
     R.Prevailing = !ObjSym.isUndefined() && B->getFile() == &F;
 
-    R.VisibleToRegularObj = Sym->IsUsedInRegularObj ||
+    // Don't strip external symbols in relocatable links.
+    bool ExternalReloc = Config->Relocatable && Sym->computeBinding() != STB_LOCAL &&
+                       Sym->Visibility == STV_DEFAULT;
+
+    R.VisibleToRegularObj = ExternalReloc || Sym->IsUsedInRegularObj ||
                             (R.Prevailing && Sym->includeInDynsym()) ||
                             UsedStartStop.count(ObjSym.getSectionName());
     if (R.Prevailing)
       undefine(Sym);
     R.LinkerRedefined = Config->RenamedSymbols.count(Sym);
   }
   checkError(LTOObj->add(std::move(F.Obj), Resols));
 }