[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-07 Thread Nicholas Mosier via lldb-commits 

https://github.com/nmosier created 
https://github.com/llvm/llvm-project/pull/77252

Fix #77251.

>From c8b55528ce76a5d3ae1736a0f73499d96173d927 Mon Sep 17 00:00:00 2001
From: Nicholas Mosier 
Date: Sun, 7 Jan 2024 20:06:55 +
Subject: [PATCH] [lldb] Fix Intel PT plugin compile errors

Fix #77251.
---
 .../intel-pt/CommandObjectTraceStartIntelPT.cpp  |  4 +---
 lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp |  5 +
 lldb/source/Plugins/Trace/intel-pt/DecodedThread.h   |  4 
 lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp |  4 ++--
 .../Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp|  4 ++--
 .../Trace/intel-pt/TraceIntelPTBundleLoader.cpp  | 12 +---
 6 files changed, 19 insertions(+), 14 deletions(-)

diff --git 
a/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp 
b/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
index d4f7dc354e9fed..44224229e625bf 100644
--- a/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
@@ -158,7 +158,7 @@ 
CommandObjectProcessTraceStartIntelPT::CommandOptions::GetDefinitions() {
   return llvm::ArrayRef(g_process_trace_start_intel_pt_options);
 }
 
-bool CommandObjectProcessTraceStartIntelPT::DoExecute(
+void CommandObjectProcessTraceStartIntelPT::DoExecute(
 Args &command, CommandReturnObject &result) {
   if (Error err = m_trace.Start(
   m_options.m_ipt_trace_size, m_options.m_process_buffer_size_limit,
@@ -167,8 +167,6 @@ bool CommandObjectProcessTraceStartIntelPT::DoExecute(
 result.SetError(Status(std::move(err)));
   else
 result.SetStatus(eReturnStatusSuccessFinishResult);
-
-  return result.Succeeded();
 }
 
 std::optional
diff --git a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp 
b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
index 17f8f51bdf0e0d..145e0386f6a023 100644
--- a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
@@ -85,6 +85,11 @@ double DecodedThread::NanosecondsRange::GetInterpolatedTime(
   return interpolate(next_range->nanos);
 }
 
+DecodedThread::TraceItemStorage::TraceItemStorage(
+const TraceItemStorage &other) {
+  std::memcpy(this, &other, sizeof *this);
+}
+
 uint64_t DecodedThread::GetItemsCount() const { return m_item_kinds.size(); }
 
 lldb::addr_t
diff --git a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h 
b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
index 5745cdb67ab68f..88bf748d186a36 100644
--- a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -276,6 +276,10 @@ class DecodedThread : public 
std::enable_shared_from_this {
 
 /// The string message of this item if it's an error
 std::string error;
+
+TraceItemStorage() {}
+~TraceItemStorage() {}
+TraceItemStorage(const TraceItemStorage &other);
   };
 
   /// Create a new trace item.
diff --git a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp 
b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
index cdf81954eee902..f8241ef6a79329 100644
--- a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
@@ -572,7 +572,7 @@ Error 
lldb_private::trace_intel_pt::DecodeSingleTraceForThread(
 Expected decoder = PSBBlockDecoder::Create(
 trace_intel_pt, block, buffer.slice(block.psb_offset, block.size),
 *decoded_thread.GetThread()->GetProcess(),
-i + 1 < blocks->size() ? blocks->at(i + 1).starting_ip : None,
+i + 1 < blocks->size() ? blocks->at(i + 1).starting_ip : std::nullopt,
 decoded_thread, std::nullopt);
 if (!decoder)
   return decoder.takeError();
@@ -640,7 +640,7 @@ Error 
lldb_private::trace_intel_pt::DecodeSystemWideTraceForThread(
   *decoded_thread.GetThread()->GetProcess(),
   j + 1 < execution.psb_blocks.size()
   ? execution.psb_blocks[j + 1].starting_ip
-  : None,
+  : std::nullopt,
   decoded_thread, execution.thread_execution.GetEndTSC());
   if (!decoder)
 return decoder.takeError();
diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp 
b/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
index 66d342196cf10d..dda6cd74343f0f 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -35,7 +35,7 @@ void TraceCursorIntelPT::Next() {
 void TraceCursorIntelPT::ClearTimingRangesIfInvalid() {
   if (m_tsc_range_calculated) {
 if (!m_tsc_range || m_pos < 0 || !m_tsc_range->InRange(m_pos)) {
-  m_tsc_range = None;
+  m_tsc_range = std::nullopt;
   m_tsc_range_calculated = false;
 }
   }
@@ -43,7 +43,7 @@ void TraceCursorIntelPT::ClearTimingRangesIfInvalid() {
   if (m_nanoseconds_range_calculated) {
 if (!m_n

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-08 Thread Nicholas Mosier via lldb-commits 


@@ -276,6 +276,10 @@ class DecodedThread : public 
std::enable_shared_from_this {
 
 /// The string message of this item if it's an error
 std::string error;
+
+TraceItemStorage() {}
+~TraceItemStorage() {}

nmosier wrote:

`TraceItemStorage()` can't be defaulted because the union member `std::string 
error` has a non-trivial default constructor (quoted from 
[here](https://en.cppreference.com/w/cpp/language/union)):
> If a union contains a non-static data member with a non-trivial default 
> constructor, the default constructor of the union is deleted by default 
> unless a variant member of the union has a default member initializer.

`~TraceItemStorage()` can't be defaulted because the union member `std::string 
error` has a non-trivial destructor (quoted from 
[here](https://en.cppreference.com/w/cpp/language/union)):
> If a union contains a non-static data member with a non-trivial special 
> member function (copy/move constructor, copy/move assignment, or destructor), 
> that function is deleted by default in the union and needs to be defined 
> explicitly by the programmer.


Anyways, if I try to default them, I get these compile errors:
```
In file included from 
/usr/include/x86_64-linux-gnu/c++/11/bits/c++allocator.h:33,
 from /usr/include/c++/11/bits/allocator.h:46,
 from /usr/include/c++/11/unordered_map:40,
 from 
/afs/cs.stanford.edu/u/nmosier/llvm/bugfix-lldb-intelpt/lldb/include/lldb/Target/Trace.h:13,
 from 
/afs/cs.stanford.edu/u/nmosier/llvm/bugfix-lldb-intelpt/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:13,
 from 
/afs/cs.stanford.edu/u/nmosier/llvm/bugfix-lldb-intelpt/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:9:
/usr/include/c++/11/ext/new_allocator.h: In instantiation of ‘void 
__gnu_cxx::new_allocator<_Tp>::construct(_Up*, _Args&& ...) [with _Up = 
lldb_private::trace_intel_pt::DecodedThread::TraceItemStorage; _Args = {}; _Tp 
= lldb_private::trace_intel_pt::DecodedThread::TraceItemStorage]’:
/usr/include/c++/11/bits/alloc_traits.h:516:17:   required from ‘static void 
std::allocator_traits 
>::construct(std::allocator_traits >::allocator_type&, 
_Up*, _Args&& ...) [with _Up = 
lldb_private::trace_intel_pt::DecodedThread::TraceItemStorage; _Args = {}; _Tp 
= lldb_private::trace_intel_pt::DecodedThread::TraceItemStorage; 
std::allocator_traits >::allocator_type = 
std::allocator]’
/usr/include/c++/11/bits/vector.tcc:115:30:   required from ‘std::vector<_Tp, 
_Alloc>::reference std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with 
_Args = {}; _Tp = 
lldb_private::trace_intel_pt::DecodedThread::TraceItemStorage; _Alloc = 
std::allocator; 
std::vector<_Tp, _Alloc>::reference = 
lldb_private::trace_intel_pt::DecodedThread::TraceItemStorage&]’
/afs/cs.stanford.edu/u/nmosier/llvm/bugfix-lldb-intelpt/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:110:27:
   required from here
/usr/include/c++/11/ext/new_allocator.h:162:11: error: use of deleted function 
‘lldb_private::trace_intel_pt::DecodedThread::TraceItemStorage::TraceItemStorage()’
  162 | { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); }
  |   ^~
In file included from 
/afs/cs.stanford.edu/u/nmosier/llvm/bugfix-lldb-intelpt/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:9:
/afs/cs.stanford.edu/u/nmosier/llvm/bugfix-lldb-intelpt/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:280:5:
 note: 
‘lldb_private::trace_intel_pt::DecodedThread::TraceItemStorage::TraceItemStorage()’
 is implicitly deleted because the default definition would be ill-formed:
  280 | TraceItemStorage() = default;
  | ^~~~
/afs/cs.stanford.edu/u/nmosier/llvm/bugfix-lldb-intelpt/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:278:17:
 error: union member 
‘lldb_private::trace_intel_pt::DecodedThread::TraceItemStorage::error’ with 
non-trivial ‘std::__cxx11::basic_string<_CharT, _Traits, 
_Alloc>::basic_string() [with _CharT = char; _Traits = std::char_traits; 
_Alloc = std::allocator]’
  278 | std::string error;
  | ^
```
and
```
In file included from /usr/include/c++/11/optional:44,
 from 
/afs/cs.stanford.edu/u/nmosier/llvm/bugfix-lldb-intelpt/lldb/include/lldb/Target/Trace.h:12,
 from 
/afs/cs.stanford.edu/u/nmosier/llvm/bugfix-lldb-intelpt/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:13,
 from 
/afs/cs.stanford.edu/u/nmosier/llvm/bugfix-lldb-intelpt/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:9:
/usr/include/c++/11/bits/stl_construct.h: In instantiation of ‘void 
std::_Destroy(_ForwardIterator, _ForwardIterator) [with _ForwardIterator = 
lldb_private::trace_intel_pt::DecodedThread::TraceItemStorage*]’:
/usr/include/c++/11/bits/alloc_traits.h:848:15:   required from ‘void 
std::_Destroy(_ForwardIterator,

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-08 Thread Nicholas Mosier via lldb-commits 


@@ -85,6 +85,11 @@ double DecodedThread::NanosecondsRange::GetInterpolatedTime(
   return interpolate(next_range->nanos);
 }
 
+DecodedThread::TraceItemStorage::TraceItemStorage(
+const TraceItemStorage &other) {
+  std::memcpy(this, &other, sizeof *this);

nmosier wrote:

`DecodedThread` does know which member is being used (it uses `enum 
TraceItemKind` to tag the union); however, it stores the tag separately in 
`DecodedThread::m_item_kinds` for space/alignment reasons.
I added this copy constructor for use by `std::vector 
DecodedThread::m_trace_data`; without it, it raises a compile error because it 
doesn't know how to copy/move `TraceItemStorage` elements when resizing the 
vector. This means that we can't add a second TraceItemKind argument to the 
copy constructor, because std::vector won't know how to use 
that.

https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-08 Thread Nicholas Mosier via lldb-commits 

https://github.com/nmosier updated 
https://github.com/llvm/llvm-project/pull/77252

>From c99dbd7f915ab996a68c4d9eb0cee2edb3b33b84 Mon Sep 17 00:00:00 2001
From: Nicholas Mosier 
Date: Sun, 7 Jan 2024 20:06:55 +
Subject: [PATCH] [lldb] Fix Intel PT plugin compile errors

Fix #77251.
---
 .../Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp   | 4 +---
 lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp| 5 +
 lldb/source/Plugins/Trace/intel-pt/DecodedThread.h  | 4 
 lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp| 4 ++--
 lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp   | 4 ++--
 .../Plugins/Trace/intel-pt/TraceIntelPTBundleLoader.cpp | 6 +++---
 6 files changed, 17 insertions(+), 10 deletions(-)

diff --git 
a/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp 
b/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
index d4f7dc354e9fed..44224229e625bf 100644
--- a/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
@@ -158,7 +158,7 @@ 
CommandObjectProcessTraceStartIntelPT::CommandOptions::GetDefinitions() {
   return llvm::ArrayRef(g_process_trace_start_intel_pt_options);
 }
 
-bool CommandObjectProcessTraceStartIntelPT::DoExecute(
+void CommandObjectProcessTraceStartIntelPT::DoExecute(
 Args &command, CommandReturnObject &result) {
   if (Error err = m_trace.Start(
   m_options.m_ipt_trace_size, m_options.m_process_buffer_size_limit,
@@ -167,8 +167,6 @@ bool CommandObjectProcessTraceStartIntelPT::DoExecute(
 result.SetError(Status(std::move(err)));
   else
 result.SetStatus(eReturnStatusSuccessFinishResult);
-
-  return result.Succeeded();
 }
 
 std::optional
diff --git a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp 
b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
index 17f8f51bdf0e0d..145e0386f6a023 100644
--- a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
@@ -85,6 +85,11 @@ double DecodedThread::NanosecondsRange::GetInterpolatedTime(
   return interpolate(next_range->nanos);
 }
 
+DecodedThread::TraceItemStorage::TraceItemStorage(
+const TraceItemStorage &other) {
+  std::memcpy(this, &other, sizeof *this);
+}
+
 uint64_t DecodedThread::GetItemsCount() const { return m_item_kinds.size(); }
 
 lldb::addr_t
diff --git a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h 
b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
index 5745cdb67ab68f..88bf748d186a36 100644
--- a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -276,6 +276,10 @@ class DecodedThread : public 
std::enable_shared_from_this {
 
 /// The string message of this item if it's an error
 std::string error;
+
+TraceItemStorage() {}
+~TraceItemStorage() {}
+TraceItemStorage(const TraceItemStorage &other);
   };
 
   /// Create a new trace item.
diff --git a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp 
b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
index cdf81954eee902..f8241ef6a79329 100644
--- a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
@@ -572,7 +572,7 @@ Error 
lldb_private::trace_intel_pt::DecodeSingleTraceForThread(
 Expected decoder = PSBBlockDecoder::Create(
 trace_intel_pt, block, buffer.slice(block.psb_offset, block.size),
 *decoded_thread.GetThread()->GetProcess(),
-i + 1 < blocks->size() ? blocks->at(i + 1).starting_ip : None,
+i + 1 < blocks->size() ? blocks->at(i + 1).starting_ip : std::nullopt,
 decoded_thread, std::nullopt);
 if (!decoder)
   return decoder.takeError();
@@ -640,7 +640,7 @@ Error 
lldb_private::trace_intel_pt::DecodeSystemWideTraceForThread(
   *decoded_thread.GetThread()->GetProcess(),
   j + 1 < execution.psb_blocks.size()
   ? execution.psb_blocks[j + 1].starting_ip
-  : None,
+  : std::nullopt,
   decoded_thread, execution.thread_execution.GetEndTSC());
   if (!decoder)
 return decoder.takeError();
diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp 
b/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
index 66d342196cf10d..dda6cd74343f0f 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -35,7 +35,7 @@ void TraceCursorIntelPT::Next() {
 void TraceCursorIntelPT::ClearTimingRangesIfInvalid() {
   if (m_tsc_range_calculated) {
 if (!m_tsc_range || m_pos < 0 || !m_tsc_range->InRange(m_pos)) {
-  m_tsc_range = None;
+  m_tsc_range = std::nullopt;
   m_tsc_range_calculated = false;
 }
   }
@@ -43,7 +43,7 @@ void TraceCursorIntelPT::ClearTimingRangesIfInvalid() {
   if (m_nanoseconds_range_calculated

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-08 Thread Nicholas Mosier via lldb-commits 


@@ -103,12 +103,10 @@ TraceIntelPTBundleLoader::CreateEmptyProcess(lldb::pid_t 
pid,
   ParsedProcess parsed_process;
   parsed_process.target_sp = target_sp;
 
-  // This should instead try to directly create an instance of ProcessTrace.
-  // ProcessSP process_sp = target_sp->CreateProcess(
-  ///*listener*/ nullptr, "trace",
-  ///*crash_file*/ nullptr,
-  ///*can_connect*/ false);
-
+  ProcessSP process_sp = target_sp->CreateProcess(
+  /*listener*/ nullptr, "trace",
+  /*crash_file*/ nullptr,
+  /*can_connect*/ false);

nmosier wrote:

I double-checked, and looks like commit 555a71be457fo3 commented out these four 
lines. Here's the commit message:
```
commit 555a71be457f351411b89c6a6a66aeecf7ca5291
Author: Walter Erquinigo 
Date:   Mon Nov 6 19:45:52 2023 -0500

[LLDB] Don't forcefully initialize the process trace plugin (#71455)

This was causing some process to wrongfully be handled by ProcessTrace.

The only place this was being used is in the intel pt plugin, but it
doesn't even build anymore, so I'm sure no one is using it.
```
I think the commit author meant to comment out the following line, 
`process_sp->SetID(static_cast(pid));`; however, they didn't catch 
the compile error because apparently the Intel PT plugin wasn't compiling back 
then anyway.

I updated my patch to keep those four lines plus the following two commented 
out. I verified that LLDB's Intel PT plugin still works just fine.

https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-08 Thread Nicholas Mosier via lldb-commits 

https://github.com/nmosier updated 
https://github.com/llvm/llvm-project/pull/77252

>From a4876e327f4e248ab4a48538f64507f60023d9d2 Mon Sep 17 00:00:00 2001
From: Nicholas Mosier 
Date: Sun, 7 Jan 2024 20:06:55 +
Subject: [PATCH] [lldb] Fix Intel PT plugin compile errors

Fix #77251.
---
 lldb/include/lldb/Target/ProcessTrace.h |  2 +-
 .../intel-pt/CommandObjectTraceStartIntelPT.cpp |  4 +---
 lldb/source/Plugins/Trace/intel-pt/DecodedThread.h  |  7 +--
 .../source/Plugins/Trace/intel-pt/LibiptDecoder.cpp |  4 ++--
 .../Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp   |  4 ++--
 .../Trace/intel-pt/TraceIntelPTBundleLoader.cpp | 13 ++---
 6 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/lldb/include/lldb/Target/ProcessTrace.h 
b/lldb/include/lldb/Target/ProcessTrace.h
index 037dea232cc024..201754e975e4fb 100644
--- a/lldb/include/lldb/Target/ProcessTrace.h
+++ b/lldb/include/lldb/Target/ProcessTrace.h
@@ -71,7 +71,7 @@ class ProcessTrace : public PostMortemProcess {
   bool DoUpdateThreadList(ThreadList &old_thread_list,
   ThreadList &new_thread_list) override;
 
-private:
+public:
   static lldb::ProcessSP CreateInstance(lldb::TargetSP target_sp,
 lldb::ListenerSP listener_sp,
 const FileSpec *crash_file_path,
diff --git 
a/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp 
b/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
index d4f7dc354e9fed..44224229e625bf 100644
--- a/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
@@ -158,7 +158,7 @@ 
CommandObjectProcessTraceStartIntelPT::CommandOptions::GetDefinitions() {
   return llvm::ArrayRef(g_process_trace_start_intel_pt_options);
 }
 
-bool CommandObjectProcessTraceStartIntelPT::DoExecute(
+void CommandObjectProcessTraceStartIntelPT::DoExecute(
 Args &command, CommandReturnObject &result) {
   if (Error err = m_trace.Start(
   m_options.m_ipt_trace_size, m_options.m_process_buffer_size_limit,
@@ -167,8 +167,6 @@ bool CommandObjectProcessTraceStartIntelPT::DoExecute(
 result.SetError(Status(std::move(err)));
   else
 result.SetStatus(eReturnStatusSuccessFinishResult);
-
-  return result.Succeeded();
 }
 
 std::optional
diff --git a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h 
b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
index 5745cdb67ab68f..ba66a09d3e762e 100644
--- a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -276,6 +276,9 @@ class DecodedThread : public 
std::enable_shared_from_this {
 
 /// The string message of this item if it's an error
 std::string error;
+
+TraceItemStorage() {}
+~TraceItemStorage() {}
   };
 
   /// Create a new trace item.
@@ -285,10 +288,10 @@ class DecodedThread : public 
std::enable_shared_from_this {
   DecodedThread::TraceItemStorage &CreateNewTraceItem(lldb::TraceItemKind 
kind);
 
   /// Most of the trace data is stored here.
-  std::vector m_item_data;
+  std::deque m_item_data;
   /// The TraceItemKind for each trace item encoded as uint8_t. We don't 
include
   /// it in TraceItemStorage to avoid padding.
-  std::vector m_item_kinds;
+  std::deque m_item_kinds;
 
   /// This map contains the TSCs of the decoded trace items. It maps
   /// `item index -> TSC`, where `item index` is the first index
diff --git a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp 
b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
index cdf81954eee902..f8241ef6a79329 100644
--- a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
@@ -572,7 +572,7 @@ Error 
lldb_private::trace_intel_pt::DecodeSingleTraceForThread(
 Expected decoder = PSBBlockDecoder::Create(
 trace_intel_pt, block, buffer.slice(block.psb_offset, block.size),
 *decoded_thread.GetThread()->GetProcess(),
-i + 1 < blocks->size() ? blocks->at(i + 1).starting_ip : None,
+i + 1 < blocks->size() ? blocks->at(i + 1).starting_ip : std::nullopt,
 decoded_thread, std::nullopt);
 if (!decoder)
   return decoder.takeError();
@@ -640,7 +640,7 @@ Error 
lldb_private::trace_intel_pt::DecodeSystemWideTraceForThread(
   *decoded_thread.GetThread()->GetProcess(),
   j + 1 < execution.psb_blocks.size()
   ? execution.psb_blocks[j + 1].starting_ip
-  : None,
+  : std::nullopt,
   decoded_thread, execution.thread_execution.GetEndTSC());
   if (!decoder)
 return decoder.takeError();
diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp 
b/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
index 66d342196cf10d..dda6cd74343f0f 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceCurso

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-08 Thread Nicholas Mosier via lldb-commits 

https://github.com/nmosier updated 
https://github.com/llvm/llvm-project/pull/77252

>From ffe2dff5783e57dfbc40b1840a784a28ea18ef26 Mon Sep 17 00:00:00 2001
From: Nicholas Mosier 
Date: Sun, 7 Jan 2024 20:06:55 +
Subject: [PATCH] [lldb] Fix Intel PT plugin compile errors

Fix #77251.
---
 lldb/include/lldb/Target/ProcessTrace.h |  2 +-
 .../intel-pt/CommandObjectTraceStartIntelPT.cpp |  4 +---
 lldb/source/Plugins/Trace/intel-pt/DecodedThread.h  |  9 ++---
 .../source/Plugins/Trace/intel-pt/LibiptDecoder.cpp |  4 ++--
 .../Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp   |  4 ++--
 .../Trace/intel-pt/TraceIntelPTBundleLoader.cpp | 13 ++---
 6 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/lldb/include/lldb/Target/ProcessTrace.h 
b/lldb/include/lldb/Target/ProcessTrace.h
index 037dea232cc024..201754e975e4fb 100644
--- a/lldb/include/lldb/Target/ProcessTrace.h
+++ b/lldb/include/lldb/Target/ProcessTrace.h
@@ -71,7 +71,7 @@ class ProcessTrace : public PostMortemProcess {
   bool DoUpdateThreadList(ThreadList &old_thread_list,
   ThreadList &new_thread_list) override;
 
-private:
+public:
   static lldb::ProcessSP CreateInstance(lldb::TargetSP target_sp,
 lldb::ListenerSP listener_sp,
 const FileSpec *crash_file_path,
diff --git 
a/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp 
b/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
index d4f7dc354e9fed..44224229e625bf 100644
--- a/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
@@ -158,7 +158,7 @@ 
CommandObjectProcessTraceStartIntelPT::CommandOptions::GetDefinitions() {
   return llvm::ArrayRef(g_process_trace_start_intel_pt_options);
 }
 
-bool CommandObjectProcessTraceStartIntelPT::DoExecute(
+void CommandObjectProcessTraceStartIntelPT::DoExecute(
 Args &command, CommandReturnObject &result) {
   if (Error err = m_trace.Start(
   m_options.m_ipt_trace_size, m_options.m_process_buffer_size_limit,
@@ -167,8 +167,6 @@ bool CommandObjectProcessTraceStartIntelPT::DoExecute(
 result.SetError(Status(std::move(err)));
   else
 result.SetStatus(eReturnStatusSuccessFinishResult);
-
-  return result.Succeeded();
 }
 
 std::optional
diff --git a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h 
b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
index 5745cdb67ab68f..0f2c6e56bd541e 100644
--- a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -14,9 +14,9 @@
 #include "lldb/Utility/TraceIntelPTGDBRemotePackets.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
+#include 
 #include 
 #include 
-#include 
 
 namespace lldb_private {
 namespace trace_intel_pt {
@@ -276,6 +276,9 @@ class DecodedThread : public 
std::enable_shared_from_this {
 
 /// The string message of this item if it's an error
 std::string error;
+
+TraceItemStorage() {}
+~TraceItemStorage() {}
   };
 
   /// Create a new trace item.
@@ -285,10 +288,10 @@ class DecodedThread : public 
std::enable_shared_from_this {
   DecodedThread::TraceItemStorage &CreateNewTraceItem(lldb::TraceItemKind 
kind);
 
   /// Most of the trace data is stored here.
-  std::vector m_item_data;
+  std::deque m_item_data;
   /// The TraceItemKind for each trace item encoded as uint8_t. We don't 
include
   /// it in TraceItemStorage to avoid padding.
-  std::vector m_item_kinds;
+  std::deque m_item_kinds;
 
   /// This map contains the TSCs of the decoded trace items. It maps
   /// `item index -> TSC`, where `item index` is the first index
diff --git a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp 
b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
index cdf81954eee902..f8241ef6a79329 100644
--- a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
@@ -572,7 +572,7 @@ Error 
lldb_private::trace_intel_pt::DecodeSingleTraceForThread(
 Expected decoder = PSBBlockDecoder::Create(
 trace_intel_pt, block, buffer.slice(block.psb_offset, block.size),
 *decoded_thread.GetThread()->GetProcess(),
-i + 1 < blocks->size() ? blocks->at(i + 1).starting_ip : None,
+i + 1 < blocks->size() ? blocks->at(i + 1).starting_ip : std::nullopt,
 decoded_thread, std::nullopt);
 if (!decoder)
   return decoder.takeError();
@@ -640,7 +640,7 @@ Error 
lldb_private::trace_intel_pt::DecodeSystemWideTraceForThread(
   *decoded_thread.GetThread()->GetProcess(),
   j + 1 < execution.psb_blocks.size()
   ? execution.psb_blocks[j + 1].starting_ip
-  : None,
+  : std::nullopt,
   decoded_thread, execution.thread_execution.GetEndTSC());
   if (!decoder)
 return decoder

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-08 Thread Nicholas Mosier via lldb-commits 


@@ -85,6 +85,11 @@ double DecodedThread::NanosecondsRange::GetInterpolatedTime(
   return interpolate(next_range->nanos);
 }
 
+DecodedThread::TraceItemStorage::TraceItemStorage(
+const TraceItemStorage &other) {
+  std::memcpy(this, &other, sizeof *this);

nmosier wrote:

Thanks for the  suggestion, I changed `DecodedThread::m_item_{kinds,data}` to 
`std::deque`s and removed the `TraceItemStorage`'s copy constructor.

https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-08 Thread Nicholas Mosier via lldb-commits 


@@ -103,12 +103,10 @@ TraceIntelPTBundleLoader::CreateEmptyProcess(lldb::pid_t 
pid,
   ParsedProcess parsed_process;
   parsed_process.target_sp = target_sp;
 
-  // This should instead try to directly create an instance of ProcessTrace.
-  // ProcessSP process_sp = target_sp->CreateProcess(
-  ///*listener*/ nullptr, "trace",
-  ///*crash_file*/ nullptr,
-  ///*can_connect*/ false);
-
+  ProcessSP process_sp = target_sp->CreateProcess(
+  /*listener*/ nullptr, "trace",
+  /*crash_file*/ nullptr,
+  /*can_connect*/ false);

nmosier wrote:

@walter-erquinigo Thanks for the suggestion! I implemented that change.

https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-08 Thread Nicholas Mosier via lldb-commits 


@@ -108,8 +108,8 @@ TraceIntelPTBundleLoader::CreateEmptyProcess(lldb::pid_t 
pid,
   ///*listener*/ nullptr, "trace",
   ///*crash_file*/ nullptr,
   ///*can_connect*/ false);
-
-  process_sp->SetID(static_cast(pid));
+  //
+  // process_sp->SetID(static_cast(pid));

nmosier wrote:

I reverted this, given your suggestion in the other thread.

https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-08 Thread Nicholas Mosier via lldb-commits 

nmosier wrote:

> Btw, how are you testing them?

Not very rigorously, ha. Just something like
```
b main
r
process trace start
c
thread trace dump instructions
```
I was using a version with my original fixes to debug other code that was 
crashing, and it worked fine for that.

Are there existing tests I should be using? 

https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-08 Thread Nicholas Mosier via lldb-commits 

nmosier wrote:

At least one test fails: `./bin/lldb -o 'trace load -v 
/llvm/lldb/test/API/commands/trace/intelpt-trace/trace_2threads.json'` crashes 
with an assertion failure on TraceIntelPTBundleLoader.cpp:127 (`*process_sp` is 
a null pointer dereference).

Do you know why `process_sp` would be null in this case? I added some extra 
assertions and it looks like `target_sp->m_process_sp` is null after the call 
to `ProcessTrace::CreateInstance` on line 111.



https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-08 Thread Nicholas Mosier via lldb-commits 

nmosier wrote:

Thanks for the suggestions. Unfortunately, it's still crashing, now on the line
```c
process_sp->SetID(static_cast(pid));
```
because the prior call to `CreateProcess` on line 107 returned null.

https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-08 Thread Nicholas Mosier via lldb-commits 

https://github.com/nmosier updated 
https://github.com/llvm/llvm-project/pull/77252

>From eece351a68e014d7a46bd1e3512298dd5dda Mon Sep 17 00:00:00 2001
From: Nicholas Mosier 
Date: Sun, 7 Jan 2024 20:06:55 +
Subject: [PATCH] [lldb] Fix Intel PT plugin compile errors

Fix #77251.
---
 lldb/include/lldb/Target/ProcessTrace.h |  2 +-
 .../intel-pt/CommandObjectTraceStartIntelPT.cpp |  4 +---
 .../source/Plugins/Trace/intel-pt/DecodedThread.cpp | 10 +-
 lldb/source/Plugins/Trace/intel-pt/DecodedThread.h  |  9 ++---
 .../source/Plugins/Trace/intel-pt/LibiptDecoder.cpp |  4 ++--
 .../Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp   |  4 ++--
 .../Trace/intel-pt/TraceIntelPTBundleLoader.cpp | 13 +++--
 lldb/source/Target/ProcessTrace.cpp |  2 ++
 8 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/lldb/include/lldb/Target/ProcessTrace.h 
b/lldb/include/lldb/Target/ProcessTrace.h
index 037dea232cc024..201754e975e4fb 100644
--- a/lldb/include/lldb/Target/ProcessTrace.h
+++ b/lldb/include/lldb/Target/ProcessTrace.h
@@ -71,7 +71,7 @@ class ProcessTrace : public PostMortemProcess {
   bool DoUpdateThreadList(ThreadList &old_thread_list,
   ThreadList &new_thread_list) override;
 
-private:
+public:
   static lldb::ProcessSP CreateInstance(lldb::TargetSP target_sp,
 lldb::ListenerSP listener_sp,
 const FileSpec *crash_file_path,
diff --git 
a/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp 
b/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
index d4f7dc354e9fed..44224229e625bf 100644
--- a/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
@@ -158,7 +158,7 @@ 
CommandObjectProcessTraceStartIntelPT::CommandOptions::GetDefinitions() {
   return llvm::ArrayRef(g_process_trace_start_intel_pt_options);
 }
 
-bool CommandObjectProcessTraceStartIntelPT::DoExecute(
+void CommandObjectProcessTraceStartIntelPT::DoExecute(
 Args &command, CommandReturnObject &result) {
   if (Error err = m_trace.Start(
   m_options.m_ipt_trace_size, m_options.m_process_buffer_size_limit,
@@ -167,8 +167,6 @@ bool CommandObjectProcessTraceStartIntelPT::DoExecute(
 result.SetError(Status(std::move(err)));
   else
 result.SetStatus(eReturnStatusSuccessFinishResult);
-
-  return result.Succeeded();
 }
 
 std::optional
diff --git a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp 
b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
index 17f8f51bdf0e0d..7497bdd3316e9e 100644
--- a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
@@ -107,7 +107,15 @@ DecodedThread::CreateNewTraceItem(lldb::TraceItemKind 
kind) {
 (*m_last_tsc)->second.items_count++;
   if (m_last_nanoseconds)
 (*m_last_nanoseconds)->second.items_count++;
-  return m_item_data.back();
+
+  TraceItemStorage &data = m_item_data.back();
+
+  // If creating an error item, then properly initialize TraceItemStorage's
+  // non-trivially-constructible union member `error`.
+  if (kind == lldb::eTraceItemKindError)
+new (&data.error) std::string();
+
+  return data;
 }
 
 void DecodedThread::NotifySyncPoint(lldb::addr_t psb_offset) {
diff --git a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h 
b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
index 5745cdb67ab68f..0f2c6e56bd541e 100644
--- a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -14,9 +14,9 @@
 #include "lldb/Utility/TraceIntelPTGDBRemotePackets.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
+#include 
 #include 
 #include 
-#include 
 
 namespace lldb_private {
 namespace trace_intel_pt {
@@ -276,6 +276,9 @@ class DecodedThread : public 
std::enable_shared_from_this {
 
 /// The string message of this item if it's an error
 std::string error;
+
+TraceItemStorage() {}
+~TraceItemStorage() {}
   };
 
   /// Create a new trace item.
@@ -285,10 +288,10 @@ class DecodedThread : public 
std::enable_shared_from_this {
   DecodedThread::TraceItemStorage &CreateNewTraceItem(lldb::TraceItemKind 
kind);
 
   /// Most of the trace data is stored here.
-  std::vector m_item_data;
+  std::deque m_item_data;
   /// The TraceItemKind for each trace item encoded as uint8_t. We don't 
include
   /// it in TraceItemStorage to avoid padding.
-  std::vector m_item_kinds;
+  std::deque m_item_kinds;
 
   /// This map contains the TSCs of the decoded trace items. It maps
   /// `item index -> TSC`, where `item index` is the first index
diff --git a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp 
b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
index cdf81954eee902..f8241ef6a79329 100644
--- a/lldb/source/Plugins/Trace/intel-pt

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-08 Thread Nicholas Mosier via lldb-commits 

nmosier wrote:

Thanks! That test passes now. Another test was crashing as well, but I fixed 
that (it was due to `TraceItemStorage.error` not being initialized properly in 
`DecodedThread::CreateNewTraceItem` when creating an error item.

49 out of 56 tests now pass, with the remaining tests being soft failures 
(e.g., unexpected output but no crashes, generally due to small differences 
like 
```
"memoryUsage": { 
  "totalInBytes": "924", 
  "avgPerItemInBytes": 33
},   
```
vs. expected
```
"memoryUsage": {
  "totalInBytes": "252",
  "avgPerItemInBytes": 9
}, 
```

https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-08 Thread Nicholas Mosier via lldb-commits 


@@ -107,7 +107,15 @@ DecodedThread::CreateNewTraceItem(lldb::TraceItemKind 
kind) {
 (*m_last_tsc)->second.items_count++;
   if (m_last_nanoseconds)
 (*m_last_nanoseconds)->second.items_count++;
-  return m_item_data.back();
+
+  TraceItemStorage &data = m_item_data.back();
+
+  // If creating an error item, then properly initialize TraceItemStorage's
+  // non-trivially-constructible union member `error`.
+  if (kind == lldb::eTraceItemKindError)
+new (&data.error) std::string();

nmosier wrote:

I don't think so (I tried it, and the crash came back). The problem is that 
before line 116, `data.error` is uninitialized. Thus `data.error = {}` 
implicitly invokes the destructor of `data.error` on uninitialized memory 
before constructing a new std::string, causing a crash. 

https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-08 Thread Nicholas Mosier via lldb-commits 

https://github.com/nmosier edited 
https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-08 Thread Nicholas Mosier via lldb-commits 

https://github.com/nmosier updated 
https://github.com/llvm/llvm-project/pull/77252

>From 77aa29a286cbf06a9959790ec26eb025ab5e3a16 Mon Sep 17 00:00:00 2001
From: Nicholas Mosier 
Date: Sun, 7 Jan 2024 20:06:55 +
Subject: [PATCH] [lldb] Fix Intel PT plugin compile errors

Fix #77251.
---
 .../CommandObjectTraceStartIntelPT.cpp|  4 +--
 .../Plugins/Trace/intel-pt/DecodedThread.cpp  | 30 +++
 .../Plugins/Trace/intel-pt/DecodedThread.h| 24 ---
 .../Plugins/Trace/intel-pt/LibiptDecoder.cpp  |  4 +--
 .../Trace/intel-pt/TraceCursorIntelPT.cpp |  4 +--
 .../intel-pt/TraceIntelPTBundleLoader.cpp | 13 
 lldb/source/Target/ProcessTrace.cpp   |  2 ++
 .../API/commands/trace/TestTraceDumpInfo.py   | 15 ++
 lldb/test/API/commands/trace/TestTraceLoad.py | 20 -
 9 files changed, 45 insertions(+), 71 deletions(-)

diff --git 
a/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp 
b/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
index d4f7dc354e9fed..44224229e625bf 100644
--- a/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
@@ -158,7 +158,7 @@ 
CommandObjectProcessTraceStartIntelPT::CommandOptions::GetDefinitions() {
   return llvm::ArrayRef(g_process_trace_start_intel_pt_options);
 }
 
-bool CommandObjectProcessTraceStartIntelPT::DoExecute(
+void CommandObjectProcessTraceStartIntelPT::DoExecute(
 Args &command, CommandReturnObject &result) {
   if (Error err = m_trace.Start(
   m_options.m_ipt_trace_size, m_options.m_process_buffer_size_limit,
@@ -167,8 +167,6 @@ bool CommandObjectProcessTraceStartIntelPT::DoExecute(
 result.SetError(Status(std::move(err)));
   else
 result.SetStatus(eReturnStatusSuccessFinishResult);
-
-  return result.Succeeded();
 }
 
 std::optional
diff --git a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp 
b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
index 17f8f51bdf0e0d..cd9ede789b11c5 100644
--- a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
@@ -85,11 +85,11 @@ double DecodedThread::NanosecondsRange::GetInterpolatedTime(
   return interpolate(next_range->nanos);
 }
 
-uint64_t DecodedThread::GetItemsCount() const { return m_item_kinds.size(); }
+uint64_t DecodedThread::GetItemsCount() const { return m_item_data.size(); }
 
 lldb::addr_t
 DecodedThread::GetInstructionLoadAddress(uint64_t item_index) const {
-  return m_item_data[item_index].load_address;
+  return std::get(m_item_data[item_index]);
 }
 
 lldb::addr_t
@@ -99,14 +99,16 @@ DecodedThread::GetSyncPointOffsetByIndex(uint64_t 
item_index) const {
 
 ThreadSP DecodedThread::GetThread() { return m_thread_sp; }
 
+template 
 DecodedThread::TraceItemStorage &
-DecodedThread::CreateNewTraceItem(lldb::TraceItemKind kind) {
-  m_item_kinds.push_back(kind);
-  m_item_data.emplace_back();
+DecodedThread::CreateNewTraceItem(lldb::TraceItemKind kind, Data &&data) {
+  m_item_data.emplace_back(data);
+  
   if (m_last_tsc)
 (*m_last_tsc)->second.items_count++;
   if (m_last_nanoseconds)
 (*m_last_nanoseconds)->second.items_count++;
+
   return m_item_data.back();
 }
 
@@ -176,27 +178,27 @@ uint64_t DecodedThread::GetTotalInstructionCount() const {
 }
 
 void DecodedThread::AppendEvent(lldb::TraceEvent event) {
-  CreateNewTraceItem(lldb::eTraceItemKindEvent).event = event;
+  CreateNewTraceItem(lldb::eTraceItemKindEvent, event);
   m_events_stats.RecordEvent(event);
 }
 
 void DecodedThread::AppendInstruction(const pt_insn &insn) {
-  CreateNewTraceItem(lldb::eTraceItemKindInstruction).load_address = insn.ip;
+  CreateNewTraceItem(lldb::eTraceItemKindInstruction, insn.ip);
   m_insn_count++;
 }
 
 void DecodedThread::AppendError(const IntelPTError &error) {
-  CreateNewTraceItem(lldb::eTraceItemKindError).error = error.message();
+  CreateNewTraceItem(lldb::eTraceItemKindError, error.message());
   m_error_stats.RecordError(/*fatal=*/false);
 }
 
 void DecodedThread::AppendCustomError(StringRef err, bool fatal) {
-  CreateNewTraceItem(lldb::eTraceItemKindError).error = err.str();
+  CreateNewTraceItem(lldb::eTraceItemKindError, err.str());
   m_error_stats.RecordError(fatal);
 }
 
 lldb::TraceEvent DecodedThread::GetEventByIndex(int item_index) const {
-  return m_item_data[item_index].event;
+  return std::get(m_item_data[item_index]);
 }
 
 const DecodedThread::EventsStats &DecodedThread::GetEventsStats() const {
@@ -233,13 +235,16 @@ const DecodedThread::ErrorStats 
&DecodedThread::GetErrorStats() const {
 
 lldb::TraceItemKind
 DecodedThread::GetItemKindByIndex(uint64_t item_index) const {
-  return static_cast(m_item_kinds[item_index]);
+  return std::visit(llvm::makeVisitor([] (const std::string &) { return 
lldb::eTraceItemKindError; },
+  [] (lldb::TraceEvent) { return 
ll

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-08 Thread Nicholas Mosier via lldb-commits 

https://github.com/nmosier updated 
https://github.com/llvm/llvm-project/pull/77252

>From 7b184bbc1ae24d548d8574d2620b642e3304423a Mon Sep 17 00:00:00 2001
From: Nicholas Mosier 
Date: Sun, 7 Jan 2024 20:06:55 +
Subject: [PATCH] [lldb] Fix Intel PT plugin compile errors

Fix #77251.
---
 .../CommandObjectTraceStartIntelPT.cpp|  4 +--
 .../Plugins/Trace/intel-pt/DecodedThread.cpp  | 32 +++
 .../Plugins/Trace/intel-pt/DecodedThread.h| 26 +--
 .../Plugins/Trace/intel-pt/LibiptDecoder.cpp  |  4 +--
 .../Trace/intel-pt/TraceCursorIntelPT.cpp |  4 +--
 .../intel-pt/TraceIntelPTBundleLoader.cpp | 13 
 lldb/source/Target/ProcessTrace.cpp   |  2 ++
 .../API/commands/trace/TestTraceDumpInfo.py   | 15 ++---
 lldb/test/API/commands/trace/TestTraceLoad.py | 20 
 9 files changed, 49 insertions(+), 71 deletions(-)

diff --git 
a/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp 
b/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
index d4f7dc354e9fed..44224229e625bf 100644
--- a/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
@@ -158,7 +158,7 @@ 
CommandObjectProcessTraceStartIntelPT::CommandOptions::GetDefinitions() {
   return llvm::ArrayRef(g_process_trace_start_intel_pt_options);
 }
 
-bool CommandObjectProcessTraceStartIntelPT::DoExecute(
+void CommandObjectProcessTraceStartIntelPT::DoExecute(
 Args &command, CommandReturnObject &result) {
   if (Error err = m_trace.Start(
   m_options.m_ipt_trace_size, m_options.m_process_buffer_size_limit,
@@ -167,8 +167,6 @@ bool CommandObjectProcessTraceStartIntelPT::DoExecute(
 result.SetError(Status(std::move(err)));
   else
 result.SetStatus(eReturnStatusSuccessFinishResult);
-
-  return result.Succeeded();
 }
 
 std::optional
diff --git a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp 
b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
index 17f8f51bdf0e0d..9c075398d54703 100644
--- a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
@@ -85,11 +85,11 @@ double DecodedThread::NanosecondsRange::GetInterpolatedTime(
   return interpolate(next_range->nanos);
 }
 
-uint64_t DecodedThread::GetItemsCount() const { return m_item_kinds.size(); }
+uint64_t DecodedThread::GetItemsCount() const { return m_item_data.size(); }
 
 lldb::addr_t
 DecodedThread::GetInstructionLoadAddress(uint64_t item_index) const {
-  return m_item_data[item_index].load_address;
+  return std::get(m_item_data[item_index]);
 }
 
 lldb::addr_t
@@ -99,14 +99,16 @@ DecodedThread::GetSyncPointOffsetByIndex(uint64_t 
item_index) const {
 
 ThreadSP DecodedThread::GetThread() { return m_thread_sp; }
 
+template 
 DecodedThread::TraceItemStorage &
-DecodedThread::CreateNewTraceItem(lldb::TraceItemKind kind) {
-  m_item_kinds.push_back(kind);
-  m_item_data.emplace_back();
+DecodedThread::CreateNewTraceItem(lldb::TraceItemKind kind, Data &&data) {
+  m_item_data.emplace_back(data);
+
   if (m_last_tsc)
 (*m_last_tsc)->second.items_count++;
   if (m_last_nanoseconds)
 (*m_last_nanoseconds)->second.items_count++;
+
   return m_item_data.back();
 }
 
@@ -176,27 +178,27 @@ uint64_t DecodedThread::GetTotalInstructionCount() const {
 }
 
 void DecodedThread::AppendEvent(lldb::TraceEvent event) {
-  CreateNewTraceItem(lldb::eTraceItemKindEvent).event = event;
+  CreateNewTraceItem(lldb::eTraceItemKindEvent, event);
   m_events_stats.RecordEvent(event);
 }
 
 void DecodedThread::AppendInstruction(const pt_insn &insn) {
-  CreateNewTraceItem(lldb::eTraceItemKindInstruction).load_address = insn.ip;
+  CreateNewTraceItem(lldb::eTraceItemKindInstruction, insn.ip);
   m_insn_count++;
 }
 
 void DecodedThread::AppendError(const IntelPTError &error) {
-  CreateNewTraceItem(lldb::eTraceItemKindError).error = error.message();
+  CreateNewTraceItem(lldb::eTraceItemKindError, error.message());
   m_error_stats.RecordError(/*fatal=*/false);
 }
 
 void DecodedThread::AppendCustomError(StringRef err, bool fatal) {
-  CreateNewTraceItem(lldb::eTraceItemKindError).error = err.str();
+  CreateNewTraceItem(lldb::eTraceItemKindError, err.str());
   m_error_stats.RecordError(fatal);
 }
 
 lldb::TraceEvent DecodedThread::GetEventByIndex(int item_index) const {
-  return m_item_data[item_index].event;
+  return std::get(m_item_data[item_index]);
 }
 
 const DecodedThread::EventsStats &DecodedThread::GetEventsStats() const {
@@ -233,13 +235,18 @@ const DecodedThread::ErrorStats 
&DecodedThread::GetErrorStats() const {
 
 lldb::TraceItemKind
 DecodedThread::GetItemKindByIndex(uint64_t item_index) const {
-  return static_cast(m_item_kinds[item_index]);
+  return std::visit(
+  llvm::makeVisitor(
+  [](const std::string &) { return lldb::eTraceItemKindError; },
+  [](lldb::TraceEvent) { return lldb::eTraceItemKi

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-08 Thread Nicholas Mosier via lldb-commits 

nmosier wrote:

I went ahead and turned `TraceItemStorage` into a variant. I also deleted the 
memory usage / bytes from the tests. Also, I noticed some error messages were 
different for some tests — I'm guessing they may have changed in newer versions 
of libipt.

Right now, only 1 test fails:
```
==  

  
FAIL: testStartPerCpuSession 
(TestTraceStartStopMultipleThreads.TestTraceStartStopMultipleThreads)   
 
--  

  
Traceback (most recent call last):  

  
  File "/llvm/lldb/packages/Python/lldbsuite/test/decorators.py", line 162, in 
wrapper 
   
return func(*args, **kwargs)

  
  File 
"/llvm/lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py", 
line 14, in wrapper 

func(*args, **kwargs)   

  
  File 
"/llvm/lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py",
 line 260, in testStartPerCpuSession

self.expect("thread trace dump instructions")   

  
  File "/llvm/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 2353, in 
expect  

self.runCmd(

  
  File "/llvm/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 2031, in 
runCmd  

self.assertTrue(self.res.Succeeded(), msg if (msg) else CMD_MSG(cmd))   

  
AssertionError: False is not True : Command 'thread trace dump instructions 

  
Error output:   

  
error: Malformed perf context switch trace for cpu 14 at offset 64. A context 
switch record doesn't happen after the previous record. Previous TSC= 
2215501248, current TSC = 22155012
48. 

  
' did not return successfully   

  
Config=x86_64-/llvm-build/bin/clang 

  
```

https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-09 Thread Nicholas Mosier via lldb-commits 

nmosier wrote:

Sounds good. My changes are ready -- only that `testStartPerCpuSession` test is 
failing now.

https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix Intel PT plugin compile errors (PR #77252)

2024-01-09 Thread Nicholas Mosier via lldb-commits 

nmosier wrote:

@walter-erquinigo Could you merge this PR for me? I don't have commit access. 
Thanks!

https://github.com/llvm/llvm-project/pull/77252
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [llvm] [flang] [clang] [clang-tools-extra] [compiler-rt] [libc] [lldb] [libcxxabi] [mlir] [lld] [openmp] [libcxx] [X86] Add support for indirect branch tracking in jump tables (PR #7767

2024-01-11 Thread Nicholas Mosier via lldb-commits 

https://github.com/nmosier updated 
https://github.com/llvm/llvm-project/pull/77679

>From 35f91b27825a81b1ba171860b47bf8b0477fd95a Mon Sep 17 00:00:00 2001
From: Nicholas Mosier 
Date: Wed, 10 Jan 2024 19:27:30 +
Subject: [PATCH] [X86] Add support for indirect branch tracking in jump tables

This patch adds support for protecting jump tables with indirect branch 
tracking (IBT).
By default, indirect jump table branches are given a 'notrack' prefix and thus 
not protected with IBT.
This default behavior is suitable for traditional threat models, assuming the 
compiler generates correct jump table code.
However, for threat models encompassing speculative execution vulnerabilities 
(e.g., Spectre),
such notrack'ed indirect branches potentially introduce Spectre-v2-style 
vulnerabilites by allowing them to mis-speculatively jump to arbitrary target 
addresses, not just ENDBRANCH instructions.

To enable indirect branch tracking for jump tables, this patch allows the 
`cf-protection-branch` module flag's value to indicate the level of indirect 
branch protection required.
If `cf-protection-branch == 0` or `cf-protection-branch` is missing entirely, 
then branch protections are applied.
If `cf-protection-branch == 1`, then branch protections are applied for all 
indirect branches except for those that cannot under any circumstances 
non-speculatively jump to the wrong target (namely, indirect jump table 
branches).
If `cf-protection-branch >= 2`, then branch protections are applied to all 
indirect branches, including indirect jump table branches.

To summarize the new interpretation of the `cf-protection-branch` module flag 
under this patch:
* `cf-protection-branch == 1` (currently emitted by clang when passed flag 
`-fcf-protection=branch`) is suitable for hardening code against 
non-speculative control-flow hijacks.
* `cf-protection-branch >= 2` is suitable for additionally hardening code 
against speculative control-flow hijacks (e.g., Spectre v2).
---
 llvm/lib/Target/X86/X86.h |  1 +
 llvm/lib/Target/X86/X86ISelLowering.cpp   | 21 +++
 .../Target/X86/X86IndirectBranchTracking.cpp  | 29 ---
 llvm/lib/Target/X86/X86TargetMachine.cpp  |  1 +
 .../X86/indirect-branch-tracking-jt.mir   | 35 +++
 5 files changed, 77 insertions(+), 10 deletions(-)
 create mode 100644 llvm/test/CodeGen/X86/indirect-branch-tracking-jt.mir

diff --git a/llvm/lib/Target/X86/X86.h b/llvm/lib/Target/X86/X86.h
index 21623a805f5568..277259c74abae2 100644
--- a/llvm/lib/Target/X86/X86.h
+++ b/llvm/lib/Target/X86/X86.h
@@ -199,6 +199,7 @@ void initializeX86ReturnThunksPass(PassRegistry &);
 void initializeX86SpeculativeExecutionSideEffectSuppressionPass(PassRegistry 
&);
 void initializeX86SpeculativeLoadHardeningPassPass(PassRegistry &);
 void initializeX86TileConfigPass(PassRegistry &);
+void initializeX86IndirectBranchTrackingPassPass(PassRegistry &);
 
 namespace X86AS {
 enum : unsigned {
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp 
b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 5f6f500e49dd2a..71a20b439b5fad 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -56365,12 +56365,21 @@ SDValue 
X86TargetLowering::expandIndirectJTBranch(const SDLoc &dl,
   int JTI,
   SelectionDAG &DAG) const {
   const Module *M = DAG.getMachineFunction().getMMI().getModule();
-  Metadata *IsCFProtectionSupported = M->getModuleFlag("cf-protection-branch");
-  if (IsCFProtectionSupported) {
-// In case control-flow branch protection is enabled, we need to add
-// notrack prefix to the indirect branch.
-// In order to do that we create NT_BRIND SDNode.
-// Upon ISEL, the pattern will convert it to jmp with NoTrack prefix.
+
+  uint64_t CFProtectionBranchLevel = 0;
+  if (Metadata *CFProtectionBranchEnabled =
+  M->getModuleFlag("cf-protection-branch"))
+CFProtectionBranchLevel =
+cast(CFProtectionBranchEnabled)
+->getValue()
+->getUniqueInteger()
+.getLimitedValue();
+
+  if (CFProtectionBranchLevel == 1) {
+// In case control-flow branch protection is enabled but we are not
+// protecting jump table branches, we need to add notrack prefix to the
+// indirect branch. In order to do that we create NT_BRIND SDNode. Upon
+// ISEL, the pattern will convert it to jmp with NoTrack prefix.
 SDValue JTInfo = DAG.getJumpTableDebugInfo(JTI, Value, dl);
 return DAG.getNode(X86ISD::NT_BRIND, dl, MVT::Other, JTInfo, Addr);
   }
diff --git a/llvm/lib/Target/X86/X86IndirectBranchTracking.cpp 
b/llvm/lib/Target/X86/X86IndirectBranchTracking.cpp
index 785bdd83cd998b..0710af3af8469a 100644
--- a/llvm/lib/Target/X86/X86IndirectBranchTracking.cpp
+++ b/llvm/lib/Target/X86/X86IndirectBranchTracking.cpp
@@ -22,11 +22,13 @@
 #include "llvm/ADT/Statistic.h"