[PATCH] D100054: Handle flags such as -m32 when computing the prefix for programs/runtime libs

2021-04-09 Thread Petr Hosek via Phabricator via cfe-commits 
phosek added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:1176-1210
+  // If command line flags such as -m32, etc. changed parts of the triple that
+  // are not just changes to normalization, we also need to update the raw
+  // triple string that is used to find tools. This ensures e.g. that clang 
-m32
+  // searches for i386-*-ld instead of x86_64-*-ld when linking (and also uses
+  // the triple-prefixed library paths).
+  EffectiveTargetTriple =
+  computeTargetTriple(*this, RawTargetTriple, *UArgs, "");

I think this should be handled by each toolchain. For example `Linux` toolchain 
already [has 
getMultiarchTriple](https://github.com/llvm/llvm-project/blob/16fe741c69429235210c03c46420f8fa6aece4a1/clang/lib/Driver/ToolChains/Linux.cpp#L40)
 which takes into account target handling on various existing platforms like 
Debian.

What I've been thinking of doing is to move that method to the `ToolChain` 
class as virtual so it could be overriden and customized by each toolchain, and 
then use it in 
[getRuntimePath](https://github.com/llvm/llvm-project/blob/16fe741c69429235210c03c46420f8fa6aece4a1/clang/lib/Driver/ToolChain.cpp#L485).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100054/new/

https://reviews.llvm.org/D100054

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100054: Handle flags such as -m32 when computing the prefix for programs/runtime libs

2021-04-09 Thread Alexander Richardson via Phabricator via cfe-commits 
arichardson updated this revision to Diff 336483.
arichardson added a comment.

- Fix Windows path regex


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100054/new/

https://reviews.llvm.org/D100054

Files:
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  
clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/i386-unknown-freebsd12.2-ld
  clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/x86_64-freebsd12.2-ld
  
clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/x86_64-unknown-freebsd12.2-ld
  clang/test/Driver/freebsd-m32.c
  clang/test/Driver/linux-per-target-runtime-dir.c

Index: clang/test/Driver/linux-per-target-runtime-dir.c
===
--- clang/test/Driver/linux-per-target-runtime-dir.c
+++ clang/test/Driver/linux-per-target-runtime-dir.c
@@ -25,3 +25,18 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-X8664 %s
 // CHECK-FILE-NAME-X8664: lib{{/|\\}}x86_64-linux-gnu{{/|\\}}libclang_rt.builtins.a
+
+/// Check that we handle flags such as -m32 when searching for the builtins:
+/// Previously clang would use the raw triple passed to -target to find builtins
+/// and sanitizer libraries, but this will result in build errors when compiling
+/// with flags such as -m32. Check that we use the adjusted triple instead:
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
+// RUN: --target=i386-linux-gnu \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-I386 %s
+// CHECK-FILE-NAME-I386: resource_dir_with_per_target_subdir{{/|\\}}lib{{/|\\}}i386-linux-gnu{{/|\\}}libclang_rt.builtins.a
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
+// RUN: --target=x86_64-linux-gnu -m32 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-X8664-M32 %s
+// CHECK-FILE-NAME-X8664-M32: resource_dir_with_per_target_subdir{{/|\\}}lib{{/|\\}}i386-linux-gnu{{/|\\}}libclang_rt.builtins.a
Index: clang/test/Driver/freebsd-m32.c
===
--- /dev/null
+++ clang/test/Driver/freebsd-m32.c
@@ -0,0 +1,76 @@
+/// Check that building with -m32 links with i386-freebsd12.2-ld/ instead of
+/// x86_64-freebsd12.2-ld and that we select the right sanitizer runtime.
+
+/// We should select x86_64-unknown-freebsd12.2-ld since it matches the triple argument
+/// Note: The paths specified by -B are not searched for triple-prefixed tools, so
+/// we also have to set $PATH.
+// RUN: env PATH=%S/Inputs/basic_freebsd64_tree/usr/bin %clang -no-canonical-prefixes \
+// RUN:   -target x86_64-unknown-freebsd12.2 %s \
+// RUN:   -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1 | FileCheck %s --check-prefix=PREFIXED-64
+// PREFIXED-64: "-cc1" "-triple" "x86_64-unknown-freebsd12.2"
+// PREFIXED-64-NEXT: "{{.+}}Inputs{{/|}}basic_freebsd64_tree{{/|}}usr{{/|}}bin{{/|}}x86_64-unknown-freebsd12.2-ld" "--eh-frame-hdr"
+// Should not be passing an explicit linker emulation for the 64-bit case
+// PREFIXED-64-NOT: "-m"
+// RUN: env PATH=/this/path/does/not/exist %clang -no-canonical-prefixes \
+// RUN:   -target x86_64-unknown-freebsd12.2 %s \
+// RUN:   -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1 | FileCheck %s --check-prefix=MINUS-B-NO-TRIPLE-PREFIX
+// MINUS-B-NO-TRIPLE-PREFIX: "-cc1" "-triple" "x86_64-unknown-freebsd12.2"
+// MINUS-B-NO-TRIPLE-PREFIX-NEXT: "ld" "--eh-frame-hdr"
+// MINUS-B-NO-TRIPLE-PREFIX-NOT: "-m"
+
+/// The triple passed to clang -cc1 should be normalized, but the prefix when searching
+/// for ld should not be normalized. Since there is no x86_64--freebsd12.2-ld, passing
+/// -target x86_64--freebsd12.2 should not find a a valid linker:
+// RUN: env PATH=%S/Inputs/basic_freebsd64_tree/usr/bin %clang -no-canonical-prefixes \
+// RUN:   -target x86_64--freebsd12.2 %s \
+// RUN:   -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1 | FileCheck %s --check-prefix=NO-NORMALIZE-LD-PREFIX-64
+// NO-NORMALIZE-LD-PREFIX-64: "-cc1" "-triple" "x86_64-unknown-freebsd12.2"
+// NO-NORMALIZE-LD-PREFIX-64-NEXT: "ld" "--eh-frame-hdr"
+// NO-NORMALIZE-LD-PREFIX-NOT: "-m"
+
+/// We should search for i386-unknown-freebsd12.2-ld when -m32 is passed (and also pass -m elf_i386_fbsd):
+// RUN: env PATH=%S/Inputs/basic_freebsd64_tree/usr/bin %clang -no-canonical-prefixes \
+// RUN:   -target x86_64-unknown-freebsd12.2 %s -m32 \
+// RUN:   -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1  | FileCheck %s --check-prefix=PREFIXED-M32
+// PREFIXED-M32: "-cc1" "-triple" "i386-unknown-freebsd12.2"
+// PREFIXED-M32-NEXT:

[PATCH] D100054: Handle flags such as -m32 when computing the prefix for programs/runtime libs

2021-04-08 Thread Alexander Richardson via Phabricator via cfe-commits 
arichardson updated this revision to Diff 336061.
arichardson added a comment.

Drop chunk that is no longer necessary


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100054/new/

https://reviews.llvm.org/D100054

Files:
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  
clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/i386-unknown-freebsd12.2-ld
  clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/x86_64-freebsd12.2-ld
  
clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/x86_64-unknown-freebsd12.2-ld
  clang/test/Driver/freebsd-m32.c
  clang/test/Driver/linux-per-target-runtime-dir.c

Index: clang/test/Driver/linux-per-target-runtime-dir.c
===
--- clang/test/Driver/linux-per-target-runtime-dir.c
+++ clang/test/Driver/linux-per-target-runtime-dir.c
@@ -25,3 +25,18 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-X8664 %s
 // CHECK-FILE-NAME-X8664: lib{{/|\\}}x86_64-linux-gnu{{/|\\}}libclang_rt.builtins.a
+
+/// Check that we handle flags such as -m32 when searching for the builtins:
+/// Previously clang would use the raw triple passed to -target to find builtins
+/// and sanitizer libraries, but this will result in build errors when compiling
+/// with flags such as -m32. Check that we use the adjusted triple instead:
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
+// RUN: --target=i386-linux-gnu \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-I386 %s
+// CHECK-FILE-NAME-I386: resource_dir_with_per_target_subdir{{/|\\}}lib{{/|\\}}i386-linux-gnu{{/|\\}}libclang_rt.builtins.a
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
+// RUN: --target=x86_64-linux-gnu -m32 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-X8664-M32 %s
+// CHECK-FILE-NAME-X8664-M32: resource_dir_with_per_target_subdir{{/|\\}}lib{{/|\\}}i386-linux-gnu{{/|\\}}libclang_rt.builtins.a
Index: clang/test/Driver/freebsd-m32.c
===
--- /dev/null
+++ clang/test/Driver/freebsd-m32.c
@@ -0,0 +1,76 @@
+/// Check that building with -m32 links with i386-freebsd12.2-ld/ instead of
+/// x86_64-freebsd12.2-ld and that we select the right sanitizer runtime.
+
+/// We should select x86_64-unknown-freebsd12.2-ld since it matches the triple argument
+/// Note: The paths specified by -B are not searched for triple-prefixed tools, so
+/// we also have to set $PATH.
+// RUN: env PATH=%S/Inputs/basic_freebsd64_tree/usr/bin %clang -no-canonical-prefixes \
+// RUN:   -target x86_64-unknown-freebsd12.2 %s \
+// RUN:   -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1 | FileCheck %s --check-prefix=PREFIXED-64
+// PREFIXED-64: "-cc1" "-triple" "x86_64-unknown-freebsd12.2"
+// PREFIXED-64-NEXT: "{{.+}}Inputs{{/|\\}}basic_freebsd64_tree{{/|\\}}usr{{/|\\}}bin{{/|\\}}x86_64-unknown-freebsd12.2-ld" "--eh-frame-hdr"
+// Should not be passing an explicit linker emulation for the 64-bit case
+// PREFIXED-64-NOT: "-m"
+// RUN: env PATH=/this/path/does/not/exist %clang -no-canonical-prefixes \
+// RUN:   -target x86_64-unknown-freebsd12.2 %s \
+// RUN:   -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1 | FileCheck %s --check-prefix=MINUS-B-NO-TRIPLE-PREFIX
+// MINUS-B-NO-TRIPLE-PREFIX: "-cc1" "-triple" "x86_64-unknown-freebsd12.2"
+// MINUS-B-NO-TRIPLE-PREFIX-NEXT: "ld" "--eh-frame-hdr"
+// MINUS-B-NO-TRIPLE-PREFIX-NOT: "-m"
+
+/// The triple passed to clang -cc1 should be normalized, but the prefix when searching
+/// for ld should not be normalized. Since there is no x86_64--freebsd12.2-ld, passing
+/// -target x86_64--freebsd12.2 should not find a a valid linker:
+// RUN: env PATH=%S/Inputs/basic_freebsd64_tree/usr/bin %clang -no-canonical-prefixes \
+// RUN:   -target x86_64--freebsd12.2 %s \
+// RUN:   -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1 | FileCheck %s --check-prefix=NO-NORMALIZE-LD-PREFIX-64
+// NO-NORMALIZE-LD-PREFIX-64: "-cc1" "-triple" "x86_64-unknown-freebsd12.2"
+// NO-NORMALIZE-LD-PREFIX-64-NEXT: "ld" "--eh-frame-hdr"
+// NO-NORMALIZE-LD-PREFIX-NOT: "-m"
+
+/// We should search for i386-unknown-freebsd12.2-ld when -m32 is passed (and also pass -m elf_i386_fbsd):
+// RUN: env PATH=%S/Inputs/basic_freebsd64_tree/usr/bin %clang -no-canonical-prefixes \
+// RUN:   -target x86_64-unknown-freebsd12.2 %s -m32 \
+// RUN:   -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1  | FileCheck %s --check-prefix=PREFIXED-M32
+// PREFIXED-M32: "-cc1" "-triple" "i386-unknown-freebsd12.2"
+// PREFIXED-M32-NEXT:

[PATCH] D100054: Handle flags such as -m32 when computing the prefix for programs/runtime libs

2021-04-08 Thread Alexander Richardson via Phabricator via cfe-commits 
arichardson updated this revision to Diff 336059.
arichardson added a comment.

Add the raw triple prefix for programs (but prefer the adjusted triple) and 
always use the adjusted triple for libraries.
Should hopefully address the feedback from @joerg.

Also improve tests to check that we don't insert additional empty/"-unknown" 
triple components.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100054/new/

https://reviews.llvm.org/D100054

Files:
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  
clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/i386-unknown-freebsd12.2-ld
  clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/x86_64-freebsd12.2-ld
  
clang/test/Driver/Inputs/basic_freebsd64_tree/usr/bin/x86_64-unknown-freebsd12.2-ld
  clang/test/Driver/freebsd-m32.c
  clang/test/Driver/linux-per-target-runtime-dir.c

Index: clang/test/Driver/linux-per-target-runtime-dir.c
===
--- clang/test/Driver/linux-per-target-runtime-dir.c
+++ clang/test/Driver/linux-per-target-runtime-dir.c
@@ -25,3 +25,18 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-X8664 %s
 // CHECK-FILE-NAME-X8664: lib{{/|\\}}x86_64-linux-gnu{{/|\\}}libclang_rt.builtins.a
+
+/// Check that we handle flags such as -m32 when searching for the builtins:
+/// Previously clang would use the raw triple passed to -target to find builtins
+/// and sanitizer libraries, but this will result in build errors when compiling
+/// with flags such as -m32. Check that we use the adjusted triple instead:
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
+// RUN: --target=i386-linux-gnu \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-I386 %s
+// CHECK-FILE-NAME-I386: resource_dir_with_per_target_subdir{{/|\\}}lib{{/|\\}}i386-linux-gnu{{/|\\}}libclang_rt.builtins.a
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
+// RUN: --target=x86_64-linux-gnu -m32 \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-X8664-M32 %s
+// CHECK-FILE-NAME-X8664-M32: resource_dir_with_per_target_subdir{{/|\\}}lib{{/|\\}}i386-linux-gnu{{/|\\}}libclang_rt.builtins.a
Index: clang/test/Driver/freebsd-m32.c
===
--- /dev/null
+++ clang/test/Driver/freebsd-m32.c
@@ -0,0 +1,76 @@
+/// Check that building with -m32 links with i386-freebsd12.2-ld/ instead of
+/// x86_64-freebsd12.2-ld and that we select the right sanitizer runtime.
+
+/// We should select x86_64-unknown-freebsd12.2-ld since it matches the triple argument
+/// Note: The paths specified by -B are not searched for triple-prefixed tools, so
+/// we also have to set $PATH.
+// RUN: env PATH=%S/Inputs/basic_freebsd64_tree/usr/bin %clang -no-canonical-prefixes \
+// RUN:   -target x86_64-unknown-freebsd12.2 %s \
+// RUN:   -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1 | FileCheck %s --check-prefix=PREFIXED-64
+// PREFIXED-64: "-cc1" "-triple" "x86_64-unknown-freebsd12.2"
+// PREFIXED-64-NEXT: "{{.+}}Inputs{{/|\\}}basic_freebsd64_tree{{/|\\}}usr{{/|\\}}bin{{/|\\}}x86_64-unknown-freebsd12.2-ld" "--eh-frame-hdr"
+// Should not be passing an explicit linker emulation for the 64-bit case
+// PREFIXED-64-NOT: "-m"
+// RUN: env PATH=/this/path/does/not/exist %clang -no-canonical-prefixes \
+// RUN:   -target x86_64-unknown-freebsd12.2 %s \
+// RUN:   -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1 | FileCheck %s --check-prefix=MINUS-B-NO-TRIPLE-PREFIX
+// MINUS-B-NO-TRIPLE-PREFIX: "-cc1" "-triple" "x86_64-unknown-freebsd12.2"
+// MINUS-B-NO-TRIPLE-PREFIX-NEXT: "ld" "--eh-frame-hdr"
+// MINUS-B-NO-TRIPLE-PREFIX-NOT: "-m"
+
+/// The triple passed to clang -cc1 should be normalized, but the prefix when searching
+/// for ld should not be normalized. Since there is no x86_64--freebsd12.2-ld, passing
+/// -target x86_64--freebsd12.2 should not find a a valid linker:
+// RUN: env PATH=%S/Inputs/basic_freebsd64_tree/usr/bin %clang -no-canonical-prefixes \
+// RUN:   -target x86_64--freebsd12.2 %s \
+// RUN:   -B%S/Inputs/basic_freebsd64_tree/usr/bin -### 2>&1 | FileCheck %s --check-prefix=NO-NORMALIZE-LD-PREFIX-64
+// NO-NORMALIZE-LD-PREFIX-64: "-cc1" "-triple" "x86_64-unknown-freebsd12.2"
+// NO-NORMALIZE-LD-PREFIX-64-NEXT: "ld" "--eh-frame-hdr"
+// NO-NORMALIZE-LD-PREFIX-NOT: "-m"
+
+/// We should search for i386-unknown-freebsd12.2-ld when -m32 is passed (and also pass -m elf_i386_fbsd):
+// RUN: env PATH=%S/Inputs/basic_freebsd64_tree/usr/bin %clang -no-canonical-prefixes \
+// RUN:   -target x86_64-unknown-freebsd12.2 %s -m32 \
+// RUN: