[clang][Driver] Fix tool path priority test failures

[DavidSpickett]

Summary

Failure type 1:
This test can fail when the path of the build includes the strings
we're checking for. E.g "/gcc" is found in ".../gcc_7.3.0/..."

To correct this look for '"' on the end of all matches. So that we
only match the end of paths printed by clang -###.
(which would be ".../gcc_7.3.0/.../gcc" for the example)

Also look for other gcc names like gcc-x.y.z in the first check.
This confirms that the copy of clang we made is isolated as expected.

Failure type 2:
If you use a triple like "powerpc64le-linux-gnu" clang actually reports
"powerpc64le-unknown-linux-gnu". Then it searches for the
former.

That combined with Mac OS adding a version number to cmake's triple
means we can't trust cmake or clang to give us the one default triple.
To fix the test, write to both names. As they don't overlap with our
fake triple, we're still showing that the lookup works.

[DavidSpickett]

• Added check for alternate gcc names in the first test.

[MaskRay]

The description appears to be wrapped at a very small column number. Can you please fix it?

clang/test/Driver/program-path-priority.c

34

\ can be removed.

[DavidSpickett]

• Removed \ from regex, not needed for POSIX syntax.
• Reflowed text in commit description.

[DavidSpickett]

• Write to cmake and clang's default triple to also fix failure reported by @stevewan.

[DavidSpickett]

For the record my logic here is that I could change the tool names to use the same name as --version. However that will break whatever mips toolchain needed this code in the first place. (https://reviews.llvm.org/D13340?id=36227#inline-108606)

This default triple lookup code had some support to be removed anyway, so I'd rather patch up this test first before doing so. So that it's easier to roll back if there are still users of it. Having tests of the existing behaviour is a good thing and I don't think writing to both potential default triples undermines that. (one will just get ignored)

[MaskRay]

clang/test/Driver/program-path-priority.c

117

If $DEFAULT_TRIPLE == %target_triple, this mv command will fail.

The same applies to the rm below.

[DavidSpickett]

• Updated rm/mv command lines to account for $DEFAULT_TRIPLE being the same as %target_triple.

[stevewan]

clang/test/Driver/program-path-priority.c

117

Maybe I'm not seeing something obvious here, but I'm not aware of the file -E usage, and on Linux I got file: invalid option -- 'E'.

[DavidSpickett]

• mv with stderr redirected instead of using non standard file -E flag

[DavidSpickett]

clang/test/Driver/program-path-priority.c

117

I was looking for a way to mimic [! -f <file>], which worked on the command line but not in a test. From my system's file:

-E      On filesystem errors (file not found etc), instead of handling the error as regu‐
         lar output as POSIX mandates and keep going, issue an error message and exit.

I didn't realise it was non-standard, so I've switched to mv || true with the error redirected so it won't confuse the verbose output.

[stevewan]

clang/test/Driver/program-path-priority.c

117

I believe having just mv || true works, but I did like the idea of pre-checking for the existence of the file, and the test is more robust with the check. Is there a reason we wouldn't use test -f here?

[DavidSpickett]

• Use more standard 'test -f' command before 'mv's

[DavidSpickett]

clang/test/Driver/program-path-priority.c

117

Honestly it slipped my mind that [ ] was the same as test. Updated to test && move || true which you're right is clearer.
(one will always fail if the two triples are the same but at least we're only hiding the error we expect, rather than all errors mv could have)

[stevewan]

This LGTM, but since I'm not most familiar with the type 1 failure in description, let's see if other reviewers have further comments.

[miyuki]

I have reproduced the type 1 failure and I confirm that this patch fixes the issue.

clang/test/Driver/program-path-priority.c

 /// Don't create symlinks on Windows
 // UNSUPPORTED: system-windows
 
 /// Check the priority used when searching for tools
 /// Names and locations are usually in this order:
 /// <triple>-tool, tool, <default triple>-tool
 /// program path, PATH
 /// (from highest to lowest priority)
 /// A higher priority name found in a lower priority
 /// location will win over a lower priority name in a
 /// higher priority location.
 /// Prefix dirs (added with -B) override the location,
 /// so only name priority is accounted for, unless we fail to find
 /// anything at all in the prefix.
 
+/// Note: All matches are expected to be at the end of file paths.
+/// So we match " on the end to account for build systems that
+/// put the name of the compiler in the build path.
+/// E.g. /build/gcc_X.Y.Z/0/...
+
 /// Symlink clang to a new dir which will be its
 /// "program path" for these tests
 // RUN: rm -rf %t && mkdir -p %t
 // RUN: ln -s %clang %t/clang
 
 /// No gccs at all, nothing is found
 // RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=NO_NOTREAL_GCC %s
-// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc
-// NO_NOTREAL_GCC-NOT: /gcc
+// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc"
+/// Some systems will have "gcc-x.y.z" so for this first check
+/// make sure we don't find "gcc" or "gcc-x.y.z". If we do find either
+/// then there is no point continuing as this copy of clang is not
+/// isolated as we expected.
+// NO_NOTREAL_GCC-NOT: {{/gcc[^/]*"}}
 
 /// <triple>-gcc in program path is found
 // RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc
 // RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=PROG_PATH_NOTREAL_GCC %s
-// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc
+// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc"
 
 /// <triple>-gcc on the PATH is found
 // RUN: mkdir -p %t/env
 // RUN: rm %t/notreal-none-elf-gcc
 // RUN: touch %t/env/notreal-none-elf-gcc && chmod +x %t/env/notreal-none-elf-gcc
 // RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=ENV_PATH_NOTREAL_GCC %s
-// ENV_PATH_NOTREAL_GCC: env/notreal-none-elf-gcc
+// ENV_PATH_NOTREAL_GCC: env/notreal-none-elf-gcc"
 
 /// <triple>-gcc in program path is preferred to one on the PATH
 // RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc
 // RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=BOTH_NOTREAL_GCC %s
-// BOTH_NOTREAL_GCC: notreal-none-elf-gcc
-// BOTH_NOTREAL_GCC-NOT: env/notreal-none-elf-gcc
+// BOTH_NOTREAL_GCC: notreal-none-elf-gcc"
+// BOTH_NOTREAL_GCC-NOT: env/notreal-none-elf-gcc"
 
 /// On program path, <triple>-gcc is preferred to plain gcc
 // RUN: touch %t/gcc && chmod +x %t/gcc
 // RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=NOTREAL_GCC_PREFERRED %s
-// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc
-// NOTREAL_GCC_PREFERRED-NOT: /gcc
+// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc"
+// NOTREAL_GCC_PREFERRED-NOT: /gcc"
 
 /// <triple>-gcc on the PATH is preferred to gcc in program path
 // RUN: rm %t/notreal-none-elf-gcc
 // RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PROG %s
-// NOTREAL_PATH_OVER_GCC_PROG: env/notreal-none-elf-gcc
-// NOTREAL_PATH_OVER_GCC_PROG-NOT: /gcc
+// NOTREAL_PATH_OVER_GCC_PROG: env/notreal-none-elf-gcc"
+// NOTREAL_PATH_OVER_GCC_PROG-NOT: /gcc"
 
 /// <triple>-gcc on the PATH is preferred to gcc on the PATH
 // RUN: rm %t/gcc
 // RUN: touch %t/env/gcc && chmod +x %t/env/gcc
 // RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PATH %s
-// NOTREAL_PATH_OVER_GCC_PATH: env/notreal-none-elf-gcc
-// NOTREAL_PATH_OVER_GCC_PATH-NOT: /gcc
+// NOTREAL_PATH_OVER_GCC_PATH: env/notreal-none-elf-gcc"
+// NOTREAL_PATH_OVER_GCC_PATH-NOT: /gcc"
+
+/// We cannot trust clang --version, or cmake's LLVM_DEFAULT_TARGET_TRIPLE
+/// to give us the one and only default triple.
+/// Can't trust cmake because on Darwin, triples have a verison appended to them.
+/// (and clang uses the versioned string to search)
+/// Can't trust --version because it will pad 3 item triples to 4 e.g.
+/// powerpc64le-linux-gnu -> powerpc64le-unknown-linux-gnu
+/// (and clang uses the former to search)
+/// So we write to both names which is a bit odd but still proves that the
+/// lookup is working.
 
 /// <default-triple>-gcc has lowest priority so <triple>-gcc
 /// on PATH beats default triple in program path
-/// Darwin triples have a version appended to them, even if set via
-/// LLVM_DEFAULT_TARGET_TRIPLE. So the only way to know for sure is to ask clang.
 // RUN: DEFAULT_TRIPLE=`%t/clang --version | grep "Target:" | cut -d ' ' -f2`
 // RUN: touch %t/$DEFAULT_TRIPLE-gcc && chmod +x %t/$DEFAULT_TRIPLE-gcc
+// RUN: touch %t/%target_triple-gcc && chmod +x %t/%target_triple-gcc
 // RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=DEFAULT_TRIPLE_GCC %s
-// DEFAULT_TRIPLE_GCC: env/notreal-none-elf-gcc
+// DEFAULT_TRIPLE_GCC: env/notreal-none-elf-gcc"
 
 /// plain gcc on PATH beats default triple in program path
 // RUN: rm %t/env/notreal-none-elf-gcc
 // RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=DEFAULT_TRIPLE_NO_NOTREAL %s
-// DEFAULT_TRIPLE_NO_NOTREAL: env/gcc
-// DEFAULT_TRIPLE_NO_NOTREAL-NOT: -gcc
+// DEFAULT_TRIPLE_NO_NOTREAL: env/gcc"
+// DEFAULT_TRIPLE_NO_NOTREAL-NOT: -gcc"
 
 /// default triple only chosen when no others are present
 // RUN: rm %t/env/gcc
 // RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \
 // RUN:   FileCheck --check-prefix=DEFAULT_TRIPLE_NO_OTHERS %s
-// DEFAULT_TRIPLE_NO_OTHERS: -gcc
-// DEFAULT_TRIPLE_NO_OTHERS-NOT: notreal-none-elf-gcc
-// DEFAULT_TRIPLE_NO_OTHERS-NOT: /gcc
+// DEFAULT_TRIPLE_NO_OTHERS: -gcc"
+// DEFAULT_TRIPLE_NO_OTHERS-NOT: notreal-none-elf-gcc"
+// DEFAULT_TRIPLE_NO_OTHERS-NOT: /gcc"
 
 /// -B paths are searched separately so default triple will win
 /// if put in one of those even if other paths have higher priority names
 // RUN: mkdir -p %t/prefix
-// RUN: mv %t/$DEFAULT_TRIPLE-gcc %t/prefix
+/// One of these will fail when $DEFAULT_TRIPLE == %target_triple
+// RUN: mv %t/$DEFAULT_TRIPLE-gcc %t/prefix 2>/dev/null || true
+// RUN: mv %t/%target_triple-gcc %t/prefix 2>/dev/null || true
 // RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc
 // RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s -B %t/prefix 2>&1 | \
 // RUN:   FileCheck --check-prefix=DEFAULT_TRIPLE_IN_PREFIX %s
-// DEFAULT_TRIPLE_IN_PREFIX: prefix/{{.*}}-gcc
-// DEFAULT_TRIPLE_IN_PREFIX-NOT: notreal-none-elf-gcc
+// DEFAULT_TRIPLE_IN_PREFIX: prefix/{{.*}}-gcc"
+// DEFAULT_TRIPLE_IN_PREFIX-NOT: notreal-none-elf-gcc"
 
 /// Only if there is nothing in the prefix will we search other paths
-// RUN: rm %t/prefix/$DEFAULT_TRIPLE-gcc
+/// -f in case $DEFAULT_TRIPLE == %target_triple
+// RUN: rm -f %t/prefix/$DEFAULT_TRIPLE-gcc
+// RUN: rm -f %t/prefix/%target_triple-gcc
 // RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s -B %t/prefix 2>&1 | \
 // RUN:   FileCheck --check-prefix=EMPTY_PREFIX_DIR %s
-// EMPTY_PREFIX_DIR: notreal-none-elf-gcc
+// EMPTY_PREFIX_DIR: notreal-none-elf-gcc"

clang/test/lit.cfg.py

 
 llvm_config.use_default_substitutions()
 
 llvm_config.use_clang()
 
 config.substitutions.append(
     ('%src_include_dir', config.clang_src_dir + '/include'))
 
+config.substitutions.append(
+    ('%target_triple', config.target_triple))
 
 # Propagate path to symbolizer for ASan/MSan.
 llvm_config.with_system_environment(
     ['ASAN_SYMBOLIZER_PATH', 'MSAN_SYMBOLIZER_PATH'])
 
 config.substitutions.append(('%PATH%', config.environment['PATH']))