This is an archive of the discontinued LLVM Phabricator instance.

Fix parent_path, used when generating libLTO path for unconditional`-lto_library', to handle compiler symlinks
ClosedPublic

Authored by jwhowarth on Oct 29 2016, 6:21 AM.

Details

Summary

Currently clang improperly handles the generation of the libLTO path, which is passed unconditionally to '-lto_library`on the linker, when the compiler is invoked via a symlink.

The attached patch solves this issue by passing D.Dir rather than D.getInstalledDir() to llvm::sys::path::parent_path() when setting P to the parent directory path.

IMHO, the nomenclature used for Dir and InstalledDir is confusing. Dir is actually the installed location of the compiler binary while InstalledDir is the unresolved path of the invoked compiler. I discovered this empirically by stepping through the execution of a stage1 bootstrap clang compiler when invoked through a direct path at /sw/src/fink.build/llvm40-4.0.0-1/build/stage3/bin/clang-4.0 and through a symlink at /tmp/clang-4.0.

Comparison of the lldb output for 'p/s D' after stepping past 'const Driver &getDriver() const { return D; }' revealed that, counterintuitively, Dir represents the path to the actual compiler binary and InstallDir represents the path to the symlink.

--- binary	2016-10-28 09:28:14.000000000 -0400
+++ symlink	2016-10-28 09:28:32.000000000 -0400
@@ -11,7 +11,7 @@
   Name = "clang-4.0"
   Dir = "/sw/src/fink.build/llvm40-4.0.0-1/build/stage3/bin"
   ClangExecutable = "/sw/src/fink.build/llvm40-4.0.0-1/build/stage3/bin/clang-4.0"
-  InstalledDir = "/sw/src/fink.build/llvm40-4.0.0-1/build/last/bin"
+  InstalledDir = "/tmp"
   ResourceDir = "/sw/src/fink.build/llvm40-4.0.0-1/build/stage3/bin/../lib/clang/4.0.0"

https://llvm.org/bugs/show_bug.cgi?id=30811#c5

Diff Detail

Repository
rL LLVM

Event Timeline

jwhowarth updated this revision to Diff 76299.Oct 29 2016, 6:21 AM
jwhowarth retitled this revision from to Fix parent_path, used when generating libLTO path for unconditional`-lto_library', to handle compiler symlinks.
jwhowarth updated this object.
jwhowarth added reviewers: mehdi_amini, dexonsmith.
jwhowarth set the repository for this revision to rL LLVM.

Proposed patch validated on x86_64-apple-darwin15 using 3-stage bootstrap with stage2/stage3 file comparison. No regressions are introduced in the compiler test suite. Also confirmed that the installed compiler properly generates the libLTO path for the unconditional '-lto_library' when invoked using the full path to the compiler binary or symlink as well as for using the compiler or symlink filename when their directories are in PATH.

mehdi_amini accepted this revision.Oct 29 2016, 10:35 AM
mehdi_amini edited edge metadata.

Great! Thanks.

This revision is now accepted and ready to land.Oct 29 2016, 10:35 AM

One clarification why the observed values of...

Dir = "/sw/src/fink.build/llvm40-4.0.0-1/build/stage3/bin"
InstalledDir = "/sw/src/fink.build/llvm40-4.0.0-1/build/last/bin"

were seen for executing the path pointing directly to the clang-4.0 compiler binary. My 3-stage bootstrap configures as...

$ cd /sw/src/fink.build/llvm40-4.0.0-1/build
$ ls -l
lrwxr-xr-x 1 fink-bld fink-bld 6 Oct 29 01:30 last -> stage3
drwxr-xr-x 32 fink-bld fink-bld 1088 Oct 29 03:18 stage3

I accidentally executed 'lldb /sw/src/fink.build/llvm40-4.0.0-1/build/last/bin/clang-4.0' rather than '/sw/src/fink.build/llvm40-4.0.0-1/build/stage3/bin/clang-4.0'. Both are paths directly to the compiler binary but the one used with lldb has an intermediate symlink in the path.

Do you have commit access or should I commit it for you?

Do you have commit access or should I commit it for you?

No I don't have commit access since I rarely code anything for LLVM.

Please go ahead and commit this so I can request that it and https://reviews.llvm.org/D25932 be back ported to clang 3.6 branch for the 3.6.1 release.

You mean 3.9.1 release, right?

You mean 3.9.1 release, right?

Doh. Yes, I meant back port to 3.9 branch and 3.9.1.

This revision was automatically updated to reflect the committed changes.