This is an archive of the discontinued LLVM Phabricator instance.

[tools][remarks-shlib] Don't build libRemarks.so without PIC
ClosedPublic

Authored by ro on Aug 10 2020, 12:04 AM.

Details

Summary

A build on sparcv9-sun-solaris2.11 with -DLLVM_ENABLE_PIC=Off failed linking libRemarks.so:

[27/2297] Linking CXX shared library lib/libRemarks.so.12git
FAILED: lib/libRemarks.so.12git
[...]
ld: fatal: relocation error: R_SPARC_H44: file lib/libLLVMRemarks.a(Remark.cpp.o): symbol _ZTVN4llvm18raw_string_ostreamE: invalid shared object relocation type: ABS44 code model unsupported
[...]

On Solaris/sparcv9 as on many other targets you cannot link non-PIC objects into a shared object.

The following patch avoids this by not building the library with PIC. It allowed the build to complete and ninja check-all showed no errors.

Diff Detail

Event Timeline

ro created this revision.Aug 10 2020, 12:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 10 2020, 12:04 AM
ro requested review of this revision.Aug 10 2020, 12:04 AM
thegameg requested changes to this revision.Sep 17 2020, 4:29 PM

Can you wrap the condition around the whole file? The rest is not useful without the library anyway.

This revision now requires changes to proceed.Sep 17 2020, 4:29 PM
ro updated this revision to Diff 292742.Sep 18 2020, 7:08 AM

Wrap whole file in if(LLVM_ENABLE_PIC).

Tested on amd64-pc-solaris2.11 with and without -DLLVM_ENABLE_PIC=Off.

lebedev.ri added inline comments.
llvm/tools/remarks-shlib/CMakeLists.txt
1–2

Is this the only LLVM shared library that was missing this if(LLVM_ENABLE_PIC) check?
If not, this comment is misleading, since clearly it worked so far everywhere else.

ro added inline comments.Sep 18 2020, 7:17 AM
llvm/tools/remarks-shlib/CMakeLists.txt
1–2

No, LLVMPolly.so had the same issue: fixed in D85627.

thegameg accepted this revision.Sep 18 2020, 3:06 PM

LGTM

llvm/tools/remarks-shlib/CMakeLists.txt
1–2

Possibly libLTO.so too? But maybe it doesn't get built during check-all on your platform. There is LLVM_TOOL_LTO_BUILD but from what I see it's never checked.

This revision is now accepted and ready to land.Sep 18 2020, 3:06 PM
ro marked an inline comment as done.Sep 20 2020, 3:39 AM
ro added inline comments.
llvm/tools/remarks-shlib/CMakeLists.txt
1–2

I do find libLTO.so in a default (i.e. PIC) build on Solaris, but it's omitted with -DLLVM_ENABLE_PIC=Off without doing anything special.

This revision was automatically updated to reflect the committed changes.
ro marked an inline comment as done.