[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
wzssyqa wrote: > > Sure. The motivation on our side is a distributed compile service where the > library doesn't exist on the remote end. This patch means we'll have to add > knowledge about path layouts at link time to the remote setup at compile > time. That's certainly doable, but kind of janky. > I guess `using new-style always` can make things simpler. Since you won't need to know how to convert the target name to tail arch name. For every arch, it uses the same archive file name. Does your `distributed compile service` is like: 1. Use upload C/C++ code 2. Your service compiles them to ASM code. 3. User downloads the ASM code 4. User assembles and link If so, it will always have a trouble for any case, as the archive's filename differs between users local setup. I think that problem is a chaos during the transition. In fact, a filename like `clang_rt.profile-x86_64.lib` also contains info about path layout at compile time. What you really need is an option for user to tell us their local path layout. https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
mstorsjo wrote: > > I would suggest we revert this - and at least collect all the observed side > > effects and declare them before considering relanding it. > > That sounds good to me. Do you have a list of PRs to revert? Not sure if there are follow-up fixes, sorry, but the discussed PRs are this one, and #81037 (where #89775 says the latter one changed functional behaviour, but it sounds mostly like your issue, i.e. caused by this one). > > ... then we do still get the old name embedded. > > Sure. The motivation on our side is a distributed compile service where the > library doesn't exist on the remote end. This patch means we'll have to add > knowledge about path layouts at link time to the remote setup at compile > time. That's certainly doable, but kind of janky. Right, I see. FWIW, with the embedding of directives, which kind of depends on knowing the runtime layout style, I guess this is an issue that needs to be tackled at one point or another, for distributed builds (especially as long as the intent is to change default towards the per-triple directories instead of per-os directories). > (What we'll actually do is use the flag from #89642 to disable all this > guessing of libs and just explicitly pass the path to the lib at link time. > So this won't actually affect us then, but reverting and relanding in one > commit with a list of side effects still sounds like a good thing independent > of that.) Ah, that probably sounds like a good flag to have in any case, for that kind of distributed setup! https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
nico wrote: > I would suggest we revert this - and at least collect all the observed side > effects and declare them before considering relanding it. That sounds good to me. Do you have a list of PRs to revert? > ... then we do still get the old name embedded. Sure. The motivation on our side is a distributed compile service where the library doesn't exist on the remote end. This patch means we'll have to add knowledge about path layouts at link time to the remote setup at compile time. That's certainly doable, but kind of janky. (What we'll actually do is use the flag from #89642 to disable all this guessing of libs and just explicitly pass the path to the lib at link time. So this won't actually affect us then, but reverting and relanding in one commit with a list of side effects still sounds like a good thing independent of that.) https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
mstorsjo wrote: > I now did build clang at > [ccdebba](https://github.com/llvm/llvm-project/commit/ccdebbae4d77d3efc236af92c22941de5d437e01) > and > [ccdebba](https://github.com/llvm/llvm-project/commit/ccdebbae4d77d3efc236af92c22941de5d437e01)^. > > [ccdebba](https://github.com/llvm/llvm-project/commit/ccdebbae4d77d3efc236af92c22941de5d437e01) > has `/DEFAULTLIB:clang_rt.profile.lib` in its output, > [ccdebba](https://github.com/llvm/llvm-project/commit/ccdebbae4d77d3efc236af92c22941de5d437e01)^ > has /DEFAULTLIB:clang_rt.profile-x86_64.lib`. So definitely due to this > change. > > It sounds like you're saying that's not intentional? I didn't author this, but as far as I understood, the intent of this patch was only to make sure to print the new style path to ease disambiguation for the cases when both are missing - i.e. the same as #81037, for some cases that wasn't covered by the former. I never saw this PR as one that had intended functional effects. At this point, with more and more functional effects popping up, that aren't mentioned as intended within the commit message, I would suggest we revert this - and at least collect all the observed side effects and declare them before considering relanding it. > Will the contents of `empty.asm` correct if > `lib//clang_rt.profile.lib` doesn't exist? I mean, will `empty.asm` > contains `/DEFAULTLIB:clang_rt.profile-x86_64.lib` then? This is in a case where this file does not exist, and neither does the new one. @nico wrote: > `foo` is a directly that contains just these two clang binaries I.e. it is a directory that contains these two binaries and nothing else. I do note that if `lib/clang/19/lib/windows/clang_rt.profile-x86_64.lib` does exist, i.e. if we add a `mkdir -p foo/lib/clang/19/lib/windows && touch foo/lib/clang/19/lib/windows/clang_rt.profile-x86_64.lib`, then we do still get the old name embedded. https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
wzssyqa wrote: Will the contents of `empty.asm` correct if `lib//clang_rt.profile.lib` doesn't exist? I mean, will `empty.asm` contains `/DEFAULTLIB:clang_rt.profile-x86_64.lib` then? https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
nico wrote: I now did build clang at ccdebbae4d77 and ccdebbae4d77^. ccdebbae4d77 has `/DEFAULTLIB:clang_rt.profile.lib` in its output, ccdebbae4d77^ has /DEFAULTLIB:clang_rt.profile-x86_64.lib`. So definitely due to this change. It sounds like you're saying that's not intentional? (It's kind of what #81037 looks like to me – it used to default to old-style, now it defaults to "new"-style. But without this chang here, #81037 doesn't have that effect.) https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
nico wrote: Here's a minimal repro: ``` % cat empty.c % foo/clang --driver-mode=cl empty.c --target=x86_64-pc-windows -fprofile-instr-generate -c /Fa && rg DEFAULTLIB empty.asm 23: .ascii " /DEFAULTLIB:libcmt.lib" 24: .ascii " /DEFAULTLIB:oldnames.lib" 25: .ascii " /DEFAULTLIB:clang_rt.profile.lib" % foo/old-clang --driver-mode=cl empty.c --target=x86_64-pc-windows -fprofile-instr-generate -c /Fa && rg DEFAULTLIB empty.asm 23: .ascii " /DEFAULTLIB:libcmt.lib" 24: .ascii " /DEFAULTLIB:oldnames.lib" 25: .ascii " /DEFAULTLIB:clang_rt.profile-x86_64.lib" ``` `foo` is a directly that contains just these two clang binaries. `clang` is built at clang-llvmorg-19-init-8943-gd8503a38. `old-clang` is built at clang-llvmorg-19-init-8091-gab037c4f. ` git log llvmorg-19-init-8091-gab037c4f..llvmorg-19-init-8795-g39bfdb7f clang/lib/Driver/ToolChain.cpp` shows this as the only change in that range. (I admittedly haven't built clang at this rev and before yet to prove that it's due to this commit. I'll do that now, but it looks pretty plausible.) https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
mstorsjo wrote: > This is a behavior change: In distributed build environments, neither lib > file exists at compile time. Previously, this would result in the "old" > style, now (together with #81037) it results in the "new" style (which we > disable everywhere since it causes all kinds of issues – from what I can > tell, we're not alone in this). Hmm, in which cases does this PR change anything of what happens at compile time? The only functional behaviour difference I'm aware of, is that `clang --print-runtime-dir` now always prints the new style path, even if the old style path exists and was expected to be used. There have been talks about embedding directives for autolinking compiler-rt builtins for msvc mode builds, and switching between old and new path style depending on what files exist on disk (which would be a problem for the kind of distributed build environment you have, admittedly!), but that's not implemented yet. Or is this already the case for embedded directives for asan? And based on your linked issue, apparently also for profiling? Can you distill out a minimal standalone command that showcases compiling, and shows the embedded directive that gets changed by this PR? https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
nico wrote: This is a behavior change: In distributed build environments, neither lib file exists at compile time. Previously, this would result in the "old" style, now (together with #81037) it results in the "new" style (which we disable everywhere since it causes all kinds of issues – from what I can tell, we're not alone in this). Is there some way we can tell clang that we always want the old style here, independent of what's on disk? [ttps://crbug.com/335997052](https://crbug.com/335997052) has details. https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
jhuber6 wrote: It's definitely not ideal that this prints a non-existent path if the per-target runtime directory configuration is off. Couldn't we just do a trivial filesystem check to make sure it exists before appending it? https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
estewart08 wrote: > > Is it expected now that `clang --print-runtime-dir` will always have the > > clang host triple appended even if `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` is > > off? I guess I was expecting to see `lib/linux` instead of > > `lib/x86_64-unknown-linux-gnu`. > > https://reviews.llvm.org/D98868 introduced `--print-runtime-dir`. The > question is whether `--print-runtime-dir` should print the legacy `lib/linux` > when `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` is off. Personally I'd hope that > `--print-runtime-dir` does not try to be smart and users should be able to > expect that it always prints the new hierarchy. > > With the old hierarchy, the user is expected to know how to derive > `libclang_rt.builtins-aarch64.a` from `libclang_rt.builtins.a`. In this case, > they can extract the directory name from `clang > --print-file-name=libclang_rt.builtins-aarch64.a`. Just odd that a legitimate configuration `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF`, unless this configuration is no longer valid, will result in a non-existent path being returned for the runtime directory. https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
mstorsjo wrote: > > Is it expected now that `clang --print-runtime-dir` will always have the > > clang host triple appended even if `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` is > > off? I guess I was expecting to see `lib/linux` instead of > > `lib/x86_64-unknown-linux-gnu`. > > https://reviews.llvm.org/D98868 introduced `--print-runtime-dir`. The > question is whether `--print-runtime-dir` should print the legacy `lib/linux` > when `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` is off. Personally I'd hope that > `--print-runtime-dir` does not try to be smart and users should be able to > expect that it always prints the new hierarchy. > > With the old hierarchy, the user is expected to know how to derive > `libclang_rt.builtins-aarch64.a` from `libclang_rt.builtins.a`. In this case, > they can extract the directory name from `clang > --print-file-name=libclang_rt.builtins-aarch64.a`. That change doesn’t seem to explicitly say that this only is intended to be used with the new layout, and it seems like a number of different users have taken up use of this option in this way, with the old layout. So this change does seem to affect a number of previously seemingly fully legitimate configurations in practice. https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
MaskRay wrote: > Is it expected now that `clang --print-runtime-dir` will always have the > clang host triple appended even if `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` is > off? I guess I was expecting to see `lib/linux` instead of > `lib/x86_64-unknown-linux-gnu`. https://reviews.llvm.org/D98868 introduced `--print-runtime-dir`. The question is whether `--print-runtime-dir` should print the legacy `lib/linux` when `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` is off. Personally I'd hope that `--print-runtime-dir` does not try to be smart and users should be able to expect that it always prints the new hierarchy. With the old hierarchy, the user is expected to know how to derive `libclang_rt.builtins-aarch64.a` from `libclang_rt.builtins.a`. In this case, they can extract the directory name from `clang --print-file-name=libclang_rt.builtins-aarch64.a`. https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
estewart08 wrote: Is it expected now that `clang --print-runtime-dir` will always have the clang host triple appended even if `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` is off? https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
wzssyqa wrote: See: https://github.com/llvm/llvm-project/pull/87866 Can you have a try to add an extra option ``` -resource-dir=%S/Inputs/resource_dir ``` https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
antmox wrote: Hello @MaskRay @bazuzi, Looks like one of the commit ([Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin) broke the clang-arm64-windows-msvc-2stage bot here: https://lab.llvm.org/buildbot/#/builders/120/builds/6489 Old compiler-rt lib seems still used "--dependent-lib=clang_rt.builtins-aarch64.lib" https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
wzssyqa wrote: New PR with my resource-dir patch: https://github.com/llvm/llvm-project/pull/88661 https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
aeubanks wrote: yeah that patch makes those test pass with this PR, lgtm (you could also test locally by touching the files I mentioned above, e.g. even just `touch lib/clang/19/lib/linux/libclang_rt.builtins-aarch64-android.a` repro'd the test failure on my machine) https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
wzssyqa wrote: [xx.patch](https://github.com/llvm/llvm-project/files/14948921/xx.patch) @aeubanks can you help to test this patch? https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
wzssyqa wrote: > > @aeubanks The problem is that in your configure, the libclang_rt is placed > > in `/lib/clang/19/lib/linux/libclang_rt.builtins-arm-android.a`, > > instead of > > `/lib/clang/19/lib/arm-unknown-linux-android/libclang_rt.builtins.a`. > > The point is that both locations were supposed to be accepted, as they were > before - this PR was not supposed to be a policy change that affects that. Yes. After the patch, both locations are accepted. This patch doesn't break it. The current problem is that in some test cases, `-resource-dir` option are missing. For details: 1. with `--sysroot` option, clang will try to find libclang_rt there. In the failure case, it fails to find. 2. then, clang try to look for libclang_rt from `lib/clang/19/lib/arm-unknown-android/libclang_rt.builtin.a`, and failed. 3. clang try to look for libclang_rt from `lib/clang/19/lib/linux/libclang_rt.builtin-arm-android.a`, and success. Thus the file name of libclang_rt is different with the one in test cases. 4. If 3) failed, clang will fallback to `lib/clang/19/lib/arm-unknown-android/libclang_rt.builtin.a`, then test case success. So, we can add `-resource-dir` to skip 3 for test case. https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
mstorsjo wrote: > @aeubanks The problem is that in your configure, the libclang_rt is placed in > `/lib/clang/19/lib/linux/libclang_rt.builtins-arm-android.a`, > instead of > `/lib/clang/19/lib/arm-unknown-linux-android/libclang_rt.builtins.a`. The point is that both locations were supposed to be accepted, as they were before - this PR was not supposed to be a policy change that affects that. https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
wzssyqa wrote: @aeubanks The problem is that in your configure, the libclang_rt is please in `/lib/clang/19/lib/linux/libclang_rt.builtins-arm-android.a`, instead of `/lib/clang/19/lib/arm-unknown-linux-android/libclang_rt.builtins.a`. https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
aeubanks wrote: it seems that the test depends on if certain android runtime libraries are present or not in the resource dir (without per-target runtime directory since that's still an issue on Android). perhaps the tests need `-resource-dir` to make them more hermetic? anyway, will revert, feel free to reland once the tests are fixed. this is what's in my resource dir if it's helpful for reproducing ``` $ ls lib/clang/19/lib/linux/ aarch64 libclang_rt.asan_cxx-aarch64-android.a libclang_rt.builtins-aarch64-android.a libclang_rt.tsan_cxx-aarch64-android.a libclang_rt.ubsan_standalone-aarch64-android.so libclang_rt.asan-aarch64-android.a libclang_rt.asan-preinit-aarch64-android.a libclang_rt.profile-aarch64-android.a libclang_rt.ubsan_minimal-aarch64-android.a libclang_rt.ubsan_standalone_cxx-aarch64-android.a libclang_rt.asan-aarch64-android.so libclang_rt.asan_static-aarch64-android.a libclang_rt.tsan-aarch64-android.a libclang_rt.ubsan_standalone-aarch64-android.a ``` https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
aeubanks wrote: this seemes to be causing some test failures for us: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/8751043232043110529/+/u/package_clang/stdout?format=raw ``` TEST 'Clang :: Driver/linux-ld.c' FAILED RUN: at line 92: ... /b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/linux-ld.c:102:25: error: CHECK-LD-RT-ANDROID: expected string not found in input // CHECK-LD-RT-ANDROID: libclang_rt.builtins.a" ^ :6:331: note: scanning from here "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/bin/ld.lld" "--sysroot=/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot" "-EL" "-z" "now" "-z" "relro" "-z" "max-page-size=4096" "-X" "--hash-style=both" "--eh-frame-hdr" "-m" "armelf_linux_eabi" "-dynamic-linker" "/system/bin/linker" "-o" "a.out" "/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib/crtbegin_dynamic.o" "-L/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib" "-L/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib" "/b/s/w/ir/x/t/lit-tmp-jnv32xv9/linux-ld-60ec02.o" "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/lib/clang/19/lib/linux/libclang_rt.builtins-arm-android.a" "-l:libunwind.a" "-ldl" "-lc" "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/lib/clang/19/lib/linux/libclang_rt.builtins-arm-android.a" "-l:libunwind.a" "-ldl" "/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib/crtend_android.o" ^ :6:866: note: possible intended match here "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/bin/ld.lld" "--sysroot=/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot" "-EL" "-z" "now" "-z" "relro" "-z" "max-page-size=4096" "-X" "--hash-style=both" "--eh-frame-hdr" "-m" "armelf_linux_eabi" "-dynamic-linker" "/system/bin/linker" "-o" "a.out" "/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib/crtbegin_dynamic.o" "-L/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib" "-L/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib" "/b/s/w/ir/x/t/lit-tmp-jnv32xv9/linux-ld-60ec02.o" "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/lib/clang/19/lib/linux/libclang_rt.builtins-arm-android.a" "-l:libunwind.a" "-ldl" "-lc" "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/lib/clang/19/lib/linux/libclang_rt.builtins-arm-android.a" "-l:libunwind.a" "-ldl" "/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib/crtend_android.o" TEST 'Clang :: Driver/sanitizer-ld.c' FAILED RUN: at line 177: ... /b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/sanitizer-ld.c:187:24: error: CHECK-ASAN-ANDROID: expected string not found in input // CHECK-ASAN-ANDROID: libclang_rt.asan.so" ^ :6:320: note: scanning from here "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/bin/ld.lld" "--sysroot=/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot" "-EL" "-z" "now" "-z" "relro" "-z" "max-page-size=4096" "-X" "--hash-style=both" "--eh-frame-hdr" "-m" "armelf_linux_eabi" "-pie" "-dynamic-linker" "/system/bin/linker" "-o" "a.out" "/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib/crtbegin_dynamic.o" "-L/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib/../lib" "-L/b/s/w/ir/cache/builder/src/third_party/llvm/clang/test/Driver/Inputs/basic_android_tree/sysroot/usr/lib" "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/lib/clang/19/lib/linux/libclang_rt.asan-arm-android.so" "--whole-archive" "/b/s/w/ir/cache/builder/src/third_party/llvm-build/Release+Asserts/lib/clang/19/lib/linux/libclang_rt.asan_static-arm-android.a" "--no-whole-archive" "/b/s/w/ir/x/t/lit-tmp-jnv32xv9/sanitizer-ld-d1ddb3.o"
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
wzssyqa wrote: For me, I think that it is a good idea to always warn/hint the full/normalized path. For compatible reason, we can still try to find libraries in old-style or non-full/normalized paths, while should not hint them. https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
mstorsjo wrote: > This seems to have had an unexpected effect. In a build where I don't use the > new path style, I used to get the old path style returned like this: > > ``` > $ clang -target x86_64-w64-mingw32 -print-runtime-dir > /home/martin/clang-nightly/lib/clang/19/lib/windows > ``` > > However after this change, now I'm getting the new style path, even if it > doesn't exist, and if the old one actually did exist: > > ``` > $ clang -target x86_64-w64-mingw32 -print-runtime-dir > /home/martin/clang-nignhtly/lib/clang/19/lib/x86_64-w64-windows-gnu > ``` > > I'm ok with changing the default if the old path style doesn't exist - but if > it does exist, we should still return that. (I haven't dig into it to see why > this is, yet.) The reason for this seems to lie here: https://github.com/llvm/llvm-project/blob/ccdebbae4d77d3efc236af92c22941de5d437e01/clang/lib/Driver/Driver.cpp#L2213-L2219 Previously, this picked the `TC.getCompilerRTPath()` case, while this now always ends up going with `TC.getRuntimePath()` as it's never empty. https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
mstorsjo wrote: This seems to have had an unexpected effect. In a build where I don't use the new path style, I used to get the old path style returned like this: ``` $ clang -target x86_64-w64-mingw32 -print-runtime-dir /home/martin/clang-nightly/lib/clang/19/lib/windows ``` However after this change, now I'm getting the new style path, even if it doesn't exist, and if the old one actually did exist: ``` $ clang -target x86_64-w64-mingw32 -print-runtime-dir /home/martin/clang-nignhtly/lib/clang/19/lib/x86_64-w64-windows-gnu ``` I'm ok with changing the default if the old path style doesn't exist - but if it does exist, we should still return that. (I haven't dig into it to see why this is, yet.) https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
https://github.com/MaskRay closed https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
https://github.com/MaskRay edited https://github.com/llvm/llvm-project/pull/87866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Ensure ToolChain::LibraryPaths is not empty for non-Darwin (PR #87866)
https://github.com/MaskRay updated
https://github.com/llvm/llvm-project/pull/87866
>From 2847b8be2e4938c03aea57609842e2fee776d916 Mon Sep 17 00:00:00 2001
From: Fangrui Song
Date: Fri, 5 Apr 2024 21:51:37 -0700
Subject: [PATCH] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20initia?=
=?UTF-8?q?l=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Created using spr 1.3.5-bogner
---
clang/lib/Driver/ToolChain.cpp| 8 -
clang/test/Driver/arm-compiler-rt.c | 14 -
clang/test/Driver/cl-link.c | 16 +-
clang/test/Driver/compiler-rt-unwind.c| 6 ++--
clang/test/Driver/coverage-ld.c | 8 ++---
clang/test/Driver/instrprof-ld.c | 16 +-
clang/test/Driver/linux-ld.c | 6 ++--
clang/test/Driver/mingw-sanitizers.c | 16 +-
clang/test/Driver/msp430-toolchain.c | 4 +--
.../Driver/print-libgcc-file-name-clangrt.c | 12
clang/test/Driver/print-runtime-dir.c | 6
clang/test/Driver/riscv32-toolchain-extra.c | 6 ++--
clang/test/Driver/riscv32-toolchain.c | 6 ++--
clang/test/Driver/riscv64-toolchain-extra.c | 6 ++--
clang/test/Driver/riscv64-toolchain.c | 6 ++--
clang/test/Driver/sanitizer-ld.c | 30 +--
clang/test/Driver/wasm-toolchain.c| 18 +--
clang/test/Driver/wasm-toolchain.cpp | 16 +-
clang/test/Driver/windows-cross.c | 18 +--
clang/test/Driver/zos-ld.c| 12
20 files changed, 115 insertions(+), 115 deletions(-)
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 03450fc0f57b93..237092ed07e5dc 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -796,7 +796,13 @@ ToolChain::getTargetSubDirPath(StringRef BaseDir) const {
std::optional ToolChain::getRuntimePath() const {
SmallString<128> P(D.ResourceDir);
llvm::sys::path::append(P, "lib");
- return getTargetSubDirPath(P);
+ if (auto Ret = getTargetSubDirPath(P))
+return Ret;
+ // Darwin does not use per-target runtime directory.
+ if (Triple.isOSDarwin())
+return {};
+ llvm::sys::path::append(P, Triple.str());
+ return std::string(P);
}
std::optional ToolChain::getStdlibPath() const {
diff --git a/clang/test/Driver/arm-compiler-rt.c
b/clang/test/Driver/arm-compiler-rt.c
index 5e9e528400d08e..cb6c29f48a7814 100644
--- a/clang/test/Driver/arm-compiler-rt.c
+++ b/clang/test/Driver/arm-compiler-rt.c
@@ -10,47 +10,47 @@
// RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
// RUN: -rtlib=compiler-rt -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix ARM-GNUEABI
-// ARM-GNUEABI: "{{.*[/\\]}}libclang_rt.builtins-arm.a"
+// ARM-GNUEABI: "{{.*[/\\]}}libclang_rt.builtins.a"
// RUN: %clang -target arm-linux-gnueabi \
// RUN: --sysroot=%S/Inputs/resource_dir_with_arch_subdir \
// RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
// RUN: -rtlib=compiler-rt -mfloat-abi=hard -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix ARM-GNUEABI-ABI
-// ARM-GNUEABI-ABI: "{{.*[/\\]}}libclang_rt.builtins-armhf.a"
+// ARM-GNUEABI-ABI: "{{.*[/\\]}}libclang_rt.builtins.a"
// RUN: %clang -target arm-linux-gnueabihf \
// RUN: --sysroot=%S/Inputs/resource_dir_with_arch_subdir \
// RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
// RUN: -rtlib=compiler-rt -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix ARM-GNUEABIHF
-// ARM-GNUEABIHF: "{{.*[/\\]}}libclang_rt.builtins-armhf.a"
+// ARM-GNUEABIHF: "{{.*[/\\]}}libclang_rt.builtins.a"
// RUN: %clang -target arm-linux-gnueabihf \
// RUN: --sysroot=%S/Inputs/resource_dir_with_arch_subdir \
// RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
// RUN: -rtlib=compiler-rt -mfloat-abi=soft -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix ARM-GNUEABIHF-ABI
-// ARM-GNUEABIHF-ABI: "{{.*[/\\]}}libclang_rt.builtins-arm.a"
+// ARM-GNUEABIHF-ABI: "{{.*[/\\]}}libclang_rt.builtins.a"
// RUN: %clang -target arm-windows-itanium \
// RUN: --sysroot=%S/Inputs/resource_dir_with_arch_subdir \
// RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
// RUN: -rtlib=compiler-rt -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix ARM-WINDOWS
-// ARM-WINDOWS: "{{.*[/\\]}}clang_rt.builtins-arm.lib"
+// ARM-WINDOWS: "{{.*[/\\]}}clang_rt.builtins.lib"
// RUN: %clang -target arm-linux-androideabi \
// RUN: --sysroot=%S/Inputs/resource_dir_with_arch_subdir \
// RUN: -resource-dir=%S/Inputs/resource_dir_with_arch_subdir \
// RUN: -rtlib=compiler-rt -### %s 2>&1 \
// RUN: | FileCheck %s -check-prefix ARM-ANDROID
-// ARM-ANDROID: "{{.*[/\\]}}libclang_rt.builtins-arm-android.a"
+// ARM-ANDROID: "{{.*[/\\]}}libclang_rt.builtins.a"
// RUN: not %clang
