If we infer the dso_local flag for -fpic, dso_local should be dropped
when we convert a GlobalVariable a declaration. dso_local causes the
generation of direct access (e.g. R_X86_64_PC32). Such relocations referencing
STB_GLOBAL STV_DEFAULT objects are not allowed in a -shared link.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Should there be a verifier check that there is no dso_local on declarations (after this and D74751 go in)?
llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp | ||
---|---|---|
186 ↗ | (On Diff #245064) | Better to do this inside convertToDeclaration, since there are other places we use that to drop definitions. Also logically it belongs there. |
llvm/test/Transforms/ThinLTOBitcodeWriter/split-dsolocal.ll | ||
9 | This checking seems unnecessary in this test. |
llvm/lib/Transforms/IPO/FunctionImport.cpp | ||
---|---|---|
993 | Don't we also need it here (alias case)? |
llvm/lib/Transforms/IPO/FunctionImport.cpp | ||
---|---|---|
993 | GlobalAlias does not need it, but I'll add a test case that an alias works. I will also add a hidden GlobalVariable case. If we don't check if (!GV.isImplicitDSOLocal()), setting a hidden GlobalVariable declaration dso_preemptable will trigger a verifier error (IR/Verifier.cpp:visitGlobalValue). |
LGTM
llvm/lib/Transforms/IPO/FunctionImport.cpp | ||
---|---|---|
993 | Oh ic why, because we create a new decl without it in the alias case. Nevermind. |
Hello, It looks like this change is breaking the test llvm/test/tools/gold/X86/thinlto_weak_library.ll . Can you please verify? Thanks!
Don't we also need it here (alias case)?