[Lldb-commits] [lldb] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and ELFLinuxPrStatus (PR #104109)

Fred Grim via lldb-commits lldb-commits at lists.llvm.org
Thu Aug 15 11:29:51 PDT 2024


https://github.com/feg208 updated https://github.com/llvm/llvm-project/pull/104109

>From 8f2f84294ea3875c88ce36a4980fd3a3afce01de Mon Sep 17 00:00:00 2001
From: Fred Grim <fgrim at apple.com>
Date: Tue, 18 Jun 2024 10:38:09 -0700
Subject: [PATCH 1/6] [lldb] Add Populate Methods for ELFLinuxPrPsInfo and
 ELFLinuxPrStatus

To create elf core files there are multiple notes in the core file that
contain these structs as the note. These populate methods take a Process
and produce fully specified structures that can be used to fill these
note sections. The pr also adds tests to ensure these structs are
correctly populated.
---
 .../Process/elf-core/ThreadElfCore.cpp        | 134 +++++++++++++++
 .../Plugins/Process/elf-core/ThreadElfCore.h  |   7 +
 lldb/unittests/Process/CMakeLists.txt         |   1 +
 .../unittests/Process/elf-core/CMakeLists.txt |  15 ++
 .../Process/elf-core/ThreadElfCoreTest.cpp    | 162 ++++++++++++++++++
 5 files changed, 319 insertions(+)
 create mode 100644 lldb/unittests/Process/elf-core/CMakeLists.txt
 create mode 100644 lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp

diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
index 2a83163884e168..fdb4a5837cd462 100644
--- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -318,6 +318,32 @@ Status ELFLinuxPrStatus::Parse(const DataExtractor &data,
   return error;
 }
 
+static struct compat_timeval
+copy_timespecs(const ProcessInstanceInfo::timespec &oth) {
+  using sec_t = decltype(compat_timeval::tv_sec);
+  using usec_t = decltype(compat_timeval::tv_usec);
+  return {static_cast<sec_t>(oth.tv_sec), static_cast<usec_t>(oth.tv_usec)};
+}
+
+std::optional<ELFLinuxPrStatus>
+ELFLinuxPrStatus::Populate(const lldb::ThreadSP &thread_sp) {
+  ELFLinuxPrStatus prstatus{};
+  prstatus.pr_pid = thread_sp->GetID();
+  lldb::ProcessSP process_sp = thread_sp->GetProcess();
+  ProcessInstanceInfo info;
+  if (!process_sp->GetProcessInfo(info)) {
+    return std::nullopt;
+  }
+  prstatus.pr_ppid = info.GetParentProcessID();
+  prstatus.pr_pgrp = info.GetProcessGroupID();
+  prstatus.pr_sid = info.GetProcessSessionID();
+  prstatus.pr_utime = copy_timespecs(info.GetUserTime());
+  prstatus.pr_stime = copy_timespecs(info.GetSystemTime());
+  prstatus.pr_cutime = copy_timespecs(info.GetCumulativeUserTime());
+  prstatus.pr_cstime = copy_timespecs(info.GetCumulativeSystemTime());
+  return prstatus;
+}
+
 // Parse PRPSINFO from NOTE entry
 ELFLinuxPrPsInfo::ELFLinuxPrPsInfo() {
   memset(this, 0, sizeof(ELFLinuxPrPsInfo));
@@ -394,6 +420,114 @@ Status ELFLinuxPrPsInfo::Parse(const DataExtractor &data,
   return error;
 }
 
+std::optional<ELFLinuxPrPsInfo>
+ELFLinuxPrPsInfo::Populate(const lldb::ProcessSP &process_sp) {
+  ELFLinuxPrPsInfo prpsinfo{};
+  prpsinfo.pr_pid = process_sp->GetID();
+  ProcessInstanceInfo info;
+  if (!process_sp->GetProcessInfo(info)) {
+    return std::nullopt;
+  }
+  prpsinfo.pr_nice = info.GetPriorityValue().value_or(0);
+  prpsinfo.pr_zomb = 0;
+  if (auto zombie_opt = info.IsZombie(); zombie_opt.value_or(false)) {
+    prpsinfo.pr_zomb = 1;
+  }
+  /**
+   * In the linux kernel this comes from:
+   * state = READ_ONCE(p->__state);
+   * i = state ? ffz(~state) + 1 : 0;
+   * psinfo->pr_sname = (i > 5) ? '.' : "RSDTZW"[i];
+   *
+   * So we replicate that here. From proc_pid_stats(5)
+   * R = Running
+   * S = Sleeping on uninterrutible wait
+   * D = Waiting on uninterruptable disk sleep
+   * T = Tracing stop
+   * Z = Zombie
+   * W = Paging
+   */
+  lldb::StateType process_state = process_sp->GetState();
+  switch (process_state) {
+  case lldb::StateType::eStateSuspended:
+    prpsinfo.pr_sname = 'S';
+    prpsinfo.pr_state = 1;
+    break;
+  case lldb::StateType::eStateStopped:
+    [[fallthrough]];
+  case lldb::StateType::eStateStepping:
+    prpsinfo.pr_sname = 'T';
+    prpsinfo.pr_state = 3;
+    break;
+  case lldb::StateType::eStateUnloaded:
+    [[fallthrough]];
+  case lldb::StateType::eStateRunning:
+    prpsinfo.pr_sname = 'R';
+    prpsinfo.pr_state = 0;
+    break;
+  default:
+    break;
+  }
+
+  /**
+   * pr_flags is left as 0. The values (in linux) are specific
+   * to the kernel. We recover them from the proc filesystem
+   * but don't put them in ProcessInfo because it would really
+   * become very linux specific and the utility here seems pretty
+   * dubious
+   */
+
+  if (info.EffectiveUserIDIsValid()) {
+    prpsinfo.pr_uid = info.GetUserID();
+  }
+  if (info.EffectiveGroupIDIsValid()) {
+    prpsinfo.pr_gid = info.GetGroupID();
+  }
+
+  if (info.ParentProcessIDIsValid()) {
+    prpsinfo.pr_ppid = info.GetParentProcessID();
+  }
+
+  if (info.ProcessGroupIDIsValid()) {
+    prpsinfo.pr_pgrp = info.GetProcessGroupID();
+  }
+
+  if (info.ProcessSessionIDIsValid()) {
+    prpsinfo.pr_sid = info.GetProcessSessionID();
+  }
+  constexpr size_t fname_len = std::extent_v<decltype(prpsinfo.pr_fname)>;
+  static_assert(fname_len > 0, "This should always be non zero");
+  const llvm::StringRef fname = info.GetNameAsStringRef();
+  auto fname_begin = fname.begin();
+  std::copy(fname_begin,
+            std::next(fname_begin, std::min(fname_len, fname.size())),
+            prpsinfo.pr_fname);
+  prpsinfo.pr_fname[fname_len - 1] = '\0';
+  auto args = info.GetArguments();
+  if (!args.empty()) {
+    constexpr size_t psargs_len = std::extent_v<decltype(prpsinfo.pr_psargs)>;
+    static_assert(psargs_len > 0, "This should always be non zero");
+    size_t i = psargs_len;
+    auto argentry_iterator = std::begin(args);
+    char *psargs = prpsinfo.pr_psargs;
+    while (i > 0 && argentry_iterator != args.end()) {
+      llvm::StringRef argentry = argentry_iterator->ref();
+      size_t len = std::min(i, argentry.size());
+      auto arg_iterator = std::begin(argentry);
+      std::copy(arg_iterator, std::next(arg_iterator, len), psargs);
+      i -= len;
+      psargs += len;
+      if (i > 0) {
+        *(psargs++) = ' ';
+        --i;
+      }
+      ++argentry_iterator;
+    }
+    prpsinfo.pr_psargs[psargs_len - 1] = '\0';
+  }
+  return prpsinfo;
+}
+
 // Parse SIGINFO from NOTE entry
 ELFLinuxSigInfo::ELFLinuxSigInfo() { memset(this, 0, sizeof(ELFLinuxSigInfo)); }
 
diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h
index 2f3ed2a017790a..31602e356850ee 100644
--- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h
+++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h
@@ -13,6 +13,7 @@
 #include "lldb/Target/Thread.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "llvm/ADT/DenseMap.h"
+#include <optional>
 #include <string>
 
 struct compat_timeval {
@@ -56,6 +57,9 @@ struct ELFLinuxPrStatus {
   lldb_private::Status Parse(const lldb_private::DataExtractor &data,
                              const lldb_private::ArchSpec &arch);
 
+  static std::optional<ELFLinuxPrStatus>
+  Populate(const lldb::ThreadSP &thread_sp);
+
   // Return the bytesize of the structure
   // 64 bit - just sizeof
   // 32 bit - hardcoded because we are reusing the struct, but some of the
@@ -112,6 +116,9 @@ struct ELFLinuxPrPsInfo {
   lldb_private::Status Parse(const lldb_private::DataExtractor &data,
                              const lldb_private::ArchSpec &arch);
 
+  static std::optional<ELFLinuxPrPsInfo>
+  Populate(const lldb::ProcessSP &process_sp);
+
   // Return the bytesize of the structure
   // 64 bit - just sizeof
   // 32 bit - hardcoded because we are reusing the struct, but some of the
diff --git a/lldb/unittests/Process/CMakeLists.txt b/lldb/unittests/Process/CMakeLists.txt
index 5fbecfcfa25ebe..d3b37e006fd899 100644
--- a/lldb/unittests/Process/CMakeLists.txt
+++ b/lldb/unittests/Process/CMakeLists.txt
@@ -1,5 +1,6 @@
 add_subdirectory(gdb-remote)
 if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
+  add_subdirectory(elf-core)
   add_subdirectory(Linux)
   add_subdirectory(POSIX)
 endif()
diff --git a/lldb/unittests/Process/elf-core/CMakeLists.txt b/lldb/unittests/Process/elf-core/CMakeLists.txt
new file mode 100644
index 00000000000000..b852a3ffb863c1
--- /dev/null
+++ b/lldb/unittests/Process/elf-core/CMakeLists.txt
@@ -0,0 +1,15 @@
+add_lldb_unittest(ProcessElfCoreTests
+  ThreadElfCoreTest.cpp
+
+  LINK_LIBS
+    lldbCore
+    lldbHost
+    lldbUtilityHelpers
+    lldbPluginProcessElfCore
+    lldbPluginPlatformLinux
+
+    LLVMTestingSupport
+
+  LINK_COMPONENTS
+    Support
+  )
diff --git a/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp b/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
new file mode 100644
index 00000000000000..6068c161ba1091
--- /dev/null
+++ b/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
@@ -0,0 +1,162 @@
+//===-- ThreadElfCoreTest.cpp ------------------------------------*- C++
+//-*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+#include "Plugins/Process/elf-core/ThreadElfCore.h"
+#include "Plugins/Platform/Linux/PlatformLinux.h"
+#include "TestingSupport/TestUtilities.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
+#include "lldb/Target/Thread.h"
+#include "lldb/Utility/Listener.h"
+#include "llvm/TargetParser/Triple.h"
+#include "gtest/gtest.h"
+
+#include <iostream>
+#include <memory>
+#include <mutex>
+#include <unistd.h>
+
+using namespace lldb_private;
+
+namespace {
+
+struct ElfCoreTest : public testing::Test {
+  static void SetUpTestCase() {
+    FileSystem::Initialize();
+    HostInfo::Initialize();
+    platform_linux::PlatformLinux::Initialize();
+    std::call_once(TestUtilities::g_debugger_initialize_flag,
+                   []() { Debugger::Initialize(nullptr); });
+  }
+  static void TearDownTestCase() {
+    platform_linux::PlatformLinux::Terminate();
+    HostInfo::Terminate();
+    FileSystem::Terminate();
+  }
+};
+
+struct DummyProcess : public Process {
+  DummyProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp)
+      : Process(target_sp, listener_sp) {
+    SetID(getpid());
+  }
+
+  bool CanDebug(lldb::TargetSP target, bool plugin_specified_by_name) override {
+    return true;
+  }
+  Status DoDestroy() override { return {}; }
+  void RefreshStateAfterStop() override {}
+  size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size,
+                      Status &error) override {
+    return 0;
+  }
+  bool DoUpdateThreadList(ThreadList &old_thread_list,
+                          ThreadList &new_thread_list) override {
+    return false;
+  }
+  llvm::StringRef GetPluginName() override { return "Dummy"; }
+};
+
+struct DummyThread : public Thread {
+  using Thread::Thread;
+
+  ~DummyThread() override { DestroyThread(); }
+
+  void RefreshStateAfterStop() override {}
+
+  lldb::RegisterContextSP GetRegisterContext() override { return nullptr; }
+
+  lldb::RegisterContextSP
+  CreateRegisterContextForFrame(StackFrame *frame) override {
+    return nullptr;
+  }
+
+  bool CalculateStopInfo() override { return false; }
+};
+
+lldb::TargetSP CreateTarget(lldb::DebuggerSP &debugger_sp, ArchSpec &arch) {
+  lldb::PlatformSP platform_sp;
+  lldb::TargetSP target_sp;
+  debugger_sp->GetTargetList().CreateTarget(
+      *debugger_sp, "", arch, eLoadDependentsNo, platform_sp, target_sp);
+  return target_sp;
+}
+
+lldb::ThreadSP CreateThread(lldb::ProcessSP &process_sp) {
+  lldb::ThreadSP thread_sp =
+      std::make_shared<DummyThread>(*process_sp.get(), gettid());
+  if (thread_sp == nullptr) {
+    return nullptr;
+  }
+  process_sp->GetThreadList().AddThread(thread_sp);
+
+  return thread_sp;
+}
+
+} // namespace
+
+TEST_F(ElfCoreTest, PopulatePrpsInfoTest) {
+  ArchSpec arch{HostInfo::GetTargetTriple()};
+  lldb::DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);
+
+  lldb::TargetSP target_sp = CreateTarget(debugger_sp, arch);
+  ASSERT_TRUE(target_sp);
+
+  lldb::ListenerSP listener_sp(Listener::MakeListener("dummy"));
+  lldb::ProcessSP process_sp =
+      std::make_shared<DummyProcess>(target_sp, listener_sp);
+  ASSERT_TRUE(process_sp);
+  auto prpsinfo_opt = ELFLinuxPrPsInfo::Populate(process_sp);
+  ASSERT_TRUE(prpsinfo_opt.has_value());
+  ASSERT_EQ(prpsinfo_opt->pr_pid, getpid());
+  ASSERT_EQ(prpsinfo_opt->pr_state, 0);
+  ASSERT_EQ(prpsinfo_opt->pr_sname, 'R');
+  ASSERT_EQ(prpsinfo_opt->pr_zomb, 0);
+  ASSERT_EQ(prpsinfo_opt->pr_nice, 0);
+  ASSERT_EQ(prpsinfo_opt->pr_flag, 0UL);
+  ASSERT_EQ(prpsinfo_opt->pr_uid, getuid());
+  ASSERT_EQ(prpsinfo_opt->pr_gid, getgid());
+  ASSERT_EQ(prpsinfo_opt->pr_pid, getpid());
+  ASSERT_EQ(prpsinfo_opt->pr_ppid, getppid());
+  ASSERT_EQ(prpsinfo_opt->pr_pgrp, getpgrp());
+  ASSERT_EQ(prpsinfo_opt->pr_sid, getsid(getpid()));
+  ASSERT_EQ(std::string{prpsinfo_opt->pr_fname}, "ProcessElfCoreT");
+  ASSERT_TRUE(std::string{prpsinfo_opt->pr_psargs}.empty());
+}
+
+TEST_F(ElfCoreTest, PopulatePrStatusTest) {
+  ArchSpec arch{HostInfo::GetTargetTriple()};
+  lldb::DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);
+
+  lldb::TargetSP target_sp = CreateTarget(debugger_sp, arch);
+  ASSERT_TRUE(target_sp);
+
+  lldb::ListenerSP listener_sp(Listener::MakeListener("dummy"));
+  lldb::ProcessSP process_sp =
+      std::make_shared<DummyProcess>(target_sp, listener_sp);
+  ASSERT_TRUE(process_sp);
+  lldb::ThreadSP thread_sp = CreateThread(process_sp);
+  ASSERT_TRUE(thread_sp);
+  auto prstatus_opt = ELFLinuxPrStatus::Populate(thread_sp);
+  ASSERT_TRUE(prstatus_opt.has_value());
+  ASSERT_EQ(prstatus_opt->si_signo, 0);
+  ASSERT_EQ(prstatus_opt->si_code, 0);
+  ASSERT_EQ(prstatus_opt->si_errno, 0);
+  ASSERT_EQ(prstatus_opt->pr_cursig, 0);
+  ASSERT_EQ(prstatus_opt->pr_sigpend, 0UL);
+  ASSERT_EQ(prstatus_opt->pr_sighold, 0UL);
+  ASSERT_EQ(prstatus_opt->pr_pid, static_cast<uint32_t>(gettid()));
+  ASSERT_EQ(prstatus_opt->pr_ppid, static_cast<uint32_t>(getppid()));
+  ASSERT_EQ(prstatus_opt->pr_pgrp, static_cast<uint32_t>(getpgrp()));
+  ASSERT_EQ(prstatus_opt->pr_sid, static_cast<uint32_t>(getsid(gettid())));
+}

>From db97ae5d9c5d128179651cd9bb156ae0fc67d7fa Mon Sep 17 00:00:00 2001
From: Fred Grim <fgrim at apple.com>
Date: Wed, 14 Aug 2024 14:46:55 -0700
Subject: [PATCH 2/6] addressed initial review comments

---
 .../Process/elf-core/ThreadElfCore.cpp        | 37 ++++++++-----------
 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
index fdb4a5837cd462..51435995021a5e 100644
--- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -499,32 +499,25 @@ ELFLinuxPrPsInfo::Populate(const lldb::ProcessSP &process_sp) {
   static_assert(fname_len > 0, "This should always be non zero");
   const llvm::StringRef fname = info.GetNameAsStringRef();
   auto fname_begin = fname.begin();
-  std::copy(fname_begin,
-            std::next(fname_begin, std::min(fname_len, fname.size())),
-            prpsinfo.pr_fname);
+  std::copy_n(fname_begin, std::min(fname_len, fname.size()),
+              prpsinfo.pr_fname);
   prpsinfo.pr_fname[fname_len - 1] = '\0';
   auto args = info.GetArguments();
-  if (!args.empty()) {
-    constexpr size_t psargs_len = std::extent_v<decltype(prpsinfo.pr_psargs)>;
-    static_assert(psargs_len > 0, "This should always be non zero");
-    size_t i = psargs_len;
-    auto argentry_iterator = std::begin(args);
-    char *psargs = prpsinfo.pr_psargs;
-    while (i > 0 && argentry_iterator != args.end()) {
-      llvm::StringRef argentry = argentry_iterator->ref();
-      size_t len = std::min(i, argentry.size());
-      auto arg_iterator = std::begin(argentry);
-      std::copy(arg_iterator, std::next(arg_iterator, len), psargs);
-      i -= len;
-      psargs += len;
-      if (i > 0) {
-        *(psargs++) = ' ';
-        --i;
-      }
-      ++argentry_iterator;
+  auto argentry_iterator = std::begin(args);
+  char *psargs = prpsinfo.pr_psargs;
+  char *psargs_end = std::end(prpsinfo.pr_psargs);
+  while (psargs < psargs_end && argentry_iterator != args.end()) {
+    llvm::StringRef argentry = argentry_iterator->ref();
+    size_t len =
+        std::min<size_t>(std::distance(psargs, psargs_end), argentry.size());
+    auto arg_iterator = std::begin(argentry);
+    psargs = std::copy_n(arg_iterator, len, psargs);
+    if (psargs != psargs_end) {
+      *(psargs++) = ' ';
     }
-    prpsinfo.pr_psargs[psargs_len - 1] = '\0';
+    ++argentry_iterator;
   }
+  *(psargs_end - 1) = '\0';
   return prpsinfo;
 }
 

>From 0655878b6ce7c916fa3e1370bf4eee1e9c9a0ad1 Mon Sep 17 00:00:00 2001
From: Fred Grim <fgrim at apple.com>
Date: Thu, 15 Aug 2024 10:21:56 -0700
Subject: [PATCH 3/6] Refines the Populate interface and adds more tests

---
 .../Plugins/Process/elf-core/ThreadElfCore.cpp     | 14 ++++++++++----
 .../Plugins/Process/elf-core/ThreadElfCore.h       |  8 ++++++++
 .../Process/elf-core/ThreadElfCoreTest.cpp         | 13 +++++++++++++
 3 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
index 51435995021a5e..f9a08ddc8581bc 100644
--- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -13,6 +13,7 @@
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
+#include "lldb/Utility/ProcessInfo.h"
 
 #include "Plugins/Process/Utility/RegisterContextFreeBSD_i386.h"
 #include "Plugins/Process/Utility/RegisterContextFreeBSD_mips64.h"
@@ -422,12 +423,18 @@ Status ELFLinuxPrPsInfo::Parse(const DataExtractor &data,
 
 std::optional<ELFLinuxPrPsInfo>
 ELFLinuxPrPsInfo::Populate(const lldb::ProcessSP &process_sp) {
-  ELFLinuxPrPsInfo prpsinfo{};
-  prpsinfo.pr_pid = process_sp->GetID();
   ProcessInstanceInfo info;
   if (!process_sp->GetProcessInfo(info)) {
     return std::nullopt;
   }
+  return Populate(info, process_sp->GetState());
+}
+
+std::optional<ELFLinuxPrPsInfo>
+ELFLinuxPrPsInfo::Populate(const lldb_private::ProcessInstanceInfo &info,
+                           lldb::StateType process_state) {
+  ELFLinuxPrPsInfo prpsinfo{};
+  prpsinfo.pr_pid = info.GetProcessID();
   prpsinfo.pr_nice = info.GetPriorityValue().value_or(0);
   prpsinfo.pr_zomb = 0;
   if (auto zombie_opt = info.IsZombie(); zombie_opt.value_or(false)) {
@@ -447,7 +454,6 @@ ELFLinuxPrPsInfo::Populate(const lldb::ProcessSP &process_sp) {
    * Z = Zombie
    * W = Paging
    */
-  lldb::StateType process_state = process_sp->GetState();
   switch (process_state) {
   case lldb::StateType::eStateSuspended:
     prpsinfo.pr_sname = 'S';
@@ -517,7 +523,7 @@ ELFLinuxPrPsInfo::Populate(const lldb::ProcessSP &process_sp) {
     }
     ++argentry_iterator;
   }
-  *(psargs_end - 1) = '\0';
+  *(psargs - 1) = '\0';
   return prpsinfo;
 }
 
diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h
index 31602e356850ee..3fa0b8b0eedb0b 100644
--- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h
+++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.h
@@ -21,6 +21,10 @@ struct compat_timeval {
   alignas(8) uint64_t tv_usec;
 };
 
+namespace lldb_private {
+class ProcessInstanceInfo;
+}
+
 // PRSTATUS structure's size differs based on architecture.
 // This is the layout in the x86-64 arch.
 // In the i386 case we parse it manually and fill it again
@@ -119,6 +123,10 @@ struct ELFLinuxPrPsInfo {
   static std::optional<ELFLinuxPrPsInfo>
   Populate(const lldb::ProcessSP &process_sp);
 
+  static std::optional<ELFLinuxPrPsInfo>
+  Populate(const lldb_private::ProcessInstanceInfo &info,
+           lldb::StateType state);
+
   // Return the bytesize of the structure
   // 64 bit - just sizeof
   // 32 bit - hardcoded because we are reusing the struct, but some of the
diff --git a/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp b/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
index 6068c161ba1091..ed08c5695b57bd 100644
--- a/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
+++ b/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
@@ -131,6 +131,19 @@ TEST_F(ElfCoreTest, PopulatePrpsInfoTest) {
   ASSERT_EQ(prpsinfo_opt->pr_sid, getsid(getpid()));
   ASSERT_EQ(std::string{prpsinfo_opt->pr_fname}, "ProcessElfCoreT");
   ASSERT_TRUE(std::string{prpsinfo_opt->pr_psargs}.empty());
+  lldb_private::ProcessInstanceInfo info;
+  ASSERT_TRUE(process_sp->GetProcessInfo(info));
+  const char *args[] = {"a.out", "--foo=bar", "--baz=boo", nullptr};
+  info.SetArguments(args, true);
+  prpsinfo_opt =
+      ELFLinuxPrPsInfo::Populate(info, lldb::StateType::eStateStopped);
+  ASSERT_TRUE(prpsinfo_opt.has_value());
+  ASSERT_EQ(prpsinfo_opt->pr_pid, getpid());
+  ASSERT_EQ(prpsinfo_opt->pr_state, 3);
+  ASSERT_EQ(prpsinfo_opt->pr_sname, 'T');
+  ASSERT_EQ(std::string{prpsinfo_opt->pr_fname}, "a.out");
+  ASSERT_FALSE(std::string{prpsinfo_opt->pr_psargs}.empty());
+  ASSERT_EQ(std::string{prpsinfo_opt->pr_psargs}, "a.out --foo=bar --baz=boo");
 }
 
 TEST_F(ElfCoreTest, PopulatePrStatusTest) {

>From 809efb3f084c068577877e6556adff54e383a257 Mon Sep 17 00:00:00 2001
From: Fred Grim <fgrim at apple.com>
Date: Thu, 15 Aug 2024 10:45:07 -0700
Subject: [PATCH 4/6] Conform to llvm coding guidelines

---
 .../Process/elf-core/ThreadElfCore.cpp        | 21 ++++++++-----------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
index f9a08ddc8581bc..728de534b56dfa 100644
--- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -424,9 +424,9 @@ Status ELFLinuxPrPsInfo::Parse(const DataExtractor &data,
 std::optional<ELFLinuxPrPsInfo>
 ELFLinuxPrPsInfo::Populate(const lldb::ProcessSP &process_sp) {
   ProcessInstanceInfo info;
-  if (!process_sp->GetProcessInfo(info)) {
+  if (!process_sp->GetProcessInfo(info))
     return std::nullopt;
-  }
+
   return Populate(info, process_sp->GetState());
 }
 
@@ -483,24 +483,21 @@ ELFLinuxPrPsInfo::Populate(const lldb_private::ProcessInstanceInfo &info,
    * dubious
    */
 
-  if (info.EffectiveUserIDIsValid()) {
+  if (info.EffectiveUserIDIsValid())
     prpsinfo.pr_uid = info.GetUserID();
-  }
-  if (info.EffectiveGroupIDIsValid()) {
+
+  if (info.EffectiveGroupIDIsValid())
     prpsinfo.pr_gid = info.GetGroupID();
-  }
 
-  if (info.ParentProcessIDIsValid()) {
+  if (info.ParentProcessIDIsValid())
     prpsinfo.pr_ppid = info.GetParentProcessID();
-  }
 
-  if (info.ProcessGroupIDIsValid()) {
+  if (info.ProcessGroupIDIsValid())
     prpsinfo.pr_pgrp = info.GetProcessGroupID();
-  }
 
-  if (info.ProcessSessionIDIsValid()) {
+  if (info.ProcessSessionIDIsValid())
     prpsinfo.pr_sid = info.GetProcessSessionID();
-  }
+
   constexpr size_t fname_len = std::extent_v<decltype(prpsinfo.pr_fname)>;
   static_assert(fname_len > 0, "This should always be non zero");
   const llvm::StringRef fname = info.GetNameAsStringRef();

>From 379a842ff9ee1048ad23fb1ff974845d09dfb8db Mon Sep 17 00:00:00 2001
From: Fred Grim <fgrim at apple.com>
Date: Thu, 15 Aug 2024 10:47:17 -0700
Subject: [PATCH 5/6] left a stray bracket pair

---
 lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
index 728de534b56dfa..99e25063f909ed 100644
--- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -515,9 +515,8 @@ ELFLinuxPrPsInfo::Populate(const lldb_private::ProcessInstanceInfo &info,
         std::min<size_t>(std::distance(psargs, psargs_end), argentry.size());
     auto arg_iterator = std::begin(argentry);
     psargs = std::copy_n(arg_iterator, len, psargs);
-    if (psargs != psargs_end) {
+    if (psargs != psargs_end)
       *(psargs++) = ' ';
-    }
     ++argentry_iterator;
   }
   *(psargs - 1) = '\0';

>From 8be18b997564bf5c6a93521f6630fe473b69c636 Mon Sep 17 00:00:00 2001
From: Fred Grim <fgrim at apple.com>
Date: Thu, 15 Aug 2024 11:29:36 -0700
Subject: [PATCH 6/6] stray include

---
 lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp b/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
index ed08c5695b57bd..ce146f62b0d826 100644
--- a/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
+++ b/lldb/unittests/Process/elf-core/ThreadElfCoreTest.cpp
@@ -19,7 +19,6 @@
 #include "llvm/TargetParser/Triple.h"
 #include "gtest/gtest.h"
 
-#include <iostream>
 #include <memory>
 #include <mutex>
 #include <unistd.h>



More information about the lldb-commits mailing list