From 36e82208c7752b5a8f5064f22c6c7232b801a607 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Mon, 29 Jan 2018 10:46:00 +0000 Subject: [PATCH] Remove ObjectFile usage from HostLinux::GetProcessInfo Summary: The ObjectFile class was used to determine the architecture of a running process by inspecting it's main executable. There were two issues with this: - it's in the wrong layer - the call can be very expensive (it can end up computing the crc of the whole file). Since the process is running on the host, ideally we would be able to just query the data straight from the OS like darwin does, but there doesn't seem to be a reasonable way to do that. So, this fixes the layering issue by using the llvm object library to inspect the file. Since we know the process is already running on the host, we just need to peek at a few bytes of the elf header to determine whether it's 32- or 64-bit (which should make this faster as well). Pretty much the same logic was implemented in NativeProcessProtocol::ResolveProcessArchitecture, so I delete this logic and replace calls with GetProcessInfo. Reviewers: eugene, krytarowski Subscribers: mgorny, hintonda, lldb-commits Differential Revision: https://reviews.llvm.org/D42488 llvm-svn: 323637 --- .../lldb/Host/common/NativeProcessProtocol.h | 5 -- lldb/include/lldb/Utility/ArchSpec.h | 1 + lldb/source/Host/CMakeLists.txt | 1 + .../Host/common/NativeProcessProtocol.cpp | 24 --------- lldb/source/Host/linux/Host.cpp | 49 +++++++++---------- .../Process/Linux/NativeProcessLinux.cpp | 23 +++++---- .../Process/NetBSD/NativeProcessNetBSD.cpp | 25 +++++----- lldb/source/Utility/ArchSpec.cpp | 5 ++ lldb/unittests/Host/linux/HostTest.cpp | 4 ++ 9 files changed, 61 insertions(+), 76 deletions(-) diff --git a/lldb/include/lldb/Host/common/NativeProcessProtocol.h b/lldb/include/lldb/Host/common/NativeProcessProtocol.h index bd8b8744b115..b1738f44890e 100644 --- a/lldb/include/lldb/Host/common/NativeProcessProtocol.h +++ b/lldb/include/lldb/Host/common/NativeProcessProtocol.h @@ -466,11 +466,6 @@ protected: NativeThreadProtocol *GetThreadByIDUnlocked(lldb::tid_t tid); - // ----------------------------------------------------------- - // Static helper methods for derived classes. - // ----------------------------------------------------------- - static Status ResolveProcessArchitecture(lldb::pid_t pid, ArchSpec &arch); - private: void SynchronouslyNotifyProcessStateChanged(lldb::StateType state); }; diff --git a/lldb/include/lldb/Utility/ArchSpec.h b/lldb/include/lldb/Utility/ArchSpec.h index 50f69606e3df..1af3d79ac4cf 100644 --- a/lldb/include/lldb/Utility/ArchSpec.h +++ b/lldb/include/lldb/Utility/ArchSpec.h @@ -617,6 +617,7 @@ protected: /// @return true if \a lhs is less than \a rhs //------------------------------------------------------------------ bool operator<(const ArchSpec &lhs, const ArchSpec &rhs); +bool operator==(const ArchSpec &lhs, const ArchSpec &rhs); bool ParseMachCPUDashSubtypeTriple(llvm::StringRef triple_str, ArchSpec &arch); diff --git a/lldb/source/Host/CMakeLists.txt b/lldb/source/Host/CMakeLists.txt index 2b6f0e48a3f2..8aeb09314622 100644 --- a/lldb/source/Host/CMakeLists.txt +++ b/lldb/source/Host/CMakeLists.txt @@ -188,5 +188,6 @@ add_lldb_library(lldbHost ${EXTRA_LIBS} LINK_COMPONENTS + Object Support ) diff --git a/lldb/source/Host/common/NativeProcessProtocol.cpp b/lldb/source/Host/common/NativeProcessProtocol.cpp index 1fcb11b8b6f5..7367ea7c436f 100644 --- a/lldb/source/Host/common/NativeProcessProtocol.cpp +++ b/lldb/source/Host/common/NativeProcessProtocol.cpp @@ -8,13 +8,11 @@ //===----------------------------------------------------------------------===// #include "lldb/Host/common/NativeProcessProtocol.h" -#include "lldb/Core/ModuleSpec.h" #include "lldb/Core/State.h" #include "lldb/Host/Host.h" #include "lldb/Host/common/NativeRegisterContext.h" #include "lldb/Host/common/NativeThreadProtocol.h" #include "lldb/Host/common/SoftwareBreakpoint.h" -#include "lldb/Symbol/ObjectFile.h" #include "lldb/Target/Process.h" #include "lldb/Utility/LLDBAssert.h" #include "lldb/Utility/Log.h" @@ -431,26 +429,4 @@ void NativeProcessProtocol::DoStopIDBumped(uint32_t /* newBumpId */) { // Default implementation does nothing. } -Status NativeProcessProtocol::ResolveProcessArchitecture(lldb::pid_t pid, - ArchSpec &arch) { - // Grab process info for the running process. - ProcessInstanceInfo process_info; - if (!Host::GetProcessInfo(pid, process_info)) - return Status("failed to get process info"); - - // Resolve the executable module. - ModuleSpecList module_specs; - if (!ObjectFile::GetModuleSpecifications(process_info.GetExecutableFile(), 0, - 0, module_specs)) - return Status("failed to get module specifications"); - lldbassert(module_specs.GetSize() == 1); - - arch = module_specs.GetModuleSpecRefAtIndex(0).GetArchitecture(); - if (arch.IsValid()) - return Status(); - else - return Status( - "failed to retrieve a valid architecture from the exe module"); -} - NativeProcessProtocol::Factory::~Factory() = default; diff --git a/lldb/source/Host/linux/Host.cpp b/lldb/source/Host/linux/Host.cpp index 1edef7aceed8..a9e0d91a2287 100644 --- a/lldb/source/Host/linux/Host.cpp +++ b/lldb/source/Host/linux/Host.cpp @@ -20,7 +20,9 @@ // C++ Includes // Other libraries and framework includes +#include "llvm/Object/ELF.h" #include "llvm/Support/ScopedPrinter.h" + // Project includes #include "lldb/Target/Process.h" #include "lldb/Utility/Log.h" @@ -29,12 +31,9 @@ #include "lldb/Host/Host.h" #include "lldb/Host/HostInfo.h" #include "lldb/Host/linux/Support.h" -#include "lldb/Utility/DataBufferHeap.h" +#include "lldb/Utility/DataBufferLLVM.h" #include "lldb/Utility/DataExtractor.h" -#include "lldb/Core/ModuleSpec.h" -#include "lldb/Symbol/ObjectFile.h" - using namespace lldb; using namespace lldb_private; @@ -123,28 +122,27 @@ static bool IsDirNumeric(const char *dname) { return true; } -static bool GetELFProcessCPUType(llvm::StringRef exe_path, - ProcessInstanceInfo &process_info) { - // Clear the architecture. - process_info.GetArchitecture().Clear(); +static ArchSpec GetELFProcessCPUType(llvm::StringRef exe_path) { + Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST); - ModuleSpecList specs; - FileSpec filespec(exe_path, false); - const size_t num_specs = - ObjectFile::GetModuleSpecifications(filespec, 0, 0, specs); - // GetModuleSpecifications() could fail if the executable has been deleted or - // is locked. - // But it shouldn't return more than 1 architecture. - assert(num_specs <= 1 && "Linux plugin supports only a single architecture"); - if (num_specs == 1) { - ModuleSpec module_spec; - if (specs.GetModuleSpecAtIndex(0, module_spec) && - module_spec.GetArchitecture().IsValid()) { - process_info.GetArchitecture() = module_spec.GetArchitecture(); - return true; - } + auto buffer_sp = DataBufferLLVM::CreateSliceFromPath(exe_path, 0x20, 0); + if (!buffer_sp) + return ArchSpec(); + + uint8_t exe_class = + llvm::object::getElfArchType( + {buffer_sp->GetChars(), size_t(buffer_sp->GetByteSize())}) + .first; + + switch (exe_class) { + case llvm::ELF::ELFCLASS32: + return HostInfo::GetArchitecture(HostInfo::eArchKind32); + case llvm::ELF::ELFCLASS64: + return HostInfo::GetArchitecture(HostInfo::eArchKind64); + default: + LLDB_LOG(log, "Unknown elf class ({0}) in file {1}", exe_class, exe_path); + return ArchSpec(); } - return false; } static bool GetProcessAndStatInfo(::pid_t pid, @@ -173,7 +171,7 @@ static bool GetProcessAndStatInfo(::pid_t pid, llvm::StringRef PathRef = ExePath; PathRef.consume_back(" (deleted)"); - GetELFProcessCPUType(PathRef, process_info); + process_info.SetArchitecture(GetELFProcessCPUType(PathRef)); // Get the process environment. auto BufferOrError = getProcFile(pid, "environ"); @@ -193,7 +191,6 @@ static bool GetProcessAndStatInfo(::pid_t pid, process_info.SetProcessID(pid); process_info.GetExecutableFile().SetFile(PathRef, false); - process_info.GetArchitecture().MergeFrom(HostInfo::GetArchitecture()); llvm::StringRef Rest = Environ->getBuffer(); while (!Rest.empty()) { diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp index 136af361af29..2d33c9a72a88 100644 --- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp +++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp @@ -245,13 +245,15 @@ NativeProcessLinux::Factory::Launch(ProcessLaunchInfo &launch_info, } LLDB_LOG(log, "inferior started, now in stopped state"); - ArchSpec arch; - if ((status = ResolveProcessArchitecture(pid, arch)).Fail()) - return status.ToError(); + ProcessInstanceInfo Info; + if (!Host::GetProcessInfo(pid, Info)) { + return llvm::make_error("Cannot get process architecture", + llvm::inconvertibleErrorCode()); + } // Set the architecture to the exe architecture. LLDB_LOG(log, "pid = {0:x}, detected architecture {1}", pid, - arch.GetArchitectureName()); + Info.GetArchitecture().GetArchitectureName()); status = SetDefaultPtraceOpts(pid); if (status.Fail()) { @@ -261,7 +263,7 @@ NativeProcessLinux::Factory::Launch(ProcessLaunchInfo &launch_info, return std::unique_ptr(new NativeProcessLinux( pid, launch_info.GetPTY().ReleaseMasterFileDescriptor(), native_delegate, - arch, mainloop, {pid})); + Info.GetArchitecture(), mainloop, {pid})); } llvm::Expected> @@ -272,17 +274,18 @@ NativeProcessLinux::Factory::Attach( LLDB_LOG(log, "pid = {0:x}", pid); // Retrieve the architecture for the running process. - ArchSpec arch; - Status status = ResolveProcessArchitecture(pid, arch); - if (!status.Success()) - return status.ToError(); + ProcessInstanceInfo Info; + if (!Host::GetProcessInfo(pid, Info)) { + return llvm::make_error("Cannot get process architecture", + llvm::inconvertibleErrorCode()); + } auto tids_or = NativeProcessLinux::Attach(pid); if (!tids_or) return tids_or.takeError(); return std::unique_ptr(new NativeProcessLinux( - pid, -1, native_delegate, arch, mainloop, *tids_or)); + pid, -1, native_delegate, Info.GetArchitecture(), mainloop, *tids_or)); } // ----------------------------------------------------------------------------- diff --git a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp index a72edd837690..76993d9a12f5 100644 --- a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp +++ b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp @@ -93,17 +93,19 @@ NativeProcessNetBSD::Factory::Launch(ProcessLaunchInfo &launch_info, } LLDB_LOG(log, "inferior started, now in stopped state"); - ArchSpec arch; - if ((status = ResolveProcessArchitecture(pid, arch)).Fail()) - return status.ToError(); + ProcessInstanceInfo Info; + if (!Host::GetProcessInfo(pid, Info)) { + return llvm::make_error("Cannot get process architecture", + llvm::inconvertibleErrorCode()); + } // Set the architecture to the exe architecture. LLDB_LOG(log, "pid = {0:x}, detected architecture {1}", pid, - arch.GetArchitectureName()); + Info.GetArchitecture().GetArchitectureName()); std::unique_ptr process_up(new NativeProcessNetBSD( pid, launch_info.GetPTY().ReleaseMasterFileDescriptor(), native_delegate, - arch, mainloop)); + Info.GetArchitecture(), mainloop)); status = process_up->ReinitializeThreads(); if (status.Fail()) @@ -124,13 +126,14 @@ NativeProcessNetBSD::Factory::Attach( LLDB_LOG(log, "pid = {0:x}", pid); // Retrieve the architecture for the running process. - ArchSpec arch; - Status status = ResolveProcessArchitecture(pid, arch); - if (!status.Success()) - return status.ToError(); + ProcessInstanceInfo Info; + if (!Host::GetProcessInfo(pid, Info)) { + return llvm::make_error("Cannot get process architecture", + llvm::inconvertibleErrorCode()); + } - std::unique_ptr process_up( - new NativeProcessNetBSD(pid, -1, native_delegate, arch, mainloop)); + std::unique_ptr process_up(new NativeProcessNetBSD( + pid, -1, native_delegate, Info.GetArchitecture(), mainloop)); status = process_up->Attach(); if (!status.Success()) diff --git a/lldb/source/Utility/ArchSpec.cpp b/lldb/source/Utility/ArchSpec.cpp index ed236f5f6414..77395200b12e 100644 --- a/lldb/source/Utility/ArchSpec.cpp +++ b/lldb/source/Utility/ArchSpec.cpp @@ -1416,6 +1416,11 @@ bool lldb_private::operator<(const ArchSpec &lhs, const ArchSpec &rhs) { return lhs_core < rhs_core; } + +bool lldb_private::operator==(const ArchSpec &lhs, const ArchSpec &rhs) { + return lhs.GetCore() == rhs.GetCore(); +} + bool ArchSpec::IsFullySpecifiedTriple() const { const auto &user_specified_triple = GetTriple(); diff --git a/lldb/unittests/Host/linux/HostTest.cpp b/lldb/unittests/Host/linux/HostTest.cpp index 36d3fd206dac..7ea702d8b18a 100644 --- a/lldb/unittests/Host/linux/HostTest.cpp +++ b/lldb/unittests/Host/linux/HostTest.cpp @@ -45,4 +45,8 @@ TEST_F(HostTest, GetProcessInfo) { ASSERT_TRUE(Info.GroupIDIsValid()); EXPECT_EQ(getegid(), Info.GetGroupID()); + + EXPECT_TRUE(Info.GetArchitecture().IsValid()); + EXPECT_EQ(HostInfo::GetArchitecture(HostInfo::eArchKindDefault), + Info.GetArchitecture()); }