[Lldb-commits] [lldb] f893b29 - [lldb] [Host/{free, net}bsd] Fix process matching by name

2020-11-03 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2020-11-03T09:45:50+01:00
New Revision: f893b2939781910fd52ad70a30484255a3cf0f6e

URL: 
https://github.com/llvm/llvm-project/commit/f893b2939781910fd52ad70a30484255a3cf0f6e
DIFF: 
https://github.com/llvm/llvm-project/commit/f893b2939781910fd52ad70a30484255a3cf0f6e.diff

LOG: [lldb] [Host/{free,net}bsd] Fix process matching by name

Fix process matching by name to make 'process attach -n ...' work.

The process finding code has an optimization that defers getting
the process name and executable format after the numeric (PID, UID...)
parameters are tested.  However, the ProcessInstanceInfoMatch.Matches()
method has been matching process name against the incomplete process
information as well, and effectively no process ever matched.

In order to fix this, create a copy of ProcessInstanceInfoMatch, set
it to ignore process name and se this copy for the initial match.
The same fix applies to FreeBSD and NetBSD host code.

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

Added: 


Modified: 
lldb/source/Host/freebsd/Host.cpp
lldb/source/Host/netbsd/HostNetBSD.cpp

Removed: 




diff  --git a/lldb/source/Host/freebsd/Host.cpp 
b/lldb/source/Host/freebsd/Host.cpp
index 09547e48afa9..460a535cf1e0 100644
--- a/lldb/source/Host/freebsd/Host.cpp
+++ b/lldb/source/Host/freebsd/Host.cpp
@@ -175,6 +175,9 @@ uint32_t Host::FindProcessesImpl(const 
ProcessInstanceInfoMatch &match_info,
 
   const size_t actual_pid_count = (pid_data_size / sizeof(struct kinfo_proc));
 
+  ProcessInstanceInfoMatch match_info_noname{match_info};
+  match_info_noname.SetNameMatchType(NameMatch::Ignore);
+
   for (size_t i = 0; i < actual_pid_count; i++) {
 const struct kinfo_proc &kinfo = kinfos[i];
 
@@ -212,7 +215,7 @@ uint32_t Host::FindProcessesImpl(const 
ProcessInstanceInfoMatch &match_info,
 process_info.SetEffectiveGroupID(kinfo.ki_svgid);
 
 // Make sure our info matches before we go fetch the name and cpu type
-if (match_info.Matches(process_info) &&
+if (match_info_noname.Matches(process_info) &&
 GetFreeBSDProcessArgs(&match_info, process_info)) {
   GetFreeBSDProcessCPUType(process_info);
   if (match_info.Matches(process_info))

diff  --git a/lldb/source/Host/netbsd/HostNetBSD.cpp 
b/lldb/source/Host/netbsd/HostNetBSD.cpp
index 38e2aa5c1e05..1945f9f9052f 100644
--- a/lldb/source/Host/netbsd/HostNetBSD.cpp
+++ b/lldb/source/Host/netbsd/HostNetBSD.cpp
@@ -200,6 +200,9 @@ uint32_t Host::FindProcessesImpl(const 
ProcessInstanceInfoMatch &match_info,
 return 0;
   }
 
+  ProcessInstanceInfoMatch match_info_noname{match_info};
+  match_info_noname.SetNameMatchType(NameMatch::Ignore);
+
   for (int i = 0; i < nproc; i++) {
 if (proc_kinfo[i].p_pid < 1)
   continue; /* not valid */
@@ -237,7 +240,7 @@ uint32_t Host::FindProcessesImpl(const 
ProcessInstanceInfoMatch &match_info,
 process_info.SetEffectiveUserID(proc_kinfo[i].p_uid);
 process_info.SetEffectiveGroupID(proc_kinfo[i].p_gid);
 // Make sure our info matches before we go fetch the name and cpu type
-if (match_info.Matches(process_info) &&
+if (match_info_noname.Matches(process_info) &&
 GetNetBSDProcessArgs(&match_info, process_info)) {
   GetNetBSDProcessCPUType(process_info);
   if (match_info.Matches(process_info))



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D90298: [lldb] [Process/FreeBSDRemote] Implement thread GetName()

2020-11-03 Thread Michał Górny via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG326d23530081: [lldb] [Process/FreeBSDRemote] Implement 
thread GetName() (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90298/new/

https://reviews.llvm.org/D90298

Files:
  lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h


Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
@@ -74,6 +74,7 @@
   using WatchpointIndexMap = std::map;
   WatchpointIndexMap m_watchpoint_index_map;
   WatchpointIndexMap m_hw_break_index_map;
+  llvm::Optional m_thread_name;
 };
 
 typedef std::shared_ptr NativeThreadFreeBSDSP;
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
@@ -22,9 +22,11 @@
 #include 
 #include 
 #include 
+#include 
 // clang-format on
 
 #include 
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
@@ -146,7 +148,43 @@
   m_stop_info.reason = StopReason::eStopReasonNone;
 }
 
-std::string NativeThreadFreeBSD::GetName() { return ""; }
+std::string NativeThreadFreeBSD::GetName() {
+  if (!m_thread_name) {
+Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_THREAD));
+
+std::vector kp;
+int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PID | KERN_PROC_INC_THREAD,
+  static_cast(GetProcess().GetID())};
+
+while (1) {
+  size_t len = kp.size() * sizeof(struct kinfo_proc);
+  void *ptr = len == 0 ? nullptr : kp.data();
+  int error = ::sysctl(mib, 4, ptr, &len, nullptr, 0);
+  if (ptr == nullptr || (error != 0 && errno == ENOMEM)) {
+kp.resize(len / sizeof(struct kinfo_proc));
+continue;
+  }
+  if (error != 0) {
+len = 0;
+LLDB_LOG(log, "tid = {0} in state {1} failed to get thread name: {2}", 
GetID(),
+ m_state, strerror(errno));
+  }
+  kp.resize(len / sizeof(struct kinfo_proc));
+  break;
+}
+
+// empty == unknown
+m_thread_name = std::string();
+for (auto& procinfo : kp) {
+  if (procinfo.ki_tid == (lwpid_t)GetID()) {
+m_thread_name = procinfo.ki_tdname;
+break;
+  }
+}
+  }
+
+  return m_thread_name.getValue();
+}
 
 lldb::StateType NativeThreadFreeBSD::GetState() { return m_state; }
 


Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
@@ -74,6 +74,7 @@
   using WatchpointIndexMap = std::map;
   WatchpointIndexMap m_watchpoint_index_map;
   WatchpointIndexMap m_hw_break_index_map;
+  llvm::Optional m_thread_name;
 };
 
 typedef std::shared_ptr NativeThreadFreeBSDSP;
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
@@ -22,9 +22,11 @@
 #include 
 #include 
 #include 
+#include 
 // clang-format on
 
 #include 
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
@@ -146,7 +148,43 @@
   m_stop_info.reason = StopReason::eStopReasonNone;
 }
 
-std::string NativeThreadFreeBSD::GetName() { return ""; }
+std::string NativeThreadFreeBSD::GetName() {
+  if (!m_thread_name) {
+Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_THREAD));
+
+std::vector kp;
+int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PID | KERN_PROC_INC_THREAD,
+  static_cast(GetProcess().GetID())};
+
+while (1) {
+  size_t len = kp.size() * sizeof(struct kinfo_proc);
+  void *ptr = len == 0 ? nullptr : kp.data();
+  int error = ::sysctl(mib, 4, ptr, &len, nullptr, 0);
+  if (ptr == nullptr || (error != 0 && errno == ENOMEM)) {
+kp.resize(len / sizeof(struct kinfo_proc));
+continue;
+  }
+  if (error != 0) {
+len = 0;
+LLDB_LOG(log, "tid = {0} in state {1} failed to get thread name: {2}", GetID(),
+ m_state, strerror(errno));
+  }
+  kp.resize(len / sizeof(struct kinfo_proc));
+  break;
+}
+
+// empty == unknown
+m_thread_name = std::string();
+for (auto& procinfo : kp) {
+  if (procinfo.ki_tid == (lwpid_t)GetID()) {
+m_thread_name = procinfo.

[Lldb-commits] [lldb] 8e6bcbb - [lldb] [Process/FreeBSDRemote] Fix attaching via lldb-server

2020-11-03 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2020-11-03T09:45:50+01:00
New Revision: 8e6bcbb41758b3a6c23f1b448fcef6f67fd8c6d0

URL: 
https://github.com/llvm/llvm-project/commit/8e6bcbb41758b3a6c23f1b448fcef6f67fd8c6d0
DIFF: 
https://github.com/llvm/llvm-project/commit/8e6bcbb41758b3a6c23f1b448fcef6f67fd8c6d0.diff

LOG: [lldb] [Process/FreeBSDRemote] Fix attaching via lldb-server

Fix two bugs that caused attaching to a process in a pre-connected
lldb-server to fail.  These are:

1. Prematurely reporting status in NativeProcessFreeBSD::Attach().
   The SetState() call defaulted to notify the process, and LLGS tried
   to send the stopped packet before the process instance was assigned
   to it.  While at it, add an assert for that in LLGS.

2. Duplicate call to ReinitializeThreads() (via SetupTrace()) that
   overwrote the stopped status in threads.  Now SetupTrace() is called
   directly by NativeProcessFreeBSD::Attach() (not the Factory) in place
   of ReinitializeThreads().

This fixes at least commands/process/attach/TestProcessAttach.py
and python_api/hello_world/TestHelloWorld.py.

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

Added: 


Modified: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
index 0a4e8ed6bcbf..a59fccb5cb65 100644
--- a/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
+++ b/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
@@ -698,8 +698,9 @@ Status NativeProcessFreeBSD::Attach() {
   0)
 return Status(errno, eErrorTypePOSIX);
 
-  /* Initialize threads */
-  status = ReinitializeThreads();
+  // Initialize threads and tracing status
+  // NB: this needs to be called before we set thread state
+  status = SetupTrace();
   if (status.Fail())
 return status;
 
@@ -707,7 +708,8 @@ Status NativeProcessFreeBSD::Attach() {
 static_cast(*thread).SetStoppedBySignal(SIGSTOP);
 
   // Let our process instance know the thread has stopped.
-  SetState(StateType::eStateStopped);
+  SetCurrentThreadID(m_threads.front()->GetID());
+  SetState(StateType::eStateStopped, false);
   return Status();
 }
 

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index d19becca823f..3b6f740a5983 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1728,6 +1728,7 @@ GDBRemoteCommunicationServerLLGS::SendStopReasonForState(
   case eStateSuspended:
   case eStateStopped:
   case eStateCrashed: {
+assert(m_debugged_process_up != nullptr);
 lldb::tid_t tid = m_debugged_process_up->GetCurrentThreadID();
 // Make sure we set the current thread so g and p packets return the data
 // the gdb will expect.



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 952ddc9 - [lldb] [Plugins/FreeBSDRemote] Disable GetMemoryRegionInfo()

2020-11-03 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2020-11-03T09:45:51+01:00
New Revision: 952ddc9866dc761e29757c262ce28ca0fa9431ed

URL: 
https://github.com/llvm/llvm-project/commit/952ddc9866dc761e29757c262ce28ca0fa9431ed
DIFF: 
https://github.com/llvm/llvm-project/commit/952ddc9866dc761e29757c262ce28ca0fa9431ed.diff

LOG: [lldb] [Plugins/FreeBSDRemote] Disable GetMemoryRegionInfo()

Disable GetMemoryRegionInfo() in order to unbreak expression parsing.
For some reason, the presence of non-stub function causes LLDB to fail
to detect system libraries correctly.  Through being unable to find
mmap() and allocate memory, this leads to expression parser being
broken.

The issue is non-trivial and it is going to require more time debugging.
On the other hand, the downsides of missing the function are minimal
(2 failing tests), and the benefit of working expression parser
justifies disabling it temporarily.  Furthermore, the old FreeBSD plugin
did not implement it anyway, so it allows us to switch to the new plugin
without major regressions.

The really curious part is that the respective code in the NetBSD plugin
yields very similar results, yet does not seem to break the expression
parser.

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

Added: 


Modified: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
index ec056b3a602c..2c22f1101597 100644
--- a/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
+++ b/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
@@ -447,6 +447,9 @@ Status NativeProcessFreeBSD::Kill() {
 Status NativeProcessFreeBSD::GetMemoryRegionInfo(lldb::addr_t load_addr,
  MemoryRegionInfo &range_info) 
{
 
+  // TODO: figure out why it breaks stuff
+  return Status("currently breaks determining module list");
+
   if (m_supports_mem_region == LazyBool::eLazyBoolNo) {
 // We're done.
 return Status("unsupported");



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 326d235 - [lldb] [Process/FreeBSDRemote] Implement thread GetName()

2020-11-03 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2020-11-03T09:45:49+01:00
New Revision: 326d235300814e2e38341580bdf04efd226ba83d

URL: 
https://github.com/llvm/llvm-project/commit/326d235300814e2e38341580bdf04efd226ba83d
DIFF: 
https://github.com/llvm/llvm-project/commit/326d235300814e2e38341580bdf04efd226ba83d.diff

LOG: [lldb] [Process/FreeBSDRemote] Implement thread GetName()

Implement NativeThreadFreeBSD::GetName().  This is based
on the equivalent code in the legacy FreeBSD plugin, except it is
modernized a bit to use llvm::Optional and std::vector for data storage.

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

Added: 


Modified: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
index c5f0edc64806..2e62b0a25ed2 100644
--- a/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
+++ b/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
@@ -22,9 +22,11 @@
 #include 
 #include 
 #include 
+#include 
 // clang-format on
 
 #include 
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
@@ -146,7 +148,43 @@ void NativeThreadFreeBSD::SetStepping() {
   m_stop_info.reason = StopReason::eStopReasonNone;
 }
 
-std::string NativeThreadFreeBSD::GetName() { return ""; }
+std::string NativeThreadFreeBSD::GetName() {
+  if (!m_thread_name) {
+Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_THREAD));
+
+std::vector kp;
+int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PID | KERN_PROC_INC_THREAD,
+  static_cast(GetProcess().GetID())};
+
+while (1) {
+  size_t len = kp.size() * sizeof(struct kinfo_proc);
+  void *ptr = len == 0 ? nullptr : kp.data();
+  int error = ::sysctl(mib, 4, ptr, &len, nullptr, 0);
+  if (ptr == nullptr || (error != 0 && errno == ENOMEM)) {
+kp.resize(len / sizeof(struct kinfo_proc));
+continue;
+  }
+  if (error != 0) {
+len = 0;
+LLDB_LOG(log, "tid = {0} in state {1} failed to get thread name: {2}", 
GetID(),
+ m_state, strerror(errno));
+  }
+  kp.resize(len / sizeof(struct kinfo_proc));
+  break;
+}
+
+// empty == unknown
+m_thread_name = std::string();
+for (auto& procinfo : kp) {
+  if (procinfo.ki_tid == (lwpid_t)GetID()) {
+m_thread_name = procinfo.ki_tdname;
+break;
+  }
+}
+  }
+
+  return m_thread_name.getValue();
+}
 
 lldb::StateType NativeThreadFreeBSD::GetState() { return m_state; }
 

diff  --git a/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
index e4d494174736..665e4ea48971 100644
--- a/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
+++ b/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
@@ -74,6 +74,7 @@ class NativeThreadFreeBSD : public NativeThreadProtocol {
   using WatchpointIndexMap = std::map;
   WatchpointIndexMap m_watchpoint_index_map;
   WatchpointIndexMap m_hw_break_index_map;
+  llvm::Optional m_thread_name;
 };
 
 typedef std::shared_ptr NativeThreadFreeBSDSP;



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 40d26bc - [lldb] [Process/FreeBSDRemote] Remove GetSharedLibraryInfoAddress override

2020-11-03 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2020-11-03T09:45:50+01:00
New Revision: 40d26bc4b1854c1e493c500a96a9c9d65a93da59

URL: 
https://github.com/llvm/llvm-project/commit/40d26bc4b1854c1e493c500a96a9c9d65a93da59
DIFF: 
https://github.com/llvm/llvm-project/commit/40d26bc4b1854c1e493c500a96a9c9d65a93da59.diff

LOG: [lldb] [Process/FreeBSDRemote] Remove GetSharedLibraryInfoAddress override

Remove the NetBSD-specific override of GetSharedLibraryInfoAddress(),
restoring the generic implementation from NativeProcessELF.

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

Added: 


Modified: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
index a59fccb5cb65..ec056b3a602c 100644
--- a/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
+++ b/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
@@ -580,11 +580,6 @@ Status NativeProcessFreeBSD::PopulateMemoryRegionCache() {
   return Status();
 }
 
-lldb::addr_t NativeProcessFreeBSD::GetSharedLibraryInfoAddress() {
-  // punt on this for now
-  return LLDB_INVALID_ADDRESS;
-}
-
 size_t NativeProcessFreeBSD::UpdateThreads() { return m_threads.size(); }
 
 Status NativeProcessFreeBSD::SetBreakpoint(lldb::addr_t addr, uint32_t size,

diff  --git a/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h
index 5900048610f5..4b6e9af140d0 100644
--- a/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h
+++ b/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h
@@ -60,8 +60,6 @@ class NativeProcessFreeBSD : public NativeProcessELF {
   Status WriteMemory(lldb::addr_t addr, const void *buf, size_t size,
  size_t &bytes_written) override;
 
-  lldb::addr_t GetSharedLibraryInfoAddress() override;
-
   size_t UpdateThreads() override;
 
   const ArchSpec &GetArchitecture() const override { return m_arch; }



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D90620: [lldb] [Process/FreeBSDRemote] Remove GetSharedLibraryInfoAddress override

2020-11-03 Thread Michał Górny via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG40d26bc4b185: [lldb] [Process/FreeBSDRemote] Remove 
GetSharedLibraryInfoAddress override (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90620/new/

https://reviews.llvm.org/D90620

Files:
  lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h


Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h
@@ -60,8 +60,6 @@
   Status WriteMemory(lldb::addr_t addr, const void *buf, size_t size,
  size_t &bytes_written) override;
 
-  lldb::addr_t GetSharedLibraryInfoAddress() override;
-
   size_t UpdateThreads() override;
 
   const ArchSpec &GetArchitecture() const override { return m_arch; }
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
@@ -580,11 +580,6 @@
   return Status();
 }
 
-lldb::addr_t NativeProcessFreeBSD::GetSharedLibraryInfoAddress() {
-  // punt on this for now
-  return LLDB_INVALID_ADDRESS;
-}
-
 size_t NativeProcessFreeBSD::UpdateThreads() { return m_threads.size(); }
 
 Status NativeProcessFreeBSD::SetBreakpoint(lldb::addr_t addr, uint32_t size,


Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h
@@ -60,8 +60,6 @@
   Status WriteMemory(lldb::addr_t addr, const void *buf, size_t size,
  size_t &bytes_written) override;
 
-  lldb::addr_t GetSharedLibraryInfoAddress() override;
-
   size_t UpdateThreads() override;
 
   const ArchSpec &GetArchitecture() const override { return m_arch; }
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
@@ -580,11 +580,6 @@
   return Status();
 }
 
-lldb::addr_t NativeProcessFreeBSD::GetSharedLibraryInfoAddress() {
-  // punt on this for now
-  return LLDB_INVALID_ADDRESS;
-}
-
 size_t NativeProcessFreeBSD::UpdateThreads() { return m_threads.size(); }
 
 Status NativeProcessFreeBSD::SetBreakpoint(lldb::addr_t addr, uint32_t size,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D90650: [lldb] [Plugins/FreeBSDRemote] Disable GetMemoryRegionInfo()

2020-11-03 Thread Michał Górny via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG952ddc9866dc: [lldb] [Plugins/FreeBSDRemote] Disable 
GetMemoryRegionInfo() (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90650/new/

https://reviews.llvm.org/D90650

Files:
  lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp


Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
@@ -447,6 +447,9 @@
 Status NativeProcessFreeBSD::GetMemoryRegionInfo(lldb::addr_t load_addr,
  MemoryRegionInfo &range_info) 
{
 
+  // TODO: figure out why it breaks stuff
+  return Status("currently breaks determining module list");
+
   if (m_supports_mem_region == LazyBool::eLazyBoolNo) {
 // We're done.
 return Status("unsupported");


Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
@@ -447,6 +447,9 @@
 Status NativeProcessFreeBSD::GetMemoryRegionInfo(lldb::addr_t load_addr,
  MemoryRegionInfo &range_info) {
 
+  // TODO: figure out why it breaks stuff
+  return Status("currently breaks determining module list");
+
   if (m_supports_mem_region == LazyBool::eLazyBoolNo) {
 // We're done.
 return Status("unsupported");
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D90525: [lldb] [Process/FreeBSDRemote] Fix attaching via lldb-server

2020-11-03 Thread Michał Górny via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8e6bcbb41758: [lldb] [Process/FreeBSDRemote] Fix attaching 
via lldb-server (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D90525?vs=302076&id=302493#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90525/new/

https://reviews.llvm.org/D90525

Files:
  lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp


Index: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1728,6 +1728,7 @@
   case eStateSuspended:
   case eStateStopped:
   case eStateCrashed: {
+assert(m_debugged_process_up != nullptr);
 lldb::tid_t tid = m_debugged_process_up->GetCurrentThreadID();
 // Make sure we set the current thread so g and p packets return the data
 // the gdb will expect.
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
@@ -698,8 +698,9 @@
   0)
 return Status(errno, eErrorTypePOSIX);
 
-  /* Initialize threads */
-  status = ReinitializeThreads();
+  // Initialize threads and tracing status
+  // NB: this needs to be called before we set thread state
+  status = SetupTrace();
   if (status.Fail())
 return status;
 
@@ -707,7 +708,8 @@
 static_cast(*thread).SetStoppedBySignal(SIGSTOP);
 
   // Let our process instance know the thread has stopped.
-  SetState(StateType::eStateStopped);
+  SetCurrentThreadID(m_threads.front()->GetID());
+  SetState(StateType::eStateStopped, false);
   return Status();
 }
 


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1728,6 +1728,7 @@
   case eStateSuspended:
   case eStateStopped:
   case eStateCrashed: {
+assert(m_debugged_process_up != nullptr);
 lldb::tid_t tid = m_debugged_process_up->GetCurrentThreadID();
 // Make sure we set the current thread so g and p packets return the data
 // the gdb will expect.
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
@@ -698,8 +698,9 @@
   0)
 return Status(errno, eErrorTypePOSIX);
 
-  /* Initialize threads */
-  status = ReinitializeThreads();
+  // Initialize threads and tracing status
+  // NB: this needs to be called before we set thread state
+  status = SetupTrace();
   if (status.Fail())
 return status;
 
@@ -707,7 +708,8 @@
 static_cast(*thread).SetStoppedBySignal(SIGSTOP);
 
   // Let our process instance know the thread has stopped.
-  SetState(StateType::eStateStopped);
+  SetCurrentThreadID(m_threads.front()->GetID());
+  SetState(StateType::eStateStopped, false);
   return Status();
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D90454: [lldb] [Host/{free, net}bsd] Fix process matching by name

2020-11-03 Thread Michał Górny via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf893b2939781: [lldb] [Host/{free,net}bsd] Fix process 
matching by name (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90454/new/

https://reviews.llvm.org/D90454

Files:
  lldb/source/Host/freebsd/Host.cpp
  lldb/source/Host/netbsd/HostNetBSD.cpp


Index: lldb/source/Host/netbsd/HostNetBSD.cpp
===
--- lldb/source/Host/netbsd/HostNetBSD.cpp
+++ lldb/source/Host/netbsd/HostNetBSD.cpp
@@ -200,6 +200,9 @@
 return 0;
   }
 
+  ProcessInstanceInfoMatch match_info_noname{match_info};
+  match_info_noname.SetNameMatchType(NameMatch::Ignore);
+
   for (int i = 0; i < nproc; i++) {
 if (proc_kinfo[i].p_pid < 1)
   continue; /* not valid */
@@ -237,7 +240,7 @@
 process_info.SetEffectiveUserID(proc_kinfo[i].p_uid);
 process_info.SetEffectiveGroupID(proc_kinfo[i].p_gid);
 // Make sure our info matches before we go fetch the name and cpu type
-if (match_info.Matches(process_info) &&
+if (match_info_noname.Matches(process_info) &&
 GetNetBSDProcessArgs(&match_info, process_info)) {
   GetNetBSDProcessCPUType(process_info);
   if (match_info.Matches(process_info))
Index: lldb/source/Host/freebsd/Host.cpp
===
--- lldb/source/Host/freebsd/Host.cpp
+++ lldb/source/Host/freebsd/Host.cpp
@@ -175,6 +175,9 @@
 
   const size_t actual_pid_count = (pid_data_size / sizeof(struct kinfo_proc));
 
+  ProcessInstanceInfoMatch match_info_noname{match_info};
+  match_info_noname.SetNameMatchType(NameMatch::Ignore);
+
   for (size_t i = 0; i < actual_pid_count; i++) {
 const struct kinfo_proc &kinfo = kinfos[i];
 
@@ -212,7 +215,7 @@
 process_info.SetEffectiveGroupID(kinfo.ki_svgid);
 
 // Make sure our info matches before we go fetch the name and cpu type
-if (match_info.Matches(process_info) &&
+if (match_info_noname.Matches(process_info) &&
 GetFreeBSDProcessArgs(&match_info, process_info)) {
   GetFreeBSDProcessCPUType(process_info);
   if (match_info.Matches(process_info))


Index: lldb/source/Host/netbsd/HostNetBSD.cpp
===
--- lldb/source/Host/netbsd/HostNetBSD.cpp
+++ lldb/source/Host/netbsd/HostNetBSD.cpp
@@ -200,6 +200,9 @@
 return 0;
   }
 
+  ProcessInstanceInfoMatch match_info_noname{match_info};
+  match_info_noname.SetNameMatchType(NameMatch::Ignore);
+
   for (int i = 0; i < nproc; i++) {
 if (proc_kinfo[i].p_pid < 1)
   continue; /* not valid */
@@ -237,7 +240,7 @@
 process_info.SetEffectiveUserID(proc_kinfo[i].p_uid);
 process_info.SetEffectiveGroupID(proc_kinfo[i].p_gid);
 // Make sure our info matches before we go fetch the name and cpu type
-if (match_info.Matches(process_info) &&
+if (match_info_noname.Matches(process_info) &&
 GetNetBSDProcessArgs(&match_info, process_info)) {
   GetNetBSDProcessCPUType(process_info);
   if (match_info.Matches(process_info))
Index: lldb/source/Host/freebsd/Host.cpp
===
--- lldb/source/Host/freebsd/Host.cpp
+++ lldb/source/Host/freebsd/Host.cpp
@@ -175,6 +175,9 @@
 
   const size_t actual_pid_count = (pid_data_size / sizeof(struct kinfo_proc));
 
+  ProcessInstanceInfoMatch match_info_noname{match_info};
+  match_info_noname.SetNameMatchType(NameMatch::Ignore);
+
   for (size_t i = 0; i < actual_pid_count; i++) {
 const struct kinfo_proc &kinfo = kinfos[i];
 
@@ -212,7 +215,7 @@
 process_info.SetEffectiveGroupID(kinfo.ki_svgid);
 
 // Make sure our info matches before we go fetch the name and cpu type
-if (match_info.Matches(process_info) &&
+if (match_info_noname.Matches(process_info) &&
 GetFreeBSDProcessArgs(&match_info, process_info)) {
   GetFreeBSDProcessCPUType(process_info);
   if (match_info.Matches(process_info))
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D90598: [lldb/DWARF] Fix implementation of DW_OP_convert

2020-11-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D90598#2369590 , @JDevlieghere 
wrote:

> In D90598#2368826 , @dblaikie wrote:
>
>> That's my understanding. It means I'm not sure (though I'm not the expert 
>> here - certainly leave final decisions up to @aprantl and @JDevlieghere ) 
>> how this will be fixed, since lldb will presumably still need to be able to 
>> parse/understand old dsyms using the incorrect absolute offsets. (we had a 
>> similar problem once with DW_OP_bit_piece, where it was implemented 
>> incorrectly in both LLVM and lldb and I'm not sure how the backwards 
>> compatibility story was addressed there).
>
> I think @aprantl has proposed this in the past, but we should make `dsymutil` 
> convert `DW_OP_convert` to `DW_OP_LLVM_convert` in its output.

How much space are we talking about saving here? A single DW_TAG_base_type DIE 
is like 4--8 bytes long (depending of whether it contains the name field, and 
how its encoded, etc.). If every compile unit needed a dozen of these entries, 
that's still like under 100 bytes per CU. Is that too much? Could we just put 
these entries into every CU that needs them? I'd expect this to be negligible 
compared to the rest of DWARF...

If the space savings are important enough, then staking out a new dwarf opcode 
seems like the best solution. However, since DWARF Linker is also coming to 
other platforms and those platforms may have consumers which may not understand 
llvm extensions, I think we may have to implement both solutions anyway. Also, 
to support non-macho use cases, I think the operand to this extension should 
not be uleb-encoded (so that it can be relocated by a linker), which can make 
it larger than usual.

Or.. here's a crazy idea. Do all compile units produced by dsymutil share the 
same abbreviation table? (If not, why not?) We could encode the 
DW_TAG_base_types "into" the abbreviation table via DW_FORM_implicit_const. 
Then the DW_TAG_base_types would be just `sizeof(uleb) ≈ 2` (plus maybe the 
size of a string form, if they have a name).

> I also don't have a good plan for backward compatibility. If there was a way 
> we could "know" that the DWARF came from a dSYM we could continue to support 
> the old behavior knowing that it has always been wrong, but I'm not aware of 
> anything like that in LLDB and I'm also not sure that's something I'd like to 
> have in the first place...

Detecting dSYM files should not be that hard. `triple.isMachO() && 
!symbol_file_dwarf->GetDebugMapSymfile()` should be a pretty good 
approximation. It won't handle case the case of opening .o files directly, but 
that's only useful for tests I believe, and we could throw in an additional 
condition for that if needed. If we want to have backward compatibility, I 
think we're going to have to have something like that at least for a 
transitional period. The question is how to ensure that this code can be 
removed eventually. One possibility would be for dsymutil to signal that it has 
this fixed via some cu-level attribute (DW_AT_LLVM_convert_is_fixed) --  after 
a certain time, we can remove the fallback, and assume the correct behavior 
everywhere...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90598/new/

https://reviews.llvm.org/D90598

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 4b9fa3b - [LLDB][NFC] treat Lua error codes in a more explicit manner

2020-11-03 Thread Pedro Tammela via lldb-commits

Author: Pedro Tammela
Date: 2020-11-03T09:39:47Z
New Revision: 4b9fa3b705c8280e71908f4cc8c115320ef48316

URL: 
https://github.com/llvm/llvm-project/commit/4b9fa3b705c8280e71908f4cc8c115320ef48316
DIFF: 
https://github.com/llvm/llvm-project/commit/4b9fa3b705c8280e71908f4cc8c115320ef48316.diff

LOG: [LLDB][NFC] treat Lua error codes in a more explicit manner

This patch is a minor suggestion to not rely on the fact
that the `LUA_OK` macro is 0.

This assumption could change in future versions of the C API.

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

Added: 


Modified: 
lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp 
b/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
index acd6128d84c5..2db44f2d29d0 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -18,7 +18,7 @@ llvm::Error Lua::Run(llvm::StringRef buffer) {
   int error =
   luaL_loadbuffer(m_lua_state, buffer.data(), buffer.size(), "buffer") ||
   lua_pcall(m_lua_state, 0, 0, 0);
-  if (!error)
+  if (error == LUA_OK)
 return llvm::Error::success();
 
   llvm::Error e = llvm::make_error(
@@ -44,7 +44,7 @@ llvm::Error Lua::LoadModule(llvm::StringRef filename) {
 
   int error = luaL_loadfile(m_lua_state, filename.data()) ||
   lua_pcall(m_lua_state, 0, 1, 0);
-  if (error) {
+  if (error != LUA_OK) {
 llvm::Error e = llvm::make_error(
 llvm::formatv("{0}\n", lua_tostring(m_lua_state, -1)),
 llvm::inconvertibleErrorCode());



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D90556: [LLDB][NFC] treat Lua error codes in a more explicit manner

2020-11-03 Thread Pedro Tammela via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4b9fa3b705c8: [LLDB][NFC] treat Lua error codes in a more 
explicit manner (authored by tammela).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90556/new/

https://reviews.llvm.org/D90556

Files:
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp


Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -18,7 +18,7 @@
   int error =
   luaL_loadbuffer(m_lua_state, buffer.data(), buffer.size(), "buffer") ||
   lua_pcall(m_lua_state, 0, 0, 0);
-  if (!error)
+  if (error == LUA_OK)
 return llvm::Error::success();
 
   llvm::Error e = llvm::make_error(
@@ -44,7 +44,7 @@
 
   int error = luaL_loadfile(m_lua_state, filename.data()) ||
   lua_pcall(m_lua_state, 0, 1, 0);
-  if (error) {
+  if (error != LUA_OK) {
 llvm::Error e = llvm::make_error(
 llvm::formatv("{0}\n", lua_tostring(m_lua_state, -1)),
 llvm::inconvertibleErrorCode());


Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -18,7 +18,7 @@
   int error =
   luaL_loadbuffer(m_lua_state, buffer.data(), buffer.size(), "buffer") ||
   lua_pcall(m_lua_state, 0, 0, 0);
-  if (!error)
+  if (error == LUA_OK)
 return llvm::Error::success();
 
   llvm::Error e = llvm::make_error(
@@ -44,7 +44,7 @@
 
   int error = luaL_loadfile(m_lua_state, filename.data()) ||
   lua_pcall(m_lua_state, 0, 1, 0);
-  if (error) {
+  if (error != LUA_OK) {
 llvm::Error e = llvm::make_error(
 llvm::formatv("{0}\n", lua_tostring(m_lua_state, -1)),
 llvm::inconvertibleErrorCode());
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D90490: [intel-pt][trace] Implement a "get supported trace type" packet

2020-11-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments.



Comment at: lldb/docs/lldb-gdb-remote.txt:247
+//
+//  {
+//"pluginName": 

Having lldb-server match the lldb trace plugin name seems a bit backward to me. 
I think it'd be more natural if the server reports a name for the technology it 
uses and then the client picks a plugin to go with that technology.

I think it's fine if the process of "picking a plugin" is implemented by 
matching the GetPluginName() string to the value returned here, so I don't 
think that the code needs to change in anyway (except maybe droping the word 
"plugin" from the field name?). My quibble is about how this is framed in the 
documentation -- I'd say this should be the source of truth and trace plugin 
should reference it instead of the other way around. (One reason for that could 
be that we can change the plugin matching logic in the client any time we want, 
but changing the packet format brings up compatibility concerns.)



Comment at: lldb/docs/lldb-gdb-remote.txt:247-248
+//
+//  The return packet is a JSON array of lldb::TraceType values represented in
+//  decimal.
+//

clayborg wrote:
> So my main question is: should a lldb-server be able to return multiple 
> tracing types? I would vote to always respond with the best tracing that is 
> available and ensure that lldb-server can only enable on kind of trace. I 
> know IntelPT can be supported along with some other format that only saves 
> that few branches (ABL?), but if IntelPT is available, should we respond with 
> only IntelPT being available? It would be also nice to have a name for each 
> returned tracing type instead of just a number? Maybe making the response a 
> dictionary of tracing integer type to name would be nice?:
> ```
> { 1: "intel-pt", 2: "abl" }
> ```
> I would rather we return just a single entry here if possible. If we can, 
> this might be better as a dictionary response:
> ```
> { "name": "intel-pt", "trace-type": 1 }
> ```
If we're expecting that lldb-server will be able to capture multiple kinds of 
traces (on the same processor architecture), then (strongly) believe 
lldb-server should be able to return/enumerate all of them. I don't think 
lldb-server should be in the business of deciding what the user wants to do 
(like, he may want deliberately use the "inferior" technology so he can compare 
the performance/features of the two trace types, or whatever). Defaulting to 
the most advanced format is of course fine, but that could be done by listing 
that format first (or giving them an explicit priority, etc.).

That said, there is still a big if on that statement -- tracing technologies 
are complex beasts, so I'd be surprised if a single processor supports two 
technologies with feature sets that overlap well enough that they may be thrown 
into the same bag. However, I don't know much about processor traces and Greg's 
comment seems to imply that they do exist.

(It's also worth noting that it'd be fairly easy to extend this packed to 
(optionally) return a list of these dictionaries if that becomes needed.)



Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:398
+  virtual llvm::Expected GetSupportedTraceType() {
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "Tracing is not supported");

We now have an UnimplementedError class you can return here.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:3462
+  StreamGDBRemote escaped_packet;
+  escaped_packet.PutCString("jTraceSupportedType");
+

wallace wrote:
> clayborg wrote:
> > So are we going to reuse all of the other "jTrace*" packets and possibly 
> > expand their usage? If so, then this name is good.  If we are going to make 
> > new packets for tracing then "jLLDBTraceSupportedType" might make more 
> > sense and all commands we would add would start with "jLLDBTrace".
> I'm glad you mention this. I'm thinking about using a completely new set of 
> packets and document the existing ones as deprecated.
> 
> The old packets include:
> 
> - jTraceMetaRead -> it's confusing and not really useful for the current 
> implementation. Even the external intel-pt plugin doesn't make use of it. It 
> exposes the data gotten when invoking the SYS_perf_event_open syscall, which 
> is too perf+linux only. It can be useful for getting more information out of 
> the kernel, like some clock tracing, but nothing useful as of now.
> -  jTraceConfigRead -> instead of having it, jTraceStart could return that 
> config once tracing has been enabled. That would avoid one round trip, as 
> decoding can't happen anyway without this information.
> -  jTraceStop -> it expects a trace ID (which is intel-pt only in the current 
> implementation), and an optional thread id. It could be simpler by just 
> asking a list of thread ids, or n

[Lldb-commits] [PATCH] D90318: Return actual type from SBType::GetArrayElementType

2020-11-03 Thread Andy Yankovsky via Phabricator via lldb-commits
werat added a comment.

Ping :) 
@teemperor @JDevlieghere


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90318/new/

https://reviews.llvm.org/D90318

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] d2700b7 - [lldb/Utility] Add unit tests for RegisterValue::GetScalarValue

2020-11-03 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2020-11-03T16:12:32+01:00
New Revision: d2700b7873e305a09a931e98b12ebab7b42deec3

URL: 
https://github.com/llvm/llvm-project/commit/d2700b7873e305a09a931e98b12ebab7b42deec3
DIFF: 
https://github.com/llvm/llvm-project/commit/d2700b7873e305a09a931e98b12ebab7b42deec3.diff

LOG: [lldb/Utility] Add unit tests for RegisterValue::GetScalarValue

Buggy cases are commented out.

Also sneak in a modernization of a RegisterValue constructor.

Added: 


Modified: 
lldb/include/lldb/Utility/RegisterValue.h
lldb/source/Expression/Materializer.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/unittests/Utility/RegisterValueTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/RegisterValue.h 
b/lldb/include/lldb/Utility/RegisterValue.h
index 4885d5806384..4211b0a59992 100644
--- a/lldb/include/lldb/Utility/RegisterValue.h
+++ b/lldb/include/lldb/Utility/RegisterValue.h
@@ -74,9 +74,9 @@ class RegisterValue {
 m_scalar = value;
   }
 
-  explicit RegisterValue(uint8_t *bytes, size_t length,
+  explicit RegisterValue(llvm::ArrayRef bytes,
  lldb::ByteOrder byte_order) {
-SetBytes(bytes, length, byte_order);
+SetBytes(bytes.data(), bytes.size(), byte_order);
   }
 
   RegisterValue::Type GetType() const { return m_type; }

diff  --git a/lldb/source/Expression/Materializer.cpp 
b/lldb/source/Expression/Materializer.cpp
index 327e15a26266..a93c127dd0d0 100644
--- a/lldb/source/Expression/Materializer.cpp
+++ b/lldb/source/Expression/Materializer.cpp
@@ -1282,9 +1282,8 @@ class EntityRegister : public Materializer::Entity {
 
 m_register_contents.reset();
 
-RegisterValue register_value(
-const_cast(register_data.GetDataStart()),
-register_data.GetByteSize(), register_data.GetByteOrder());
+RegisterValue register_value(register_data.GetData(),
+ register_data.GetByteOrder());
 
 if (!reg_context_sp->WriteRegister(&m_register_info, register_value)) {
   err.SetErrorStringWithFormat("couldn't write the value of register %s",

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 3b6f740a5983..02b6ca40fc9b 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2091,7 +2091,7 @@ 
GDBRemoteCommunicationServerLLGS::Handle_P(StringExtractorGDBRemote &packet) {
   StreamGDBRemote response;
 
   RegisterValue reg_value(
-  reg_bytes, reg_size,
+  makeArrayRef(reg_bytes, reg_size),
   m_debugged_process_up->GetArchitecture().GetByteOrder());
   Status error = reg_context.WriteRegister(reg_info, reg_value);
   if (error.Fail()) {

diff  --git a/lldb/unittests/Utility/RegisterValueTest.cpp 
b/lldb/unittests/Utility/RegisterValueTest.cpp
index 70d79ea0024a..bbefb96519b1 100644
--- a/lldb/unittests/Utility/RegisterValueTest.cpp
+++ b/lldb/unittests/Utility/RegisterValueTest.cpp
@@ -10,6 +10,8 @@
 #include "gtest/gtest.h"
 
 using namespace lldb_private;
+using llvm::APInt;
+using llvm::ArrayRef;
 
 TEST(RegisterValueTest, GetSet8) {
   RegisterValue R8(uint8_t(47));
@@ -20,3 +22,37 @@ TEST(RegisterValueTest, GetSet8) {
   EXPECT_EQ(42u, R8.GetAsUInt32());
   EXPECT_EQ(42u, R8.GetAsUInt64());
 }
+
+TEST(RegisterValueTest, GetScalarValue) {
+  using RV = RegisterValue;
+  const auto &Get = [](const RV &V) -> llvm::Optional {
+Scalar S;
+if (V.GetScalarValue(S))
+  return S;
+return llvm::None;
+  };
+  EXPECT_EQ(Get(RV(uint8_t(47))), Scalar(47));
+  EXPECT_EQ(Get(RV(uint16_t(4747))), Scalar(4747));
+  EXPECT_EQ(Get(RV(uint32_t(47474242))), Scalar(47474242));
+  EXPECT_EQ(Get(RV(uint64_t(4747424247474242))), Scalar(4747424247474242));
+  EXPECT_EQ(Get(RV(APInt::getMaxValue(128))), Scalar(APInt::getMaxValue(128)));
+  EXPECT_EQ(Get(RV(47.5f)), Scalar(47.5f));
+  EXPECT_EQ(Get(RV(47.5)), Scalar(47.5));
+  EXPECT_EQ(Get(RV(47.5L)), Scalar(47.5L));
+  EXPECT_EQ(Get(RV({0xff, 0xee, 0xdd, 0xcc}, lldb::eByteOrderLittle)),
+Scalar(0xccddeeff));
+  //  EXPECT_EQ(Get(RV({0xff, 0xee, 0xdd, 0xcc}, lldb::eByteOrderBig)),
+  //Scalar(0xffeeddcc));
+  EXPECT_EQ(Get(RV({0xff, 0xee, 0xdd, 0xcc, 0xbb, 0xaa, 0x99, 0x88, 0x77, 0x66,
+0x55, 0x44, 0x33, 0x22, 0x11, 0x00},
+   lldb::eByteOrderLittle)),
+Scalar((APInt(128, 0x0011223344556677ull) << 64) |
+   APInt(128, 0x8899aabbccddeeff)));
+#if 0
+  EXPECT_EQ(Get(RV({0xff, 0xee, 0xdd, 0xcc, 0xbb, 0xaa, 0x99, 0x88, 0x77, 0x66,
+0x55, 0x44, 0x33, 0x22, 0x11, 0x00},
+   lldb::eByteOrderBig)),
+Scalar((APInt(128, 0xffeeddccbbaa9988ull) << 64) 

[Lldb-commits] [lldb] fbc0d41 - [lldb] [Process/FreeBSDRemote] Fix "Fix attaching via lldb-server"

2020-11-03 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2020-11-03T17:16:57+01:00
New Revision: fbc0d41bb0e465a0e2a799e7e3b86be4319019f2

URL: 
https://github.com/llvm/llvm-project/commit/fbc0d41bb0e465a0e2a799e7e3b86be4319019f2
DIFF: 
https://github.com/llvm/llvm-project/commit/fbc0d41bb0e465a0e2a799e7e3b86be4319019f2.diff

LOG: [lldb] [Process/FreeBSDRemote] Fix "Fix attaching via lldb-server"

One of the changes seems to have been lost in rebase.  Reapply.

Added: 


Modified: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
index 2c22f1101597..d132e37d38fa 100644
--- a/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
+++ b/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
@@ -125,10 +125,6 @@ NativeProcessFreeBSD::Factory::Attach(
   if (!status.Success())
 return status.ToError();
 
-  status = process_up->SetupTrace();
-  if (status.Fail())
-return status.ToError();
-
   return std::move(process_up);
 }
 



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 4b84682 - [crashlog] Move crash log parsing into its own class

2020-11-03 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-11-03T09:04:35-08:00
New Revision: 4b846820445ef33a099a19b5df983ed2f9d6e067

URL: 
https://github.com/llvm/llvm-project/commit/4b846820445ef33a099a19b5df983ed2f9d6e067
DIFF: 
https://github.com/llvm/llvm-project/commit/4b846820445ef33a099a19b5df983ed2f9d6e067.diff

LOG: [crashlog] Move crash log parsing into its own class

Move crash log parsing out of the CrashLog class and into its own class
and add more tests.

Differential revision: https://reviews.llvm.org/D90664

Added: 
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/crashlog.test
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive.test
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/lit.local.cfg

Modified: 
lldb/examples/python/crashlog.py

Removed: 
lldb/test/Shell/ScriptInterpreter/Python/crashlog.test



diff  --git a/lldb/examples/python/crashlog.py 
b/lldb/examples/python/crashlog.py
index 85387756ce18..373065f165ca 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -71,41 +71,7 @@ def read_plist(s):
 else:
 return plistlib.readPlistFromString(s)
 
-class CrashLogParseMode:
-NORMAL = 0
-THREAD = 1
-IMAGES = 2
-THREGS = 3
-SYSTEM = 4
-INSTRS = 5
-
-
 class CrashLog(symbolication.Symbolicator):
-"""Class that does parses darwin crash logs"""
-parent_process_regex = re.compile('^Parent Process:\s*(.*)\[(\d+)\]')
-thread_state_regex = re.compile('^Thread ([0-9]+) crashed with')
-thread_instrs_regex = re.compile('^Thread ([0-9]+) instruction stream')
-thread_regex = re.compile('^Thread ([0-9]+)([^:]*):(.*)')
-app_backtrace_regex = re.compile(
-'^Application Specific Backtrace ([0-9]+)([^:]*):(.*)')
-version = r'(\(.+\)|(arm|x86_)[0-9a-z]+)\s+'
-frame_regex = re.compile(r'^([0-9]+)' r'\s'# id
- r'+(.+?)'r'\s+'   # img_name
- r'(' +version+ r')?'  # img_version
- r'(0x[0-9a-fA-F]{7}[0-9a-fA-F]+)' # addr
- r' +(.*)' # offs
-)
-null_frame_regex = re.compile(r'^([0-9]+)\s+\?\?\?\s+(0{7}0+) +(.*)')
-image_regex_uuid = re.compile(r'(0x[0-9a-fA-F]+)'# img_lo
-  r'\s+' '-' r'\s+'  #   -
-  r'(0x[0-9a-fA-F]+)' r'\s+' # img_hi
-  r'[+]?(.+?)'r'\s+' # img_name
-  r'(' +version+ ')?'# img_version
-  r'(<([-0-9a-fA-F]+)>\s+)?' # img_uuid
-  r'(/.*)'   # img_path
- )
-empty_line_regex = re.compile('^$')
-
 class Thread:
 """Class that represents a thread in a darwin crash log"""
 
@@ -355,88 +321,174 @@ def __init__(self, path, verbose):
 self.idents = list()  # A list of the required identifiers for doing 
all stack backtraces
 self.crashed_thread_idx = -1
 self.version = -1
-self.error = None
 self.target = None
 self.verbose = verbose
-# With possible initial component of ~ or ~user replaced by that user's
-# home directory.
-try:
-f = open(self.path)
-except IOError:
-self.error = 'error: cannot open "%s"' % self.path
-return
 
-self.file_lines = f.read().splitlines()
-parse_mode = CrashLogParseMode.NORMAL
-thread = None
-app_specific_backtrace = False
-for line in self.file_lines:
-# print line
+def dump(self):
+print("Crash Log File: %s" % (self.path))
+if self.backtraces:
+print("\nApplication Specific Backtraces:")
+for thread in self.backtraces:
+thread.dump('  ')
+print("\nThreads:")
+for thread in self.threads:
+thread.dump('  ')
+print("\nImages:")
+for image in self.images:
+image.dump('  ')
+
+def find_image_with_identifier(self, identifier):
+for image in self.images:
+if image.identifier == identifier:
+return image
+regex_text = '^.*\.%s$' % (re.escape(identifier))
+regex = re.compile(regex_text)
+for image in self.images:
+if regex.match(image.identifier):
+return image
+return None
+
+def create_target(self):
+if self.target is None:
+self.target = symbolication.Symbolicator.create_target(self)
+if self.target:
+return self.target
+# We weren't able to open the main executable as, but we can still
+# symbolicat

[Lldb-commits] [PATCH] D90664: [crashlog] Move crash log parsing into its own class

2020-11-03 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4b846820445e: [crashlog] Move crash log parsing into its own 
class (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D90664?vs=302453&id=302604#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90664/new/

https://reviews.llvm.org/D90664

Files:
  lldb/examples/python/crashlog.py
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/crashlog.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive.test
  lldb/test/Shell/ScriptInterpreter/Python/Crashlog/lit.local.cfg
  lldb/test/Shell/ScriptInterpreter/Python/crashlog.test

Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/lit.local.cfg
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/lit.local.cfg
@@ -0,0 +1,5 @@
+if 'system-darwin' not in config.available_features:
+  config.unsupported = True
+
+if 'lldb-repro' in config.available_features:
+  config.unsupported = True
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive.test
@@ -0,0 +1,8 @@
+# RUN: echo "quit" | %S/../../../../../examples/python/crashlog.py -i %s 2>&1 | FileCheck %s
+# CHECK: 1 crash logs are loaded:
+# CHECK: [0] = {{.*}}interactive.test
+# CHECK: Interactive crashlogs prompt, type "help" to see a list of supported commands.
+
+Binary Images:
+   0x10ab87000 -0x10abdafff +lldb (10.0.0) <87BD1384-BAE9-3625-A838-9D241CBAEF87> /Volumes/VOLUME/*/lldb
+   0x10ac45000 -0x10ae94fff  com.apple.python3 (3.8.2 - 3.8.2) <20BC3FC4-CAAD-3002-ACDF-423A3188F24C> /Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.8/Python3
Index: lldb/test/Shell/ScriptInterpreter/Python/Crashlog/crashlog.test
===
--- lldb/test/Shell/ScriptInterpreter/Python/Crashlog/crashlog.test
+++ lldb/test/Shell/ScriptInterpreter/Python/Crashlog/crashlog.test
@@ -1,12 +1,11 @@
 # -*- python -*-
-# REQUIRES: system-darwin
-# UNSUPPORTED: lldb-repro
-# DEBUG: cd %S/../../../../examples/python && cat %s | %lldb && false
-# RUN: cd %S/../../../../examples/python && cat %s | %lldb | FileCheck %s
+# DEBUG: cd %S/../../../../../examples/python && cat %s | %lldb && false
+# RUN: cd %S/../../../../../examples/python && cat %s | %lldb | FileCheck %s
 # CHECK-LABEL: {{S}}KIP BEYOND CHECKS
 script
 import crashlog
-cl = crashlog.CrashLog
+crash_log_parser = crashlog.CrashLogParser
+crash_log = crashlog.CrashLog
 images = [
 "0x10b60b000 - 0x10f707fff com.apple.LLDB.framework (1.1000.11.38.2 - 1000.11.38.2) <96E36F5C-1A83-39A1-8713-5FDD9701C3F1> /Applications/Xcode.app/Contents/SharedFrameworks/LLDB.framework/LLDB",
 # CHECK: 0x10b60b000
@@ -120,7 +119,7 @@
 for image in images:
 print('"%s"'%image)
 print("--")
-match = cl.image_regex_uuid.search(image)
+match = crash_log_parser.image_regex_uuid.search(image)
 for group in match.groups():
 print(group)
 
@@ -128,7 +127,7 @@
 for frame in frames:
 print('"%s"'%frame)
 print("--")
-match = cl.frame_regex.search(frame)
+match = crash_log_parser.frame_regex.search(frame)
 for group in match.groups():
 print(group)
 
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -71,41 +71,7 @@
 else:
 return plistlib.readPlistFromString(s)
 
-class CrashLogParseMode:
-NORMAL = 0
-THREAD = 1
-IMAGES = 2
-THREGS = 3
-SYSTEM = 4
-INSTRS = 5
-
-
 class CrashLog(symbolication.Symbolicator):
-"""Class that does parses darwin crash logs"""
-parent_process_regex = re.compile('^Parent Process:\s*(.*)\[(\d+)\]')
-thread_state_regex = re.compile('^Thread ([0-9]+) crashed with')
-thread_instrs_regex = re.compile('^Thread ([0-9]+) instruction stream')
-thread_regex = re.compile('^Thread ([0-9]+)([^:]*):(.*)')
-app_backtrace_regex = re.compile(
-'^Application Specific Backtrace ([0-9]+)([^:]*):(.*)')
-version = r'(\(.+\)|(arm|x86_)[0-9a-z]+)\s+'
-frame_regex = re.compile(r'^([0-9]+)' r'\s'# id
- r'+(.+?)'r'\s+'   # img_name
- r'(' +version+ r')?'  # img_version
- r'(0x[0-9a-fA-F]{7}[0-9a-fA-F]+)' # addr
- r' +(.*)' # offs
-)
-null_frame_regex = re.compile(r'^([0-9]+)\s+\?\?

[Lldb-commits] [PATCH] D90665: [crashlog] Modularize parser

2020-11-03 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG16dd69347dfc: [crashlog] Modularize parser (authored by 
JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90665/new/

https://reviews.llvm.org/D90665

Files:
  lldb/examples/python/crashlog.py

Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -414,10 +414,18 @@
 def __init__(self, path, verbose):
 self.path = os.path.expanduser(path)
 self.verbose = verbose
-self.parse_mode = CrashLogParseMode.NORMAL
 self.thread = None
 self.app_specific_backtrace = False
 self.crashlog = CrashLog(self.path, self.verbose)
+self.parse_mode = CrashLogParseMode.NORMAL
+self.parsers = {
+CrashLogParseMode.NORMAL : self.parse_normal,
+CrashLogParseMode.THREAD : self.parse_thread,
+CrashLogParseMode.IMAGES : self.parse_images,
+CrashLogParseMode.THREGS : self.parse_thread_registers,
+CrashLogParseMode.SYSTEM : self.parse_system,
+CrashLogParseMode.INSTRS : self.parse_instructions,
+}
 
 def parse(self):
 with open(self.path,'r') as f:
@@ -445,145 +453,153 @@
 if len(self.crashlog.info_lines) > 0 and len(self.crashlog.info_lines[-1]):
 self.crashlog.info_lines.append(line)
 self.parse_mode = CrashLogParseMode.NORMAL
-elif self.parse_mode == CrashLogParseMode.NORMAL:
-if line.startswith('Process:'):
-(self.crashlog.process_name, pid_with_brackets) = line[
-8:].strip().split(' [')
-self.crashlog.process_id = pid_with_brackets.strip('[]')
-elif line.startswith('Path:'):
-self.crashlog.process_path = line[5:].strip()
-elif line.startswith('Identifier:'):
-self.crashlog.process_identifier = line[11:].strip()
-elif line.startswith('Version:'):
-version_string = line[8:].strip()
-matched_pair = re.search("(.+)\((.+)\)", version_string)
-if matched_pair:
-self.crashlog.process_version = matched_pair.group(1)
-self.crashlog.process_compatability_version = matched_pair.group(
-2)
-else:
-self.crashlog.process = version_string
-self.crashlog.process_compatability_version = version_string
-elif self.parent_process_regex.search(line):
-parent_process_match = self.parent_process_regex.search(
-line)
-self.crashlog.parent_process_name = parent_process_match.group(1)
-self.crashlog.parent_process_id = parent_process_match.group(2)
-elif line.startswith('Exception Type:'):
-self.crashlog.thread_exception = line[15:].strip()
-continue
-elif line.startswith('Exception Codes:'):
-self.crashlog.thread_exception_data = line[16:].strip()
-continue
-elif line.startswith('Exception Subtype:'): # iOS
-self.crashlog.thread_exception_data = line[18:].strip()
-continue
-elif line.startswith('Crashed Thread:'):
-self.crashlog.crashed_thread_idx = int(line[15:].strip().split()[0])
-continue
-elif line.startswith('Triggered by Thread:'): # iOS
-self.crashlog.crashed_thread_idx = int(line[20:].strip().split()[0])
-continue
-elif line.startswith('Report Version:'):
-self.crashlog.version = int(line[15:].strip())
-continue
-elif line.startswith('System Profile:'):
-self.parse_mode = CrashLogParseMode.SYSTEM
-continue
-elif (line.startswith('Interval Since Last Report:') or
-  line.startswith('Crashes Since Last Report:') or
-  line.startswith('Per-App Interval Since Last Report:') or
-  line.startswith('Per-App Crashes Since Last Report:') or
-  line.startswith('Sleep/Wake UUID:') or
-  line.startswith('Anonymous UUID:')):
-# ignore these
-continue
-elif line.startswith('Thread'):
-thread_state_match = self.thread_state_regex.search(line)
-if thread_state_match:
-

[Lldb-commits] [lldb] 16dd693 - [crashlog] Modularize parser

2020-11-03 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-11-03T10:21:21-08:00
New Revision: 16dd69347dfc80ec52f656b971102c31ccf78449

URL: 
https://github.com/llvm/llvm-project/commit/16dd69347dfc80ec52f656b971102c31ccf78449
DIFF: 
https://github.com/llvm/llvm-project/commit/16dd69347dfc80ec52f656b971102c31ccf78449.diff

LOG: [crashlog] Modularize parser

Instead of parsing the crashlog in one big loop, use methods that
correspond to the different parsing modes.

Differential revision: https://reviews.llvm.org/D90665

Added: 


Modified: 
lldb/examples/python/crashlog.py

Removed: 




diff  --git a/lldb/examples/python/crashlog.py 
b/lldb/examples/python/crashlog.py
index 373065f165ca..76538715f17e 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -414,10 +414,18 @@ class CrashLogParser:
 def __init__(self, path, verbose):
 self.path = os.path.expanduser(path)
 self.verbose = verbose
-self.parse_mode = CrashLogParseMode.NORMAL
 self.thread = None
 self.app_specific_backtrace = False
 self.crashlog = CrashLog(self.path, self.verbose)
+self.parse_mode = CrashLogParseMode.NORMAL
+self.parsers = {
+CrashLogParseMode.NORMAL : self.parse_normal,
+CrashLogParseMode.THREAD : self.parse_thread,
+CrashLogParseMode.IMAGES : self.parse_images,
+CrashLogParseMode.THREGS : self.parse_thread_registers,
+CrashLogParseMode.SYSTEM : self.parse_system,
+CrashLogParseMode.INSTRS : self.parse_instructions,
+}
 
 def parse(self):
 with open(self.path,'r') as f:
@@ -445,145 +453,153 @@ def parse(self):
 if len(self.crashlog.info_lines) > 0 and 
len(self.crashlog.info_lines[-1]):
 self.crashlog.info_lines.append(line)
 self.parse_mode = CrashLogParseMode.NORMAL
-elif self.parse_mode == CrashLogParseMode.NORMAL:
-if line.startswith('Process:'):
-(self.crashlog.process_name, pid_with_brackets) = line[
-8:].strip().split(' [')
-self.crashlog.process_id = pid_with_brackets.strip('[]')
-elif line.startswith('Path:'):
-self.crashlog.process_path = line[5:].strip()
-elif line.startswith('Identifier:'):
-self.crashlog.process_identifier = line[11:].strip()
-elif line.startswith('Version:'):
-version_string = line[8:].strip()
-matched_pair = re.search("(.+)\((.+)\)", version_string)
-if matched_pair:
-self.crashlog.process_version = matched_pair.group(1)
-self.crashlog.process_compatability_version = 
matched_pair.group(
-2)
-else:
-self.crashlog.process = version_string
-self.crashlog.process_compatability_version = 
version_string
-elif self.parent_process_regex.search(line):
-parent_process_match = self.parent_process_regex.search(
-line)
-self.crashlog.parent_process_name = 
parent_process_match.group(1)
-self.crashlog.parent_process_id = 
parent_process_match.group(2)
-elif line.startswith('Exception Type:'):
-self.crashlog.thread_exception = line[15:].strip()
-continue
-elif line.startswith('Exception Codes:'):
-self.crashlog.thread_exception_data = line[16:].strip()
-continue
-elif line.startswith('Exception Subtype:'): # iOS
-self.crashlog.thread_exception_data = line[18:].strip()
-continue
-elif line.startswith('Crashed Thread:'):
-self.crashlog.crashed_thread_idx = 
int(line[15:].strip().split()[0])
-continue
-elif line.startswith('Triggered by Thread:'): # iOS
-self.crashlog.crashed_thread_idx = 
int(line[20:].strip().split()[0])
-continue
-elif line.startswith('Report Version:'):
-self.crashlog.version = int(line[15:].strip())
-continue
-elif line.startswith('System Profile:'):
-self.parse_mode = CrashLogParseMode.SYSTEM
-continue
-elif (line.startswith('Interval Since Last Report:') or
-  line.startswith('Crashes Since Last Report:') or
-  line.startswith('Per-App Interval Since Last Report:') or
-  line.startswith('Per-App Crashes Since Last Report:') or
-  

[Lldb-commits] [PATCH] D90318: Return actual type from SBType::GetArrayElementType

2020-11-03 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

In D90318#2362321 , @werat wrote:

> Btw, I don't have commit access, so would be great if someone could land this 
> for me :) (if everything is OK with this patch).

I'll land this for you once the test suite finishes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90318/new/

https://reviews.llvm.org/D90318

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] f35a823 - Return actual type from SBType::GetArrayElementType

2020-11-03 Thread Jonas Devlieghere via lldb-commits

Author: Andy Yankovsky
Date: 2020-11-03T10:53:44-08:00
New Revision: f35a82384d9ebeff3bb5ffd7a5ebe30436c1b401

URL: 
https://github.com/llvm/llvm-project/commit/f35a82384d9ebeff3bb5ffd7a5ebe30436c1b401
DIFF: 
https://github.com/llvm/llvm-project/commit/f35a82384d9ebeff3bb5ffd7a5ebe30436c1b401.diff

LOG: Return actual type from SBType::GetArrayElementType

SBType::GetArrayElementType should return the actual type, not the
canonical type (e.g. int32_t, not the underlying int).

Added a test case to validate the new behavior. I also ran all other
tests on Linux (ninja check-lldb), they all pass.

Differential revision: https://reviews.llvm.org/D90318

Added: 


Modified: 
lldb/source/API/SBType.cpp
lldb/test/API/python_api/type/TestTypeList.py
lldb/test/API/python_api/type/main.cpp

Removed: 




diff  --git a/lldb/source/API/SBType.cpp b/lldb/source/API/SBType.cpp
index 772ac87145bb..0a99ac0f2292 100644
--- a/lldb/source/API/SBType.cpp
+++ b/lldb/source/API/SBType.cpp
@@ -213,10 +213,8 @@ SBType SBType::GetArrayElementType() {
 
   if (!IsValid())
 return LLDB_RECORD_RESULT(SBType());
-  CompilerType canonical_type =
-  m_opaque_sp->GetCompilerType(true).GetCanonicalType();
-  return LLDB_RECORD_RESULT(SBType(
-  TypeImplSP(new TypeImpl(canonical_type.GetArrayElementType(nullptr);
+  return LLDB_RECORD_RESULT(SBType(TypeImplSP(new TypeImpl(
+  m_opaque_sp->GetCompilerType(true).GetArrayElementType(nullptr);
 }
 
 SBType SBType::GetArrayType(uint64_t size) {

diff  --git a/lldb/test/API/python_api/type/TestTypeList.py 
b/lldb/test/API/python_api/type/TestTypeList.py
index 75a793a95b29..901ddc62f4e5 100644
--- a/lldb/test/API/python_api/type/TestTypeList.py
+++ b/lldb/test/API/python_api/type/TestTypeList.py
@@ -131,3 +131,16 @@ def test(self):
 # (lldb-enumerations.h).
 int_type = id_type.GetBasicType(lldb.eBasicTypeInt)
 self.assertTrue(id_type == int_type)
+
+# Find 'myint_arr' and check the array element type.
+myint_arr = frame0.FindVariable('myint_arr')
+self.assertTrue(myint_arr, VALID_VARIABLE)
+self.DebugSBValue(myint_arr)
+myint_arr_type = myint_arr.GetType()
+self.DebugSBType(myint_arr_type)
+self.assertTrue(myint_arr_type.IsArrayType())
+myint_arr_element_type = myint_arr_type.GetArrayElementType()
+self.DebugSBType(myint_arr_element_type)
+myint_type = target.FindFirstType('myint')
+self.DebugSBType(myint_type)
+self.assertTrue(myint_arr_element_type == myint_type)

diff  --git a/lldb/test/API/python_api/type/main.cpp 
b/lldb/test/API/python_api/type/main.cpp
index 40799abe36d3..13e6bbc127ba 100644
--- a/lldb/test/API/python_api/type/main.cpp
+++ b/lldb/test/API/python_api/type/main.cpp
@@ -56,5 +56,8 @@ int main (int argc, char const *argv[])
 // This corresponds to an empty task list.
 Task *empty_task_head = new Task(-1, NULL);
 
+typedef int myint;
+myint myint_arr[] = {1, 2, 3};
+
 return 0; // Break at this line
 }



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D90318: Return actual type from SBType::GetArrayElementType

2020-11-03 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf35a82384d9e: Return actual type from 
SBType::GetArrayElementType (authored by werat, committed by JDevlieghere).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90318/new/

https://reviews.llvm.org/D90318

Files:
  lldb/source/API/SBType.cpp
  lldb/test/API/python_api/type/TestTypeList.py
  lldb/test/API/python_api/type/main.cpp


Index: lldb/test/API/python_api/type/main.cpp
===
--- lldb/test/API/python_api/type/main.cpp
+++ lldb/test/API/python_api/type/main.cpp
@@ -56,5 +56,8 @@
 // This corresponds to an empty task list.
 Task *empty_task_head = new Task(-1, NULL);
 
+typedef int myint;
+myint myint_arr[] = {1, 2, 3};
+
 return 0; // Break at this line
 }
Index: lldb/test/API/python_api/type/TestTypeList.py
===
--- lldb/test/API/python_api/type/TestTypeList.py
+++ lldb/test/API/python_api/type/TestTypeList.py
@@ -131,3 +131,16 @@
 # (lldb-enumerations.h).
 int_type = id_type.GetBasicType(lldb.eBasicTypeInt)
 self.assertTrue(id_type == int_type)
+
+# Find 'myint_arr' and check the array element type.
+myint_arr = frame0.FindVariable('myint_arr')
+self.assertTrue(myint_arr, VALID_VARIABLE)
+self.DebugSBValue(myint_arr)
+myint_arr_type = myint_arr.GetType()
+self.DebugSBType(myint_arr_type)
+self.assertTrue(myint_arr_type.IsArrayType())
+myint_arr_element_type = myint_arr_type.GetArrayElementType()
+self.DebugSBType(myint_arr_element_type)
+myint_type = target.FindFirstType('myint')
+self.DebugSBType(myint_type)
+self.assertTrue(myint_arr_element_type == myint_type)
Index: lldb/source/API/SBType.cpp
===
--- lldb/source/API/SBType.cpp
+++ lldb/source/API/SBType.cpp
@@ -213,10 +213,8 @@
 
   if (!IsValid())
 return LLDB_RECORD_RESULT(SBType());
-  CompilerType canonical_type =
-  m_opaque_sp->GetCompilerType(true).GetCanonicalType();
-  return LLDB_RECORD_RESULT(SBType(
-  TypeImplSP(new TypeImpl(canonical_type.GetArrayElementType(nullptr);
+  return LLDB_RECORD_RESULT(SBType(TypeImplSP(new TypeImpl(
+  m_opaque_sp->GetCompilerType(true).GetArrayElementType(nullptr);
 }
 
 SBType SBType::GetArrayType(uint64_t size) {


Index: lldb/test/API/python_api/type/main.cpp
===
--- lldb/test/API/python_api/type/main.cpp
+++ lldb/test/API/python_api/type/main.cpp
@@ -56,5 +56,8 @@
 // This corresponds to an empty task list.
 Task *empty_task_head = new Task(-1, NULL);
 
+typedef int myint;
+myint myint_arr[] = {1, 2, 3};
+
 return 0; // Break at this line
 }
Index: lldb/test/API/python_api/type/TestTypeList.py
===
--- lldb/test/API/python_api/type/TestTypeList.py
+++ lldb/test/API/python_api/type/TestTypeList.py
@@ -131,3 +131,16 @@
 # (lldb-enumerations.h).
 int_type = id_type.GetBasicType(lldb.eBasicTypeInt)
 self.assertTrue(id_type == int_type)
+
+# Find 'myint_arr' and check the array element type.
+myint_arr = frame0.FindVariable('myint_arr')
+self.assertTrue(myint_arr, VALID_VARIABLE)
+self.DebugSBValue(myint_arr)
+myint_arr_type = myint_arr.GetType()
+self.DebugSBType(myint_arr_type)
+self.assertTrue(myint_arr_type.IsArrayType())
+myint_arr_element_type = myint_arr_type.GetArrayElementType()
+self.DebugSBType(myint_arr_element_type)
+myint_type = target.FindFirstType('myint')
+self.DebugSBType(myint_type)
+self.assertTrue(myint_arr_element_type == myint_type)
Index: lldb/source/API/SBType.cpp
===
--- lldb/source/API/SBType.cpp
+++ lldb/source/API/SBType.cpp
@@ -213,10 +213,8 @@
 
   if (!IsValid())
 return LLDB_RECORD_RESULT(SBType());
-  CompilerType canonical_type =
-  m_opaque_sp->GetCompilerType(true).GetCanonicalType();
-  return LLDB_RECORD_RESULT(SBType(
-  TypeImplSP(new TypeImpl(canonical_type.GetArrayElementType(nullptr);
+  return LLDB_RECORD_RESULT(SBType(TypeImplSP(new TypeImpl(
+  m_opaque_sp->GetCompilerType(true).GetArrayElementType(nullptr);
 }
 
 SBType SBType::GetArrayType(uint64_t size) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D90490: [intel-pt][trace] Implement a "get supported trace type" packet

2020-11-03 Thread walter erquinigo via Phabricator via lldb-commits
wallace updated this revision to Diff 302630.
wallace added a comment.

- Use name instead of pluginName and UnimplementedError, as @labath suggested
- Still keep the packet returning one single trace technology. It'll be a long 
time until two technologies collide, but if that ever happens, we can extend 
the packet to return a list or a dictionary. As of now, the only technology 
that could potentially collide with intel-pt is LBR, which traces only the last 
32 branches and is pretty much defunct in new applications, as Intel PT is 
actually the modern version of LBR. Arm has Coresight, Windows also has 
intel-pt. AMD has IBS. For now, I'm in favor of keeping it simple.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90490/new/

https://reviews.llvm.org/D90490

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
  lldb/source/Plugins/Process/Linux/ProcessorTrace.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/Trace.cpp
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -362,6 +362,45 @@
   EXPECT_FALSE(result.get().Success());
 }
 
+TEST_F(GDBRemoteCommunicationClientTest, SendTraceSupportedTypePacket) {
+  // Success response
+  {
+std::future> result = std::async(
+std::launch::async, [&] { return client.SendGetSupportedTraceType(); });
+
+HandlePacket(
+server, "jLLDBTraceSupportedType",
+R"({"name":"intel-pt","description":"Intel Processor Trace"}])");
+
+llvm::Expected trace_type_or_err = result.get();
+EXPECT_THAT_EXPECTED(trace_type_or_err, llvm::Succeeded());
+ASSERT_STREQ(trace_type_or_err->name.c_str(), "intel-pt");
+ASSERT_STREQ(trace_type_or_err->description.c_str(),
+ "Intel Processor Trace");
+  }
+
+  // Error response - wrong json
+  {
+std::future> result = std::async(
+std::launch::async, [&] { return client.SendGetSupportedTraceType(); });
+
+HandlePacket(server, "jLLDBTraceSupportedType", R"({"type":"intel-pt"}])");
+
+llvm::Expected trace_type_or_err = result.get();
+EXPECT_THAT_EXPECTED(trace_type_or_err, llvm::Failed());
+  }
+
+  // Error response
+  {
+std::future> result = std::async(
+std::launch::async, [&] { return client.SendGetSupportedTraceType(); });
+
+HandlePacket(server, "jLLDBTraceSupportedType", "E23");
+llvm::Expected trace_type_or_err = result.get();
+ASSERT_THAT_EXPECTED(trace_type_or_err, llvm::Failed());
+  }
+}
+
 TEST_F(GDBRemoteCommunicationClientTest, SendStartTracePacket) {
   TraceOptions options;
   Status error;
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -310,6 +310,8 @@
   return eServerPacketType_jTraceStart;
 if (PACKET_STARTS_WITH("jTraceStop:"))
   return eServerPacketType_jTraceStop;
+if (PACKET_MATCHES("jLLDBTraceSupportedType"))
+  return eServerPacketType_jLLDBTraceSupportedType;
 break;
 
   case 'v':
Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -266,3 +266,18 @@
 return index < end_position;
   });
 }
+
+namespace llvm {
+namespace json {
+
+bool fromJSON(const Value &value, lldb_private::TraceTypeInfo &info,
+  Path path) {
+  ObjectMapper o(value, path);
+  if (!o)
+return false;
+  o.map("description", info.description);
+  return o.map("name", info.name);
+}
+
+} // namespace json
+} // namespace llvm
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-r

[Lldb-commits] [lldb] f0fd434 - [crashlog] Print the actual exception in the CommandReturnObject

2020-11-03 Thread Jonas Devlieghere via lldb-commits

Author: Jonas Devlieghere
Date: 2020-11-03T11:51:40-08:00
New Revision: f0fd4349a78aff99b7e2214bf1ffe9e352201de3

URL: 
https://github.com/llvm/llvm-project/commit/f0fd4349a78aff99b7e2214bf1ffe9e352201de3
DIFF: 
https://github.com/llvm/llvm-project/commit/f0fd4349a78aff99b7e2214bf1ffe9e352201de3.diff

LOG: [crashlog] Print the actual exception in the CommandReturnObject

Before:

  error: python exception 

After:

  error: python exception: 'DarwinImage' object has no attribute 'debugger'

Added: 


Modified: 
lldb/examples/python/crashlog.py

Removed: 




diff  --git a/lldb/examples/python/crashlog.py 
b/lldb/examples/python/crashlog.py
index 76538715f17e..d3943fb3f0fe 100755
--- a/lldb/examples/python/crashlog.py
+++ b/lldb/examples/python/crashlog.py
@@ -849,8 +849,8 @@ def save_crashlog(debugger, command, exe_ctx, result, dict):
 def Symbolicate(debugger, command, result, dict):
 try:
 SymbolicateCrashLogs(shlex.split(command))
-except:
-result.PutCString("error: python exception %s" % sys.exc_info()[0])
+except Exception as e:
+result.PutCString("error: python exception: %s" % e)
 
 
 def SymbolicateCrashLog(crash_log, options):



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D90706: [crashlog] Pass the debugger around instead of relying on lldb.debugger

2020-11-03 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added a reviewer: clayborg.
JDevlieghere requested review of this revision.

The `lldb.debugger` et al convenience variables are only available from the 
interactive script interpreter. In all other scenarios, they are `None` (since 
fc1fd6bf9fcfac412b10b4193805ec5de0e8df57) before that they were default 
initialized.

The `crashlog` script was hacking around that by setting the `lldb.debugger` to 
a newly created debugger instance when running outside of the script 
interpreter, which works fine until lldb creates a script interpreter instance 
under the hood and clears the variables. This was resulting in an 
`AttributeError` when invoking the script directly (from outside of lldb):

  AttributeError: 'NoneType' object has no attribute 'GetSourceManager'

This patch fixes that by passing around the debugger instance.


https://reviews.llvm.org/D90706

Files:
  lldb/examples/python/crashlog.py
  lldb/examples/python/symbolication.py

Index: lldb/examples/python/symbolication.py
===
--- lldb/examples/python/symbolication.py
+++ lldb/examples/python/symbolication.py
@@ -419,7 +419,7 @@
 resolved_path = self.get_resolved_path()
 path_spec = lldb.SBFileSpec(resolved_path)
 error = lldb.SBError()
-target = lldb.debugger.CreateTarget(
+target = self.debugger.CreateTarget(
 resolved_path, self.arch, None, False, error)
 if target:
 self.module = target.FindModule(path_spec)
@@ -437,8 +437,9 @@
 
 class Symbolicator:
 
-def __init__(self):
+def __init__(self, debugger):
 """A class the represents the information needed to symbolicate addresses in a program"""
+self.debugger = debugger
 self.target = None
 self.images = list()  # a list of images to be used when symbolicating
 self.addr_mask = 0x
@@ -632,7 +633,7 @@
 print(sym)
 
 
-def Symbolicate(command_args):
+def Symbolicate(debugger, command_args):
 
 usage = "usage: %prog [options]  [addr2 ...]"
 description = '''Symbolicate one or more addresses using LLDB's python scripting API..'''
@@ -686,7 +687,7 @@
 (options, args) = parser.parse_args(command_args)
 except:
 return
-symbolicator = Symbolicator()
+symbolicator = Symbolicator(debugger)
 images = list()
 if options.file:
 image = Image(options.file)
@@ -720,5 +721,6 @@
 
 if __name__ == '__main__':
 # Create a new debugger instance
-lldb.debugger = lldb.SBDebugger.Create()
-Symbolicate(sys.argv[1:])
+debugger = lldb.SBDebugger.Create()
+Symbolicate(debugger, sys.argv[1:])
+SBDebugger.Destroy(debugger)
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -131,7 +131,7 @@
 if line_entry.IsValid():
 strm = lldb.SBStream()
 if line_entry:
-lldb.debugger.GetSourceManager().DisplaySourceLinesWithLineNumbers(
+crash_log.debugger.GetSourceManager().DisplaySourceLinesWithLineNumbers(
 line_entry.file, line_entry.line, source_context, source_context, "->", strm)
 source_text = strm.GetData()
 if source_text:
@@ -310,9 +310,9 @@
 self.unavailable = True
 return False
 
-def __init__(self, path, verbose):
+def __init__(self, debugger, path, verbose):
 """CrashLog constructor that take a path to a darwin crash log file"""
-symbolication.Symbolicator.__init__(self)
+symbolication.Symbolicator.__init__(self, debugger)
 self.path = os.path.expanduser(path)
 self.info_lines = list()
 self.system_profile = list()
@@ -411,12 +411,12 @@
  )
 
 
-def __init__(self, path, verbose):
+def __init__(self, debugger, path, verbose):
 self.path = os.path.expanduser(path)
 self.verbose = verbose
 self.thread = None
 self.app_specific_backtrace = False
-self.crashlog = CrashLog(self.path, self.verbose)
+self.crashlog = CrashLog(debugger, self.path, self.verbose)
 self.parse_mode = CrashLogParseMode.NORMAL
 self.parsers = {
 CrashLogParseMode.NORMAL : self.parse_normal,
@@ -711,7 +711,7 @@
 return False
 
 
-def interactive_crashlogs(options, args):
+def interactive_crashlogs(debugger, options, args):
 crash_log_files = list()
 for arg in args:
 for resolved_path in glob.glob(arg):
@@ -720,7 +720,7 @@
 crash_logs = list()
 for crash_log_file in crash_log_fil

[Lldb-commits] [PATCH] D90706: [crashlog] Pass the debugger around instead of relying on lldb.debugger

2020-11-03 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere updated this revision to Diff 302655.
JDevlieghere edited the summary of this revision.
JDevlieghere added a comment.

Fix partial upload.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90706/new/

https://reviews.llvm.org/D90706

Files:
  lldb/examples/python/crashlog.py
  lldb/examples/python/symbolication.py

Index: lldb/examples/python/symbolication.py
===
--- lldb/examples/python/symbolication.py
+++ lldb/examples/python/symbolication.py
@@ -410,7 +410,7 @@
 return str(self.uuid).upper()
 return None
 
-def create_target(self):
+def create_target(self, debugger):
 '''Create a target using the information in this Image object.'''
 if self.unavailable:
 return None
@@ -419,7 +419,7 @@
 resolved_path = self.get_resolved_path()
 path_spec = lldb.SBFileSpec(resolved_path)
 error = lldb.SBError()
-target = lldb.debugger.CreateTarget(
+target = debugger.CreateTarget(
 resolved_path, self.arch, None, False, error)
 if target:
 self.module = target.FindModule(path_spec)
@@ -437,8 +437,9 @@
 
 class Symbolicator:
 
-def __init__(self):
+def __init__(self, debugger):
 """A class the represents the information needed to symbolicate addresses in a program"""
+self.debugger = debugger
 self.target = None
 self.images = list()  # a list of images to be used when symbolicating
 self.addr_mask = 0x
@@ -496,7 +497,7 @@
 
 if self.images:
 for image in self.images:
-self.target = image.create_target()
+self.target = image.create_target(self.debugger)
 if self.target:
 if self.target.GetAddressByteSize() == 4:
 triple = self.target.triple
@@ -632,7 +633,7 @@
 print(sym)
 
 
-def Symbolicate(command_args):
+def Symbolicate(debugger, command_args):
 
 usage = "usage: %prog [options]  [addr2 ...]"
 description = '''Symbolicate one or more addresses using LLDB's python scripting API..'''
@@ -686,7 +687,7 @@
 (options, args) = parser.parse_args(command_args)
 except:
 return
-symbolicator = Symbolicator()
+symbolicator = Symbolicator(debugger)
 images = list()
 if options.file:
 image = Image(options.file)
@@ -720,5 +721,6 @@
 
 if __name__ == '__main__':
 # Create a new debugger instance
-lldb.debugger = lldb.SBDebugger.Create()
-Symbolicate(sys.argv[1:])
+debugger = lldb.SBDebugger.Create()
+Symbolicate(debugger, sys.argv[1:])
+SBDebugger.Destroy(debugger)
Index: lldb/examples/python/crashlog.py
===
--- lldb/examples/python/crashlog.py
+++ lldb/examples/python/crashlog.py
@@ -131,7 +131,7 @@
 if line_entry.IsValid():
 strm = lldb.SBStream()
 if line_entry:
-lldb.debugger.GetSourceManager().DisplaySourceLinesWithLineNumbers(
+crash_log.debugger.GetSourceManager().DisplaySourceLinesWithLineNumbers(
 line_entry.file, line_entry.line, source_context, source_context, "->", strm)
 source_text = strm.GetData()
 if source_text:
@@ -310,9 +310,9 @@
 self.unavailable = True
 return False
 
-def __init__(self, path, verbose):
+def __init__(self, debugger, path, verbose):
 """CrashLog constructor that take a path to a darwin crash log file"""
-symbolication.Symbolicator.__init__(self)
+symbolication.Symbolicator.__init__(self, debugger)
 self.path = os.path.expanduser(path)
 self.info_lines = list()
 self.system_profile = list()
@@ -360,12 +360,12 @@
 for ident in self.idents:
 image = self.find_image_with_identifier(ident)
 if image:
-self.target = image.create_target()
+self.target = image.create_target(self.debugger)
 if self.target:
 return self.target  # success
 print('crashlog.create_target()...3')
 for image in self.images:
-self.target = image.create_target()
+self.target = image.create_target(self.debugger)
 if self.target:
 return self.target  # success
 print('crashlog.create_target()...4')
@@ -411,12 +411,12 @@
  )
 
 
-def __init__(self, path, verbose):
+def __init__(self, debugger, path, verbose):
 self.path = os.p

[Lldb-commits] [lldb] b7de7be - [lldb] [test] Remove xfail from tests that pass on FreeBSD

2020-11-03 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2020-11-03T22:01:59+01:00
New Revision: b7de7be098d7f167d10acb713fe601ad15f37071

URL: 
https://github.com/llvm/llvm-project/commit/b7de7be098d7f167d10acb713fe601ad15f37071
DIFF: 
https://github.com/llvm/llvm-project/commit/b7de7be098d7f167d10acb713fe601ad15f37071.diff

LOG: [lldb] [test] Remove xfail from tests that pass on FreeBSD

Added: 


Modified: 
lldb/test/API/api/multiple-targets/TestMultipleTargets.py
lldb/test/API/functionalities/avoids-fd-leak/TestFdLeak.py

lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/TestDataFormatterSkipSummary.py
lldb/test/API/lang/cpp/enum_types/TestCPP11EnumTypes.py
lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
lldb/test/API/python_api/thread/TestThreadAPI.py

Removed: 




diff  --git a/lldb/test/API/api/multiple-targets/TestMultipleTargets.py 
b/lldb/test/API/api/multiple-targets/TestMultipleTargets.py
index 1268ff9306a3..eebaea3c4fd2 100644
--- a/lldb/test/API/api/multiple-targets/TestMultipleTargets.py
+++ b/lldb/test/API/api/multiple-targets/TestMultipleTargets.py
@@ -19,7 +19,7 @@ class TestMultipleTargets(TestBase):
 @skipIfNoSBHeaders
 @skipIfHostIncompatibleWithRemote
 @expectedFailureAll(
-oslist=["windows", "freebsd"],
+oslist=["windows"],
 bugnumber="llvm.org/pr20282")
 def test_multiple_targets(self):
 env = {self.dylibPath: self.getLLDBLibraryEnvVal()}

diff  --git a/lldb/test/API/functionalities/avoids-fd-leak/TestFdLeak.py 
b/lldb/test/API/functionalities/avoids-fd-leak/TestFdLeak.py
index bb0a9f99b0db..85772582f6bd 100644
--- a/lldb/test/API/functionalities/avoids-fd-leak/TestFdLeak.py
+++ b/lldb/test/API/functionalities/avoids-fd-leak/TestFdLeak.py
@@ -25,9 +25,6 @@ class AvoidsFdLeakTestCase(TestBase):
 mydir = TestBase.compute_mydir(__file__)
 
 @expectedFailureIfFn(python_leaky_fd_version, "bugs.freebsd.org/197376")
-@expectedFailureAll(
-oslist=['freebsd'],
-bugnumber="llvm.org/pr25624 still failing with Python 2.7.10")
 # The check for descriptor leakage needs to be implemented 
diff erently
 # here.
 @skipIfWindows

diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/TestDataFormatterSkipSummary.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/TestDataFormatterSkipSummary.py
index aa5e20bd2f75..cafd51c20947 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/TestDataFormatterSkipSummary.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-skip-summary/TestDataFormatterSkipSummary.py
@@ -13,9 +13,6 @@ class SkipSummaryDataFormatterTestCase(TestBase):
 
 mydir = TestBase.compute_mydir(__file__)
 
-@expectedFailureAll(
-oslist=['freebsd'],
-bugnumber="llvm.org/pr20548 fails to build on lab.llvm.org buildbot")
 @expectedFailureAll(
 oslist=["windows"],
 bugnumber="llvm.org/pr24462, Data formatters have problems on Windows")

diff  --git a/lldb/test/API/lang/cpp/enum_types/TestCPP11EnumTypes.py 
b/lldb/test/API/lang/cpp/enum_types/TestCPP11EnumTypes.py
index d40eee0cb1b0..b0a52e1552a6 100644
--- a/lldb/test/API/lang/cpp/enum_types/TestCPP11EnumTypes.py
+++ b/lldb/test/API/lang/cpp/enum_types/TestCPP11EnumTypes.py
@@ -37,7 +37,6 @@ def check_enum(self, suffix):
 self.expect_expr("var_below_" + suffix, result_type=enum_name, 
result_value="-3")
 self.expect_expr("var_above_" + suffix, result_type=enum_name, 
result_value="1")
 
-@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr36527')
 @skipIf(dwarf_version=['<', '4'])
 def test(self):
 self.build()

diff  --git a/lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py 
b/lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
index fa8e2ddbbed5..5761ab7ef170 100644
--- a/lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
+++ b/lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
@@ -38,9 +38,6 @@ def runToBkpt(self, command):
 substrs=['stopped',
  'stop reason = breakpoint'])
 
-@expectedFailureAll(
-oslist=["freebsd"],
-bugnumber="llvm.org/pr25819")
 @skipIfWindows # This is flakey on Windows: llvm.org/pr38373
 def test_scope_lookup_with_run_command(self):
 """Test scope lookup of functions in lldb."""
@@ -225,9 +222,6 @@ def test_scope_lookup_before_using_with_run_command(self):
 compiler="gcc",
 oslist=["linux"],
 debug_info=["dwo"])  # Skip to avoid crash
-@expectedFailureAll(
-oslist=["freebsd"],
-bugnumber="llvm.org/pr25819")
 def test_scope_after_using_directive_lookup_with_run_command(self):
 """Test scope lookup after using directive in lldb."""
 self.build()
@@ -2

[Lldb-commits] [lldb] 051da2b - [lldb] [test/Shell] Pass -pthread to host toolchain on FreeBSD too

2020-11-03 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2020-11-03T22:01:59+01:00
New Revision: 051da2bede4ba7231d009b5708d1191dde1e5dde

URL: 
https://github.com/llvm/llvm-project/commit/051da2bede4ba7231d009b5708d1191dde1e5dde
DIFF: 
https://github.com/llvm/llvm-project/commit/051da2bede4ba7231d009b5708d1191dde1e5dde.diff

LOG: [lldb] [test/Shell] Pass -pthread to host toolchain on FreeBSD too

Added: 


Modified: 
lldb/test/Shell/helper/toolchain.py

Removed: 




diff  --git a/lldb/test/Shell/helper/toolchain.py 
b/lldb/test/Shell/helper/toolchain.py
index 2b075d5523d4..cda66b652961 100644
--- a/lldb/test/Shell/helper/toolchain.py
+++ b/lldb/test/Shell/helper/toolchain.py
@@ -116,7 +116,7 @@ def use_support_substitutions(config):
 sdk_path = lit.util.to_string(out)
 llvm_config.lit_config.note('using SDKROOT: %r' % sdk_path)
 host_flags += ['-isysroot', sdk_path]
-elif platform.system() in ['NetBSD', 'OpenBSD', 'Linux']:
+elif platform.system() in ['FreeBSD', 'NetBSD', 'OpenBSD', 'Linux']:
 host_flags += ['-pthread']
 
 if sys.platform.startswith('netbsd'):



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 98257c3 - [lldb] [test] Update XFAILs/skips for FreeBSD

2020-11-03 Thread Michał Górny via lldb-commits

Author: Michał Górny
Date: 2020-11-03T22:01:59+01:00
New Revision: 98257c30065a7c85685a60653df7075cf95281dd

URL: 
https://github.com/llvm/llvm-project/commit/98257c30065a7c85685a60653df7075cf95281dd
DIFF: 
https://github.com/llvm/llvm-project/commit/98257c30065a7c85685a60653df7075cf95281dd.diff

LOG: [lldb] [test] Update XFAILs/skips for FreeBSD

Update expected failures and test skips based on common results
for the old and new FreeBSD plugins.

Added: 


Modified: 
lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py
lldb/test/API/api/multithreaded/TestMultithreaded.py

lldb/test/API/commands/expression/multiline-navigation/TestMultilineNavigation.py
lldb/test/API/commands/log/basic/TestLogging.py
lldb/test/API/commands/platform/process/list/TestProcessList.py
lldb/test/API/commands/process/launch/TestProcessLaunch.py
lldb/test/API/commands/target/create-deps/TestTargetCreateDeps.py

lldb/test/API/functionalities/breakpoint/breakpoint_callback_command_source/TestBreakpointCallbackCommandSource.py

lldb/test/API/functionalities/breakpoint/global_constructor/TestBreakpointInGlobalConstructor.py

lldb/test/API/functionalities/dlopen_other_executable/TestDlopenOtherExecutable.py
lldb/test/API/functionalities/memory-region/TestMemoryRegion.py

lldb/test/API/functionalities/thread/exit_during_expression/TestExitDuringExpression.py

lldb/test/API/functionalities/thread/state_after_expression/TestStateAfterExpression.py
lldb/test/API/lang/c/conflicting-symbol/TestConflictingSymbol.py
lldb/test/API/lang/cpp/thread_local/TestThreadLocal.py
lldb/test/API/tools/lldb-vscode/breakpoint/TestVSCode_setBreakpoints.py
lldb/test/API/tools/lldb-vscode/launch/TestVSCode_launch.py
lldb/test/Shell/Unwind/thread-step-out-ret-addr-check.test

Removed: 




diff  --git a/lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py 
b/lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py
index d3a69a10baa5..1b120114f710 100644
--- a/lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py
+++ b/lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py
@@ -19,6 +19,7 @@ class TestMultipleSimultaneousDebuggers(TestBase):
 
 @skipIfNoSBHeaders
 @skipIfWindows
+@expectedFailureAll(oslist=['freebsd'])
 def test_multiple_debuggers(self):
 env = {self.dylibPath: self.getLLDBLibraryEnvVal()}
 

diff  --git a/lldb/test/API/api/multithreaded/TestMultithreaded.py 
b/lldb/test/API/api/multithreaded/TestMultithreaded.py
index 60c2c3b372cb..1dc44b9a9448 100644
--- a/lldb/test/API/api/multithreaded/TestMultithreaded.py
+++ b/lldb/test/API/api/multithreaded/TestMultithreaded.py
@@ -31,6 +31,7 @@ def setUp(self):
 @skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
+@expectedFailureAll(oslist=['freebsd'])
 def test_python_stop_hook(self):
 """Test that you can run a python command in a stop-hook when stdin is 
File based. """
 self.build_and_test('driver.cpp test_stop-hook.cpp',

diff  --git 
a/lldb/test/API/commands/expression/multiline-navigation/TestMultilineNavigation.py
 
b/lldb/test/API/commands/expression/multiline-navigation/TestMultilineNavigation.py
index 743e79945364..b932abab82a2 100644
--- 
a/lldb/test/API/commands/expression/multiline-navigation/TestMultilineNavigation.py
+++ 
b/lldb/test/API/commands/expression/multiline-navigation/TestMultilineNavigation.py
@@ -18,6 +18,7 @@ class TestCase(PExpectTest):
 # under ASAN on a loaded machine..
 @skipIfAsan
 @skipIfEditlineSupportMissing
+@expectedFailureAll(oslist=['freebsd'])
 def test_nav_arrow_up(self):
 """Tests that we can navigate back to the previous line with the up 
arrow"""
 self.launch()
@@ -40,6 +41,7 @@ def test_nav_arrow_up(self):
 
 @skipIfAsan
 @skipIfEditlineSupportMissing
+@expectedFailureAll(oslist=['freebsd'])
 def test_nav_arrow_down(self):
 """Tests that we can navigate to the next line with the down arrow"""
 self.launch()

diff  --git a/lldb/test/API/commands/log/basic/TestLogging.py 
b/lldb/test/API/commands/log/basic/TestLogging.py
index 4ba67f8794b6..da1a3e8a50cf 100644
--- a/lldb/test/API/commands/log/basic/TestLogging.py
+++ b/lldb/test/API/commands/log/basic/TestLogging.py
@@ -93,6 +93,8 @@ def test_log_append(self):
 
 # Enable all log options and check that nothing crashes.
 @skipIfWindows
+# TODO: figure out why it segfaults
+@skipIfFreeBSD
 def test_all_log_options(self):
 if (os.path.exists(self.log_file)):
 os.remove(self.log_file)

diff  --git a/lldb/test/API/commands/platform/process/list/TestProcessList.py 
b/lldb/test/API/commands/platform/process/list/TestProcessList.py
index fe2ed74916eb..1bc37b542b19 100644
--- a/lldb/test/API/commands/platform/proc

[Lldb-commits] [lldb] 33945cd - [NFC] Fix call to lldb RegisterValue constructor

2020-11-03 Thread Jordan Rupprecht via lldb-commits

Author: Jordan Rupprecht
Date: 2020-11-03T13:24:06-08:00
New Revision: 33945cdd62c40ea4ce381d4c3d49b22f8a2cc015

URL: 
https://github.com/llvm/llvm-project/commit/33945cdd62c40ea4ce381d4c3d49b22f8a2cc015
DIFF: 
https://github.com/llvm/llvm-project/commit/33945cdd62c40ea4ce381d4c3d49b22f8a2cc015.diff

LOG: [NFC] Fix call to lldb RegisterValue constructor

Added: 


Modified: 
lldb/source/Plugins/ABI/ARC/ABISysV_arc.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ABI/ARC/ABISysV_arc.cpp 
b/lldb/source/Plugins/ABI/ARC/ABISysV_arc.cpp
index 540270fb9544..be8586722d8f 100644
--- a/lldb/source/Plugins/ABI/ARC/ABISysV_arc.cpp
+++ b/lldb/source/Plugins/ABI/ARC/ABISysV_arc.cpp
@@ -272,7 +272,8 @@ bool ABISysV_arc::PrepareTrivialCall(Thread &thread, addr_t 
sp, addr_t pc,
 reg_value[byte_index++] = 0;
   }
 
-  RegisterValue reg_val_obj(reg_value, reg_size, eByteOrderLittle);
+  RegisterValue reg_val_obj(llvm::makeArrayRef(reg_value, reg_size),
+eByteOrderLittle);
   if (!reg_ctx->WriteRegister(
 reg_ctx->GetRegisterInfo(eRegisterKindGeneric, reg_index),
 reg_val_obj))



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D90729: [trace][intel-pt] Scaffold the 'thread trace start | stop' commands

2020-11-03 Thread walter erquinigo via Phabricator via lldb-commits
wallace created this revision.
wallace added a reviewer: clayborg.
Herald added subscribers: lldb-commits, dang, mgorny.
Herald added a reviewer: JDevlieghere.
Herald added a project: LLDB.
wallace requested review of this revision.

Depends on D90490 .

The stop command is simple and invokes the new method 
Trace::StopTracingThread(thread).

On the other hand, the start command works by delegating its implementation to 
a CommandObject provided by the Trace plugin. This is necessary because each 
trace plugin needs different options for this command. There's even the chance 
that a Trace plugin can't support live tracing, but instead supports offline 
decoding and analysis, which means that "thread trace dump instructions" works 
but "thread trace start" doest. Because of this and a few other reasons, it's 
better to have each plugin provide this implementation.

Besides, I'm using the GetSupportedTraceType method introduced in D90490 
 to quickly infer what's the trace plug-in 
that works for the current process.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90729

Files:
  lldb/include/lldb/Core/PluginManager.h
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/CommandObjectThread.h
  lldb/source/Core/PluginManager.cpp
  lldb/source/Interpreter/CommandObject.cpp
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py
  lldb/test/API/commands/trace/TestTraceStartStop.py

Index: lldb/test/API/commands/trace/TestTraceStartStop.py
===
--- /dev/null
+++ lldb/test/API/commands/trace/TestTraceStartStop.py
@@ -0,0 +1,67 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+
+class TestTraceLoad(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def setUp(self):
+TestBase.setUp(self)
+if 'intel-pt' not in configuration.enabled_plugins:
+self.skipTest("The intel-pt test plugin is not enabled")
+
+def expectGenericHelpMessageForStartCommand(self):
+self.expect("help thread trace start",
+substrs=["Syntax: thread trace start []"])
+
+def testStartStopSessionFileThreads(self):
+# it should fail for processes from json session files
+self.expect("trace load -v " + os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"))
+self.expect("thread trace start", error=True,
+substrs=["error: tracing is not available for the current process. Not implemented"])
+
+# the help command should be the generic one, as it's not a live process
+self.expectGenericHelpMessageForStartCommand()
+
+# this should fail because 'trace stop' is not yet implemented
+self.expect("thread trace stop", error=True,
+substrs=["error: failed stopping thread 3842849"])
+
+@skipIf(oslist=no_match(['linux']), archs=no_match(['i386', 'x86_64']))
+def testStartStopLiveThreads(self):
+# The help command should be the generic one if there's no process running
+self.expectGenericHelpMessageForStartCommand()
+
+self.expect("file " + os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+self.expect("b main")
+
+# The help command should be the generic one if there's still no process running
+self.expectGenericHelpMessageForStartCommand()
+
+self.expect("r")
+
+# the help command should be the intel-pt one now
+self.expect("help thread trace start",
+substrs=["Start tracing one or more threads with intel-pt.",
+ "Syntax: thread trace start [  ...] []"])
+
+self.expect("thread trace start",
+patterns=["would trace tid .* with size_in_kb 4 and custom_config 0"])
+
+self.expect("thread trace start --size 20 --custom-config 1",
+patterns=["would trace tid .* with size_in_kb 20 and custom_config 1"])
+
+# This fails because "trace stop" is not yet implemented.
+self.expect("thread trace stop", error=True,
+substrs=["error: Process is not being traced"])
+
+self.expect("c")
+# Now the process has finished, so the commands should fail
+self.expect("thread trace start", error=True,
+substrs=["error: Process must be launched."])
+
+self.expect("thread trace stop", error=True,
+ 

[Lldb-commits] [PATCH] D90741: [LLDB] Fix SVE reginfo for sequential offset in g packet

2020-11-03 Thread Muhammad Omair Javaid via Phabricator via lldb-commits
omjavaid created this revision.
omjavaid added a reviewer: labath.
Herald added subscribers: kristof.beyls, tschuett.
Herald added a reviewer: rengolin.
omjavaid requested review of this revision.

This moves in the direction of our effort to synchronize register descriptions 
between LLDB and GDB xml description. We want to able to send registers in a 
way that their offset fields can be re-constructed based on register sizes in 
the increasing order of register number.

In context to Arm64 SVE, FPCR and FPSR are same registers in FPU regset and SVE 
regset. Previously FPSR/FPCR offset was set at the end of SVE data because 
Linux ptrace data placed FPCR and FPSR at the end of SVE register set.

Now we have two options

1. to include 2 additional primary FPCR/FPSR at the end of SVE regset and make 
existing FPCR/FPSR in FPU regset as their value regs.
2. use the same register for both SVE and FPU register set.

I have taken the second choice and considering interoperability with other 
stubs like QEMU and that g packets should generate register data in increasing 
order of register numbers. We have to move FPCR/FPSR offset up to its original 
location according to register numbering scheme of ARM64 registers with SVE 
registers included.


https://reviews.llvm.org/D90741

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp


Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
@@ -89,7 +89,7 @@
 const uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
 sve_reg_offset = sve::ptrace_fpsimd_offset + (reg - GetRegNumSVEZ0()) * 16;
   } else if (m_sve_state == SVEState::Full) {
-uint32_t sve_z0_offset = GetGPRSize() + 8;
+uint32_t sve_z0_offset = GetGPRSize() + 16;
 sve_reg_offset =
 sve::SigRegsOffset() + reg_info->byte_offset - sve_z0_offset;
   }
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -288,8 +288,10 @@
 
 uint32_t offset = SVE_REGS_DEFAULT_OFFSET_LINUX;
 
-reg_info_ref[sve_vg].byte_offset = offset;
-offset += reg_info_ref[sve_vg].byte_size;
+reg_info_ref[fpu_fpsr].byte_offset = offset;
+reg_info_ref[fpu_fpcr].byte_offset = offset + 4;
+reg_info_ref[sve_vg].byte_offset = offset + 8;
+offset += 16;
 
 // Update Z registers size and offset
 uint32_t s_reg_base = fpu_s0;
@@ -314,8 +316,7 @@
   offset += reg_info_ref[it].byte_size;
 }
 
-reg_info_ref[fpu_fpsr].byte_offset = offset;
-reg_info_ref[fpu_fpcr].byte_offset = offset + 4;
+m_per_vq_reg_infos[sve_vq] = reg_info_ref;
   }
 
   m_register_info_p = reg_info_ref.data();
Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -,7 +,7 @@
 sve_reg_offset =
 SVE_PT_FPSIMD_OFFSET + (reg - GetRegisterInfo().GetRegNumSVEZ0()) * 16;
   } else if (m_sve_state == SVEState::Full) {
-uint32_t sve_z0_offset = GetGPRSize() + 8;
+uint32_t sve_z0_offset = GetGPRSize() + 16;
 sve_reg_offset =
 SVE_SIG_REGS_OFFSET + reg_info->byte_offset - sve_z0_offset;
   }


Index: lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
===
--- lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
+++ lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
@@ -89,7 +89,7 @@
 const uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
 sve_reg_offset = sve::ptrace_fpsimd_offset + (reg - GetRegNumSVEZ0()) * 16;
   } else if (m_sve_state == SVEState::Full) {
-uint32_t sve_z0_offset = GetGPRSize() + 8;
+uint32_t sve_z0_offset = GetGPRSize() + 16;
 sve_reg_offset =
 sve::SigRegsOffset() + reg_info->byte_offset - sve_z0_offset;
   }
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -288,8 +288,10 @@
 
 uint32_t offset = SVE_REGS_DEFAULT_OFFSET_LINUX;
 
-reg_info_ref[sve_vg].byte_offset = offset;
-offset += reg_i