[Lldb-commits] [PATCH] D123358: [trace][intelpt] Remove code smell when printing the raw trace size

2022-04-07 Thread walter erquinigo via Phabricator via lldb-commits 
wallace created this revision.
wallace added reviewers: jj10306, zrthxn.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Something ugly I did was to report the trace buffer size to the DecodedThread,
which is later used as part of the `dump info` command. Instead of doing that,
we can just directly ask the trace for the raw buffer and print its size.

I thought about not asking for the entire trace but instead just for its size,
but in this case, as our traces as not extremely big, I prefer to ask for the
entire trace, ensuring it could be fetched, and then print its size.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123358

Files:
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h

Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
@@ -73,7 +73,7 @@
 
   void DumpTraceInfo(Thread , Stream , bool verbose) override;
 
-  llvm::Optional GetRawTraceSize(Thread );
+  llvm::Expected GetRawTraceSize(Thread );
 
   void DoRefreshLiveProcessState(
   llvm::Expected state) override;
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -106,20 +106,27 @@
 }
 
 void TraceIntelPT::DumpTraceInfo(Thread , Stream , bool verbose) {
-  Optional raw_size = GetRawTraceSize(thread);
+  lldb::tid_t tid = thread.GetID();
   s.Format("\nthread #{0}: tid = {1}", thread.GetIndexID(), thread.GetID());
-  if (!raw_size) {
+  if (!IsTraced(tid)) {
 s << ", not traced\n";
 return;
   }
   s << "\n";
+
+  Expected raw_size = GetRawTraceSize(thread);
+  if (!raw_size) {
+s.Format("  {0}\n", toString(raw_size.takeError()));
+return;
+  }
+
   DecodedThreadSP decoded_trace_sp = Decode(thread);
   size_t insn_len = decoded_trace_sp->GetInstructionsCount();
   size_t mem_used = decoded_trace_sp->CalculateApproximateMemoryUsage();
 
   s.Format("  Total number of instructions: {0}\n", insn_len);
 
-  s.PutCString("\n  Memory usage:\n");
+  s << "\n  Memory usage:\n";
   s.Format("Raw trace size: {0} KiB\n", *raw_size / 1024);
   s.Format(
   "Total approximate memory usage (excluding raw trace): {0:2} KiB\n",
@@ -129,15 +136,13 @@
  "{0:2} bytes\n",
  (double)mem_used / insn_len);
 
-  s.PutCString("\n  Timing:\n");
-  GetTimer()
-  .ForThread(thread.GetID())
-  .ForEachTimedTask(
-  [&](const std::string , std::chrono::milliseconds duration) {
-s.Format("{0}: {1:2}s\n", name, duration.count() / 1000.0);
-  });
+  s << "\n  Timing:\n";
+  GetTimer().ForThread(tid).ForEachTimedTask(
+  [&](const std::string , std::chrono::milliseconds duration) {
+s.Format("{0}: {1:2}s\n", name, duration.count() / 1000.0);
+  });
 
-  s.PutCString("\n  Errors:\n");
+  s << "\n  Errors:\n";
   const DecodedThread::LibiptErrors _errors =
   decoded_trace_sp->GetTscErrors();
   s.Format("Number of TSC decoding errors: {0}\n", tsc_errors.total_count);
@@ -147,11 +152,16 @@
   }
 }
 
-Optional TraceIntelPT::GetRawTraceSize(Thread ) {
-  if (IsTraced(thread.GetID()))
-return Decode(thread)->GetRawTraceSize();
-  else
-return None;
+llvm::Expected TraceIntelPT::GetRawTraceSize(Thread ) {
+  size_t size;
+  auto callback = [&](llvm::ArrayRef data) {
+size = data.size();
+return Error::success();
+  };
+  if (Error err = OnThreadBufferRead(thread.GetID(), callback))
+return std::move(err);
+
+  return size;
 }
 
 Expected TraceIntelPT::GetCPUInfoForLiveProcess() {
Index: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
+++ lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
@@ -290,8 +290,6 @@
 void lldb_private::trace_intel_pt::DecodeTrace(DecodedThread _thread,
TraceIntelPT _intel_pt,
ArrayRef buffer) {
-  decoded_thread.SetRawTraceSize(buffer.size());
-
   Expected decoder_up =
   CreateInstructionDecoder(decoded_thread, trace_intel_pt, buffer);
   if (!decoder_up)
Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
===
--- lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@

[Lldb-commits] [PATCH] D123357: [trace][intelpt] Add task timer classes

2022-04-07 Thread walter erquinigo via Phabricator via lldb-commits 
wallace created this revision.
Herald added a subscriber: mgorny.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

I'm adding two new classes that can be used to measure the duration of long
tasks as process and thread level, e.g. decoding, fetching data from
lldb-server, etc. In this first patch, I'm using it to measure the time it takes
to decode each thread, which is printed out with the `dump info` command. In a
later patch I'll start adding process-level tasks and I might move these
classes to the upper Trace level, instead of having them in the intel-pt
plugin. I might need to do that anyway in the future when we have to
measure HTR. For now, I want to keep the impact of this change minimal.

With it, I was able to generate the following info of a very big trace:

  (lldb) thread trace dump info 
   Trace technology: 
intel-pt
  
  thread #1: tid = 616081
Total number of instructions: 9729366
  
Memory usage:
  Raw trace size: 1024 KiB
  Total approximate memory usage (excluding raw trace): 123517.34 KiB
  Average memory usage per instruction (excluding raw trace): 13.00 bytes
  
Timing:
  Decoding instructions: 1.62s
  
Errors:
  Number of TSC decoding errors: 0

As seen above, it took 1.62 seconds to decode 9.7M instructions. This is great
news, as we don't need to do any optimization work in this area.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123357

Files:
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/TaskTimer.cpp
  lldb/source/Plugins/Trace/intel-pt/TaskTimer.h
  lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/test/API/commands/trace/TestTraceDumpInfo.py
  lldb/test/API/commands/trace/TestTraceLoad.py

Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -36,12 +36,18 @@
 self.expect("thread trace dump info", substrs=['''Trace technology: intel-pt
 
 thread #1: tid = 3842849
-  Raw trace size: 4 KiB
   Total number of instructions: 21
-  Total approximate memory usage: 0.27 KiB
-  Average memory usage per instruction: 13.00 bytes
 
-  Number of TSC decoding errors: 0'''])
+  Memory usage:
+Raw trace size: 4 KiB
+Total approximate memory usage (excluding raw trace): 0.27 KiB
+Average memory usage per instruction (excluding raw trace): 13.00 bytes
+
+  Timing:
+Decoding instructions: ''', '''s
+
+  Errors:
+Number of TSC decoding errors: 0'''])
 
 def testLoadInvalidTraces(self):
 src_dir = self.getSourceDir()
Index: lldb/test/API/commands/trace/TestTraceDumpInfo.py
===
--- lldb/test/API/commands/trace/TestTraceDumpInfo.py
+++ lldb/test/API/commands/trace/TestTraceDumpInfo.py
@@ -38,9 +38,16 @@
 substrs=['''Trace technology: intel-pt
 
 thread #1: tid = 3842849
-  Raw trace size: 4 KiB
   Total number of instructions: 21
-  Total approximate memory usage: 0.27 KiB
-  Average memory usage per instruction: 13.00 bytes
 
-  Number of TSC decoding errors: 0'''])
+  Memory usage:
+Raw trace size: 4 KiB
+Total approximate memory usage (excluding raw trace): 0.27 KiB
+Average memory usage per instruction (excluding raw trace): 13.00 bytes
+
+  Timing:
+Decoding instructions: ''', '''s
+
+  Errors:
+Number of TSC decoding errors: 0'''],
+patterns=["Decoding instructions: \d.\d\ds"])
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
@@ -9,6 +9,7 @@
 #ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACEINTELPT_H
 #define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACEINTELPT_H
 
+#include "TaskTimer.h"
 #include "ThreadDecoder.h"
 #include "TraceIntelPTSessionFileParser.h"
 #include "lldb/Utility/FileSpec.h"
@@ -150,6 +151,10 @@
   /// return \a nullptr.
   Process *GetLiveProcess();
 
+  /// \return
+  /// The timer object for this trace.
+  TaskTimer ();
+
 private:
   friend class TraceIntelPTSessionFileParser;
 
@@ -184,6 +189,7 @@
   std::map> m_thread_decoders;
   /// Error gotten after a failed live process update, if any.
   llvm::Optional m_live_refresh_error;
+  TaskTimer m_task_timer;
 };
 
 } // namespace trace_intel_pt
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp

[Lldb-commits] [PATCH] D123356: add task timer

2022-04-07 Thread walter erquinigo via Phabricator via lldb-commits 
wallace created this revision.
Herald added a subscriber: mgorny.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123356

Files:
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/TaskTimer.cpp
  lldb/source/Plugins/Trace/intel-pt/TaskTimer.h
  lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/test/API/commands/trace/TestTraceDumpInfo.py
  lldb/test/API/commands/trace/TestTraceLoad.py

Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -36,12 +36,18 @@
 self.expect("thread trace dump info", substrs=['''Trace technology: intel-pt
 
 thread #1: tid = 3842849
-  Raw trace size: 4 KiB
   Total number of instructions: 21
-  Total approximate memory usage: 0.27 KiB
-  Average memory usage per instruction: 13.00 bytes
 
-  Number of TSC decoding errors: 0'''])
+  Memory usage:
+Raw trace size: 4 KiB
+Total approximate memory usage (excluding raw trace): 0.27 KiB
+Average memory usage per instruction (excluding raw trace): 13.00 bytes
+
+  Timing:
+Decoding instructions: ''', '''s
+
+  Errors:
+Number of TSC decoding errors: 0'''])
 
 def testLoadInvalidTraces(self):
 src_dir = self.getSourceDir()
Index: lldb/test/API/commands/trace/TestTraceDumpInfo.py
===
--- lldb/test/API/commands/trace/TestTraceDumpInfo.py
+++ lldb/test/API/commands/trace/TestTraceDumpInfo.py
@@ -38,9 +38,16 @@
 substrs=['''Trace technology: intel-pt
 
 thread #1: tid = 3842849
-  Raw trace size: 4 KiB
   Total number of instructions: 21
-  Total approximate memory usage: 0.27 KiB
-  Average memory usage per instruction: 13.00 bytes
 
-  Number of TSC decoding errors: 0'''])
+  Memory usage:
+Raw trace size: 4 KiB
+Total approximate memory usage (excluding raw trace): 0.27 KiB
+Average memory usage per instruction (excluding raw trace): 13.00 bytes
+
+  Timing:
+Decoding instructions: ''', '''s
+
+  Errors:
+Number of TSC decoding errors: 0'''],
+patterns=["Decoding instructions: \d.\d\ds"])
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
@@ -9,6 +9,7 @@
 #ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACEINTELPT_H
 #define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACEINTELPT_H
 
+#include "TaskTimer.h"
 #include "ThreadDecoder.h"
 #include "TraceIntelPTSessionFileParser.h"
 #include "lldb/Utility/FileSpec.h"
@@ -150,6 +151,10 @@
   /// return \a nullptr.
   Process *GetLiveProcess();
 
+  /// \return
+  /// The timer object for this trace.
+  TaskTimer ();
+
 private:
   friend class TraceIntelPTSessionFileParser;
 
@@ -184,6 +189,7 @@
   std::map> m_thread_decoders;
   /// Error gotten after a failed live process update, if any.
   llvm::Optional m_live_refresh_error;
+  TaskTimer m_task_timer;
 };
 
 } // namespace trace_intel_pt
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -117,19 +117,32 @@
   size_t insn_len = decoded_trace_sp->GetInstructionsCount();
   size_t mem_used = decoded_trace_sp->CalculateApproximateMemoryUsage();
 
-  s.Format("  Raw trace size: {0} KiB\n", *raw_size / 1024);
   s.Format("  Total number of instructions: {0}\n", insn_len);
-  s.Format("  Total approximate memory usage: {0:2} KiB\n",
-   (double)mem_used / 1024);
+
+  s.PutCString("\n  Memory usage:\n");
+  s.Format("Raw trace size: {0} KiB\n", *raw_size / 1024);
+  s.Format(
+  "Total approximate memory usage (excluding raw trace): {0:2} KiB\n",
+  (double)mem_used / 1024);
   if (insn_len != 0)
-s.Format("  Average memory usage per instruction: {0:2} bytes\n",
+s.Format("Average memory usage per instruction (excluding raw trace): "
+ "{0:2} bytes\n",
  (double)mem_used / insn_len);
 
+  s.PutCString("\n  Timing:\n");
+  GetTimer()
+  .ForThread(thread.GetID())
+  .ForEachTimedTask(
+  [&](const std::string , std::chrono::milliseconds duration) {
+s.Format("{0}: {1:2}s\n", name, duration.count() / 1000.0);
+  });
+
+  s.PutCString("\n  Errors:\n");
   const DecodedThread::LibiptErrors _errors =

[Lldb-commits] [PATCH] D123281: [trace][intel pt] Create a common accessor for live and postmortem data

2022-04-07 Thread 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 rGe0cfe20ad2fb: [trace][intel pt] Create a common accessor for 
live and postmortem data (authored by Walter Erquinigo wall...@fb.com).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123281

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp
  lldb/source/Plugins/Trace/common/TraceSessionSaver.h
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
  lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
  lldb/source/Target/Trace.cpp

Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -215,3 +215,64 @@
   RefreshLiveProcessState();
   return m_stop_id;
 }
+
+llvm::Expected
+Trace::GetPostMortemThreadDataFile(lldb::tid_t tid, llvm::StringRef kind) {
+  auto NotFoundError = [&]() {
+return createStringError(
+inconvertibleErrorCode(),
+formatv("The thread with tid={0} doesn't have the tracing data {1}",
+tid, kind));
+  };
+
+  auto it = m_postmortem_thread_data.find(tid);
+  if (it == m_postmortem_thread_data.end())
+return NotFoundError();
+
+  std::unordered_map _kind_to_file_spec_map =
+  it->second;
+  auto it2 = data_kind_to_file_spec_map.find(kind.str());
+  if (it2 == data_kind_to_file_spec_map.end())
+return NotFoundError();
+  return it2->second;
+}
+
+void Trace::SetPostMortemThreadDataFile(lldb::tid_t tid, llvm::StringRef kind,
+FileSpec file_spec) {
+  m_postmortem_thread_data[tid][kind.str()] = file_spec;
+}
+
+llvm::Error
+Trace::OnLiveThreadBinaryDataRead(lldb::tid_t tid, llvm::StringRef kind,
+  OnBinaryDataReadCallback callback) {
+  Expected> data = GetLiveThreadBinaryData(tid, kind);
+  if (!data)
+return data.takeError();
+  return callback(*data);
+}
+
+llvm::Error
+Trace::OnPostMortemThreadBinaryDataRead(lldb::tid_t tid, llvm::StringRef kind,
+OnBinaryDataReadCallback callback) {
+  Expected file = GetPostMortemThreadDataFile(tid, kind);
+  if (!file)
+return file.takeError();
+  ErrorOr> trace_or_error =
+  MemoryBuffer::getFile(file->GetPath());
+  if (std::error_code err = trace_or_error.getError())
+return errorCodeToError(err);
+
+  MemoryBuffer  = **trace_or_error;
+  ArrayRef array_ref(
+  reinterpret_cast(data.getBufferStart()),
+  data.getBufferSize());
+  return callback(array_ref);
+}
+
+llvm::Error Trace::OnThreadBinaryDataRead(lldb::tid_t tid, llvm::StringRef kind,
+  OnBinaryDataReadCallback callback) {
+  if (m_live_process)
+return OnLiveThreadBinaryDataRead(tid, kind, callback);
+  else
+return OnPostMortemThreadBinaryDataRead(tid, kind, callback);
+}
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
@@ -48,15 +48,8 @@
 return json_intel_pt_trace.takeError();
 
   llvm::Expected json_session_description =
-  TraceSessionSaver::BuildProcessesSection(
-  *live_process,
-  [&](lldb::tid_t tid)
-  -> llvm::Expected>> {
-if (!trace_ipt.IsTraced(tid))
-  return None;
-return trace_ipt.GetLiveThreadBuffer(tid);
-  },
-  directory);
+  TraceSessionSaver::BuildProcessesSection(*live_process,
+   "threadTraceBuffer", directory);
 
   if (!json_session_description)
 return json_session_description.takeError();
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
@@ -137,8 +137,9 @@
 StructuredData::ObjectSP configuration =
 StructuredData::ObjectSP()) override;
 
-  /// Get the thread buffer content for a live thread
-  llvm::Expected> GetLiveThreadBuffer(lldb::tid_t tid);
+  /// See \a Trace::OnThreadBinaryDataRead().
+  llvm::Error OnThreadBufferRead(lldb::tid_t tid,
+ OnBinaryDataReadCallback callback);
 
   llvm::Expected GetCPUInfo();
 
Index:

[Lldb-commits] [lldb] e0cfe20 - [trace][intel pt] Create a common accessor for live and postmortem data

2022-04-07 Thread Walter Erquinigo via lldb-commits 

Author: Walter Erquinigo
Date: 2022-04-07T15:58:44-07:00
New Revision: e0cfe20ad2fb8c7aab3d6e82c42649eacf595d9f

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

LOG: [trace][intel pt] Create a common accessor for live and postmortem data

Some parts of the code have to distinguish between live and postmortem threads
to figure out how to get some data, e.g. thread trace buffers. This makes the
code less generic and more error prone. An example of that is that we have
two different decoders: LiveThreadDecoder and PostMortemThreadDecoder. They
exist because getting the trace bufer is different for each case.

The problem doesn't stop there. Soon we'll have even more kinds of data, like
the context switch trace, whose fetching will be different for live and post-
mortem processes.

As a way to fix this, I'm creating a common API for accessing thread data,
which is able to figure out how to handle the postmortem and live cases on
behalf of the caller. As a result of that, I was able to eliminate the two
decoders and unify them into a simpler one. Not only that, our TraceSave
functionality only worked for live threads, but now it can also work for
postmortem processes, which might be useful now, but it might in the future.

This common API is OnThreadBinaryDataRead. More information in the inline
documentation.

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

Added: 


Modified: 
lldb/include/lldb/Target/Trace.h
lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp
lldb/source/Plugins/Trace/common/TraceSessionSaver.h
lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
lldb/source/Target/Trace.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Trace.h 
b/lldb/include/lldb/Target/Trace.h
index c4ca192a1c263..78575119b680b 100644
--- a/lldb/include/lldb/Target/Trace.h
+++ b/lldb/include/lldb/Target/Trace.h
@@ -233,15 +233,76 @@ class Trace : public PluginInterface,
   /// \a llvm::Error otherwise.
   llvm::Error Stop();
 
-  /// Get the trace file of the given post mortem thread.
-  llvm::Expected GetPostMortemTraceFile(lldb::tid_t tid);
-
   /// \return
   /// The stop ID of the live process being traced, or an invalid stop ID
   /// if the trace is in an error or invalid state.
   uint32_t GetStopID();
 
+  using OnBinaryDataReadCallback =
+  std::function data)>;
+  /// Fetch binary data associated with a thread, either live or postmortem, 
and
+  /// pass it to the given callback. The reason of having a callback is to free
+  /// the caller from having to manage the life cycle of the data and to hide
+  /// the 
diff erent data fetching procedures that exist for live and post mortem
+  /// threads.
+  ///
+  /// The fetched data is not persisted after the callback is invoked.
+  ///
+  /// \param[in] tid
+  /// The tid who owns the data.
+  ///
+  /// \param[in] kind
+  /// The kind of data to read.
+  ///
+  /// \param[in] callback
+  /// The callback to be invoked once the data was successfully read. Its
+  /// return value, which is an \a llvm::Error, is returned by this
+  /// function.
+  ///
+  /// \return
+  /// An \a llvm::Error if the data couldn't be fetched, or the return 
value
+  /// of the callback, otherwise.
+  llvm::Error OnThreadBinaryDataRead(lldb::tid_t tid, llvm::StringRef kind,
+ OnBinaryDataReadCallback callback);
+
 protected:
+  /// Implementation of \a OnThreadBinaryDataRead() for live threads.
+  llvm::Error OnLiveThreadBinaryDataRead(lldb::tid_t tid, llvm::StringRef kind,
+ OnBinaryDataReadCallback callback);
+
+  /// Implementation of \a OnThreadBinaryDataRead() for post mortem threads.
+  llvm::Error
+  OnPostMortemThreadBinaryDataRead(lldb::tid_t tid, llvm::StringRef kind,
+   OnBinaryDataReadCallback callback);
+
+  /// Get the file path containing data of a postmortem thread given a data
+  /// identifier.
+  ///
+  /// \param[in] tid
+  /// The thread whose data is requested.
+  ///
+  /// \param[in] kind
+  /// The kind of data requested.
+  ///
+  /// \return
+  /// The file spec containing the requested data, or an \a llvm::Error in
+  /// case of failures.
+  llvm::Expected GetPostMortemThreadDataFile(lldb::tid_t tid,
+   llvm::StringRef kind);
+
+  ///

[Lldb-commits] [lldb] 6423b50 - [trace][intel pt] Create a class for the libipt decoder wrapper

2022-04-07 Thread Walter Erquinigo via lldb-commits 

Author: Walter Erquinigo
Date: 2022-04-07T15:58:34-07:00
New Revision: 6423b50235212db4c3a2e673b2b59fa5f6e07ec0

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

LOG: [trace][intel pt] Create a class for the libipt decoder wrapper

As we soon will need to decode multiple raw traces for the same thread,
having a class that encapsulates the decoding of a single raw trace is
a stepping stone that will make the coming features easier to implement.

So, I'm creating a LibiptDecoder class with that purpose. I refactored
the code and it's now much more readable. Besides that, more comments
were added. With this new structure, it's also easier to implement unit
tests.

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

Added: 
lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h

Modified: 
lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h

Removed: 
lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h



diff  --git a/lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt 
b/lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
index 58f7f904814ae..78d64d509b8cc 100644
--- a/lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
+++ b/lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
@@ -16,7 +16,8 @@ lldb_tablegen(TraceIntelPTCommandOptions.inc 
-gen-lldb-option-defs
 add_lldb_library(lldbPluginTraceIntelPT PLUGIN
   CommandObjectTraceStartIntelPT.cpp
   DecodedThread.cpp
-  IntelPTDecoder.cpp
+  LibiptDecoder.cpp
+  ThreadDecoder.cpp
   TraceCursorIntelPT.cpp
   TraceIntelPT.cpp
   TraceIntelPTJSONStructs.cpp

diff  --git a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp 
b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
index f1a5a78ef0d11..d08c50e60cdb7 100644
--- a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
@@ -9,10 +9,10 @@
 #include "DecodedThread.h"
 
 #include 
-#include 
 
 #include "TraceCursorIntelPT.h"
-#include "lldb/Utility/StreamString.h"
+
+#include 
 
 using namespace lldb;
 using namespace lldb_private;

diff  --git a/lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp 
b/lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
deleted file mode 100644
index 7af56f13cb250..0
--- a/lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
+++ /dev/null
@@ -1,308 +0,0 @@
-//===-- IntelPTDecoder.cpp 
--=====//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "IntelPTDecoder.h"
-
-#include "llvm/Support/MemoryBuffer.h"
-
-#include "../common/ThreadPostMortemTrace.h"
-#include "DecodedThread.h"
-#include "TraceIntelPT.h"
-#include "lldb/Core/Module.h"
-#include "lldb/Core/Section.h"
-#include "lldb/Target/Target.h"
-#include "lldb/Utility/StringExtractor.h"
-#include 
-
-using namespace lldb;
-using namespace lldb_private;
-using namespace lldb_private::trace_intel_pt;
-using namespace llvm;
-
-/// Move the decoder forward to the next synchronization point (i.e. next PSB
-/// packet).
-///
-/// Once the decoder is at that sync. point, it can start decoding 
instructions.
-///
-/// \return
-///   A negative number with the libipt error if we couldn't synchronize.
-///   Otherwise, a positive number with the synchronization status will be
-///   returned.
-static int FindNextSynchronizationPoint(pt_insn_decoder ) {
-  // Try to sync the decoder. If it fails, then get
-  // the decoder_offset and try to sync again from
-  // the next synchronization point. If the
-  // new_decoder_offset is same as decoder_offset
-  // then we can't move to the next synchronization
-  // point. Otherwise, keep resyncing until either
-  // end of trace stream (eos) is reached or
-  // pt_insn_sync_forward() passes.
-  int errcode = pt_insn_sync_forward();
-
-  if (errcode != -pte_eos && errcode < 0) {
-uint64_t decoder_offset = 0;
-int errcode_off = pt_insn_get_offset(, _offset);
-if (errcode_off >= 0) { // we could get the offset
-  while (true) {
-errcode = pt_insn_sync_forward();
-if (errcode >= 0 || errcode == -pte_eos)
-  break;
-
-uint64_t new_decoder_offset = 0;
-

[Lldb-commits] [PATCH] D123106: [trace][intel pt] Create a class for the libipt decoder wrapper

2022-04-07 Thread Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6423b5023521: [trace][intel pt] Create a class for the 
libipt decoder wrapper (authored by Walter Erquinigo wall...@fb.com).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123106

Files:
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
  lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h

Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
@@ -9,7 +9,7 @@
 #ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACEINTELPT_H
 #define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACEINTELPT_H
 
-#include "IntelPTDecoder.h"
+#include "ThreadDecoder.h"
 #include "TraceIntelPTSessionFileParser.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/lldb-types.h"
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -9,7 +9,7 @@
 #ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACECURSORINTELPT_H
 #define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACECURSORINTELPT_H
 
-#include "IntelPTDecoder.h"
+#include "ThreadDecoder.h"
 #include "TraceIntelPTSessionFileParser.h"
 
 namespace lldb_private {
Index: lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
===
--- lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
+++ lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
@@ -1,4 +1,4 @@
-//===-- IntelPTDecoder.h --==*- C++ -*-===//
+//===-- ThreadDecoder.h --==-*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,8 +6,8 @@
 //
 //===--===//
 
-#ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODER_H
-#define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODER_H
+#ifndef LLDB_SOURCE_PLUGINS_TRACE_THREAD_DECODER_H
+#define LLDB_SOURCE_PLUGINS_TRACE_THREAD_DECODER_H
 
 #include "intel-pt.h"
 
@@ -84,4 +84,4 @@
 } // namespace trace_intel_pt
 } // namespace lldb_private
 
-#endif // LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODER_H
+#endif // LLDB_SOURCE_PLUGINS_TRACE_THREAD_DECODER_H
Index: lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
===
--- /dev/null
+++ lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
@@ -0,0 +1,79 @@
+//===-- ThreadDecoder.cpp --==-===//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ThreadDecoder.h"
+
+#include "llvm/Support/MemoryBuffer.h"
+
+#include "../common/ThreadPostMortemTrace.h"
+#include "LibiptDecoder.h"
+#include "TraceIntelPT.h"
+
+#include 
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::trace_intel_pt;
+using namespace llvm;
+
+// ThreadDecoder 
+
+DecodedThreadSP ThreadDecoder::Decode() {
+  if (!m_decoded_thread.hasValue())
+m_decoded_thread = DoDecode();
+  return *m_decoded_thread;
+}
+
+// LiveThreadDecoder 
+
+LiveThreadDecoder::LiveThreadDecoder(Thread , TraceIntelPT )
+: m_thread_sp(thread.shared_from_this()), m_trace(trace) {}
+
+DecodedThreadSP LiveThreadDecoder::DoDecode() {
+  DecodedThreadSP decoded_thread_sp =
+  std::make_shared(m_thread_sp);
+
+  Expected> buffer =
+  m_trace.GetLiveThreadBuffer(m_thread_sp->GetID());
+  if (!buffer) {
+decoded_thread_sp->AppendError(buffer.takeError());
+return decoded_thread_sp;
+  }
+
+  decoded_thread_sp->SetRawTraceSize(buffer->size());
+  DecodeTrace(*decoded_thread_sp, m_trace, MutableArrayRef(*buffer));
+  return decoded_thread_sp;
+}
+
+// PostMortemThreadDecoder ===
+
+PostMortemThreadDecoder::PostMortemThreadDecoder(
+const lldb::ThreadPostMortemTraceSP _thread, TraceIntelPT )
+:

[Lldb-commits] [PATCH] D123281: [trace][intel pt] Create a common accessor for live and postmortem data

2022-04-07 Thread walter erquinigo via Phabricator via lldb-commits 
wallace added inline comments.



Comment at: lldb/include/lldb/Target/Trace.h:386-396
+  llvm::DenseMap>
   m_live_thread_data;
+
   /// data kind -> size
   std::unordered_map m_live_process_data;
+  /// \}
+

jj10306 wrote:
> Why not change all the maps to DenseMap while we're at it?
interestingly DenseMap doesn't know how to work with std::strings as key, only 
with const char *. Maybe they want to force a comparison by pointer and not by 
string content.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123281

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

[Lldb-commits] [PATCH] D123281: [trace][intel pt] Create a common accessor for live and postmortem data

2022-04-07 Thread Jakob Johnson via Phabricator via lldb-commits 
jj10306 accepted this revision.
jj10306 added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/include/lldb/Target/Trace.h:386-396
+  llvm::DenseMap>
   m_live_thread_data;
+
   /// data kind -> size
   std::unordered_map m_live_process_data;
+  /// \}
+

Why not change all the maps to DenseMap while we're at it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123281

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

[Lldb-commits] [PATCH] D123340: Applying clang-tidy modernize-use-override over LLDB

2022-04-07 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM modulo the redundant comment




Comment at: lldb/unittests/Target/RemoteAwarePlatformTest.cpp:40
   const ModuleSpec _spec, lldb::ModuleSP _module_sp,
-  const FileSpecList *module_search_paths_ptr) /*override*/ {
+  const FileSpecList *module_search_paths_ptr) override /*override*/ {
 auto pair = ResolveRemoteExecutable(module_spec, module_search_paths_ptr);




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

https://reviews.llvm.org/D123340

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

[Lldb-commits] [PATCH] D123020: increase timeouts if running on valgrind

2022-04-07 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added a comment.

In D123020#3437512 , @clayborg wrote:

> So the setting should take care of everything and we should be able to 
> increase it when running with valgrind right? I would rather not have code 
> all over LLDB making valgrind tests and doing something in response if 
> possible

Yup that's exactly what I suggested because I have the same concern


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123020

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

[Lldb-commits] [PATCH] D123020: increase timeouts if running on valgrind

2022-04-07 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

So the setting should take care of everything and we should be able to increase 
it when running with valgrind right? I would rather not have code all over LLDB 
making valgrind tests and doing something in response if possible


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123020

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

[Lldb-commits] [PATCH] D123092: [LLDB][NativePDB] Fix inline line info in line table

2022-04-07 Thread Zequan Wu via Phabricator via lldb-commits 
zequanwu updated this revision to Diff 421352.
zequanwu added a comment.

Modity inline_sites.test to do non-live test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123092

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/test/Shell/SymbolFile/NativePDB/Inputs/inline_sites.lldbinit
  lldb/test/Shell/SymbolFile/NativePDB/Inputs/inline_sites.s
  lldb/test/Shell/SymbolFile/NativePDB/inline_sites.test

Index: lldb/test/Shell/SymbolFile/NativePDB/inline_sites.test
===
--- lldb/test/Shell/SymbolFile/NativePDB/inline_sites.test
+++ lldb/test/Shell/SymbolFile/NativePDB/inline_sites.test
@@ -21,7 +21,8 @@
 # CHECK-NEXT: 0x000140001035: /tmp/c.h:7
 # CHECK-NEXT: 0x000140001039: /tmp/a.cpp:3
 # CHECK-NEXT: 0x00014000103d: /tmp/a.cpp:4
-# CHECK-NEXT: 0x000140001044: /tmp/a.h:8, is_start_of_statement = TRUE
+# CHECK-NEXT: 0x00014000103f: /tmp/a.h:20
+# CHECK-NEXT: 0x000140001044: /tmp/a.h:8
 # CHECK-NEXT: 0x000140001046: /tmp/a.cpp:4, is_terminal_entry = TRUE
 
 #CHECK: (lldb) b a.h:5
@@ -31,7 +32,7 @@
 #CHECK: (lldb) b a.h:7
 #CHECK: Breakpoint 3: where = {{.*}}`main + 16 [inlined] Namespace1::foo + 12 at a.h:7, address = 0x000140001010
 #CHECK: (lldb) b a.h:8
-#CHECK: Breakpoint 4: where = {{.*}}`main + 68 [inlined] Namespace1::foo at a.h:8, address = 0x000140001044
+#CHECK: Breakpoint 4: where = {{.*}}`main + 68 [inlined] Namespace1::foo + 5 at a.h:8, address = 0x000140001044
 #CHECK: (lldb) b a.h:9
 #CHECK: Breakpoint 5: where = {{.*}}`main + 24 [inlined] Namespace1::foo + 20 at a.h:9, address = 0x000140001018
 #CHECK: (lldb) b b.h:5
@@ -50,8 +51,6 @@
 #CHECK: Breakpoint 12: where = {{.*}}`main + 57 at a.cpp:3, address = 0x000140001039
 #CHECK: (lldb) b a.cpp:4
 #CHECK: Breakpoint 13: where = {{.*}}`main + 61 at a.cpp:4, address = 0x00014000103d
-#CHECK: (lldb) b a.h:8
-#CHECK: Breakpoint 14: where = {{.*}}`main + 68 [inlined] Namespace1::foo at a.h:8, address = 0x000140001044
 
 # CEHCK-LABEL: (lldb) image lookup -a 0x140001003 -v
 # CHECK:   Summary: {{.*}}`main + 3 at a.cpp:2
@@ -66,7 +65,7 @@
 # CHECK-NEXT:   {{.*}}`main + 4 at a.cpp:3
 # CHECK:   Function: id = {{.*}}, name = "main", range = [0x000140001000-0x000140001046)
 # CHECK: Blocks: id = {{.*}}, range = [0x140001000-0x140001046)
-# CHECK-NEXT:id = {{.*}}, ranges = [0x140001004-0x140001039)[0x140001044-0x140001046), name = "Namespace1::foo", decl = a.h:4
+# CHECK-NEXT:id = {{.*}}, ranges = [0x140001004-0x140001039)[0x14000103f-0x140001046), name = "Namespace1::foo", decl = a.h:4
 # CHECK:   LineEntry: [0x000140001004-0x00014000100c): /tmp/a.h:5
 # CHECK-NEXT:  Variable: id = {{.*}}, name = "foo_local", type = "int", valid ranges = [0x000140001004-0x000140001039)
 # CHECK-NEXT:  Variable: id = {{.*}}, name = "argc", type = "int", valid ranges = [0x000140001000-0x00014000102d)
@@ -78,7 +77,7 @@
 # CHECK-NEXT:   {{.*}}`main + 4 at a.cpp:3
 # CHECK:   Function: id = {{.*}}, name = "main", range = [0x000140001000-0x000140001046)
 # CHECK: Blocks: id = {{.*}}, range = [0x140001000-0x140001046)
-# CHECK-NEXT:id = {{.*}}, ranges = [0x140001004-0x140001039)[0x140001044-0x140001046), name = "Namespace1::foo", decl = a.h:4
+# CHECK-NEXT:id = {{.*}}, ranges = [0x140001004-0x140001039)[0x14000103f-0x140001046), name = "Namespace1::foo", decl = a.h:4
 # CHECK:   LineEntry: [0x000140001010-0x000140001018): /tmp/a.h:7
 # CHECK-NEXT:  Variable: id = {{.*}}, name = "foo_local", type = "int", valid ranges = [0x000140001004-0x000140001039)
 # CHECK-NEXT:  Variable: id = {{.*}}, name = "argc", type = "int", valid ranges = [0x000140001000-0x00014000102d)
@@ -91,7 +90,7 @@
 # CHECK-NEXT:   {{.*}}`main + 4 at a.cpp:3
 # CHECK:   Function: id = {{.*}}, name = "main", range = [0x000140001000-0x000140001046)
 # CHECK: Blocks: id = {{.*}}, range = [0x140001000-0x140001046)
-# CHECK-NEXT:id = {{.*}}, ranges = [0x140001004-0x140001039)[0x140001044-0x140001046), name = "Namespace1::foo", decl = a.h:4
+# CHECK-NEXT:id = {{.*}}, ranges = [0x140001004-0x140001039)[0x14000103f-0x140001046), name = "Namespace1::foo", decl = a.h:4
 # CHECK-NEXT:id = {{.*}}, range = [0x14000101c-0x140001039), name = "Class1::bar", decl = b.h:4
 # CHECK:   LineEntry: [0x00014000101c-0x000140001022): /tmp/b.h:5
 # CHECK-NEXT:  Variable: id = {{.*}}, name = "x", type = "int", valid ranges = [0x00014000101c-0x00014000101e)
@@ -108,7 +107,7 @@
 # CHECK-NEXT:   {{.*}}`main + 4 at a.cpp:3
 # CHECK:   Function: id = {{.*}}, name = "main", range = [0x000140001000-0x000140001046)
 # CHECK: Blocks: id =

[Lldb-commits] [PATCH] D123340: Applying clang-tidy modernize-use-override over LLDB

2022-04-07 Thread Shafik Yaghmour via Phabricator via lldb-commits 
shafik created this revision.
shafik added reviewers: labath, JDevlieghere, aprantl.
Herald added a project: All.
shafik requested review of this revision.
Herald added a subscriber: aheejin.

Applied clang-tidy `modernize-use-override` over LLDB and added it to the LLDB 
`.clang-tidy` config.


https://reviews.llvm.org/D123340

Files:
  lldb/.clang-tidy
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
  lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
  lldb/unittests/API/SBCommandInterpreterTest.cpp
  lldb/unittests/Interpreter/TestCommandPaths.cpp
  lldb/unittests/Interpreter/TestOptionValue.cpp
  lldb/unittests/Target/RemoteAwarePlatformTest.cpp

Index: lldb/unittests/Target/RemoteAwarePlatformTest.cpp
===
--- lldb/unittests/Target/RemoteAwarePlatformTest.cpp
+++ lldb/unittests/Target/RemoteAwarePlatformTest.cpp
@@ -37,7 +37,7 @@
const FileSpecList *));
   Status ResolveRemoteExecutable(
   const ModuleSpec _spec, lldb::ModuleSP _module_sp,
-  const FileSpecList *module_search_paths_ptr) /*override*/ {
+  const FileSpecList *module_search_paths_ptr) override /*override*/ {
 auto pair = ResolveRemoteExecutable(module_spec, module_search_paths_ptr);
 exe_module_sp = pair.second;
 return pair.first;
Index: lldb/unittests/Interpreter/TestOptionValue.cpp
===
--- lldb/unittests/Interpreter/TestOptionValue.cpp
+++ lldb/unittests/Interpreter/TestOptionValue.cpp
@@ -111,7 +111,7 @@
   }
 
 private:
-  lldb::OptionValueSP Clone() const {
+  lldb::OptionValueSP Clone() const override {
 return std::make_shared(*this);
   }
 
Index: lldb/unittests/Interpreter/TestCommandPaths.cpp
===
--- lldb/unittests/Interpreter/TestCommandPaths.cpp
+++ lldb/unittests/Interpreter/TestCommandPaths.cpp
@@ -51,7 +51,7 @@
   }
 
 protected:
-  virtual bool DoExecute(Args , CommandReturnObject ) {
+  bool DoExecute(Args , CommandReturnObject ) override {
 result.SetStatus(eReturnStatusSuccessFinishResult);
 result.AppendMessage("I did nothing");
 return true;
Index: lldb/unittests/API/SBCommandInterpreterTest.cpp
===
--- lldb/unittests/API/SBCommandInterpreterTest.cpp
+++ lldb/unittests/API/SBCommandInterpreterTest.cpp
@@ -34,7 +34,7 @@
   DummyCommand(const char *message) : m_message(message) {}
 
   bool DoExecute(SBDebugger dbg, char **command,
- SBCommandReturnObject ) {
+ SBCommandReturnObject ) override {
 result.PutCString(m_message.c_str());
 result.SetStatus(eReturnStatusSuccessFinishResult);
 return result.Succeeded();
Index: lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
===
--- lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
+++ lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
@@ -74,7 +74,7 @@
 m_collection_sp->Initialize(g_processkdp_properties);
   }
 
-  virtual ~PluginProperties() = default;
+  ~PluginProperties() override = default;
 
   uint64_t GetPacketTimeout() {
 const uint32_t idx = ePropertyKDPPacketTimeout;
@@ -877,7 +877,7 @@
 m_option_group.Finalize();
   }
 
-  ~CommandObjectProcessKDPPacketSend() = default;
+  ~CommandObjectProcessKDPPacketSend() override = default;
 
   bool DoExecute(Args , CommandReturnObject ) override {
 const size_t argc = command.GetArgumentCount();
@@ -978,7 +978,7 @@
 CommandObjectSP(new CommandObjectProcessKDPPacketSend(interpreter)));
   }
 
-  ~CommandObjectProcessKDPPacket() = default;
+  ~CommandObjectProcessKDPPacket() override = default;
 };
 
 class CommandObjectMultiwordProcessKDP : public CommandObjectMultiword {
@@ -992,7 +992,7 @@
  interpreter)));
   }
 
-  ~CommandObjectMultiwordProcessKDP() = default;
+  ~CommandObjectMultiwordProcessKDP() override = default;
 };
 
 CommandObject *ProcessKDP::GetPluginCommandObject() {
Index: lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
===
--- lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
@@ -193,7 +193,7 @@
 m_collection_sp->Initialize(g_platformdarwinkernel_properties);
   }
 
-  virtual ~PlatformDarwinKernelProperties() = default;
+  ~PlatformDarwinKernelProperties() override = default;
 
   FileSpecList GetKextDirectories() const {
 const uint32_t idx = ePropertyKextDirectories;
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
---

[Lldb-commits] [lldb] 8ece6b7 - [lldb] Use getMainExecutable in SBDebugger::PrintStackTraceOnError

2022-04-07 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2022-04-07T13:53:23-07:00
New Revision: 8ece6b78c0425ba587c34bbb046d1cb6529a4569

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

LOG: [lldb] Use getMainExecutable in SBDebugger::PrintStackTraceOnError

Implement Pavel's suggestion to use llvm::sys::fs::getMainExecutable to
find the executable name for llvm::sys::PrintStackTraceOnErrorSignal.

Added: 


Modified: 
lldb/source/API/SBDebugger.cpp

Removed: 




diff  --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 4d92a0a9b2805..c82ff0f1e878d 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -212,9 +212,9 @@ void SBDebugger::PrintStackTraceOnError() {
   LLDB_INSTRUMENT();
 
   llvm::EnablePrettyStackTrace();
-  // We don't have a meaningful argv[0] to use, so use "SBDebugger" as a
-  // substitute.
-  llvm::sys::PrintStackTraceOnErrorSignal("SBDebugger");
+  static std::string executable =
+  llvm::sys::fs::getMainExecutable(nullptr, nullptr);
+  llvm::sys::PrintStackTraceOnErrorSignal(executable);
 }
 
 void SBDebugger::Terminate() {



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

[Lldb-commits] [PATCH] D123020: increase timeouts if running on valgrind

2022-04-07 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added a comment.

In D123020#3437246 , @llunak wrote:

> In D123020#3426867 , @JDevlieghere 
> wrote:
>
>> FWIW the official policy is outlined here: 
>> https://llvm.org/docs/CodeReview.html
>
> I'm aware of it, but as far as I can judge I was following it. Even reading 
> it now again I see nothing that I would understand as mandating review for 
> everything.
>
>> The GDB remote packet timeout is customizable:
>> ...
>> So maybe we don't need this patch?
>
> The timeout in GDBRemoteCommunicationClient::QueryNoAckModeSupported() is 
> hardcoded to 6 seconds, so I doubt the setting matters there.

This is incorrect. It's hardcoded to be `std::max(GetPacketTimeout(), 
seconds(6))` where `GetPacketTimeout()` is the value from the setting.

> And even if it did, I'd find it sometwhat silly having to look for a 
> please-unbreak-valgrind setting, especially given that there is already LLVM 
> support for detecting Valgrind.

The suggestion was to increase the timeout globally (through the setting) when 
running under Valgrind.

> But this all is too much for something so simple.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123020

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

[Lldb-commits] [PATCH] D123020: increase timeouts if running on valgrind

2022-04-07 Thread Luboš Luňák via Phabricator via lldb-commits 
llunak abandoned this revision.
llunak added a comment.

In D123020#3426867 , @JDevlieghere 
wrote:

> FWIW the official policy is outlined here: 
> https://llvm.org/docs/CodeReview.html

I'm aware of it, but as far as I can judge I was following it. Even reading it 
now again I see nothing that I would understand as mandating review for 
everything.

> The GDB remote packet timeout is customizable:
> ...
> So maybe we don't need this patch?

The timeout in GDBRemoteCommunicationClient::QueryNoAckModeSupported() is 
hardcoded to 6 seconds, so I doubt the setting matters there. And even if it 
did, I'd find it sometwhat silly having to look for a please-unbreak-valgrind 
setting, especially given that there is already LLVM support for detecting 
Valgrind.

But this all is too much for something so simple.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123020

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

[Lldb-commits] [PATCH] D123008: remove the "expand" diamond for variables where expanding fails

2022-04-07 Thread Luboš Luňák via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc29a51b3a257: [lldb][gui] remove the expand 
diamond for variables where expanding fails (authored by llunak).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123008

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp


Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -4537,7 +4537,8 @@
 if (parent)
   parent->DrawTreeForChild(window, this, 0);
 
-if (might_have_children) {
+if (might_have_children &&
+(!calculated_children || !GetChildren().empty())) {
   // It we can get UTF8 characters to work we should try to use the
   // "symbol" UTF8 string below
   //const char *symbol = "";
@@ -5824,9 +5825,11 @@
 ++m_num_rows;
   }
 
-  auto  = row.GetChildren();
-  if (row.expanded && !children.empty()) {
-DisplayRows(window, children, options);
+  if (row.expanded) {
+auto  = row.GetChildren();
+if (!children.empty()) {
+  DisplayRows(window, children, options);
+}
   }
 }
   }
@@ -5847,11 +5850,13 @@
 return 
   else {
 --row_index;
-auto  = row.GetChildren();
-if (row.expanded && !children.empty()) {
-  Row *result = GetRowForRowIndexImpl(children, row_index);
-  if (result)
-return result;
+if (row.expanded) {
+  auto  = row.GetChildren();
+  if (!children.empty()) {
+Row *result = GetRowForRowIndexImpl(children, row_index);
+if (result)
+  return result;
+  }
 }
   }
 }


Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -4537,7 +4537,8 @@
 if (parent)
   parent->DrawTreeForChild(window, this, 0);
 
-if (might_have_children) {
+if (might_have_children &&
+(!calculated_children || !GetChildren().empty())) {
   // It we can get UTF8 characters to work we should try to use the
   // "symbol" UTF8 string below
   //const char *symbol = "";
@@ -5824,9 +5825,11 @@
 ++m_num_rows;
   }
 
-  auto  = row.GetChildren();
-  if (row.expanded && !children.empty()) {
-DisplayRows(window, children, options);
+  if (row.expanded) {
+auto  = row.GetChildren();
+if (!children.empty()) {
+  DisplayRows(window, children, options);
+}
   }
 }
   }
@@ -5847,11 +5850,13 @@
 return 
   else {
 --row_index;
-auto  = row.GetChildren();
-if (row.expanded && !children.empty()) {
-  Row *result = GetRowForRowIndexImpl(children, row_index);
-  if (result)
-return result;
+if (row.expanded) {
+  auto  = row.GetChildren();
+  if (!children.empty()) {
+Row *result = GetRowForRowIndexImpl(children, row_index);
+if (result)
+  return result;
+  }
 }
   }
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D123015: handle Ctrl+C to stop a running process

2022-04-07 Thread Luboš Luňák via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf42f21746cb8: [lldb][gui] handle Ctrl+C to stop a running 
process (authored by llunak).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123015

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/Core/IOHandlerCursesGUI.cpp


Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -7710,7 +7710,9 @@
 
 void IOHandlerCursesGUI::Cancel() {}
 
-bool IOHandlerCursesGUI::Interrupt() { return false; }
+bool IOHandlerCursesGUI::Interrupt() {
+  return m_debugger.GetCommandInterpreter().IOHandlerInterrupt(*this);
+}
 
 void IOHandlerCursesGUI::GotEOF() {}
 
Index: lldb/include/lldb/Interpreter/CommandInterpreter.h
===
--- lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -610,6 +610,8 @@
 
   bool IsInteractive();
 
+  bool IOHandlerInterrupt(IOHandler _handler) override;
+
 protected:
   friend class Debugger;
 
@@ -623,8 +625,6 @@
 return ConstString();
   }
 
-  bool IOHandlerInterrupt(IOHandler _handler) override;
-
   void GetProcessOutput();
 
   bool DidProcessStopAbnormally() const;


Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -7710,7 +7710,9 @@
 
 void IOHandlerCursesGUI::Cancel() {}
 
-bool IOHandlerCursesGUI::Interrupt() { return false; }
+bool IOHandlerCursesGUI::Interrupt() {
+  return m_debugger.GetCommandInterpreter().IOHandlerInterrupt(*this);
+}
 
 void IOHandlerCursesGUI::GotEOF() {}
 
Index: lldb/include/lldb/Interpreter/CommandInterpreter.h
===
--- lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -610,6 +610,8 @@
 
   bool IsInteractive();
 
+  bool IOHandlerInterrupt(IOHandler _handler) override;
+
 protected:
   friend class Debugger;
 
@@ -623,8 +625,6 @@
 return ConstString();
   }
 
-  bool IOHandlerInterrupt(IOHandler _handler) override;
-
   void GetProcessOutput();
 
   bool DidProcessStopAbnormally() const;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] c29a51b - [lldb][gui] remove the "expand" diamond for variables where expanding fails

2022-04-07 Thread Luboš Luňák via lldb-commits 

Author: Luboš Luňák
Date: 2022-04-07T21:59:18+02:00
New Revision: c29a51b3a257908aebc01cd7c4655665db317d66

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

LOG: [lldb][gui] remove the "expand" diamond for variables where expanding fails

If the variables view shows a variable that is a struct that has
MightHaveChildren(), the expand diamond is shown, but if trying to expand
it and it's not possible (e.g. incomplete type), remove the expand diamond
to visualize that it can't be in fact expanded. Otherwise it feels kinda
weird that a tree item cannot be expanded even though it "should".

I guess the MightHaveChildren() checking means that GetChildren() may
be expensive, so also do not call it for rows that are not expanded.

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

Added: 


Modified: 
lldb/source/Core/IOHandlerCursesGUI.cpp

Removed: 




diff  --git a/lldb/source/Core/IOHandlerCursesGUI.cpp 
b/lldb/source/Core/IOHandlerCursesGUI.cpp
index c8d86f0e1128a..0151255631bf8 100644
--- a/lldb/source/Core/IOHandlerCursesGUI.cpp
+++ b/lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -4537,7 +4537,8 @@ struct Row {
 if (parent)
   parent->DrawTreeForChild(window, this, 0);
 
-if (might_have_children) {
+if (might_have_children &&
+(!calculated_children || !GetChildren().empty())) {
   // It we can get UTF8 characters to work we should try to use the
   // "symbol" UTF8 string below
   //const char *symbol = "";
@@ -5824,9 +5825,11 @@ class ValueObjectListDelegate : public WindowDelegate {
 ++m_num_rows;
   }
 
-  auto  = row.GetChildren();
-  if (row.expanded && !children.empty()) {
-DisplayRows(window, children, options);
+  if (row.expanded) {
+auto  = row.GetChildren();
+if (!children.empty()) {
+  DisplayRows(window, children, options);
+}
   }
 }
   }
@@ -5847,11 +5850,13 @@ class ValueObjectListDelegate : public WindowDelegate {
 return 
   else {
 --row_index;
-auto  = row.GetChildren();
-if (row.expanded && !children.empty()) {
-  Row *result = GetRowForRowIndexImpl(children, row_index);
-  if (result)
-return result;
+if (row.expanded) {
+  auto  = row.GetChildren();
+  if (!children.empty()) {
+Row *result = GetRowForRowIndexImpl(children, row_index);
+if (result)
+  return result;
+  }
 }
   }
 }



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

[Lldb-commits] [lldb] f42f217 - [lldb][gui] handle Ctrl+C to stop a running process

2022-04-07 Thread Luboš Luňák via lldb-commits 

Author: Luboš Luňák
Date: 2022-04-07T21:58:37+02:00
New Revision: f42f21746cb8b940518bf37e5917d61542d278b7

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

LOG: [lldb][gui] handle Ctrl+C to stop a running process

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

Added: 


Modified: 
lldb/include/lldb/Interpreter/CommandInterpreter.h
lldb/source/Core/IOHandlerCursesGUI.cpp

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h 
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 641e651e18909..f1f715c891a5c 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -610,6 +610,8 @@ class CommandInterpreter : public Broadcaster,
 
   bool IsInteractive();
 
+  bool IOHandlerInterrupt(IOHandler _handler) override;
+
 protected:
   friend class Debugger;
 
@@ -623,8 +625,6 @@ class CommandInterpreter : public Broadcaster,
 return ConstString();
   }
 
-  bool IOHandlerInterrupt(IOHandler _handler) override;
-
   void GetProcessOutput();
 
   bool DidProcessStopAbnormally() const;

diff  --git a/lldb/source/Core/IOHandlerCursesGUI.cpp 
b/lldb/source/Core/IOHandlerCursesGUI.cpp
index caf88c7fdf376..c8d86f0e1128a 100644
--- a/lldb/source/Core/IOHandlerCursesGUI.cpp
+++ b/lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -7710,7 +7710,9 @@ IOHandlerCursesGUI::~IOHandlerCursesGUI() = default;
 
 void IOHandlerCursesGUI::Cancel() {}
 
-bool IOHandlerCursesGUI::Interrupt() { return false; }
+bool IOHandlerCursesGUI::Interrupt() {
+  return m_debugger.GetCommandInterpreter().IOHandlerInterrupt(*this);
+}
 
 void IOHandlerCursesGUI::GotEOF() {}
 



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

[Lldb-commits] [PATCH] D91835: [lldb] Add Python bindings to print stack traces on crashes.

2022-04-07 Thread Pavel Labath via Phabricator via lldb-commits 
labath added inline comments.



Comment at: lldb/source/API/SBDebugger.cpp:215-216
+  llvm::EnablePrettyStackTrace();
+  // We don't have a meaningful argv[0] to use, so use "SBDebugger" as a
+  // substitute.
+  llvm::sys::PrintStackTraceOnErrorSignal("SBDebugger");

if you really wanted to, you could call something  like 
`llvm::sys::fs::getMainExecutable(nullptr, nullptr)` -- it quite often finds 
the executable name even without the argv0 and MainAddr hints.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91835

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

[Lldb-commits] [PATCH] D91835: [lldb] Add Python bindings to print stack traces on crashes.

2022-04-07 Thread Jonas Devlieghere via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGee2d9b872356: [lldb] Add Python bindings to print stack 
traces on crashes. (authored by JDevlieghere).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91835

Files:
  lldb/bindings/interface/SBDebugger.i
  lldb/include/lldb/API/SBDebugger.h
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/source/API/SBDebugger.cpp


Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -57,6 +57,8 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/ManagedStatic.h"
+#include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Signals.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -206,6 +208,15 @@
   return error;
 }
 
+void SBDebugger::PrintStackTraceOnError() {
+  LLDB_INSTRUMENT();
+
+  llvm::EnablePrettyStackTrace();
+  // We don't have a meaningful argv[0] to use, so use "SBDebugger" as a
+  // substitute.
+  llvm::sys::PrintStackTraceOnErrorSignal("SBDebugger");
+}
+
 void SBDebugger::Terminate() {
   LLDB_INSTRUMENT();
 
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -880,6 +880,7 @@
 
 import lldb
 lldb.SBDebugger.Initialize()
+lldb.SBDebugger.PrintStackTraceOnError()
 
 checkLibcxxSupport()
 checkLibstdcxxSupport()
Index: lldb/include/lldb/API/SBDebugger.h
===
--- lldb/include/lldb/API/SBDebugger.h
+++ lldb/include/lldb/API/SBDebugger.h
@@ -92,6 +92,8 @@
 
   static lldb::SBError InitializeWithErrorHandling();
 
+  static void PrintStackTraceOnError();
+
   static void Terminate();
 
   // Deprecated, use the one that takes a source_init_files bool.
Index: lldb/bindings/interface/SBDebugger.i
===
--- lldb/bindings/interface/SBDebugger.i
+++ lldb/bindings/interface/SBDebugger.i
@@ -141,6 +141,8 @@
 static SBError
 InitializeWithErrorHandling();
 
+static void PrintStackTraceOnError();
+
 static void
 Terminate();
 


Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -57,6 +57,8 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/ManagedStatic.h"
+#include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Signals.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -206,6 +208,15 @@
   return error;
 }
 
+void SBDebugger::PrintStackTraceOnError() {
+  LLDB_INSTRUMENT();
+
+  llvm::EnablePrettyStackTrace();
+  // We don't have a meaningful argv[0] to use, so use "SBDebugger" as a
+  // substitute.
+  llvm::sys::PrintStackTraceOnErrorSignal("SBDebugger");
+}
+
 void SBDebugger::Terminate() {
   LLDB_INSTRUMENT();
 
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -880,6 +880,7 @@
 
 import lldb
 lldb.SBDebugger.Initialize()
+lldb.SBDebugger.PrintStackTraceOnError()
 
 checkLibcxxSupport()
 checkLibstdcxxSupport()
Index: lldb/include/lldb/API/SBDebugger.h
===
--- lldb/include/lldb/API/SBDebugger.h
+++ lldb/include/lldb/API/SBDebugger.h
@@ -92,6 +92,8 @@
 
   static lldb::SBError InitializeWithErrorHandling();
 
+  static void PrintStackTraceOnError();
+
   static void Terminate();
 
   // Deprecated, use the one that takes a source_init_files bool.
Index: lldb/bindings/interface/SBDebugger.i
===
--- lldb/bindings/interface/SBDebugger.i
+++ lldb/bindings/interface/SBDebugger.i
@@ -141,6 +141,8 @@
 static SBError
 InitializeWithErrorHandling();
 
+static void PrintStackTraceOnError();
+
 static void
 Terminate();
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] ee2d9b8 - [lldb] Add Python bindings to print stack traces on crashes.

2022-04-07 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2022-04-07T11:21:02-07:00
New Revision: ee2d9b8723561fb6a81b9c2c4fd7a93300c6154f

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

LOG: [lldb] Add Python bindings to print stack traces on crashes.

As noticed in D87637, when LLDB crashes, we only print stack traces if
LLDB is directly executed, not when used via Python bindings. Enabling
this by default may be undesirable (libraries shouldn't be messing with
signal handlers), so make this an explicit opt-in.

I "commandeered" this patch from Jordan Rupprecht who put this up for
review originally.

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

Added: 


Modified: 
lldb/bindings/interface/SBDebugger.i
lldb/include/lldb/API/SBDebugger.h
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/source/API/SBDebugger.cpp

Removed: 




diff  --git a/lldb/bindings/interface/SBDebugger.i 
b/lldb/bindings/interface/SBDebugger.i
index e9a6168d0c093..2239f049c6514 100644
--- a/lldb/bindings/interface/SBDebugger.i
+++ b/lldb/bindings/interface/SBDebugger.i
@@ -141,6 +141,8 @@ public:
 static SBError
 InitializeWithErrorHandling();
 
+static void PrintStackTraceOnError();
+
 static void
 Terminate();
 

diff  --git a/lldb/include/lldb/API/SBDebugger.h 
b/lldb/include/lldb/API/SBDebugger.h
index 0893bc60315e6..6a23077175b92 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -92,6 +92,8 @@ class LLDB_API SBDebugger {
 
   static lldb::SBError InitializeWithErrorHandling();
 
+  static void PrintStackTraceOnError();
+
   static void Terminate();
 
   // Deprecated, use the one that takes a source_init_files bool.

diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index 5cf75391d8440..489e6ce7bd7a1 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -880,6 +880,7 @@ def run_suite():
 
 import lldb
 lldb.SBDebugger.Initialize()
+lldb.SBDebugger.PrintStackTraceOnError()
 
 checkLibcxxSupport()
 checkLibstdcxxSupport()

diff  --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 3391665786d56..4d92a0a9b2805 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -57,6 +57,8 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/ManagedStatic.h"
+#include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Signals.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -206,6 +208,15 @@ lldb::SBError SBDebugger::InitializeWithErrorHandling() {
   return error;
 }
 
+void SBDebugger::PrintStackTraceOnError() {
+  LLDB_INSTRUMENT();
+
+  llvm::EnablePrettyStackTrace();
+  // We don't have a meaningful argv[0] to use, so use "SBDebugger" as a
+  // substitute.
+  llvm::sys::PrintStackTraceOnErrorSignal("SBDebugger");
+}
+
 void SBDebugger::Terminate() {
   LLDB_INSTRUMENT();
 



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

[Lldb-commits] [PATCH] D91835: [lldb] Add Python bindings to print stack traces on crashes.

2022-04-07 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor accepted this revision.
teemperor added a comment.

This is beautiful


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

https://reviews.llvm.org/D91835

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

[Lldb-commits] [PATCH] D123281: [trace][intel pt] Create a common accessor for live and postmortem data

2022-04-07 Thread walter erquinigo via Phabricator via lldb-commits 
wallace updated this revision to Diff 421291.
wallace added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123281

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/source/Plugins/Trace/common/TraceSessionSaver.cpp
  lldb/source/Plugins/Trace/common/TraceSessionSaver.h
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
  lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
  lldb/source/Target/Trace.cpp

Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -215,3 +215,64 @@
   RefreshLiveProcessState();
   return m_stop_id;
 }
+
+llvm::Expected
+Trace::GetPostMortemThreadDataFile(lldb::tid_t tid, llvm::StringRef kind) {
+  auto NotFoundError = [&]() {
+return createStringError(
+inconvertibleErrorCode(),
+formatv("The thread with tid={0} doesn't have the tracing data {1}",
+tid, kind));
+  };
+
+  auto it = m_postmortem_thread_data.find(tid);
+  if (it == m_postmortem_thread_data.end())
+return NotFoundError();
+
+  std::unordered_map _kind_to_file_spec_map =
+  it->second;
+  auto it2 = data_kind_to_file_spec_map.find(kind.str());
+  if (it2 == data_kind_to_file_spec_map.end())
+return NotFoundError();
+  return it2->second;
+}
+
+void Trace::SetPostMortemThreadDataFile(lldb::tid_t tid, llvm::StringRef kind,
+FileSpec file_spec) {
+  m_postmortem_thread_data[tid][kind.str()] = file_spec;
+}
+
+llvm::Error
+Trace::OnLiveThreadBinaryDataRead(lldb::tid_t tid, llvm::StringRef kind,
+  OnBinaryDataReadCallback callback) {
+  Expected> data = GetLiveThreadBinaryData(tid, kind);
+  if (!data)
+return data.takeError();
+  return callback(*data);
+}
+
+llvm::Error
+Trace::OnPostMortemThreadBinaryDataRead(lldb::tid_t tid, llvm::StringRef kind,
+OnBinaryDataReadCallback callback) {
+  Expected file = GetPostMortemThreadDataFile(tid, kind);
+  if (!file)
+return file.takeError();
+  ErrorOr> trace_or_error =
+  MemoryBuffer::getFile(file->GetPath());
+  if (std::error_code err = trace_or_error.getError())
+return errorCodeToError(err);
+
+  MemoryBuffer  = **trace_or_error;
+  ArrayRef array_ref(
+  reinterpret_cast(data.getBufferStart()),
+  data.getBufferSize());
+  return callback(array_ref);
+}
+
+llvm::Error Trace::OnThreadBinaryDataRead(lldb::tid_t tid, llvm::StringRef kind,
+  OnBinaryDataReadCallback callback) {
+  if (m_live_process)
+return OnLiveThreadBinaryDataRead(tid, kind, callback);
+  else
+return OnPostMortemThreadBinaryDataRead(tid, kind, callback);
+}
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionSaver.cpp
@@ -48,15 +48,8 @@
 return json_intel_pt_trace.takeError();
 
   llvm::Expected json_session_description =
-  TraceSessionSaver::BuildProcessesSection(
-  *live_process,
-  [&](lldb::tid_t tid)
-  -> llvm::Expected>> {
-if (!trace_ipt.IsTraced(tid))
-  return None;
-return trace_ipt.GetLiveThreadBuffer(tid);
-  },
-  directory);
+  TraceSessionSaver::BuildProcessesSection(*live_process,
+   "threadTraceBuffer", directory);
 
   if (!json_session_description)
 return json_session_description.takeError();
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
@@ -137,8 +137,9 @@
 StructuredData::ObjectSP configuration =
 StructuredData::ObjectSP()) override;
 
-  /// Get the thread buffer content for a live thread
-  llvm::Expected> GetLiveThreadBuffer(lldb::tid_t tid);
+  /// See \a Trace::OnThreadBinaryDataRead().
+  llvm::Error OnThreadBufferRead(lldb::tid_t tid,
+ OnBinaryDataReadCallback callback);
 
   llvm::Expected GetCPUInfo();
 
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++

[Lldb-commits] [PATCH] D91835: [lldb] Add Python bindings to print stack traces on crashes.

2022-04-07 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib accepted this revision.
mib added a comment.
This revision is now accepted and ready to land.

Pretty straightforward, LGTM!


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

https://reviews.llvm.org/D91835

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

[Lldb-commits] [PATCH] D123106: [trace][intel pt] Create a class for the libipt decoder wrapper

2022-04-07 Thread walter erquinigo via Phabricator via lldb-commits 
wallace updated this revision to Diff 421283.
wallace marked 6 inline comments as done.
wallace added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123106

Files:
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
  lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h

Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
@@ -9,7 +9,7 @@
 #ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACEINTELPT_H
 #define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACEINTELPT_H
 
-#include "IntelPTDecoder.h"
+#include "ThreadDecoder.h"
 #include "TraceIntelPTSessionFileParser.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/lldb-types.h"
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -9,7 +9,7 @@
 #ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACECURSORINTELPT_H
 #define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_TRACECURSORINTELPT_H
 
-#include "IntelPTDecoder.h"
+#include "ThreadDecoder.h"
 #include "TraceIntelPTSessionFileParser.h"
 
 namespace lldb_private {
Index: lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
===
--- lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
+++ lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.h
@@ -1,4 +1,4 @@
-//===-- IntelPTDecoder.h --==*- C++ -*-===//
+//===-- ThreadDecoder.h --==-*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,8 +6,8 @@
 //
 //===--===//
 
-#ifndef LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODER_H
-#define LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODER_H
+#ifndef LLDB_SOURCE_PLUGINS_TRACE_THREAD_DECODER_H
+#define LLDB_SOURCE_PLUGINS_TRACE_THREAD_DECODER_H
 
 #include "intel-pt.h"
 
@@ -84,4 +84,4 @@
 } // namespace trace_intel_pt
 } // namespace lldb_private
 
-#endif // LLDB_SOURCE_PLUGINS_TRACE_INTEL_PT_DECODER_H
+#endif // LLDB_SOURCE_PLUGINS_TRACE_THREAD_DECODER_H
Index: lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
===
--- /dev/null
+++ lldb/source/Plugins/Trace/intel-pt/ThreadDecoder.cpp
@@ -0,0 +1,79 @@
+//===-- ThreadDecoder.cpp --==-===//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ThreadDecoder.h"
+
+#include "llvm/Support/MemoryBuffer.h"
+
+#include "../common/ThreadPostMortemTrace.h"
+#include "LibiptDecoder.h"
+#include "TraceIntelPT.h"
+
+#include 
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::trace_intel_pt;
+using namespace llvm;
+
+// ThreadDecoder 
+
+DecodedThreadSP ThreadDecoder::Decode() {
+  if (!m_decoded_thread.hasValue())
+m_decoded_thread = DoDecode();
+  return *m_decoded_thread;
+}
+
+// LiveThreadDecoder 
+
+LiveThreadDecoder::LiveThreadDecoder(Thread , TraceIntelPT )
+: m_thread_sp(thread.shared_from_this()), m_trace(trace) {}
+
+DecodedThreadSP LiveThreadDecoder::DoDecode() {
+  DecodedThreadSP decoded_thread_sp =
+  std::make_shared(m_thread_sp);
+
+  Expected> buffer =
+  m_trace.GetLiveThreadBuffer(m_thread_sp->GetID());
+  if (!buffer) {
+decoded_thread_sp->AppendError(buffer.takeError());
+return decoded_thread_sp;
+  }
+
+  decoded_thread_sp->SetRawTraceSize(buffer->size());
+  DecodeTrace(*decoded_thread_sp, m_trace, MutableArrayRef(*buffer));
+  return decoded_thread_sp;
+}
+
+// PostMortemThreadDecoder ===
+
+PostMortemThreadDecoder::PostMortemThreadDecoder(
+const lldb::ThreadPostMortemTraceSP _thread, TraceIntelPT )
+: m_trace_thread(trace_thread), m_trace(trace) {}
+
+DecodedThreadSP

[Lldb-commits] [PATCH] D122943: [LLDB][NativePDB] Fix a crash when S_DEFRANGE_SUBFIELD_REGISTER descirbes a simple type

2022-04-07 Thread Zequan Wu via Phabricator via lldb-commits 
zequanwu added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp:54
 struct FindMembersSize : public TypeVisitorCallbacks {
-  FindMembersSize(std::vector> _info,
+  FindMembersSize(llvm::SmallVector> 
_info,
   TpiStream )

aganea wrote:
> labath wrote:
> > I think `SmallVectorImpl` is still the official way to take SmallVector 
> > references.
> `ArrayRef` would be even better! :) ie. `llvm::ArrayRef uint32_t>> members_info;`
Using `SmallVectorImpl` because we want to insert elements in 
`visitKnownMember`.



Comment at: 
lldb/test/Shell/SymbolFile/NativePDB/subfield_register_simple_type.s:27
+# CHECK:  LineEntry: [0x00401026-0x00401039): C:\src\a.cpp:3
+# CHECK-NEXT: Variable: id = {{.*}}, name = "x", type = "int64_t", valid 
ranges = [0x0040102f-0x00401036), location = DW_OP_reg0 EAX, DW_OP_piece 0x4, 
DW_OP_reg2 EDX, DW_OP_piece 0x4, decl =
+

aganea wrote:
> nitpick: Is anything after (and including) ", valid ranges" necessary to 
> cover this change?
Yes, the structure of `location` is basically what we are testing against 
though the register number maybe incorrect due to the `FIXME` above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122943

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

[Lldb-commits] [PATCH] D122943: [LLDB][NativePDB] Fix a crash when S_DEFRANGE_SUBFIELD_REGISTER descirbes a simple type

2022-04-07 Thread Zequan Wu via Phabricator via lldb-commits 
zequanwu updated this revision to Diff 421281.
zequanwu marked 2 inline comments as done.
zequanwu added a comment.

Update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122943

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
  
lldb/test/Shell/SymbolFile/NativePDB/Inputs/subfield_register_simple_type.lldbinit
  lldb/test/Shell/SymbolFile/NativePDB/subfield_register_simple_type.s

Index: lldb/test/Shell/SymbolFile/NativePDB/subfield_register_simple_type.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/subfield_register_simple_type.s
@@ -0,0 +1,433 @@
+# clang-format off
+# REQUIRES: lld, x86
+
+# RUN: %clang_cl --target=i386-windows-msvc -c /Fo%t.obj %s
+# RUN: lld-link /debug:full /nodefaultlib /entry:main %t.obj /out:%t.exe /base:0x40
+# RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
+# RUN: %p/Inputs/subfield_register_simple_type.lldbinit 2>&1 | FileCheck %s
+
+# This file is compiled from following source file:
+# clang-cl --target=i386-windows-msvc /Z7 /O1 -c /Fa a.cpp
+# __int64 __attribute__((optnone)) bar(__int64 x) { return x; };
+# __int64 foo(__int64 x) {
+#   return bar(x);
+# }
+#
+# int main(int argc, char** argv) {
+#   foo(argc);
+#   return 0;
+# }
+
+# FIXME: The following variable location have wrong register numbers due to
+# https://github.com/llvm/llvm-project/issues/53575. Fix them after resolving
+# the issue.
+
+# CHECK:  (lldb) image lookup -a 0x40102f -v
+# CHECK:  LineEntry: [0x00401026-0x00401039): C:\src\a.cpp:3
+# CHECK-NEXT: Variable: id = {{.*}}, name = "x", type = "int64_t", valid ranges = [0x0040102f-0x00401036), location = DW_OP_reg0 EAX, DW_OP_piece 0x4, DW_OP_reg2 EDX, DW_OP_piece 0x4, decl =
+
+	.text
+	.def	@feat.00;
+	.scl	3;
+	.type	0;
+	.endef
+	.globl	@feat.00
+.set @feat.00, 1
+	.intel_syntax noprefix
+	.file	"a.cpp"
+	.def	"?bar@@YA_J_J@Z";
+	.scl	2;
+	.type	32;
+	.endef
+	.section	.text,"xr",one_only,"?bar@@YA_J_J@Z"
+	.globl	"?bar@@YA_J_J@Z"# -- Begin function ?bar@@YA_J_J@Z
+	.p2align	4, 0x90
+"?bar@@YA_J_J@Z":   # @"?bar@@YA_J_J@Z"
+Lfunc_begin0:
+	.cv_func_id 0
+	.cv_file	1 "C:\\src\\a.cpp" "CB99424BC3DD1AB059A2DBC6841147F2" 1
+	.cv_loc	0 1 1 0 # a.cpp:1:0
+	.cv_fpo_proc	"?bar@@YA_J_J@Z" 8
+# %bb.0:# %entry
+	push	ebp
+	.cv_fpo_pushreg	ebp
+	mov	ebp, esp
+	.cv_fpo_setframe	ebp
+	and	esp, -8
+	.cv_fpo_stackalign	8
+	sub	esp, 8
+	.cv_fpo_stackalloc	8
+	.cv_fpo_endprologue
+	mov	eax, dword ptr [ebp + 8]
+	mov	ecx, dword ptr [ebp + 12]
+	mov	dword ptr [esp], eax
+	mov	dword ptr [esp + 4], ecx
+Ltmp0:
+	mov	eax, dword ptr [esp]
+	mov	edx, dword ptr [esp + 4]
+	mov	esp, ebp
+	pop	ebp
+	ret
+Ltmp1:
+	.cv_fpo_endproc
+Lfunc_end0:
+# -- End function
+	.def	"?foo@@YA_J_J@Z";
+	.scl	2;
+	.type	32;
+	.endef
+	.section	.text,"xr",one_only,"?foo@@YA_J_J@Z"
+	.globl	"?foo@@YA_J_J@Z"# -- Begin function ?foo@@YA_J_J@Z
+"?foo@@YA_J_J@Z":   # @"?foo@@YA_J_J@Z"
+Lfunc_begin1:
+	.cv_func_id 1
+	.cv_fpo_proc	"?foo@@YA_J_J@Z" 8
+# %bb.0:# %entry
+	#DEBUG_VALUE: foo:x <- [DW_OP_plus_uconst 4] [$esp+0]
+	.cv_loc	1 1 3 0 # a.cpp:3:0
+	jmp	"?bar@@YA_J_J@Z"# TAILCALL
+Ltmp2:
+	.cv_fpo_endproc
+Lfunc_end1:
+# -- End function
+	.def	_main;
+	.scl	2;
+	.type	32;
+	.endef
+	.section	.text,"xr",one_only,_main
+	.globl	_main   # -- Begin function main
+_main:  # @main
+Lfunc_begin2:
+	.cv_func_id 2
+	.cv_loc	2 1 6 0 # a.cpp:6:0
+	.cv_fpo_proc	_main 8
+# %bb.0:# %entry
+	#DEBUG_VALUE: main:argv <- [DW_OP_plus_uconst 8] [$esp+0]
+	#DEBUG_VALUE: main:argc <- [DW_OP_plus_uconst 4] [$esp+0]
+	.cv_inline_site_id 3 within 2 inlined_at 1 7 0
+	.cv_loc	3 1 3 0 # a.cpp:3:0
+	mov	eax, dword ptr [esp + 4]
+	mov	ecx, eax
+	sar	ecx, 31
+Ltmp3:
+	#DEBUG_VALUE: foo:x <- [DW_OP_LLVM_fragment 0 32] $eax
+	#DEBUG_VALUE: foo:x <- [DW_OP_LLVM_fragment 32 32] $ecx
+	push	ecx
+Ltmp4:
+	push	eax
+	call	"?bar@@YA_J_J@Z"
+Ltmp5:
+	add	esp, 8
+Ltmp6:
+	.cv_loc	2 1 8 0 # a.cpp:8:0
+	xor	eax, eax
+	ret
+Ltmp7:
+	.cv_fpo_endproc
+Lfunc_end2:
+# -- End function
+	.section	.drectve,"yn"
+	.ascii	" /DEFAULTLIB:libcmt.lib"
+	.ascii	" /DEFAULTLIB:oldnames.lib"
+	.section	.debug$S,"dr"
+	.p2align	2
+	.long	4   # Debug section magic
+	.long	241
+	.long	Ltmp9-Ltmp8 # Subsection size
+Ltmp8:
+	.short	Ltmp11-Ltmp10   # Record length
+Ltmp10:
+	.short	4353# Record

[Lldb-commits] [PATCH] D123106: [trace][intel pt] Create a class for the libipt decoder wrapper

2022-04-07 Thread walter erquinigo via Phabricator via lldb-commits 
wallace added a comment.

thanks for the gotchas




Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:41
+
+/// Class that decodes a raw buffer for a single thread using the low level
+/// libipt library.

jj10306 wrote:
> "for a single thread"
> thinking ahead for the multi-CPU buffer case - this class will be responsible 
> for decoding a single trace buffer, not necessarily a single thread's buffer, 
> right? Also, come multi-buffer time, the notion of `DecodeThread` in this 
> class will need to be changed to `DecodedBuffer` or something similar that 
> decouples the decoder from a particular thread.
> 
> No need to change this now but wanted to make sure I'm understanding the plan 
> correctly.
> 
> Also, come multi-buffer time, the notion of DecodeThread in this class will 
> need to be changed to DecodedBuffer or something similar that decouples the 
> decoder from a particular thread.

Not necessarily. A possibility is that the constructor doesn't accept a 
DecodedThread. Instead, we could create a new method 
`DecodeUntilThreadIsScheduledOut(decoded_thread, end_tsc_for_this_thread)`. If 
we have something like that, we ask this decoder to put all the instructions 
into the given DecodedThread until a TSC mark is reached, which signals the end 
of a continuous execution of this thread.



Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h:23
+/// library underneath.
+void DecodeInMemoryTrace(DecodedThread _thread,
+ TraceIntelPT _intel_pt,

jj10306 wrote:
> If this file is just a wrapper around Libipt and doesn’t have any context 
> about how LLDB gets a trace (from the aux buffer or a file), should we rename 
> it to ‘DecodeTrace’ and the caller is responsible for providing the trace 
> data in a byte buffer?
> In the case of in memory trace, the caller already has the byte buffer. In 
> the case of a file, they could mmap the file and cast it to the appropriate 
> type?
DecodeTrace is definitely a better name because now we are not showing anymore 
whether the data comes from a file or just from memory


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123106

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

[Lldb-commits] [PATCH] D123281: [trace][intel pt] Create a common accessor for live and postmortem data

2022-04-07 Thread walter erquinigo via Phabricator via lldb-commits 
wallace added inline comments.



Comment at: lldb/include/lldb/Target/Trace.h:265
+  lldb::tid_t tid, llvm::StringRef kind,
+  std::function data)> callback);
+

jj10306 wrote:
> typedef the callback to be cleaner and make the intention more clear?
good idea



Comment at: lldb/include/lldb/Target/Trace.h:385
+  /// tid -> data kind -> file
+  std::map>
+  m_postmortem_thread_data;

jj10306 wrote:
> Can you explain what "kind" represents and why we need the nested map? Also, 
> I see that for live tracing we have a map for both processes and threads, why 
> is this not the case for post mortem?
those "kinds" are just arbitrarily tagged data. If you see the 
jLLDBTraceGetState, there's a mention to those data kinds. This is the client 
side of those data kinds. The difference in usage is that lldb-server will 
return it as a data blob in memory, and in the post-mortem case the data will 
be in files.

I should make those maps DenseMaps, now that I think of.

We still don't have a map for post mortem process because we haven't needed it 
yet, but we might soon.



Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:271-274
+  // The libipt library does not modify the trace buffer, hence the
+  // following casts are safe.
+  config.begin = const_cast(buffer.data());
+  config.end = const_cast(buffer.data() + buffer.size());

jj10306 wrote:
> The couple minor changes to Libiptdecoder aren't related to this diff, maybe 
> move these to D123106.
these are needed because ArrayRef and MutableArrayRef have different const 
modifiers for their pointers. I like the fact that we are now passing an 
ArrayRef here instead of a mutable one for safety



Comment at: lldb/source/Target/Trace.cpp:265
+
+  MemoryBuffer  = **trace_or_error;
+  ArrayRef array_ref(

jj10306 wrote:
> is guaranteed that the UP will be non-null? 
yes, otherwise there would have been an error in the previous if


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123281

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

[Lldb-commits] [PATCH] D91835: [lldb] Add Python bindings to print stack traces on crashes.

2022-04-07 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere updated this revision to Diff 421256.
JDevlieghere added a comment.

Rebase


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

https://reviews.llvm.org/D91835

Files:
  lldb/bindings/interface/SBDebugger.i
  lldb/include/lldb/API/SBDebugger.h
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/source/API/SBDebugger.cpp


Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -57,6 +57,8 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/ManagedStatic.h"
+#include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Signals.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -206,6 +208,15 @@
   return error;
 }
 
+void SBDebugger::PrintStackTraceOnError() {
+  LLDB_INSTRUMENT();
+
+  llvm::EnablePrettyStackTrace();
+  // We don't have a meaningful argv[0] to use, so use "SBDebugger" as a
+  // substitute.
+  llvm::sys::PrintStackTraceOnErrorSignal("SBDebugger");
+}
+
 void SBDebugger::Terminate() {
   LLDB_INSTRUMENT();
 
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -880,6 +880,7 @@
 
 import lldb
 lldb.SBDebugger.Initialize()
+lldb.SBDebugger.PrintStackTraceOnError()
 
 checkLibcxxSupport()
 checkLibstdcxxSupport()
Index: lldb/include/lldb/API/SBDebugger.h
===
--- lldb/include/lldb/API/SBDebugger.h
+++ lldb/include/lldb/API/SBDebugger.h
@@ -92,6 +92,8 @@
 
   static lldb::SBError InitializeWithErrorHandling();
 
+  static void PrintStackTraceOnError();
+
   static void Terminate();
 
   // Deprecated, use the one that takes a source_init_files bool.
Index: lldb/bindings/interface/SBDebugger.i
===
--- lldb/bindings/interface/SBDebugger.i
+++ lldb/bindings/interface/SBDebugger.i
@@ -141,6 +141,8 @@
 static SBError
 InitializeWithErrorHandling();
 
+static void PrintStackTraceOnError();
+
 static void
 Terminate();
 


Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -57,6 +57,8 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/DynamicLibrary.h"
 #include "llvm/Support/ManagedStatic.h"
+#include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Signals.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -206,6 +208,15 @@
   return error;
 }
 
+void SBDebugger::PrintStackTraceOnError() {
+  LLDB_INSTRUMENT();
+
+  llvm::EnablePrettyStackTrace();
+  // We don't have a meaningful argv[0] to use, so use "SBDebugger" as a
+  // substitute.
+  llvm::sys::PrintStackTraceOnErrorSignal("SBDebugger");
+}
+
 void SBDebugger::Terminate() {
   LLDB_INSTRUMENT();
 
Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -880,6 +880,7 @@
 
 import lldb
 lldb.SBDebugger.Initialize()
+lldb.SBDebugger.PrintStackTraceOnError()
 
 checkLibcxxSupport()
 checkLibstdcxxSupport()
Index: lldb/include/lldb/API/SBDebugger.h
===
--- lldb/include/lldb/API/SBDebugger.h
+++ lldb/include/lldb/API/SBDebugger.h
@@ -92,6 +92,8 @@
 
   static lldb::SBError InitializeWithErrorHandling();
 
+  static void PrintStackTraceOnError();
+
   static void Terminate();
 
   // Deprecated, use the one that takes a source_init_files bool.
Index: lldb/bindings/interface/SBDebugger.i
===
--- lldb/bindings/interface/SBDebugger.i
+++ lldb/bindings/interface/SBDebugger.i
@@ -141,6 +141,8 @@
 static SBError
 InitializeWithErrorHandling();
 
+static void PrintStackTraceOnError();
+
 static void
 Terminate();
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D120485: [lldb][Process/FreeBSD] Add support for address masks on aarch64

2022-04-07 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.

Thanks for enabling the test, great to see support for this outside Linux.




Comment at: lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp:817
+  return address_mask;
+}
+

I was going to suggest deduping this and the Linux function but then I found a 
problem with the Linux one that needs fixing. 
https://reviews.llvm.org/D122411#change-cwOvKZ9JBiEU

We could probably move everything within the `if thread_sp` into a function to 
reuse it but I can do that after I get the Linux one correct.



Comment at: 
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp:104
+
+
 Status NativeRegisterContextFreeBSD_arm64::ReadGPR() {

Remove the extra newline here.



Comment at: 
lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py:37-38
+backtrace = ["func_c", "func_b", "func_a", "main"] + backtrace_tail
+# This doesn't work as we get a ___lldb_unnamed_symbol on FreeBSD
+#self.assertEqual(thread.GetNumFrames(), len(backtrace))
 for frame_idx, frame in enumerate(thread.frames):

Might as well delete these lines then. Perhaps a comment before `backtrace = ` 
"# Between Linux and FreeBSD the last frames differ". (first frames? I get them 
mixed up sometimes)



Comment at: 
lldb/test/API/functionalities/unwind/aarch64_unwind_pac/TestAArch64UnwindPAC.py:42
+lldb_unnamed = "___lldb_unnamed_symbol"
+if frame.GetFunctionName()[:len(lldb_unnamed)] == lldb_unnamed:
+break

You could do:
```
if frame.GetFunctionName().startswith(""___lldb_unnamed_symbol"):
```

If I understand correctly, on FreeBSD this unamed symbol marks the end of the 
backtrace basically? If so please comment before the if to say so.



Comment at: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c:19
+
+  return (hwcap & HWCAP_PACA) != 0;
+}

Do you have any worry that `HWCAP_PACA` might not be defined or has it been 
around long enough at this point to be ok? I don't know what toolchains you 
usually build with.

For Linux we've sometimes done #ifndef something #define it to what we know the 
constant should be, but other times we just use it, with mixed results.



Comment at: lldb/test/API/functionalities/unwind/aarch64_unwind_pac/main.c:22
+#else
+static bool pac_supported(void) {
+  return true;

Add a comment like:
```
// We expect Linux to check for PAC up front using /proc/cpuinfo.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120485

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

[Lldb-commits] [PATCH] D123281: [trace][intel pt] Create a common accessor for live and postmortem data

2022-04-07 Thread Jakob Johnson via Phabricator via lldb-commits 
jj10306 added inline comments.



Comment at: lldb/include/lldb/Target/Trace.h:265
+  lldb::tid_t tid, llvm::StringRef kind,
+  std::function data)> callback);
+

typedef the callback to be cleaner and make the intention more clear?



Comment at: lldb/include/lldb/Target/Trace.h:385
+  /// tid -> data kind -> file
+  std::map>
+  m_postmortem_thread_data;

Can you explain what "kind" represents and why we need the nested map? Also, I 
see that for live tracing we have a map for both processes and threads, why is 
this not the case for post mortem?



Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:271-274
+  // The libipt library does not modify the trace buffer, hence the
+  // following casts are safe.
+  config.begin = const_cast(buffer.data());
+  config.end = const_cast(buffer.data() + buffer.size());

The couple minor changes to Libiptdecoder aren't related to this diff, maybe 
move these to D123106.



Comment at: lldb/source/Target/Trace.cpp:265
+
+  MemoryBuffer  = **trace_or_error;
+  ArrayRef array_ref(

is guaranteed that the UP will be non-null? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123281

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

[Lldb-commits] [PATCH] D123106: [trace][intel pt] Create a class for the libipt decoder wrapper

2022-04-07 Thread Jakob Johnson via Phabricator via lldb-commits 
jj10306 added a comment.

Overall looks good, just a couple cosmetic things and comments about the plan 
for multi-buffer, single thread decoding




Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:41
+
+/// Class that decodes a raw buffer for a single thread using the low level
+/// libipt library.

"for a single thread"
thinking ahead for the multi-CPU buffer case - this class will be responsible 
for decoding a single trace buffer, not necessarily a single thread's buffer, 
right? Also, come multi-buffer time, the notion of `DecodeThread` in this class 
will need to be changed to `DecodedBuffer` or something similar that decouples 
the decoder from a particular thread.

No need to change this now but wanted to make sure I'm understanding the plan 
correctly.




Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:103
+  ///   The libipt decoder status after moving to the next PSB. Negative if
+  ///   not PSB was found.
+  int FindNextSynchronizationPoint() {





Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:112
+
+if (status != -pte_eos && IsLibiptError(status)) {
+  uint64_t decoder_offset = 0;





Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:152
+  status = pt_insn_event(_decoder, , sizeof(event));
+  if (status < 0)
+break;





Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp:253
+using PtInsnDecoderUP =
+std::unique_ptr;
+

nice custom deleter (:



Comment at: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h:23
+/// library underneath.
+void DecodeInMemoryTrace(DecodedThread _thread,
+ TraceIntelPT _intel_pt,

If this file is just a wrapper around Libipt and doesn’t have any context about 
how LLDB gets a trace (from the aux buffer or a file), should we rename it to 
‘DecodeTrace’ and the caller is responsible for providing the trace data in a 
byte buffer?
In the case of in memory trace, the caller already has the byte buffer. In the 
case of a file, they could mmap the file and cast it to the appropriate type?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123106

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

[Lldb-commits] [lldb] e22a60b - Revert "Reland "[Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON"""

2022-04-07 Thread Nico Weber via lldb-commits 

Author: Nico Weber
Date: 2022-04-07T10:07:07-04:00
New Revision: e22a60b1c898a760a73417fa225c2fbe0609a69f

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

LOG: Revert "Reland "[Driver] Default CLANG_DEFAULT_PIE_ON_LINUX to ON"""

This reverts commit 2aca33baf15926afe2520a06b1427a9894226fd2.
Broke tests on several bots, see comments on https://reviews.llvm.org/D120305

Added: 


Modified: 
clang/CMakeLists.txt
clang/docs/ReleaseNotes.rst
clang/test/Driver/hip-fpie-option.hip

lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
lldb/test/Shell/ObjectFile/ELF/minidebuginfo-set-and-hit-breakpoint.test
llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn
utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h

Removed: 




diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 3a77e7b0c8d60..c0b1a20e8e8a0 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -245,7 +245,7 @@ set(PPC_LINUX_DEFAULT_IEEELONGDOUBLE OFF CACHE BOOL
 set(CLANG_SPAWN_CC1 OFF CACHE BOOL
 "Whether clang should use a new process for the CC1 invocation")
 
-option(CLANG_DEFAULT_PIE_ON_LINUX "Default to -fPIE and -pie on linux-gnu" ON)
+option(CLANG_DEFAULT_PIE_ON_LINUX "Default to -fPIE and -pie on Linux" OFF)
 
 # Manually handle default so we can change the meaning of a cached default.
 set(CLANG_ENABLE_OPAQUE_POINTERS "DEFAULT" CACHE STRING

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a64d9d383d957..df6a7d7539ee6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -279,12 +279,6 @@ Internal API Changes
 Build System Changes
 
 
-* CMake ``-DCLANG_DEFAULT_PIE_ON_LINUX=ON`` is now the default. This is used by
-  linux-gnu systems to decide whether ``-fPIE -pie`` is the default (instead of
-  ``-fno-pic -no-pie``). This matches GCC installations on many Linux distros.
-  Note: linux-android and linux-musl always default to ``-fPIE -pie``, ignoring
-  this variable. ``-DCLANG_DEFAULT_PIE_ON_LINUX`` may be removed in the future.
-
 AST Matchers
 
 

diff  --git a/clang/test/Driver/hip-fpie-option.hip 
b/clang/test/Driver/hip-fpie-option.hip
index ffd639dd5a6de..2e296a099dea5 100644
--- a/clang/test/Driver/hip-fpie-option.hip
+++ b/clang/test/Driver/hip-fpie-option.hip
@@ -1,15 +1,15 @@
-// REQUIRES: clang-driver, amdgpu-registered-target, default-pie-on-linux
+// REQUIRES: clang-driver, amdgpu-registered-target
 
 // -fPIC and -fPIE only affects host relocation model.
 // device compilation always uses PIC. 
 
 // RUN: %clang -### -target x86_64-unknown-linux-gnu \
 // RUN:   --offload-arch=gfx906 %s -nogpulib -nogpuinc \
-// RUN:   2>&1 | FileCheck -check-prefixes=DEV,HOST-PIE %s
+// RUN:   2>&1 | FileCheck -check-prefixes=DEV,HOST-STATIC %s
 
 // RUN: %clang -### -target x86_64-unknown-linux-gnu \
 // RUN:   -fgpu-rdc --offload-arch=gfx906 %s -nogpulib -nogpuinc \
-// RUN:   2>&1 | FileCheck -check-prefixes=DEV,HOST-PIE %s
+// RUN:   2>&1 | FileCheck -check-prefixes=DEV,HOST-STATIC %s
 
 // RUN: %clang -### -target x86_64-unknown-linux-gnu \
 // RUN:   --offload-arch=gfx906 %s -nogpulib -nogpuinc \
@@ -32,6 +32,7 @@
 // RUN:   2>&1 | FileCheck -check-prefixes=DEV,HOST-PIE %s
 
 // DEV-DAG: {{".*clang.*".* "-triple" "amdgcn-amd-amdhsa".* 
"-mrelocation-model" "pic" "-pic-level" "[1|2]".* "-mframe-pointer=all"}}
+// HOST-STATIC-DAG: {{".*clang.*".* "-triple" "x86_64-unknown-linux-gnu".* 
"-mrelocation-model" "static"}}
 // HOST-PIC-DAG: {{".*clang.*".* "-triple" "x86_64-unknown-linux-gnu".* 
"-mrelocation-model" "pic" "-pic-level" "2"}}
 // HOST-PIC-NOT: "-pic-is-pie"
 // HOST-PIE-DAG: {{".*clang.*".* "-triple" "x86_64-unknown-linux-gnu".* 
"-mrelocation-model" "pic" "-pic-level" "2" "-pic-is-pie"}}

diff  --git 
a/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
 
b/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
index cef500f0e7754..19aad2ab1ec32 100644
--- 
a/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
+++ 
b/lldb/test/API/functionalities/tail_call_frames/unambiguous_sequence/TestUnambiguousTailCalls.py
@@ -2,7 +2,6 @@
 from lldbsuite.test import decorators
 
 decor = [decorators.skipUnlessHasCallSiteInfo,
- decorators.skipIf(archs=['arm'],oslist=["linux"]),
  decorators.skipIf(dwarf_version=['<', '4']),
  decorators.skipIf(compiler="clang", compiler_version=['<', '11.0'])]
 lldbinline.MakeInlineTest(__file__, globals(), name="UnambiguousTailCalls_V5",

diff  --git 
a/lldb/test/Shell/ObjectFile/ELF/minidebuginfo-set-and-hit-breakpoint.test

[Lldb-commits] [PATCH] D118794: [lldb] Remove non-address bits from read/write addresses in lldb

2022-04-07 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.

> Do you have any explanation on this from AARM

Yes I do.

  linux arch/arm64/kernel/ptrace.c:
/*
 * The PAC bits can differ across data and instruction pointers
 * depending on TCR_EL1.TBID*, which we may make use of in future, so
 * we expose separate masks.
 */
unsigned long mask = ptrauth_user_pac_mask();
struct user_pac_mask uregs = {
  .data_mask = mask,
  .insn_mask = mask,
};

So currently we'll only ever see one value, in both masks. The control bit this 
refers to is:

  D13.2.131 TCR_EL1, Translation Control Register (EL1)
  
  TBID0, bit [51]
  
  0b0 TCR_EL1.TBI0 applies to Instruction and Data accesses.
  0b1 TCR_EL1.TBI0 applies to Data accesses only.

This is talked about earlier in the docs:

  Supported PAC field and relation to the use of address tagging
  
  When address tagging is used
  The PAC field is Xn[54:bottom_PAC_bit].
  
  When address tagging is not used
  The PAC field is Xn[63:56, 54:bottom_PAC_bit].

The upshot of that is that you could have top byte ignore and PAC for data, but 
only PAC for instruction addresses.

PAC itself is all or nothing, at the hardware level it's on or off. If you 
wanted to not use it for one of code or data
your runtime simply chooses not to sign any pointers. Like arm64e appears to do 
for data
(https://developer.apple.com/documentation/security/preparing_your_app_to_work_with_pointer_authentication).

The current masks that lldb shows, which have top byte ignore included already:

  (lldb) process status --verbose
  <...>
  Addressable code address mask: 0xff7f
  Addressable data address mask: 0xff7f

So the end result is the same for us. What could happen is a future extension 
that isn't top byte ignore could use
those bits instead of PAC, making the PAC specific mask 0x007f...

Though I don't know how Linux would reconsile enabling TBI for userspace then 
doing that. Maybe the amount of top byte
use is small enough it could be changed (especially top byte of code 
addresses). But chances are slim it seems to me.

So back to my ideas in the previous comment.

> Assume that they're the same, which does work for Linux, for now.

Would work fine for Linux for now and probably for a long time given that 
changing the TBI setting would be seen as an ABI issue.
And if someone decided to disable TBI completely and only use PAC, this still 
works because PAC extends into the top byte.

If they do decide to disable TBI for instructions then we're still fine given 
that the mask to extract the virtual address remains
the same. Yes the PAC mask has changed but the debugger is looking to remove 
*all* non-address bits.

E.g. If we disable TBI for instruction accesses the mask is 0xff7f 
because PAC claims the top byte.
Then the mask for data accesses is 0x007f but we add TBI to get 
0xff7f. Same result in the end.

So we could just pick one of the methods and standardise on that for sitautions 
where you don't know for sure it'll be a code address.
This will have to be `FixDataAddress` due to Arm Thumb's mode bit 0. We don't 
want to be aligning all reads to 2 bytes.
(FWIW this matches what I've done so far, though that was unintentional)

Perhaps we add a third method to make that clear (name subject to change) 
`FixAnyAddress`. Then the Arm code can forward that to fixdata and AArch64
can pick either data or code. For situations where you're sure you can pick 
code or data e.g. code breakpoint on an address.

> Add a method that does both fixes, on the assumption that the virtual address 
> size for code and data is the same so no harm done and all bits will be 
> removed either way.

The Arm Thumb problem means this is not going to work. (not that those targets 
are likely to care about non-address bits but these Fix calls are made from 
generic code
so it does still matter)

> Extensively track whether addresses refer to code or data

Isn't realistic a lot of the time. Though there are some clear situations where 
FixCode or FixData makes more sense so we can do some of this, just not an lldb 
wide tracking
framework sort of thing.

So my suggestion for a solution would be to add a FixAnyAddress alongside 
FixCode and FixData, and use that whenever it could be either. Tricky things 
like Arm Thumb can
then choose what the most "safe" fix is.

Tell me if that logic makes sense.

> Which will mean we actually dont need two separate functions.

At the ABI plugin level we do simply due to Arm Thumb existing. Lower down yeah 
you could get away with reading just one of the PAC masks but it's not much of 
a saving.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118794

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

[Lldb-commits] [lldb] 3031fa8 - [lldb] Fix building standalone LLDB on Windows.

2022-04-07 Thread Martin Storsjö via lldb-commits 

Author: Mehdi Chinoune
Date: 2022-04-07T12:30:33+03:00
New Revision: 3031fa88f01e59dcacb0a3020fb9a27ccf2b7615

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

LOG: [lldb] Fix building standalone LLDB on Windows.

It was broken since https://reviews.llvm.org/D110172

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
llvm/include/llvm/Config/config.h.cmake
llvm/include/llvm/Config/llvm-config.h.cmake

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp 
b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
index 16a8ad8a10422..130353dfc67a6 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -57,7 +57,7 @@
 #include "Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h"
 
 #if defined(_WIN32)
-#include "llvm/Config/config.h"
+#include "llvm/Config/llvm-config.h"
 #endif
 
 using namespace lldb;

diff  --git a/llvm/include/llvm/Config/config.h.cmake 
b/llvm/include/llvm/Config/config.h.cmake
index 2098c249e20db..cc514b17f8f0e 100644
--- a/llvm/include/llvm/Config/config.h.cmake
+++ b/llvm/include/llvm/Config/config.h.cmake
@@ -50,9 +50,6 @@
don't. */
 #cmakedefine01 HAVE_DECL_STRERROR_S
 
-/* Define to 1 if you have the DIA SDK installed, and to 0 if you don't. */
-#cmakedefine01 LLVM_ENABLE_DIA_SDK
-
 /* Define to 1 if you have the  header file. */
 #cmakedefine HAVE_DLFCN_H ${HAVE_DLFCN_H}
 

diff  --git a/llvm/include/llvm/Config/llvm-config.h.cmake 
b/llvm/include/llvm/Config/llvm-config.h.cmake
index f15eef3a4dea9..7d420bbc71645 100644
--- a/llvm/include/llvm/Config/llvm-config.h.cmake
+++ b/llvm/include/llvm/Config/llvm-config.h.cmake
@@ -113,4 +113,7 @@
  * in non assert builds */
 #cmakedefine01 LLVM_UNREACHABLE_OPTIMIZE
 
+/* Define to 1 if you have the DIA SDK installed, and to 0 if you don't. */
+#cmakedefine01 LLVM_ENABLE_DIA_SDK
+
 #endif



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

[Lldb-commits] [PATCH] D123254: [lldb] Disable GCC's -Wstringop-truncation warning. NFC.

2022-04-07 Thread Martin Storsjö 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 rG5560b9e88423: [lldb] [CMake] Disable GCCs 
-Wstringop-truncation warning. NFC. (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123254

Files:
  lldb/cmake/modules/LLDBConfig.cmake


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -183,6 +183,9 @@
 check_cxx_compiler_flag("-Wstrict-aliasing" CXX_SUPPORTS_STRICT_ALIASING)
 append_if(CXX_SUPPORTS_STRICT_ALIASING "-Wno-strict-aliasing" CMAKE_CXX_FLAGS)
 
+check_cxx_compiler_flag("-Wstringop-truncation" 
CXX_SUPPORTS_STRINGOP_TRUNCATION)
+append_if(CXX_SUPPORTS_STRINGOP_TRUNCATION "-Wno-stringop-truncation" 
CMAKE_CXX_FLAGS)
+
 # Disable Clang warnings
 check_cxx_compiler_flag("-Wdeprecated-register" 
CXX_SUPPORTS_DEPRECATED_REGISTER)
 append_if(CXX_SUPPORTS_DEPRECATED_REGISTER "-Wno-deprecated-register" 
CMAKE_CXX_FLAGS)


Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -183,6 +183,9 @@
 check_cxx_compiler_flag("-Wstrict-aliasing" CXX_SUPPORTS_STRICT_ALIASING)
 append_if(CXX_SUPPORTS_STRICT_ALIASING "-Wno-strict-aliasing" CMAKE_CXX_FLAGS)
 
+check_cxx_compiler_flag("-Wstringop-truncation" CXX_SUPPORTS_STRINGOP_TRUNCATION)
+append_if(CXX_SUPPORTS_STRINGOP_TRUNCATION "-Wno-stringop-truncation" CMAKE_CXX_FLAGS)
+
 # Disable Clang warnings
 check_cxx_compiler_flag("-Wdeprecated-register" CXX_SUPPORTS_DEPRECATED_REGISTER)
 append_if(CXX_SUPPORTS_DEPRECATED_REGISTER "-Wno-deprecated-register" CMAKE_CXX_FLAGS)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 5560b9e - [lldb] [CMake] Disable GCC's -Wstringop-truncation warning. NFC.

2022-04-07 Thread Martin Storsjö via lldb-commits 

Author: Martin Storsjö
Date: 2022-04-07T12:09:01+03:00
New Revision: 5560b9e88423dbe044f4e82d4dc3d4b6dbd0e678

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

LOG: [lldb] [CMake] Disable GCC's -Wstringop-truncation warning. NFC.

This warning gives false positives about lldb's correct use of
strncpy to fill fixed length fields that don't need null termination,
in lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp, like this:

In file included from /usr/include/string.h:495,
 from /usr/include/c++/9/cstring:42,
 from ../include/llvm/ADT/StringRef.h:19,
 from 
../tools/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:10:
In function ‘char* strncpy(char*, const char*, size_t)’,
inlined from ‘lldb::offset_t CreateAllImageInfosPayload(const 
ProcessSP&, lldb::offset_t, lldb_private::StreamString&, lldb::SaveCoreStyle)’ 
at ../tools/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:6341:16:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:34: warning: 
‘char* __builtin_strncpy(char*, const char*, long unsigned int)’ specified 
bound 16 equals destination size [-Wstringop-truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos 
(__dest));
  |  ^~~

The warning could be squelched locally with

#pragma GCC diagnostic ignored "-Wstringop-truncation"

too, but Clang also interprets those GCC pragmas, and produces
a -Wunknown-warning-option warning instead. That could be remedied
by wrapping the pragma in an "#ifndef __clang__" - but that makes
things even more messy. Instead, just silence this warning entirely.

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

Added: 


Modified: 
lldb/cmake/modules/LLDBConfig.cmake

Removed: 




diff  --git a/lldb/cmake/modules/LLDBConfig.cmake 
b/lldb/cmake/modules/LLDBConfig.cmake
index 25d1cc526ab03..987353517d0d5 100644
--- a/lldb/cmake/modules/LLDBConfig.cmake
+++ b/lldb/cmake/modules/LLDBConfig.cmake
@@ -183,6 +183,9 @@ append_if(CXX_SUPPORTS_UNKNOWN_PRAGMAS 
"-Wno-unknown-pragmas" CMAKE_CXX_FLAGS)
 check_cxx_compiler_flag("-Wstrict-aliasing" CXX_SUPPORTS_STRICT_ALIASING)
 append_if(CXX_SUPPORTS_STRICT_ALIASING "-Wno-strict-aliasing" CMAKE_CXX_FLAGS)
 
+check_cxx_compiler_flag("-Wstringop-truncation" 
CXX_SUPPORTS_STRINGOP_TRUNCATION)
+append_if(CXX_SUPPORTS_STRINGOP_TRUNCATION "-Wno-stringop-truncation" 
CMAKE_CXX_FLAGS)
+
 # Disable Clang warnings
 check_cxx_compiler_flag("-Wdeprecated-register" 
CXX_SUPPORTS_DEPRECATED_REGISTER)
 append_if(CXX_SUPPORTS_DEPRECATED_REGISTER "-Wno-deprecated-register" 
CMAKE_CXX_FLAGS)



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

[Lldb-commits] [PATCH] D118794: [lldb] Remove non-address bits from read/write addresses in lldb

2022-04-07 Thread Muhammad Omair Javaid via Phabricator via lldb-commits 
omjavaid added a comment.

In D118794#3405724 , @DavidSpickett 
wrote:

> Switch to removing non-address bits in lldb instead of lldb-server.
>
> The breakpoint issues I mention only really happen if you try to break on a 
> tagged
> function pointer. Which is pretty niche, but I hope to address it later 
> anyway.
>
> On the issue of whether to use FixData vs FixCode there's 2 maybe 3 ways to 
> go:
>
> - Assume that they're the same, which does work for Linux, for now.
> - Add a method that does both fixes, on the assumption that the virtual 
> address size for code and data is the same so no harm done and all bits will 
> be removed either way.
> - Extensively track whether addresses refer to code or data. In some 
> situations this is possible (looking at the exec bits of a memory mapping for 
> example) but I don't have a great idea what that looks like at this time.
>
> Option 2 seems like a good way to go for now.

So on the topic of separate code/data address masks (Linux specific). I dont 
recall if the actual position of the mask in the address changes or not? It may 
be the case that we have separate code and address masks but their position in 
the address bits is fixed for both. Which will mean we actually dont need two 
separate functions. I tried fidning it out in Linux documentation but it only 
says "Separate masks are exposed for data pointers and instruction pointers". 
It doesnt specifically says if the location of the both can be different or not.
Do you have any explanation on this from AARM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118794

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

[Lldb-commits] [PATCH] D123269: debugserver would never write modified xmm/ymm/zmm register values into the inferior

2022-04-07 Thread Jason Molenda via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4d3cc2783138: Correct debugserver to write xmm/ymm/zmm reg 
values (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123269

Files:
  lldb/test/Shell/Register/x86-64-write.test
  lldb/test/Shell/Register/x86-64-ymm-write.test
  lldb/test/Shell/Register/x86-64-zmm-write.test
  lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp


Index: lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
===
--- lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+++ lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
@@ -2632,7 +2632,8 @@
>value.uint8, 16);
 memcpy(_state.context.fpu.avx.__fpu_ymmh0 + (reg - fpu_ymm0),
(>value.uint8) + 16, 16);
-return true;
+success = true;
+break;
   case fpu_k0:
   case fpu_k1:
   case fpu_k2:
@@ -2643,7 +2644,8 @@
   case fpu_k7:
 memcpy(_state.context.fpu.avx512f.__fpu_k0 + (reg - fpu_k0),
>value.uint8, 8);
-return true;
+success = true;
+break;
   case fpu_zmm0:
   case fpu_zmm1:
   case fpu_zmm2:
@@ -2666,7 +2668,8 @@
>value.uint8 + 16, 16);
 memcpy(_state.context.fpu.avx512f.__fpu_zmmh0 + (reg - fpu_zmm0),
>value.uint8 + 32, 32);
-return true;
+success = true;
+break;
   case fpu_zmm16:
   case fpu_zmm17:
   case fpu_zmm18:
@@ -2685,7 +2688,8 @@
   case fpu_zmm31:
 memcpy(_state.context.fpu.avx512f.__fpu_zmm16 + (reg - fpu_zmm16),
>value.uint8, 64);
-return true;
+success = true;
+break;
   }
   break;
 
Index: lldb/test/Shell/Register/x86-64-zmm-write.test
===
--- lldb/test/Shell/Register/x86-64-zmm-write.test
+++ lldb/test/Shell/Register/x86-64-zmm-write.test
@@ -1,4 +1,9 @@
-# XFAIL: system-darwin
+# xfail with system debugserver until the fix for
+# https://reviews.llvm.org/D123269 in
+# lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+# has made it into released tools.
+# XFAIL: system-debugserver
+
 # XFAIL: system-freebsd
 # XFAIL: system-linux
 # XFAIL: system-netbsd
Index: lldb/test/Shell/Register/x86-64-ymm-write.test
===
--- lldb/test/Shell/Register/x86-64-ymm-write.test
+++ lldb/test/Shell/Register/x86-64-ymm-write.test
@@ -1,4 +1,9 @@
-# XFAIL: system-darwin
+# xfail with system debugserver until the fix for
+# https://reviews.llvm.org/D123269 in
+# lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+# has made it into released tools.
+# XFAIL: system-debugserver
+
 # XFAIL: system-windows
 # REQUIRES: native && target-x86_64 && native-cpu-avx
 # RUN: %clangxx_host %p/Inputs/x86-ymm-write.cpp -o %t
Index: lldb/test/Shell/Register/x86-64-write.test
===
--- lldb/test/Shell/Register/x86-64-write.test
+++ lldb/test/Shell/Register/x86-64-write.test
@@ -1,4 +1,9 @@
-# XFAIL: system-darwin
+# xfail with system debugserver until the fix for
+# https://reviews.llvm.org/D123269 in
+# lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+# has made it into released tools.
+# XFAIL: system-debugserver
+
 # XFAIL: system-windows
 # REQUIRES: native && target-x86_64
 # RUN: %clangxx_host %p/Inputs/x86-64-write.cpp -o %t


Index: lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
===
--- lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+++ lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
@@ -2632,7 +2632,8 @@
>value.uint8, 16);
 memcpy(_state.context.fpu.avx.__fpu_ymmh0 + (reg - fpu_ymm0),
(>value.uint8) + 16, 16);
-return true;
+success = true;
+break;
   case fpu_k0:
   case fpu_k1:
   case fpu_k2:
@@ -2643,7 +2644,8 @@
   case fpu_k7:
 memcpy(_state.context.fpu.avx512f.__fpu_k0 + (reg - fpu_k0),
>value.uint8, 8);
-return true;
+success = true;
+break;
   case fpu_zmm0:
   case fpu_zmm1:
   case fpu_zmm2:
@@ -2666,7 +2668,8 @@
>value.uint8 + 16, 16);
 memcpy(_state.context.fpu.avx512f.__fpu_zmmh0 + (reg - fpu_zmm0),
>value.uint8 + 32, 32);
-return true;
+success = true;
+break;
   case fpu_zmm16:
   case fpu_zmm17:
   case fpu_zmm18:
@@ -2685,7 +2688,8 @@
   case fpu_zmm31:
 memcpy(_state.context.fpu.avx512f.__fpu_zmm16 +

[Lldb-commits] [lldb] 4d3cc27 - Correct debugserver to write xmm/ymm/zmm reg values

2022-04-07 Thread Jason Molenda via lldb-commits 

Author: Jason Molenda
Date: 2022-04-06T23:36:52-07:00
New Revision: 4d3cc27831383987b38e60875573982af5e0538b

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

LOG: Correct debugserver to write xmm/ymm/zmm reg values

debugserver does not call thread_set_state when changing xmm/ymm/zmm
register values, so the register contents are never updated.  Fix
that.  Mark the shell tests which xfail'ed these tests on darwin systems
to xfail them when the system debugserver, they will pass when using
the in-tree debugserver.  When this makes it into the installed
system debugservers, we'll remove the xfails.

Differential Revision: https://reviews.llvm.org/D123269
rdar://91258333
rdar://31294382

Added: 


Modified: 
lldb/test/Shell/Register/x86-64-write.test
lldb/test/Shell/Register/x86-64-ymm-write.test
lldb/test/Shell/Register/x86-64-zmm-write.test
lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp

Removed: 




diff  --git a/lldb/test/Shell/Register/x86-64-write.test 
b/lldb/test/Shell/Register/x86-64-write.test
index 29dd7bc71b3b2..1b6b1e8fb1806 100644
--- a/lldb/test/Shell/Register/x86-64-write.test
+++ b/lldb/test/Shell/Register/x86-64-write.test
@@ -1,4 +1,9 @@
-# XFAIL: system-darwin
+# xfail with system debugserver until the fix for
+# https://reviews.llvm.org/D123269 in
+# lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+# has made it into released tools.
+# XFAIL: system-debugserver
+
 # XFAIL: system-windows
 # REQUIRES: native && target-x86_64
 # RUN: %clangxx_host %p/Inputs/x86-64-write.cpp -o %t

diff  --git a/lldb/test/Shell/Register/x86-64-ymm-write.test 
b/lldb/test/Shell/Register/x86-64-ymm-write.test
index 05b3c2f52d2c6..9d9f3451b41a8 100644
--- a/lldb/test/Shell/Register/x86-64-ymm-write.test
+++ b/lldb/test/Shell/Register/x86-64-ymm-write.test
@@ -1,4 +1,9 @@
-# XFAIL: system-darwin
+# xfail with system debugserver until the fix for
+# https://reviews.llvm.org/D123269 in
+# lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+# has made it into released tools.
+# XFAIL: system-debugserver
+
 # XFAIL: system-windows
 # REQUIRES: native && target-x86_64 && native-cpu-avx
 # RUN: %clangxx_host %p/Inputs/x86-ymm-write.cpp -o %t

diff  --git a/lldb/test/Shell/Register/x86-64-zmm-write.test 
b/lldb/test/Shell/Register/x86-64-zmm-write.test
index 4b22235c1b926..6304205779234 100644
--- a/lldb/test/Shell/Register/x86-64-zmm-write.test
+++ b/lldb/test/Shell/Register/x86-64-zmm-write.test
@@ -1,4 +1,9 @@
-# XFAIL: system-darwin
+# xfail with system debugserver until the fix for
+# https://reviews.llvm.org/D123269 in
+# lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+# has made it into released tools.
+# XFAIL: system-debugserver
+
 # XFAIL: system-freebsd
 # XFAIL: system-linux
 # XFAIL: system-netbsd

diff  --git a/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp 
b/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
index 7270ecb55cc06..5a62e2a8d12e2 100644
--- a/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
@@ -2632,7 +2632,8 @@ bool DNBArchImplX86_64::SetRegisterValue(uint32_t set, 
uint32_t reg,
>value.uint8, 16);
 memcpy(_state.context.fpu.avx.__fpu_ymmh0 + (reg - fpu_ymm0),
(>value.uint8) + 16, 16);
-return true;
+success = true;
+break;
   case fpu_k0:
   case fpu_k1:
   case fpu_k2:
@@ -2643,7 +2644,8 @@ bool DNBArchImplX86_64::SetRegisterValue(uint32_t set, 
uint32_t reg,
   case fpu_k7:
 memcpy(_state.context.fpu.avx512f.__fpu_k0 + (reg - fpu_k0),
>value.uint8, 8);
-return true;
+success = true;
+break;
   case fpu_zmm0:
   case fpu_zmm1:
   case fpu_zmm2:
@@ -2666,7 +2668,8 @@ bool DNBArchImplX86_64::SetRegisterValue(uint32_t set, 
uint32_t reg,
>value.uint8 + 16, 16);
 memcpy(_state.context.fpu.avx512f.__fpu_zmmh0 + (reg - fpu_zmm0),
>value.uint8 + 32, 32);
-return true;
+success = true;
+break;
   case fpu_zmm16:
   case fpu_zmm17:
   case fpu_zmm18:
@@ -2685,7 +2688,8 @@ bool DNBArchImplX86_64::SetRegisterValue(uint32_t set, 
uint32_t reg,
   case fpu_zmm31:
 memcpy(_state.context.fpu.avx512f.__fpu_zmm16 + (reg - fpu_zmm16),
>value.uint8, 64);
-return true;
+success = true;
+break;
   }
   break;
 



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