[lldb] Move host platform implementations into the base class

About half of our host platform code was implemented in the Platform
class, while the rest was it RemoteAwarePlatform. Most of the time, this
did not matter, as nearly all our platforms are also
RemoteAwarePlatforms. It makes a difference for PlatformQemu, which
descends directly from the base class (as it is local-only).

This patch moves all host code paths into the base class, and marks
PlatformQemu as a "host" platform so it can make use of them (it sounds
slightly strange, but that is consistent with what the apple simulator
platforms are doing). Not all of the host implementations make sense for
this platform, but it can always override those that don't.

I add some basic tests using the platform file apis to exercise this
functionality.

Differential Revision: https://reviews.llvm.org/D122898

lldb/include/lldb/Target/Platform.h

@@ -278,7 +278,7 @@ public:
 
   virtual bool SetRemoteWorkingDirectory(const FileSpec &working_dir);
 
-  virtual UserIDResolver &GetUserIDResolver() = 0;
+  virtual UserIDResolver &GetUserIDResolver();
 
   /// Locate a file for a platform.
   ///
@@ -516,34 +516,20 @@ public:
 
   virtual lldb::user_id_t OpenFile(const FileSpec &file_spec,
                                    File::OpenOptions flags, uint32_t mode,
-                                   Status &error) {
-    return UINT64_MAX;
-  }
+                                   Status &error);
 
-  virtual bool CloseFile(lldb::user_id_t fd, Status &error) { return false; }
+  virtual bool CloseFile(lldb::user_id_t fd, Status &error);
 
-  virtual lldb::user_id_t GetFileSize(const FileSpec &file_spec) {
-    return UINT64_MAX;
-  }
+  virtual lldb::user_id_t GetFileSize(const FileSpec &file_spec);
 
   virtual void AutoCompleteDiskFileOrDirectory(CompletionRequest &request,
                                                bool only_dir) {}
 
   virtual uint64_t ReadFile(lldb::user_id_t fd, uint64_t offset, void *dst,
-                            uint64_t dst_len, Status &error) {
-    error.SetErrorStringWithFormatv(
-        "Platform::ReadFile() is not supported in the {0} platform",
-        GetPluginName());
-    return -1;
-  }
+                            uint64_t dst_len, Status &error);
 
   virtual uint64_t WriteFile(lldb::user_id_t fd, uint64_t offset,
-                             const void *src, uint64_t src_len, Status &error) {
-    error.SetErrorStringWithFormatv(
-        "Platform::WriteFile() is not supported in the {0} platform",
-        GetPluginName());
-    return -1;
-  }
+                             const void *src, uint64_t src_len, Status &error);
 
   virtual Status GetFile(const FileSpec &source, const FileSpec &destination);
 

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

@@ -60,7 +60,7 @@ private:
   static lldb::PlatformSP CreateInstance(bool force, const ArchSpec *arch);
   static void DebuggerInitialize(Debugger &debugger);
 
-  PlatformQemuUser() : Platform(/*is_host=*/false) {}
+  PlatformQemuUser() : Platform(/*is_host=*/true) {}
 };
 
 } // namespace lldb_private

lldb/source/Target/Platform.cpp

@@ -19,6 +19,7 @@
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/StreamFile.h"
+#include "lldb/Host/FileCache.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
@@ -790,6 +791,56 @@ Status Platform::SetFilePermissions(const FileSpec &file_spec,
   }
 }
 
+user_id_t Platform::OpenFile(const FileSpec &file_spec,
+                                   File::OpenOptions flags, uint32_t mode,
+                                   Status &error) {
+  if (IsHost())
+    return FileCache::GetInstance().OpenFile(file_spec, flags, mode, error);
+  return UINT64_MAX;
+}
+
+bool Platform::CloseFile(user_id_t fd, Status &error) {
+  if (IsHost())
+    return FileCache::GetInstance().CloseFile(fd, error);
+  return false;
+}
+
+user_id_t Platform::GetFileSize(const FileSpec &file_spec) {
+  if (!IsHost())
+    return UINT64_MAX;
+
+  uint64_t Size;
+  if (llvm::sys::fs::file_size(file_spec.GetPath(), Size))
+    return 0;
+  return Size;
+}
+
+uint64_t Platform::ReadFile(lldb::user_id_t fd, uint64_t offset, void *dst,
+                            uint64_t dst_len, Status &error) {
+  if (IsHost())
+    return FileCache::GetInstance().ReadFile(fd, offset, dst, dst_len, error);
+  error.SetErrorStringWithFormatv(
+      "Platform::ReadFile() is not supported in the {0} platform",
+      GetPluginName());
+  return -1;
+}
+
+uint64_t Platform::WriteFile(lldb::user_id_t fd, uint64_t offset,
+                             const void *src, uint64_t src_len, Status &error) {
+  if (IsHost())
+    return FileCache::GetInstance().WriteFile(fd, offset, src, src_len, error);
+  error.SetErrorStringWithFormatv(
+      "Platform::WriteFile() is not supported in the {0} platform",
+      GetPluginName());
+  return -1;
+}
+
+UserIDResolver &Platform::GetUserIDResolver() {
+  if (IsHost())
+    return HostInfo::GetUserIDResolver();
+  return UserIDResolver::GetNoopResolver();
+}
+
 const char *Platform::GetHostname() {
   if (IsHost())
     return "127.0.0.1";
@@ -1381,17 +1432,21 @@ Status
 Platform::CreateSymlink(const FileSpec &src, // The name of the link is in src
                         const FileSpec &dst) // The symlink points to dst
 {
-  Status error("unimplemented");
-  return error;
+  if (IsHost())
+    return FileSystem::Instance().Symlink(src, dst);
+  return Status("unimplemented");
 }
 
 bool Platform::GetFileExists(const lldb_private::FileSpec &file_spec) {
+  if (IsHost())
+    return FileSystem::Instance().Exists(file_spec);
   return false;
 }
 
 Status Platform::Unlink(const FileSpec &path) {
-  Status error("unimplemented");
-  return error;
+  if (IsHost())
+    return llvm::sys::fs::remove(path.GetPath());
+  return Status("unimplemented");
 }
 
 MmapArgList Platform::GetMmapArgumentList(const ArchSpec &arch, addr_t addr,
@@ -1437,8 +1492,7 @@ lldb_private::Status Platform::RunShellCommand(
   if (IsHost())
     return Host::RunShellCommand(shell, command, working_dir, status_ptr,
                                  signo_ptr, command_output, timeout);
-  else
-    return Status("unimplemented");
+  return Status("unable to run a remote command without a platform");
 }
 
 bool Platform::CalculateMD5(const FileSpec &file_spec, uint64_t &low,
@@ -1597,7 +1651,11 @@ lldb_private::Status OptionGroupPlatformCaching::SetOptionValue(
   return error;
 }
 
-Environment Platform::GetEnvironment() { return Environment(); }
+Environment Platform::GetEnvironment() {
+  if (IsHost())
+    return Host::GetEnvironment();
+  return Environment();
+}
 
 const std::vector<ConstString> &Platform::GetTrapHandlerSymbolNames() {
   if (!m_calculated_trap_handlers) {

lldb/source/Target/RemoteAwarePlatform.cpp

@@ -10,7 +10,6 @@
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleList.h"
 #include "lldb/Core/ModuleSpec.h"
-#include "lldb/Host/FileCache.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
@@ -180,14 +179,12 @@ Status RemoteAwarePlatform::RunShellCommand(
     llvm::StringRef shell, llvm::StringRef command, const FileSpec &working_dir,
     int *status_ptr, int *signo_ptr, std::string *command_output,
     const Timeout<std::micro> &timeout) {
-  if (IsHost())
-    return Host::RunShellCommand(shell, command, working_dir, status_ptr,
-                                 signo_ptr, command_output, timeout);
   if (m_remote_platform_sp)
     return m_remote_platform_sp->RunShellCommand(shell, command, working_dir,
                                                  status_ptr, signo_ptr,
                                                  command_output, timeout);
-  return Status("unable to run a remote command without a platform");
+  return Platform::RunShellCommand(shell, command, working_dir, status_ptr,
+                                   signo_ptr, command_output, timeout);
 }
 
 Status RemoteAwarePlatform::MakeDirectory(const FileSpec &file_spec,
@@ -216,16 +213,12 @@ Status RemoteAwarePlatform::SetFilePermissions(const FileSpec &file_spec,
 lldb::user_id_t RemoteAwarePlatform::OpenFile(const FileSpec &file_spec,
                                               File::OpenOptions flags,
                                               uint32_t mode, Status &error) {
-  if (IsHost())
-    return FileCache::GetInstance().OpenFile(file_spec, flags, mode, error);
   if (m_remote_platform_sp)
     return m_remote_platform_sp->OpenFile(file_spec, flags, mode, error);
   return Platform::OpenFile(file_spec, flags, mode, error);
 }
 
 bool RemoteAwarePlatform::CloseFile(lldb::user_id_t fd, Status &error) {
-  if (IsHost())
-    return FileCache::GetInstance().CloseFile(fd, error);
   if (m_remote_platform_sp)
     return m_remote_platform_sp->CloseFile(fd, error);
   return Platform::CloseFile(fd, error);
@@ -234,8 +227,6 @@ bool RemoteAwarePlatform::CloseFile(lldb::user_id_t fd, Status &error) {
 uint64_t RemoteAwarePlatform::ReadFile(lldb::user_id_t fd, uint64_t offset,
                                        void *dst, uint64_t dst_len,
                                        Status &error) {
-  if (IsHost())
-    return FileCache::GetInstance().ReadFile(fd, offset, dst, dst_len, error);
   if (m_remote_platform_sp)
     return m_remote_platform_sp->ReadFile(fd, offset, dst, dst_len, error);
   return Platform::ReadFile(fd, offset, dst, dst_len, error);
@@ -244,20 +235,12 @@ uint64_t RemoteAwarePlatform::ReadFile(lldb::user_id_t fd, uint64_t offset,
 uint64_t RemoteAwarePlatform::WriteFile(lldb::user_id_t fd, uint64_t offset,
                                         const void *src, uint64_t src_len,
                                         Status &error) {
-  if (IsHost())
-    return FileCache::GetInstance().WriteFile(fd, offset, src, src_len, error);
   if (m_remote_platform_sp)
     return m_remote_platform_sp->WriteFile(fd, offset, src, src_len, error);
   return Platform::WriteFile(fd, offset, src, src_len, error);
 }
 
 lldb::user_id_t RemoteAwarePlatform::GetFileSize(const FileSpec &file_spec) {
-  if (IsHost()) {
-    uint64_t Size;
-    if (llvm::sys::fs::file_size(file_spec.GetPath(), Size))
-      return 0;
-    return Size;
-  }
   if (m_remote_platform_sp)
     return m_remote_platform_sp->GetFileSize(file_spec);
   return Platform::GetFileSize(file_spec);
@@ -265,24 +248,18 @@ lldb::user_id_t RemoteAwarePlatform::GetFileSize(const FileSpec &file_spec) {
 
 Status RemoteAwarePlatform::CreateSymlink(const FileSpec &src,
                                           const FileSpec &dst) {
-  if (IsHost())
-    return FileSystem::Instance().Symlink(src, dst);
   if (m_remote_platform_sp)
     return m_remote_platform_sp->CreateSymlink(src, dst);
   return Platform::CreateSymlink(src, dst);
 }
 
 bool RemoteAwarePlatform::GetFileExists(const FileSpec &file_spec) {
-  if (IsHost())
-    return FileSystem::Instance().Exists(file_spec);
   if (m_remote_platform_sp)
     return m_remote_platform_sp->GetFileExists(file_spec);
   return Platform::GetFileExists(file_spec);
 }
 
 Status RemoteAwarePlatform::Unlink(const FileSpec &file_spec) {
-  if (IsHost())
-    return llvm::sys::fs::remove(file_spec.GetPath());
   if (m_remote_platform_sp)
     return m_remote_platform_sp->Unlink(file_spec);
   return Platform::Unlink(file_spec);
@@ -290,11 +267,9 @@ Status RemoteAwarePlatform::Unlink(const FileSpec &file_spec) {
 
 bool RemoteAwarePlatform::CalculateMD5(const FileSpec &file_spec, uint64_t &low,
                                        uint64_t &high) {
-  if (IsHost())
-    return Platform::CalculateMD5(file_spec, low, high);
   if (m_remote_platform_sp)
     return m_remote_platform_sp->CalculateMD5(file_spec, low, high);
-  return false;
+  return Platform::CalculateMD5(file_spec, low, high);
 }
 
 FileSpec RemoteAwarePlatform::GetRemoteWorkingDirectory() {
@@ -349,55 +324,42 @@ ArchSpec RemoteAwarePlatform::GetRemoteSystemArchitecture() {
 }
 
 const char *RemoteAwarePlatform::GetHostname() {
-  if (IsHost())
-    return Platform::GetHostname();
   if (m_remote_platform_sp)
     return m_remote_platform_sp->GetHostname();
-  return nullptr;
+  return Platform::GetHostname();
 }
 
 UserIDResolver &RemoteAwarePlatform::GetUserIDResolver() {
-  if (IsHost())
-    return HostInfo::GetUserIDResolver();
   if (m_remote_platform_sp)
     return m_remote_platform_sp->GetUserIDResolver();
-  return UserIDResolver::GetNoopResolver();
+  return Platform::GetUserIDResolver();
 }
 
 Environment RemoteAwarePlatform::GetEnvironment() {
-  if (IsRemote()) {
-    if (m_remote_platform_sp)
-      return m_remote_platform_sp->GetEnvironment();
-    return Environment();
-  }
-  return Host::GetEnvironment();
+  if (m_remote_platform_sp)
+    return m_remote_platform_sp->GetEnvironment();
+  return Platform::GetEnvironment();
 }
 
 bool RemoteAwarePlatform::IsConnected() const {
-  if (IsHost())
-    return true;
-  else if (m_remote_platform_sp)
+  if (m_remote_platform_sp)
     return m_remote_platform_sp->IsConnected();
-  return false;
+  return Platform::IsConnected();
 }
 
 bool RemoteAwarePlatform::GetProcessInfo(lldb::pid_t pid,
                                          ProcessInstanceInfo &process_info) {
-  if (IsHost())
-    return Platform::GetProcessInfo(pid, process_info);
   if (m_remote_platform_sp)
     return m_remote_platform_sp->GetProcessInfo(pid, process_info);
-  return false;
+  return Platform::GetProcessInfo(pid, process_info);
 }
 
 uint32_t
 RemoteAwarePlatform::FindProcesses(const ProcessInstanceInfoMatch &match_info,
                                    ProcessInstanceInfoList &process_infos) {
-  if (IsHost())
-    return Platform::FindProcesses(match_info, process_infos);
   if (m_remote_platform_sp)
     return m_remote_platform_sp->FindProcesses(match_info, process_infos);
-  return 0;
+  return Platform::FindProcesses(match_info, process_infos);
 }
 
 lldb::ProcessSP RemoteAwarePlatform::ConnectProcess(llvm::StringRef connect_url,
@@ -413,25 +375,15 @@ lldb::ProcessSP RemoteAwarePlatform::ConnectProcess(llvm::StringRef connect_url,
 }
 
 Status RemoteAwarePlatform::LaunchProcess(ProcessLaunchInfo &launch_info) {
-  Status error;
-
-  if (IsHost()) {
-    error = Platform::LaunchProcess(launch_info);
-  } else {
-    if (m_remote_platform_sp)
-      error = m_remote_platform_sp->LaunchProcess(launch_info);
-    else
-      error.SetErrorString("the platform is not currently connected");
-  }
-  return error;
+  if (m_remote_platform_sp)
+    return m_remote_platform_sp->LaunchProcess(launch_info);
+  return Platform::LaunchProcess(launch_info);
 }
 
 Status RemoteAwarePlatform::KillProcess(const lldb::pid_t pid) {
-  if (IsHost())
-    return Platform::KillProcess(pid);
   if (m_remote_platform_sp)
     return m_remote_platform_sp->KillProcess(pid);
-  return Status("the platform is not currently connected");
+  return Platform::KillProcess(pid);
 }
 
 size_t RemoteAwarePlatform::ConnectToWaitingProcesses(Debugger &debugger,

lldb/test/API/qemu/TestQemuAPI.py

@@ -0,0 +1,28 @@
+from __future__ import print_function
+import lldb
+import os
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+
+
+@skipIfRemote
+class TestQemuAPI(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def test_file_api(self):
+        qemu = lldb.SBPlatform("qemu-user")
+        host = lldb.SBPlatform.GetHostPlatform()
+
+        target = self.getBuildArtifact("target.c")
+        main_c = lldb.SBFileSpec(self.getSourcePath("main.c"))
+
+        self.assertSuccess(qemu.Put(main_c, lldb.SBFileSpec(target)))
+        self.assertTrue(os.path.exists(target))
+        self.assertEqual(qemu.GetFilePermissions(target),
+                host.GetFilePermissions(target))
+
+        self.assertSuccess(qemu.MakeDirectory(
+            self.getBuildArtifact("target_dir")))
+        self.assertTrue(os.path.isdir(self.getBuildArtifact("target_dir")))