This is an archive of the discontinued LLVM Phabricator instance.

[Host] Respect LLVM_LIBDIR_SUFFIX when looking for LLDB plugins on Linux
ClosedPublic

Authored by mgorny on Jan 19 2018, 2:34 PM.

Details

Summary

Fix the Linux plugin lookup path to include appropriate libdir suffix
for the system. To accomplish this, store the value of
LLVM_LIBDIR_SUFFIX in lldb/Host/Config.h as LLDB_LIBDIR_SUFFIX,
and use this variable when defining the plugin path.

Diff Detail

Repository
rL LLVM

Event Timeline

mgorny created this revision.Jan 19 2018, 2:34 PM

Disclaimer: I have never seen a plugin for LLDB. I've just noticed the wrong path in strace output and fixed it ;-).

labath accepted this revision.Jan 29 2018, 6:37 AM
labath added a subscriber: labath.

We have one plugin in-tree (tools/intel-features), but I'm not sure if anyone uses/installs it.

In any case, the change seems like the right thing to do.

This revision is now accepted and ready to land.Jan 29 2018, 6:37 AM
clayborg added inline comments.
source/Host/linux/HostInfoLinux.cpp
208 ↗(On Diff #130695)

Is there no setting that sets the start of the path here ("/usr/lib")? Not sure how open source projects handle this, but it seems that this should be a build setting that defaults to "/usr/lib" and can be modified?

See inlined comment

mgorny added inline comments.Jan 29 2018, 8:56 AM
source/Host/linux/HostInfoLinux.cpp
208 ↗(On Diff #130695)

Well, yes, this is something that could be possibly be done but TBH I don't really know how exactly it should be done. For autoconf projects, you generally do something like -Dbindir=$(bindir) in the Makefile. For CMake projects, I suppose this could be CMAKE_INSTALL_PREFIX, except I don't know if it has any corner cases. AFAIK clang uses path relative to executable to find compiler-rt, so maybe that would be revelant here as well.

In other words, I went for fixing the most obvious bug, and leaving the harder (yet less frequent) problem to somebody else.

clayborg accepted this revision.Jan 29 2018, 10:19 AM

Looks good.

This revision was automatically updated to reflect the committed changes.