Page MenuHomePhabricator

[Driver] Add CLANG_PREFER_GCC_LIBRARIES which can be disabled
AbandonedPublic

Authored by Hahnfeld on Nov 2 2016, 7:17 AM.

Details

Summary

Currently, libraries installed next to GCC are preferred over those next to Clang. Make this behaviour configurable during build.

This is for example needed to make sure we get LLVM's libunwind.so.1 and don't end up with the system default non-GNU libunwind.so.8 as reported in D25008.

Diff Detail

Event Timeline

Hahnfeld updated this revision to Diff 76700.Nov 2 2016, 7:17 AM
Hahnfeld retitled this revision from to [Driver] Prefer libraries installed next to Clang over those from GCC.
Hahnfeld updated this object.
Hahnfeld added reviewers: mgorny, rsmith, EricWF.
Hahnfeld added a subscriber: cfe-commits.
emaste added a subscriber: emaste.Nov 3 2016, 6:12 AM
EricWF resigned from this revision.Dec 30 2016, 1:58 AM
EricWF removed a reviewer: EricWF.

I don't feel comfortable reviewing driver changes, since I never work on that code.

Ping. Looks like Chandler originally added this logic...

Ping. Could I get comments before the branching this week?

rsmith edited edge metadata.Jan 9 2017, 12:17 AM

This makes sense to me in principle, but I'd like @chandlerc to weigh in.

chandlerc requested changes to this revision.Jan 10 2017, 12:14 AM
chandlerc edited edge metadata.

There are some issues with this patch currently.

At a basic level, it needs well crafted test cases. You can find out how to write them by looking at other Driver tests.

But at a deeper level, the change here will be an very significant and potentially very disruptive change. While libunwind is your motivation, it could well have a dramatic impact on the selected set of libraries for links. I'm very concerned to make this kind of change without a *lot* of careful testing, and a very clear rationale about why this is the right search order *for all libraries*, not just libunwind. Because this applies to all libraries. =]

This revision now requires changes to proceed.Jan 10 2017, 12:14 AM

This change doesn't break existing tests but maybe I should extend linux-ld.c and so on?

About the impact: Which libraries are there next to clang that could conflict with the system? When I look at my current installation dir:

  • libLLVM*, libclang*, liblld* libraries which should be fine to use
  • libc++, libc++experimental, libc++abi and libunwind which we really want to use IMO
  • OpenMP runtime(s) which are fine to use (or even better than the system default)
  • I'm not sure about libLTO but I *think* that users do not link to this library directly?
Hahnfeld updated this revision to Diff 83784.Jan 10 2017, 2:07 AM
Hahnfeld edited edge metadata.

Add test case to check that

  • bin/../lib is the first library path by default
  • user-specified -L preceeds

This change doesn't break existing tests but maybe I should extend linux-ld.c and so on?

About the impact: Which libraries are there next to clang that could conflict with the system? When I look at my current installation dir:

  • libLLVM*, libclang*, liblld* libraries which should be fine to use
  • libc++, libc++experimental, libc++abi and libunwind which we really want to use IMO
  • OpenMP runtime(s) which are fine to use (or even better than the system default)
  • I'm not sure about libLTO but I *think* that users do not link to this library directly?

Those are just the libriaries LLVM is installing.

If it is installed into a prefix, say, /opt/mumble/{bin,lib,...} along with other libraries, then suddenly that directory is searched in a very different place.

I'm not arguing one way or the other here. I'm pointing out that this is a *huge* change to make. It will cause a large number of users to silently have a different library search path than they have had before, something that has been fairly stable for several years. So whatever we do must be considered very carefully.

Those are just the libriaries LLVM is installing.

If it is installed into a prefix, say, /opt/mumble/{bin,lib,...} along with other libraries, then suddenly that directory is searched in a very different place.

I'm not arguing one way or the other here. I'm pointing out that this is a *huge* change to make. It will cause a large number of users to silently have a different library search path than they have had before, something that has been fairly stable for several years. So whatever we do must be considered very carefully.

Haven't thought about that! If I don't install to /usr/lib, I usually only install one software per prefix but there might be cases where that is not true.

What do you propose? Should I send an RFC to cfe-dev?

mgorny edited edge metadata.Jan 10 2017, 5:33 AM

I think the easiest solution would be for you to use a path related to clang runtime directory, and install the runtimes there.

However, this would require rethinking the structure a bit to cover multilib, different ABIs, maybe cross. We have a cheap hack for this in clang-suite (I can give you link when I get home) but this really requires some smart thinking.

Hahnfeld updated this revision to Diff 86396.Jan 31 2017, 1:37 AM
Hahnfeld retitled this revision from [Driver] Prefer libraries installed next to Clang over those from GCC to [Driver] Add CLANG_PREFER_GCC_LIBRARIES which can be disabled.
Hahnfeld edited the summary of this revision. (Show Details)
Hahnfeld added a reviewer: hfinkel.
Hahnfeld abandoned this revision.Oct 19 2017, 6:24 PM

Abandoning as I lost interest in this.