[llvm-ar] Improve tool selection logic

[MaskRay]

Summary

If llvm-ar is installed at arm-pokymllib32-linux-gnueabi-llvm-ar, it may
think it is llvm-lib due to the "lib" substring.

Update the heuristic to make all the following work as intended:

llvm-ar-9
llvm-ranlib.exe
arm-pokymllib32-linux-gnueabi-llvm-ar
arm-pokymllib32-linux-gnueabi-llvm-ar-9

[rupprecht]

Update the heuristic to make all the following work as intended:

llvm-ar-9
llvm-ranlib.exe
arm-pokymllib32-linux-gnueabi-llvm-ar
arm-pokymllib32-linux-gnueabi-llvm-ar-9

This was manually verified, but can you add a test for it?
You could probably write a lit test like:

# RUN: mkdir -p bin
# RUN: cp llvm-ar bin/llvm-ar-9
# RUN: bin/llvm-ar-9 --help | FileCheck %s --check-prefix=AR
# RUN: cp llvm-ar bin/llvm-ranlib.exe
# RUN: bin/llvm-ranlib.exe --help | FileCheck %s --check-prefix=RANLIB
...

llvm/tools/llvm-ar/llvm-ar.cpp

1184

Needs to be lower-cased (+ a test case -- e.g. LLVM-AR.EXE should work)

1185–1186

This is pretty opaque, I think a more obvious algorithm should be used even if it's possibly more inefficient -- there's no need for this to be fast, it called at most 4 times.

Is the intention to match all these?
"...ar$"
"...ar-..."
"...ar.exe"

[MaskRay]

In D71302#1778295, @rupprecht wrote:

Update the heuristic to make all the following work as intended:

llvm-ar-9
llvm-ranlib.exe
arm-pokymllib32-linux-gnueabi-llvm-ar
arm-pokymllib32-linux-gnueabi-llvm-ar-9

This was manually verified, but can you add a test for it?
You could probably write a lit test like:

# RUN: mkdir -p bin
# RUN: cp llvm-ar bin/llvm-ar-9
# RUN: bin/llvm-ar-9 --help | FileCheck %s --check-prefix=AR
# RUN: cp llvm-ar bin/llvm-ranlib.exe
# RUN: bin/llvm-ranlib.exe --help | FileCheck %s --check-prefix=RANLIB
...

The path of llvm-ar is not known. Even if it does, I don't know copying an executable to a directory that may be mounted with the noexec flag will work...

llvm/tools/llvm-ar/llvm-ar.cpp

1184

I think LLVM-AR.EXE is a DOSism that we don't need to care in 2019.

1185–1186

Yes.

[rupprecht]

In D71302#1778352, @MaskRay wrote:

In D71302#1778295, @rupprecht wrote:

Update the heuristic to make all the following work as intended:

llvm-ar-9
llvm-ranlib.exe
arm-pokymllib32-linux-gnueabi-llvm-ar
arm-pokymllib32-linux-gnueabi-llvm-ar-9

This was manually verified, but can you add a test for it?
You could probably write a lit test like:

# RUN: mkdir -p bin
# RUN: cp llvm-ar bin/llvm-ar-9
# RUN: bin/llvm-ar-9 --help | FileCheck %s --check-prefix=AR
# RUN: cp llvm-ar bin/llvm-ranlib.exe
# RUN: bin/llvm-ranlib.exe --help | FileCheck %s --check-prefix=RANLIB
...

The path of llvm-ar is not known.

lit substitutes llvm-ar with the full path; see llvm/test/tools/llvm-ar/case-detection.test

Even if it does, I don't know copying an executable to a directory that may be mounted with the noexec flag will work...

Hmm, fair point. There's probably some other trick that would work... would creating a symlink work on a noexec mount?

llvm/tools/llvm-ar/llvm-ar.cpp

1184

Per D44808, the lowercasing is needed

[MaskRay]

Add tests.

Detect Lib.exe

[raj.khem]

LGTM now. I am testing it in yocto multilib SDK builds.

[MaskRay]

Fix a copy-n-paste problem

[rupprecht]

Looks good. Thanks for adding the tests!

[thakis]

llvm/test/tools/llvm-dlltool/tool-name.test

5

FYI, this test never ran because llvm/test/tools/llvm-dlltool/lit.local.cfg has config.suffixes = ['.def'] (instead of config.suffixes.add('.def')). (And if I change the lit.local.cfg file to make it run, the test fails.)

[thakis]

llvm/test/tools/llvm-dlltool/tool-name.test

5

fixed in af9bf14e6b0

[thakis]

llvm/test/tools/llvm-dlltool/tool-name.test

5

(maybe lit should diag if there are files in a dir that contains RUN lines but that are skipped because of config.suffixes?)

llvm/test/tools/llvm-ar/tool-name.test

+## Don't make symlinks on Windows.
+# UNSUPPORTED: system-windows
+
+# RUN: rm -rf %t
+# RUN: mkdir %t
+# RUN: ln -s llvm-ar %t/llvm-ar-9
+# RUN: ln -s llvm-ar %t/ar.exe
+# RUN: ln -s llvm-ar %t/arm-pokymllib32-linux-gnueabi-llvm-ar-9
+
+# RUN: llvm-ar h | FileCheck %s
+# RUN: %t/llvm-ar-9 h | FileCheck %s
+# RUN: %t/ar.exe h | FileCheck %s
+# RUN: %t/arm-pokymllib32-linux-gnueabi-llvm-ar-9 h | FileCheck %s
+
+# CHECK: USAGE: llvm-ar

llvm/test/tools/llvm-dlltool/tool-name.test

+## Don't make symlinks on Windows.
+# UNSUPPORTED: system-windows
+
+# RUN: rm -rf %t
+# RUN: mkdir %t
+# RUN: ln -s llvm-dlltool %t/dlltool.exe
+# RUN: ln -s llvm-dlltool %t/dlltool-10
+
+# RUN: not llvm-dlltool 2>&1 | FileCheck %s
+# RUN: not %t/dlltool.exe 2>&1 | FileCheck %s
+# RUN: not %t/dlltool-10 2>&1 | FileCheck %s
+
+# CHECK: USAGE: llvm-dlltool

llvm/test/tools/llvm-lib/tool-name.test

+## Don't make symlinks on Windows.
+# UNSUPPORTED: system-windows
+
+# RUN: rm -rf %t
+# RUN: mkdir %t
+## See D44808, MSBuild runs Lib.exe
+# RUN: ln -s llvm-lib %t/Lib.exe
+# RUN: ln -s llvm-lib %t/llvm-lib-10
+
+# RUN: llvm-lib '/?' | FileCheck %s
+# RUN: %t/Lib.exe '/?' | FileCheck %s
+# RUN: %t/llvm-lib-10 '/?' | FileCheck %s
+
+# CHECK: USAGE: llvm-lib

llvm/test/tools/llvm-ranlib/tool-name.test

+## Don't make symlinks on Windows.
+# UNSUPPORTED: system-windows
+
+# RUN: rm -rf %t
+# RUN: mkdir %t
+# RUN: ln -s llvm-ranlib %t/llvm-ranlib-9
+# RUN: ln -s llvm-ranlib %t/ranlib.exe
+
+# RUN: llvm-ranlib -h | FileCheck %s
+# RUN: %t/llvm-ranlib-9 -h | FileCheck %s
+# RUN: %t/ranlib.exe -h | FileCheck %s
+
+# CHECK: USAGE: llvm-ranlib

llvm/tools/llvm-ar/llvm-ar.cpp

 //===-- llvm-ar.cpp - LLVM archive librarian utility ----------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Builds up (relatively) standard unix archive files (.a) containing LLVM
 // bitcode or other files.
 //
 //===----------------------------------------------------------------------===//
 
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/ArchiveWriter.h"
 #include "llvm/Object/MachO.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/Chrono.h"
int main(int argc, char **argv) {
   InitLLVM X(argc, argv);
   ToolName = argv[0];
 
   llvm::InitializeAllTargetInfos();
   llvm::InitializeAllTargetMCs();
   llvm::InitializeAllAsmParsers();
 
   Stem = sys::path::stem(ToolName);
-  if (Stem.contains_lower("dlltool"))
-    return dlltoolDriverMain(makeArrayRef(argv, argc));
+  auto Is = [](StringRef Tool) {
+    // We need to recognize the following filenames.
+    //
+    // Lib.exe -> lib (see D44808, MSBuild runs Lib.exe)
+    // dlltool.exe -> dlltool
+    // arm-pokymllib32-linux-gnueabi-llvm-ar-10 -> ar
+    auto I = Stem.rfind_lower(Tool);
+    return I != StringRef::npos &&
+           (I + Tool.size() == Stem.size() || !isAlnum(Stem[I + Tool.size()]));
+  };
 
-  if (Stem.contains_lower("ranlib"))
+  if (Is("dlltool"))
+    return dlltoolDriverMain(makeArrayRef(argv, argc));
+  if (Is("ranlib"))
     return ranlib_main(argc, argv);
-
-  if (Stem.contains_lower("lib"))
+  if (Is("lib"))
     return libDriverMain(makeArrayRef(argv, argc));
-
-  if (Stem.contains_lower("ar"))
+  if (Is("ar"))
     return ar_main(argc, argv);
+
   fail("not ranlib, ar, lib or dlltool");
 }