This is an archive of the discontinued LLVM Phabricator instance.

[OpenEmbedded] Fix lib paths for OpenEmbedded targets
ClosedPublic

Authored by mgrang on Jul 2 2018, 6:35 PM.

Details

Summary

The lib paths are not correctly picked up for OpenEmbedded sysroots (like arm-oe-linux-gnueabi) for 2 reasons:

  1. OpenEmbedded sysroots are of the form <sysroot>/usr/lib/<triple>/x.y.z. This form is handled in clang but only for Freescale vendor.
  1. 64-bit OpenEmbedded sysroots may not have a /usr/lib dir. So they cannot find /usr/lib64 as it is referenced as /usr/lib/../lib64 in clang.

This is a follow-up to the llvm patch: D48861

Diff Detail

Repository
rL LLVM

Event Timeline

mgrang created this revision.Jul 2 2018, 6:35 PM
mgrang updated this revision to Diff 153845.Jul 2 2018, 6:40 PM
mgrang added a comment.Jul 5 2018, 4:47 PM

The prerequisite llvm patch has been committed: r336401

Ping for reviews, please.

mgrang updated this revision to Diff 155112.Jul 11 2018, 9:14 PM
mgrang added a reviewer: hfinkel.

Added tests for C++ headers search.

mgrang retitled this revision from Fix lib paths for OpenEmbedded targets to [OpenEmbedded] Fix lib paths for OpenEmbedded targets.Jul 13 2018, 12:12 PM

Ping 2 for reviews please.

Ping 3 for reviews please.

rsmith accepted this revision.Jul 19 2018, 1:29 PM
rsmith added a subscriber: rsmith.
rsmith added inline comments.
lib/Driver/ToolChains/Gnu.cpp
2205–2209 ↗(On Diff #155112)

Combine this with the Freescale case rather than adding a separate entry to the suffix table.

lib/Driver/ToolChains/Linux.cpp
386–390 ↗(On Diff #155112)

It would make more sense to include this as an alternative to the "/usr/lib/../" + OSLibDir path above, rather than adding *both* if /usr/lib does exist on an OpenEmbedded system.

This revision is now accepted and ready to land.Jul 19 2018, 1:29 PM
mgrang updated this revision to Diff 157106.Jul 24 2018, 12:44 PM

Thanks @rsmith. I have addressed your comments.

mgrang marked 2 inline comments as done.Jul 24 2018, 12:44 PM
mgrang updated this revision to Diff 157391.Jul 25 2018, 4:25 PM

Rebased patch and fixed unit test on Windows.

@rsmith Could you please take a look at the updated patch? I would like to commit this. Thanks.

I would commit this today unless there any comments. Thanks.

This revision was automatically updated to reflect the committed changes.
phosek added a subscriber: phosek.Jul 31 2018, 12:20 AM

This has been breaking our bots since it has landed (both Linux and macOS ones), here's a log: https://logs.chromium.org/v/?s=fuchsia%2Fbuildbucket%2Fcr-buildbucket.appspot.com%2F8939527016533487696%2F%2B%2Fsteps%2Fclang%2F0%2Fsteps%2Fcheck-clang%2F0%2Fstdout

In our Clang toolchain, we set libc++ to be the default C++ library and compiler-rt to be the default runtime library. In your case you need to explicitly set -stdlib=libstdc++ and -rtlib=libgcc if you're relying on those being used in your tests.

I fixed linux-header-search.cpp by adding -stdlib=libstdc++ in r338360 because I was seeing the same failure and that's what agreed to do in these cases. If you can verify that it fixes your problems, I think it's safe to add -rtlib=libgcc to the other test.