Platform qemu-user: Build path to qemu automatically if not specified

[ted]

Summary

Get the path to qemu in the following order:

1. From the property platform.plugin.qemu-user.emulator-path
2. If that property is not set, from PATH, building the name of the qemu binary from the triple in property platform.plugin.qemu-user.architecture
3. If that property is not set, from PATH, building the name of the qemu binary from the triple in the target

This will allow a user to load a target and run without setting properties,
if qemu is on the PATH and named qemu-<ArchName>

[labath]

I am wondering if we actually need the second step (the architecture setting) here. The main reason it exists is the usage in GetSupportedArchitectures (which is called before a target is created) it seems like the value derived from the target should always be more correct. WDYT? What are the values you get in steps 2 and 3 for your use case?

[ted]

In D155117#4505538, @labath wrote:

I am wondering if we actually need the second step (the architecture setting) here. The main reason it exists is the usage in GetSupportedArchitectures (which is called before a target is created) it seems like the value derived from the target should always be more correct. WDYT? What are the values you get in steps 2 and 3 for your use case?

I don't think we need to specify the architecture, because I think we can always get it from the triple. There might be a case where someone is using a qemu that isn't named the same as the Triple ArchName, but that case could be covered by emulator-path.

In my case, I'm loading a target, then running, letting the platform get ArchName from the target Triple, then turning that into qemu-riscv32 (or riscv64), and launching that from my PATH.

An example (with my experimental RISC-V ABI plugin):

> bin/lldb
(lldb) platform select qemu-user
  Platform: qemu-user
    Triple: x86_64-*-linux-gnu
OS Version: 5.4.0 (5.4.0-136-generic)
  Hostname: 127.0.0.1
WorkingDir: /local/mnt/ted/8.8/riscv
    Kernel: #153~18.04.1-Ubuntu SMP Wed Nov 30 15:47:57 UTC 2022
(lldb) file ~/lldb_test/factrv32
Current executable set to '/usr2/tedwood/lldb_test/factrv32' (riscv32).
(lldb) b main
Breakpoint 1: where = factrv32`main + 28 at factorial.c:32:8, address = 0x000104ea
(lldb) r
Process 1 launched: '/usr2/tedwood/lldb_test/factrv32' (riscv32)
Process 1 stopped
* thread #1, stop reason = breakpoint 1.1
    frame #0: 0x000104ea factrv32`main(argc=1, argv=0x408005a4) at factorial.c:32:8
   29  	  }
   30  	*/
   31  	
-> 32  	  base = 10;
   33  	
   34  	  printf("Factorial of %d is %d\n", base, factorial(base));
   35  	  return 0;
(lldb) target list
Current targets:
* target #0: /usr2/tedwood/lldb_test/factrv32 ( arch=riscv32-*-linux, platform=qemu-user, pid=1, state=stopped )
(lldb)

[ted]

As for the GetSupportedArchitectures case, downstream I left in the code that checks the property, but changed the return {}; to

return {ArchSpec(llvm::Triple("riscv32-unknown-linux")),
        ArchSpec(llvm::Triple("riscv64-unknown-linux"))};

I don't think we want that upstream, but downstream the only time we're targetting riscv linux is when using qemu.

That lets me run lldb, load a riscv file, then run, without setting any properties.

[labath]

In D155117#4510512, @ted wrote:

In D155117#4505538, @labath wrote:

I am wondering if we actually need the second step (the architecture setting) here. The main reason it exists is the usage in GetSupportedArchitectures (which is called before a target is created) it seems like the value derived from the target should always be more correct. WDYT? What are the values you get in steps 2 and 3 for your use case?

I don't think we need to specify the architecture, because I think we can always get it from the triple. There might be a case where someone is using a qemu that isn't named the same as the Triple ArchName, but that case could be covered by emulator-path.

Yeah, that's my thinking as well. Given this, I think we should drop the setting part and just go with the target value directly.

In D155117#4511968, @ted wrote:

As for the GetSupportedArchitectures case, downstream I left in the code that checks the property, but changed the return {}; to

return {ArchSpec(llvm::Triple("riscv32-unknown-linux")),
        ArchSpec(llvm::Triple("riscv64-unknown-linux"))};

I don't think we want that upstream, but downstream the only time we're targetting riscv linux is when using qemu.

That lets me run lldb, load a riscv file, then run, without setting any properties.

FWIW, the way we do that is by putting something along the lines of:

settings set platform.plugin.qemu-user.architecture aarch64
...
platform select qemu-user
platform select host

into our global lldbinit file. This causes lldb's platform selection logic to pick qemu-user (instead of remote-linux) when encountering an aarch64 executable, without needing to change the source code.

[ted]

In D155117#4521393, @labath wrote:

In D155117#4510512, @ted wrote:

In D155117#4505538, @labath wrote:

I am wondering if we actually need the second step (the architecture setting) here. The main reason it exists is the usage in GetSupportedArchitectures (which is called before a target is created) it seems like the value derived from the target should always be more correct. WDYT? What are the values you get in steps 2 and 3 for your use case?

I don't think we need to specify the architecture, because I think we can always get it from the triple. There might be a case where someone is using a qemu that isn't named the same as the Triple ArchName, but that case could be covered by emulator-path.

Yeah, that's my thinking as well. Given this, I think we should drop the setting part and just go with the target value directly.

Do you want me to remove the setting entirely?

lldb/source/Plugins/Platform/QemuUser/PlatformQemuUser.cpp

 //===-- PlatformQemuUser.cpp ----------------------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
static Environment ComputeLaunchEnvironment(Environment target,
   return host;
 }
 
 lldb::ProcessSP PlatformQemuUser::DebugProcess(ProcessLaunchInfo &launch_info,
                                                Debugger &debugger,
                                                Target &target, Status &error) {
   Log *log = GetLog(LLDBLog::Platform);
 
+  // If platform.plugin.qemu-user.emulator-path is set, use it.
   FileSpec qemu = GetGlobalProperties().GetEmulatorPath();
-  if (!qemu)
-    qemu.SetPath(("qemu-" + GetGlobalProperties().GetArchitecture()).str());
+  // If platform.plugin.qemu-user.emulator-path is not set, build the
+  // executable name from platform.plugin.qemu-user.architecture.
+  if (!qemu) {
+    llvm::StringRef arch = GetGlobalProperties().GetArchitecture();
+    // If platform.plugin.qemu-user.architecture is not set, build the
+    // executable name from the target Triple's ArchName
+    if (arch.empty())
+      arch = target.GetArchitecture().GetTriple().getArchName();
+    qemu.SetPath(("qemu-" + arch).str());
+  }
   FileSystem::Instance().ResolveExecutableLocation(qemu);
 
   llvm::SmallString<0> socket_model, socket_path;
   HostInfo::GetProcessTempDir().GetPath(socket_model);
   llvm::sys::path::append(socket_model, "qemu-%%%%%%%%.socket");
   do {
     llvm::sys::fs::createUniquePath(socket_model, socket_path, false);
   } while (FileSystem::Instance().Exists(socket_path));