[lldb] Resolve exe location for `target create`

[alvinhochun]

Summary

This fixes an issue that, when you start lldb or use target create
with a program name which is on $PATH, or not specify the .exe suffix of
a program in the working directory on Windows, you get a confusing
error, for example:

(lldb) target create notepad
error: 'C:\WINDOWS\SYSTEM32\notepad.exe' doesn't contain any 'host'
platform architectures: i686, x86_64, i386, i386

Fixes https://github.com/mstorsjo/llvm-mingw/issues/265

[DavidSpickett]

Is it easy to test this?

Ideally this behaviour works with a path to a file as well as PATH itself e.g. C:\foo becomes C:\foo.exe and you could write a test that only runs on Windows quite easily. If not I guess you could do a shell test that compiles a program then runs lldb with PATH set to the current dir? (though it sounds like you can use the current dir by default anyway)

[alvinhochun]

I suppose it is possible to have a Windows-only test for this. (Someone will need to verify them for me though as I don't have a build that runs tests on Windows.)

This doesn't currently work with a path without the suffix though (like C:\Windows\System32\notepad would not work). ResolveExecutableLocation calls llvm::sys::findProgramByName (from llvm/lib/Support/Windows/Program.inc), which doesn't do anything if the argument is a path. I can change this behaviour, but it may affect more than just target create... I don't feel comfortable making this change here for now.

[alvinhochun]

Does this test look good:

diff --git a/lldb/test/Shell/Commands/command-target-create-resolve-exe.test b/lldb/test/Shell/Commands/command-target-create-resolve-exe.test
new file mode 100644
index 000000000000..3dfa7f0d9853
--- /dev/null
+++ b/lldb/test/Shell/Commands/command-target-create-resolve-exe.test
@@ -0,0 +1,22 @@
+# REQUIRES: system-windows
+# RUN: mkdir "%t.dir"
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c -o %t.dir\testmain.exe
+
+## Test with full path to exe
+# RUN: %lldb %t.dir\testmain.exe -b | FileCheck %s
+
+## Test with exe on path, with .exe suffix
+# RUN: set "PATH=%t.dir;%%PATH%%" && %lldb testmain.exe -b | FileCheck %s
+
+## Test with exe on path, without .exe suffix
+# RUN: set "PATH=%t.dir;%%PATH%%" && %lldb testmain -b | FileCheck %s
+
+## Test in cwd, with .exe suffix
+# RUN: cd "%t.dir" && %lldb testmain.exe -b | FileCheck %s
+
+## Test in cwd, without .exe suffix
+# RUN: cd "%t.dir" && %lldb testmain -b | FileCheck %s
+
+# LABEL: target create
+# CHECK-NEXT: Current executable set to '{{.*[/\\]}}testmain.exe'
+# CHECK-NOT: Error

[mstorsjo]

In D127436#3574289, @alvinhochun wrote:

Does this test look good:

With a bunch of minor fixups, this testcase does work - I uploaded a working copy of it at https://martin.st/temp/command-target-create-resolve-exe.test.

[alvinhochun]

Added Windows-specific test

[DavidSpickett]

LGTM

[stella.stamenova]

This broke the Windows LLDB bot:

https://lab.llvm.org/buildbot/#/builders/83/builds/20295/steps/7/logs/stdio

[mstorsjo]

In D127436#3602224, @stella.stamenova wrote:

This broke the Windows LLDB bot:

https://lab.llvm.org/buildbot/#/builders/83/builds/20295/steps/7/logs/stdio

Yep, noted. It worked for me in my MSVC build configuration - I'll investigate a bit more and either push another attempt at fixing it, or revert, in a little while.

[mstorsjo]

In D127436#3602224, @stella.stamenova wrote:

This broke the Windows LLDB bot:

https://lab.llvm.org/buildbot/#/builders/83/builds/20295/steps/7/logs/stdio

The second fix attempt, in https://github.com/llvm/llvm-project/commit/a1ee0b947d46c9be1cc2ea8db21603bac84efb18, seems to have fixed this test now - however the latest test run seems to have some other failures: https://lab.llvm.org/buildbot/#/builders/83/builds/20304 (Not sure if these are spurious failures or if they are other regressions that happened meanwhile)

lldb/source/Commands/CommandObjectTarget.cpp

if (argc == 1 || core_file || remote_file) {
                                         symfile.GetPath(),
                                         llvm::toString(file.takeError()));
           return false;
         }
       }
 
       const char *file_path = command.GetArgumentAtIndex(0);
       LLDB_SCOPED_TIMERF("(lldb) target create '%s'", file_path);
-      FileSpec file_spec;
-
-      if (file_path) {
-        file_spec.SetFile(file_path, FileSpec::Style::native);
-        FileSystem::Instance().Resolve(file_spec);
-      }
 
       bool must_set_platform_path = false;
 
       Debugger &debugger = GetDebugger();
 
       TargetSP target_sp;
       llvm::StringRef arch_cstr = m_arch_option.GetArchitectureName();
       Status error(debugger.GetTargetList().CreateTarget(
if (argc == 1 || core_file || remote_file) {
           });
 
       // Only get the platform after we create the target because we might
       // have switched platforms depending on what the arguments were to
       // CreateTarget() we can't rely on the selected platform.
 
       PlatformSP platform_sp = target_sp->GetPlatform();
 
+      FileSpec file_spec;
+      if (file_path) {
+        file_spec.SetFile(file_path, FileSpec::Style::native);
+        FileSystem::Instance().Resolve(file_spec);
+
+        // Try to resolve the exe based on PATH and/or platform-specific
+        // suffixes, but only if using the host platform.
+        if (platform_sp && platform_sp->IsHost() &&
+            !FileSystem::Instance().Exists(file_spec))
+          FileSystem::Instance().ResolveExecutableLocation(file_spec);
+      }
+
       if (remote_file) {
         if (platform_sp) {
           // I have a remote file.. two possible cases
           if (file_spec && FileSystem::Instance().Exists(file_spec)) {
             // if the remote file does not exist, push it there
             if (!platform_sp->GetFileExists(remote_file)) {
               Status err = platform_sp->PutFile(file_spec, remote_file);
               if (err.Fail()) {

lldb/source/Target/TargetList.cpp

if (!platform_arch.TripleOSWasSpecified() ||
       prefer_platform_arch = true;
       platform_arch = module_arch;
     }
   };
 
   if (!user_exe_path.empty()) {
     ModuleSpec module_spec(FileSpec(user_exe_path, FileSpec::Style::native));
     FileSystem::Instance().Resolve(module_spec.GetFileSpec());
+
+    // Try to resolve the exe based on PATH and/or platform-specific suffixes,
+    // but only if using the host platform.
+    if (platform_sp->IsHost() &&
+        !FileSystem::Instance().Exists(module_spec.GetFileSpec()))
+      FileSystem::Instance().ResolveExecutableLocation(
+          module_spec.GetFileSpec());
+
     // Resolve the executable in case we are given a path to a application
     // bundle like a .app bundle on MacOSX.
     Host::ResolveExecutableInBundle(module_spec.GetFileSpec());
 
     lldb::offset_t file_offset = 0;
     lldb::offset_t file_size = 0;
     ModuleSpecList module_specs;
     const size_t num_specs = ObjectFile::GetModuleSpecifications(

lldb/test/Shell/Commands/command-target-create-resolve-exe.test

+# REQUIRES: system-windows
+
+## This checks that when starting lldb (or using `target create`) with a
+## program name which is on $PATH, or not specify the .exe suffix of a program
+## in the working directory on Windows, lldb can still detect the target
+## architecture correctly instead of producing an error.
+
+# RUN: mkdir -p "%t.dir"
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c -o %t.dir/testmain.exe
+
+## Test with full path to exe
+# RUN: %lldb %t.dir/testmain.exe -b | FileCheck %s
+
+## Test with exe on path, with .exe suffix
+# RUN: env "PATH=%t.dir;%PATH%" %lldb testmain.exe -b | FileCheck %s
+
+## Test with exe on path, without .exe suffix
+# RUN: env "PATH=%t.dir;%PATH%" %lldb testmain -b | FileCheck %s
+
+## Test in cwd, with .exe suffix
+# RUN: cd "%t.dir" && %lldb testmain.exe -b | FileCheck %s
+
+## Test in cwd, without .exe suffix
+# RUN: cd "%t.dir" && %lldb testmain -b | FileCheck %s
+
+# CHECK-LABEL: target create
+# CHECK-NEXT: Current executable set to '{{.*[/\\]}}testmain.exe'
+# CHECK-NOT: Error