jyknight added a comment.
Since this change is not android-only, please update change description, and
update some non-android tests.
E.g. maybe the tests in clang/test/Driver/linux-ld.c for
ubuntu_14.04_multiarch_tree should check the path at which ld is found. While
the existing Inputs/ubuntu_14.04_multiarch_tree doesn't have any "ld" files
populated, and the tests aren't checking for ld's path at all, it would be
reasonable to add such files and then check the result.
This is what the layout of the "ld" files look like in current ubuntu versions
on x86_64. It seems that it was almost the same back in 2014 -- except the
triple-less names were the primary file, and the others linked to them. That's
not really relevant to the test, anyways, just the filenames existing and being
executable.
lrwxrwxrwx /usr/bin/ld.bfd -> x86_64-linux-gnu-ld.bfd
lrwxrwxrwx /usr/bin/ld.gold -> x86_64-linux-gnu-ld.gold
lrwxrwxrwx /usr/bin/ld -> x86_64-linux-gnu-ld
-rwxr-xr-x /usr/bin/x86_64-linux-gnu-ld.bfd
-rwxr-xr-x /usr/bin/x86_64-linux-gnu-ld.gold
-rwxr-xr-x /usr/bin/x86_64-linux-gnu-ld -> x86_64-linux-gnu-ld.bfd
-rwxr-xr-x /usr/bin/powerpc64le-linux-gnu-ld.bfd
-rwxr-xr-x /usr/bin/powerpc64le-linux-gnu-ld.gold
-rwxr-xr-x /usr/bin/powerpc64le-linux-gnu-ld -> powerpc64le-linux-gnu-ld.bfd
Before this clang patch, we would've defaulted to /usr/bin/ld unless the target
string was specified exactly as "x86_64-linux-gnu" or "powerpc64le-linux-gnu".
Afterwards, we can also chose those tools for any other triple which we can
detect as matching those gcc installation triples.
This would even have the useful effect of allowing an invocation like "clang
-target powerpc64le-unknown-linux-gnu" to work, which it doesn't do currently
-- because it defaults to /usr/bin/ld, which is an x86_64 linker.
================
Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2578
getProgramPaths().push_back(getDriver().Dir);
+ getProgramPrefixes().push_back(Twine(GCCInstallation.getTriple().str() +
"-").str());
+
----------------
I suspect this doesn't actually work here, since GCCInstallation.init() hasn't
yet been called. But you have the same in Linux.cpp, after the init() call,
which seems better.
================
Comment at: clang/lib/Driver/ToolChains/Linux.cpp:242
.str());
+ PPrefixes.push_back(Twine(GCCInstallation.getTriple().str() + "-").str());
}
----------------
Probably should check that GCCInstallation.getTriple() != Triple before adding,
in order to avoid duplicating entries in the search-list.
================
Comment at: clang/test/Driver/android-triple-version.c:1
+// Android's target triples can contain a version number in the environment
+// field (e.g. arm-linux-androideabi9).
----------------
This seems like too many test-cases. It doesn't seem to me that they're
actually adding any meaningful coverage, after the first 3?
================
Comment at: clang/test/Driver/android-triple-version.c:6
+// Ensure no execute permissions on .../bin/{target-triple}/ld.
+// RUN: chmod -x %S/Inputs/basic_android_ndk_tree/arm-linux-androideabi/bin/ld
+// RUN: chmod -x %S/Inputs/basic_android_ndk_tree/aarch64-linux-android/bin/ld
----------------
You shouldn't edit files in the Inputs tree (or add/remove files there either).
If this script stops, it may be left in an edited state, which would be bad.
Also, sometimes it might be read-only.
Another way to achieve this test goal may be with -fuse-ld=prefixtest, and then
create bin/$triple-ld.prefixtest, but not $triple/bin/ld.prefixtest
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71848/new/
https://reviews.llvm.org/D71848
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
