[Driver] Use arch type to find compiler-rt libraries (on Linux)

[mgorny]

Summary

Use llvm::Triple::getArchTypeName() when looking for compiler-rt
libraries, rather than the exact arch string from the triple. This is
more correct as it matches the values used when building compiler-rt
(builtin-config-ix.cmake) which are the subset of the values allowed
in triples.

For example, this fixes an issue when the compiler set for
i686-pc-linux-gnu triple would not find an i386 compiler-rt library,
while this is the exact arch that is detected by compiler-rt. The same
applies to any other i?86 variant allowed by LLVM.

This also makes the special case for MSVC unnecessary, since now i386
will be used reliably for all 32-bit x86 variants.

[jroelofs]

Testcase?

[mgorny]

Updated expected output for existing tests and added an additional test to linux-ld.c.

[jroelofs]

test/Driver/print-libgcc-file-name-clangrt.c

11

This CHECK's name is now a lie.

[mgorny]

test/Driver/print-libgcc-file-name-clangrt.c

11

Right. I'll change it.

[mgorny]

Updated the -print-libgcc-file-name test name. Additionally, I've added another subvariant of that test using i686-* target to ensure that the mapping works for that function too, in case we ever decide to split it.

[mgorny]

Ping.

[rengolin]

I don't know enough about the x86 environment to be able to review this patch. Sorry.

[mgorny]

Second ping.

[eugenis]

LGTM

[compnerd]

This would need an upgrade path, since the output name can actually be different from the AchTypeName (e.g. i386 vs i686).

[mgorny]

In D26796#632602, @compnerd wrote:

This would need an upgrade path, since the output name can actually be different from the AchTypeName (e.g. i386 vs i686).

I was considering committing this alongside with the i686 removal, and assuming that people are going to upgrade clang and compiler-rt in lockstep. Is there any other case that needs explicit consideration? Do you have another suggestion on handling this?

[mgorny]

@compnerd, ping. Do you really think this is a real issue? As I said on the other patch, the only case when compiler-rt would generate i686-suffixed library is if the builder specifically used -march=i686 for the compiler-rt build, and in that case the i386 variant (which is identical to the i686 variant since both are built with the same flags) is built anyway.

[compnerd]

I do think that this is an issue. Particularly for Windows targets, which have usually had the different architecture value.

[mgorny]

Another ping. Since 4.0.0 final has been tagged, I think we should get back to working on this. @compnerd, any suggestion how to proceed here?

[mgorny]

Rebased the patch since it no longer applied.

[compnerd]

I don't think that there is a guarantee that compiler-rt and clang are upgraded in lockstep. At least for the builtins, there is a relatively stable interface. However, I don't think that at this point the Windows MSVC environment depends on compiler-rt much (except for complex multiplication/division in C99+). That can be satisfied with vcclang.lib. So, this is probably better off now than before.

[mgorny]

Thanks a lot. Let's hope no buildbot complains now ;-).

[mgorny]

@eugenis, it seems that this broke the Android buildbot. Could you suggest an appropriate course of action? I suppose we either have to fix Android-something to use i386, or keep using i686 for Android. The latter I know how to do but for the former, I have no clue where the issue might be.

[mgorny]

The problems:

1. http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-android/builds/2140/steps/annotate/logs/stdio -- purely this change,
2. http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-android/builds/2146/steps/run%20instrumented%20asan%20tests%20%5Bi686%2Ffugu-userdebug%2FN2G48C%5D/logs/stdio -- after D26764.

[mgorny]

I have reverted the changes for now to fix Android.

[eugenis]

In D26796#853446, @mgorny wrote:

The problems:

1. http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-android/builds/2140/steps/annotate/logs/stdio -- purely this change,

This one still builds i686 libraries. Maybe it needs a clean cmake? Try "clobber" "1" in build properties.

[171/178] Linking CXX shared library /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm_build64/lib/clang/6.0.0/lib/linux/libclang_rt.asan-i686-android.so
[172/178] Linking CXX static library /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm_build64/lib/clang/6.0.0/lib/linux/libclang_rt.asan-i686-android.a

2. http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-android/builds/2146/steps/run%20instrumented%20asan%20tests%20%5Bi686%2Ffugu-userdebug%2FN2G48C%5D/logs/stdio -- after D26764.

This probably needs an update of zorg/buildbot/builders/sanitizers/buildbot_android.sh in the zorg repo.

[mgorny]

@eugenis, thanks for the suggestion. I've found the problematic push (that attempts to move the wrong file) and filed D37226 for review.

lib/Driver/ToolChain.cpp

Tool *ToolChain::getTool(Action::ActionClass AC) const {
   llvm_unreachable("Invalid tool kind.");
 }
 
 static StringRef getArchNameForCompilerRTLib(const ToolChain &TC,
                                              const ArgList &Args) {
   const llvm::Triple &Triple = TC.getTriple();
   bool IsWindows = Triple.isOSWindows();
 
-  if (Triple.isWindowsMSVCEnvironment() && TC.getArch() == llvm::Triple::x86)
-    return "i386";
-
   if (TC.getArch() == llvm::Triple::arm || TC.getArch() == llvm::Triple::armeb)
     return (arm::getARMFloatABI(TC, Args) == arm::FloatABI::Hard && !IsWindows)
                ? "armhf"
                : "arm";
 
-  return TC.getArchName();
+  return llvm::Triple::getArchTypeName(TC.getArch());
 }
 
 std::string ToolChain::getCompilerRT(const ArgList &Args, StringRef Component,
                                      bool Shared) const {
   const llvm::Triple &TT = getTriple();
   const char *Env = TT.isAndroid() ? "-android" : "";
   bool IsITANMSVCWindows =
       TT.isWindowsMSVCEnvironment() || TT.isWindowsItaniumEnvironment();

test/Driver/linux-ld.c

 // CHECK-LD-RT: "-L[[SYSROOT]]/usr/lib/gcc/x86_64-unknown-linux/4.6.0/../../.."
 // CHECK-LD-RT: "-L[[SYSROOT]]/lib"
 // CHECK-LD-RT: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD-RT: libclang_rt.builtins-x86_64.a"
 // CHECK-LD-RT: "-lc"
 // CHECK-LD-RT: libclang_rt.builtins-x86_64.a"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN:     --target=i686-unknown-linux \
+// RUN:     --gcc-toolchain="" \
+// RUN:     --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:     --rtlib=compiler-rt \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-RT-I686 %s
+// CHECK-LD-RT-I686-NOT: warning:
+// CHECK-LD-RT-I686: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-LD-RT-I686: "--eh-frame-hdr"
+// CHECK-LD-RT-I686: "-m" "elf_i386"
+// CHECK-LD-RT-I686: "-dynamic-linker"
+// CHECK-LD-RT-I686: "{{.*}}/usr/lib/gcc/i686-unknown-linux/4.6.0{{/|\\\\}}crtbegin.o"
+// CHECK-LD-RT-I686: "-L[[SYSROOT]]/usr/lib/gcc/i686-unknown-linux/4.6.0"
+// CHECK-LD-RT-I686: "-L[[SYSROOT]]/usr/lib/gcc/i686-unknown-linux/4.6.0/../../../../i686-unknown-linux/lib"
+// CHECK-LD-RT-I686: "-L[[SYSROOT]]/usr/lib/gcc/i686-unknown-linux/4.6.0/../../.."
+// CHECK-LD-RT-I686: "-L[[SYSROOT]]/lib"
+// CHECK-LD-RT-I686: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD-RT-I686: libclang_rt.builtins-i386.a"
+// CHECK-LD-RT-I686: "-lc"
+// CHECK-LD-RT-I686: libclang_rt.builtins-i386.a"
+//
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN:     --target=arm-linux-androideabi \
 // RUN:     --gcc-toolchain="" \
 // RUN:     --sysroot=%S/Inputs/basic_android_tree/sysroot \
 // RUN:     --rtlib=compiler-rt \
 // RUN:   | FileCheck --check-prefix=CHECK-LD-RT-ANDROID %s
 // CHECK-LD-RT-ANDROID-NOT: warning:
 // CHECK-LD-RT-ANDROID: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
 // CHECK-LD-RT-ANDROID: "--eh-frame-hdr"

test/Driver/nostdlib.c

 // RUN: %clang -target i686-pc-linux-gnu -### -nostdlib %s 2> %t
 // RUN: FileCheck < %t %s
 //
 // CHECK-NOT: start-group
 
 // Most of the toolchains would check for -nostartfiles and -nostdlib
 // in a short-circuiting boolean expression, so if both of the preceding
 // options were present, the second would warn about being unused.
 // RUN: %clang -### -Wno-liblto -nostartfiles -nostdlib -target i386-apple-darwin %s \
 // RUN:   2>&1 | FileCheck %s -check-prefix=ARGSCLAIMED
 // ARGSCLAIMED-NOT: warning:
 
 // In the presence of -nostdlib, the standard libraries should not be
 // passed down to link line
 // RUN: %clang -no-canonical-prefixes %s -### -Wno-liblto -o %t.o 2>&1 \
 // RUN:     -target i686-pc-linux-gnu -nostdlib --rtlib=compiler-rt \
-// RUN:     -resource-dir=%S/Inputs/resource_dir -lclang_rt.builtins-i686 \
+// RUN:     -resource-dir=%S/Inputs/resource_dir -lclang_rt.builtins-i386 \
 // RUN:   | FileCheck --check-prefix=CHECK-LINUX-NOSTDLIB %s
 //
 // RUN: %clang -no-canonical-prefixes %s -### -Wno-liblto -o %t.o 2>&1 \
 // RUN:     -target i686-pc-linux-gnu --rtlib=compiler-rt -nostdlib \
-// RUN:     -resource-dir=%S/Inputs/resource_dir -lclang_rt.builtins-i686 \
+// RUN:     -resource-dir=%S/Inputs/resource_dir -lclang_rt.builtins-i386 \
 // RUN:   | FileCheck --check-prefix=CHECK-LINUX-NOSTDLIB %s
 //
 // RUN: %clang -target x86_64-pc-windows-msvc -nostdlib --rtlib=compiler-rt -### -Wno-liblto %s 2>&1 | FileCheck %s -check-prefix CHECK-MSVC-NOSTDLIB
 // RUN: %clang -target x86_64-pc-windows-msvc --rtlib=compiler-rt -nostdlib -### -Wno-liblto %s 2>&1 | FileCheck %s -check-prefix CHECK-MSVC-NOSTDLIB
 //
 // CHECK-LINUX-NOSTDLIB: warning: argument unused during compilation: '--rtlib=compiler-rt'
 // CHECK-LINUX-NOSTDLIB: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}"
-// CHECK-LINUX-NOSTDLIB-NOT: "{{.*}}/Inputs/resource_dir{{/|\\\\}}lib{{/|\\\\}}linux{{/|\\\\}}libclang_rt.builtins-i686.a"
+// CHECK-LINUX-NOSTDLIB-NOT: "{{.*}}/Inputs/resource_dir{{/|\\\\}}lib{{/|\\\\}}linux{{/|\\\\}}libclang_rt.builtins-i386.a"
 // CHECK-MSVC-NOSTDLIB: warning: argument unused during compilation: '--rtlib=compiler-rt'

test/Driver/print-libgcc-file-name-clangrt.c

 // Test that -print-libgcc-file-name correctly respects -rtlib=compiler-rt.
 
 // RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name 2>&1 \
 // RUN:     --target=x86_64-pc-linux \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-X8664 %s
 // CHECK-CLANGRT-X8664: libclang_rt.builtins-x86_64.a
 
 // RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name 2>&1 \
-// RUN:     --target=i686-pc-linux \
+// RUN:     --target=i386-pc-linux \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-I686 %s
-// CHECK-CLANGRT-I686: libclang_rt.builtins-i686.a
+// CHECK-CLANGRT-I686: libclang_rt.builtins-i386.a

test/Driver/windows-cross.c

 
 // CHECK-SANITIZE-ADDRESS-EXE: "-fsanitize=address"
 // CHECK-SANITIZE-ADDRESS-EXE: "{{.*}}clang_rt.asan_dynamic-arm.lib" "{{.*}}clang_rt.asan_dynamic_runtime_thunk-arm.lib" "--undefined" "__asan_seh_interceptor"
 
 // RUN: %clang -### -target i686-windows-itanium -B %S/Inputs/Windows/ARM/8.1/usr/bin -fuse-ld=lld-link2 -o test.exe -fsanitize=address -x c++ %s 2>&1 \
 // RUN:    | FileCheck %s --check-prefix CHECK-SANITIZE-ADDRESS-EXE-X86
 
 // CHECK-SANITIZE-ADDRESS-EXE-X86: "-fsanitize=address"
-// CHECK-SANITIZE-ADDRESS-EXE-X86: "{{.*}}clang_rt.asan_dynamic-i686.lib" "{{.*}}clang_rt.asan_dynamic_runtime_thunk-i686.lib" "--undefined" "___asan_seh_interceptor"
+// CHECK-SANITIZE-ADDRESS-EXE-X86: "{{.*}}clang_rt.asan_dynamic-i386.lib" "{{.*}}clang_rt.asan_dynamic_runtime_thunk-i386.lib" "--undefined" "___asan_seh_interceptor"
 
 // RUN: %clang -### -target armv7-windows-itanium --sysroot %S/Inputs/Windows/ARM/8.1 -B %S/Inputs/Windows/ARM/8.1/usr/bin -fuse-ld=lld-link2 -shared -o shared.dll -fsanitize=tsan -x c++ %s 2>&1 \
 // RUN:    | FileCheck %s --check-prefix CHECK-SANITIZE-TSAN
 
 // CHECK-SANITIZE-TSAN: error: unsupported argument 'tsan' to option 'fsanitize='
 // CHECK-SANITIZE-TSAN-NOT: "-fsanitize={{.*}}"
 
 // RUN: %clang -### -target armv7-windows-itanium -isystem-after "Windows Kits/10/Include/10.0.10586.0/ucrt" -isystem-after "Windows Kits/10/Include/10.0.10586.0/um" -isystem-after "Windows Kits/10/Include/10.0.10586.0/shared" -c %s -o /dev/null 2>&1 \
 // RUN:     | FileCheck %s --check-prefix CHECK-ISYSTEM-AFTER
 // CHECK-ISYSTEM-AFTER: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-ISYSTEM-AFTER: "-internal-isystem" "[[RESOURCE_DIR]]{{(/|\\\\)}}include"
 // CHECK-ISYSTEM-AFTER: "-internal-isystem" "Windows Kits{{[/\\]}}10{{[/\\]}}Include{{[/\\]}}10.0.10586.0{{[/\\]}}ucrt"
 // CHECK-ISYSTEM-AFTER: "-internal-isystem" "Windows Kits{{[/\\]}}10{{[/\\]}}Include{{[/\\]}}10.0.10586.0{{[/\\]}}um"
 // CHECK-ISYSTEM-AFTER: "-internal-isystem" "Windows Kits{{[/\\]}}10{{[/\\]}}Include{{[/\\]}}10.0.10586.0{{[/\\]}}shared"