[PATCH] D135897: [LLD][ELF] --wrap: __real_foo references should trigger archive extraction for foo

Mon Oct 17 17:11:01 PDT 2022

MaskRay added inline comments.

================
Comment at: lld/test/ELF/wrap-extract-real.ll:19
+# REAL-NEXT:        {{.*}}           {{.*}} FUNC    GLOBAL DEFAULT     2 _start
+# REAL-NEXT:        {{.*}}           {{.*}} FUNC    WEAK   DEFAULT     2 foo
+# REAL-NEXT:        {{.*}}           {{.*}} NOTYPE  GLOBAL DEFAULT   UND __wrap_foo
----------------
smeenai wrote:
> bd1976llvm wrote:
> > smeenai wrote:
> > > Why does this become weak? (It's global for the assembly test case, which is what I'd expect.)
> > Good question! It seems to be a bug. Weak binding is assigned by this piece of code:
> > 
> > llvm\lib\LTO\LTO.cpp:
> >         // For symbols re-defined with linker -wrap and -defsym options,
> >         // set the linkage to weak to inhibit IPO. The linkage will be
> >         // restored by the linker.
> >         if (Res.LinkerRedefined)
> >           GV->setLinkage(GlobalValue::WeakAnyLinkage);
> > 
> > However, LLD doesn't seem to have fulfilled its side of the bargain alluded to in this comment.
> > 
> > I'll look at this more tomorrow.
> Ah, I remember this now; that's https://github.com/llvm/llvm-project/issues/51346. It's independent of your change though, so you can certainly land this without needing to fix that.
So this is related to a WeakAnyLinkage workaround D33621. It'd be difficult to fix properly but it's good to re-think how to handle it.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135897/new/

https://reviews.llvm.org/D135897