[PATCH] D145208: [COFF] Add MC support for emitting IMAGE_WEAK_EXTERN_ANTI_DEPENDENCY symbols
Eli Friedman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 26 20:02:44 PDT 2023
efriedma added inline comments.
================
Comment at: llvm/lib/MC/MCExpr.cpp:764
static bool canExpand(const MCSymbol &Sym, bool InSet) {
+ if (Sym.isWeakExternal())
+ return false;
----------------
aeubanks wrote:
> efriedma wrote:
> > bcl5980 wrote:
> > > Do we need to limit it to AntiDep only? This code looks means any weak symbol can form a cycle.
> > isWeakExternal should be true for any symbol where we call setIsWeakExternal, which should be all weak symbols on Windows.
> >
> > The whole recursion this is just much less likely to be relevant for non-AntiDep weak symbols; I've never seen this sort of usage of aliases elsewhere.
> in https://crbug.com/1435480 we're getting undefined symbol errors with symbols introduced via whole program devirtualization. removing this change resolves the issue. any thoughts?
>
> the undefined symbol is a hidden alias to a weak_odr alias to a private ptr array constant
So you're talking about something like the following, I guess?
```
baz:
.word 1
.weak bar
.set bar, baz
.globl foo
.set foo, bar
```
Without the patch, we resolve "foo" to point directly at "baz". That seems dubious; we're basically ignoring the linkage of the alias "bar".
With the patch, "foo" is undefined, which is pretty clearly wrong. I'll need to take another look; I guess the code that handles aliases isn't expecting a non-weak alias to point at a weak alias? You can revert in the meantime if you need to...
What I'd expect is that we actually just emit the alias into the object file.
Do you know what code is emitting this construct? It seems like whatever the original IR was trying to do, it wasn't actually getting the effect it wanted.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145208/new/
https://reviews.llvm.org/D145208
More information about the llvm-commits
mailing list