[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

2022-03-31 Thread walter erquinigo via Phabricator via lldb-commits
wallace added a comment.

Don't forget to update the description of this diff and of the commit before 
pushing (you need to do both). Include the avg instruction size for a trace of 
at least 10k instructions as well :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122603

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


[Lldb-commits] [PATCH] D122867: [trace][intel pt] Handle better tsc in the decoder

2022-03-31 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.

A problem that I introduced in the decoder is that I was considering TSC 
decoding
errors as actual instruction errors, which mean that the trace has a gap. This 
is
wrong because a TSC decoding error doesn't mean that there's a gap in the trace.
Instead, now I'm just counting how many of these errors happened and I'm using
the `dump info` command to check for this number.

Besides that, I refactored the decoder a little bit to make it simpler, more
readable, and to handle TSCs in a cleaner way.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122867

Files:
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/test/API/commands/trace/TestTraceDumpInfo.py

Index: lldb/test/API/commands/trace/TestTraceDumpInfo.py
===
--- lldb/test/API/commands/trace/TestTraceDumpInfo.py
+++ lldb/test/API/commands/trace/TestTraceDumpInfo.py
@@ -41,4 +41,5 @@
   Raw trace size: 4 KiB
   Total number of instructions: 21
   Total approximate memory usage: 0.98 KiB
-  Average memory usage per instruction: 48.00 bytes'''])
+  Average memory usage per instruction: 48.00 bytes
+  Number of TSC decoding errors: 0'''])
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
@@ -111,9 +111,9 @@
 return;
   }
   s.Printf("\n");
-
-  size_t insn_len = Decode(thread)->GetInstructions().size();
-  size_t mem_used = Decode(thread)->CalculateApproximateMemoryUsage();
+  DecodedThreadSP decoded_trace_sp = Decode(thread);
+  size_t insn_len = decoded_trace_sp->GetInstructions().size();
+  size_t mem_used = decoded_trace_sp->CalculateApproximateMemoryUsage();
 
   s.Printf("  Raw trace size: %zu KiB\n", *raw_size / 1024);
   s.Printf("  Total number of instructions: %zu\n", insn_len);
@@ -122,6 +122,8 @@
   if (insn_len != 0)
 s.Printf("  Average memory usage per instruction: %0.2lf bytes\n",
  (double)mem_used / insn_len);
+  s.Printf("  Number of TSC decoding errors: %d\n",
+   decoded_trace_sp->GetTscErrorsCount());
   return;
 }
 
Index: lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
+++ lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
@@ -90,6 +90,59 @@
   return 0;
 }
 
+// Simple struct used by the decoder to keep the state of the most
+// recent TSC and a flag indicating whether TSCs are enabled, not enabled
+// or we just don't yet.
+struct TscInfo {
+  uint64_t tsc = 0;
+  LazyBool has_tsc = eLazyBoolCalculate;
+
+  explicit operator bool() const { return has_tsc == eLazyBoolYes; }
+};
+
+/// Query the decoder for the most recent TSC timestamp and update
+/// tsc_info accordingly.
+void RefreshTscInfo(TscInfo _info, pt_insn_decoder ,
+DecodedThreadSP decoded_thread_sp) {
+  if (tsc_info.has_tsc == eLazyBoolNo)
+return;
+
+  uint64_t new_tsc;
+  if (int tsc_error = pt_insn_time(, _tsc, nullptr, nullptr)) {
+if (tsc_error == -pte_no_time) {
+  // We now know that the trace doesn't support TSC, so we won't try again.
+  // See
+  // https://github.com/intel/libipt/blob/master/doc/man/pt_qry_time.3.md
+  tsc_info.has_tsc = eLazyBoolNo;
+} else {
+  // We don't add TSC decoding errors in the decoded trace itself to prevent
+  // creating unnecessary gaps, but we can count how many of these errors
+  // happened. In this case we reuse the previous correct TSC we saw, as
+  // it's better than no TSC at all.
+  decoded_thread_sp->ReportTscError(tsc_error);
+}
+  } else {
+tsc_info.tsc = new_tsc;
+tsc_info.has_tsc = eLazyBoolYes;
+  }
+}
+
+static void AppendError(DecodedThreadSP _thread_sp, Error &,
+TscInfo _info) {
+  if (tsc_info)
+decoded_thread_sp->AppendError(std::move(error), tsc_info.tsc);
+  else
+decoded_thread_sp->AppendError(std::move(error));
+}
+
+static void AppendInstruction(DecodedThreadSP _thread_sp,
+  const pt_insn , TscInfo _info) {
+  if (tsc_info)
+decoded_thread_sp->AppendInstruction(insn, tsc_info.tsc);
+  else
+decoded_thread_sp->AppendInstruction(insn);
+}
+
 /// Decode all the instructions from a configured decoder.
 /// The decoding flow is based on
 /// https://github.com/intel/libipt/blob/master/doc/howto_libipt.md#the-instruction-flow-decode-loop
@@ -98,60 

[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

2022-03-31 Thread walter erquinigo via Phabricator via lldb-commits
wallace accepted this revision.
wallace added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122603

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


[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

2022-03-31 Thread Alisamar Husain via Phabricator via lldb-commits
zrthxn added inline comments.



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:145
+  private:
+friend class DecodedThread;
+

wallace wrote:
> jj10306 wrote:
> > nit: No need to friend the enclosing class since C++11 - 
> > https://en.cppreference.com/w/cpp/language/nested_types
> TIL!
We need the friend because we are using a private constructor from outside, in 
DecodedThread::CalculateTscRange and a couple other places. The idea is to let 
only DecodedThread create TscRange. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122603

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


[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

2022-03-31 Thread Alisamar Husain via Phabricator via lldb-commits
zrthxn updated this revision to Diff 419619.
zrthxn marked 3 inline comments as done.
zrthxn added a comment.

Dont use auto for simple types


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122603

Files:
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  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
@@ -38,8 +38,8 @@
 thread #1: tid = 3842849
   Raw trace size: 4 KiB
   Total number of instructions: 21
-  Total approximate memory usage: 5.31 KiB
-  Average memory usage per instruction: 259 bytes'''])
+  Total approximate memory usage: 0.98 KiB
+  Average memory usage per instruction: 48.00 bytes'''])
 
 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
@@ -40,5 +40,5 @@
 thread #1: tid = 3842849
   Raw trace size: 4 KiB
   Total number of instructions: 21
-  Total approximate memory usage: 5.31 KiB
-  Average memory usage per instruction: 259 bytes'''])
+  Total approximate memory usage: 0.98 KiB
+  Average memory usage per instruction: 48.00 bytes'''])
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
@@ -119,8 +119,9 @@
   s.Printf("  Total number of instructions: %zu\n", insn_len);
   s.Printf("  Total approximate memory usage: %0.2lf KiB\n",
(double)mem_used / 1024);
-  s.Printf("  Average memory usage per instruction: %zu bytes\n",
-   mem_used / insn_len);
+  if (insn_len != 0)
+s.Printf("  Average memory usage per instruction: %0.2lf bytes\n",
+ (double)mem_used / insn_len);
   return;
 }
 
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
@@ -42,6 +42,8 @@
   DecodedThreadSP m_decoded_thread_sp;
   /// Internal instruction index currently pointing at.
   size_t m_pos;
+  /// Tsc range covering the current instruction.
+  llvm::Optional m_tsc_range;
 };
 
 } // namespace trace_intel_pt
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -23,6 +23,7 @@
   assert(!m_decoded_thread_sp->GetInstructions().empty() &&
  "a trace should have at least one instruction or error");
   m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+  m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos);
 }
 
 size_t TraceCursorIntelPT::GetInternalInstructionSize() {
@@ -40,6 +41,10 @@
 
   while (canMoveOne()) {
 m_pos += IsForwards() ? 1 : -1;
+
+if (m_tsc_range && !m_tsc_range->InRange(m_pos))
+  m_tsc_range = IsForwards() ? m_tsc_range->Next() : m_tsc_range->Prev();
+
 if (!m_ignore_errors && IsError())
   return true;
 if (GetInstructionControlFlowType() & m_granularity)
@@ -58,23 +63,29 @@
 return std::min(std::max((int64_t)0, raw_pos), last_index);
   };
 
-  switch (origin) {
-  case TraceCursor::SeekType::Set:
-m_pos = fitPosToBounds(offset);
-return m_pos;
-  case TraceCursor::SeekType::End:
-m_pos = fitPosToBounds(offset + last_index);
-return last_index - m_pos;
-  case TraceCursor::SeekType::Current:
-int64_t new_pos = fitPosToBounds(offset + m_pos);
-int64_t dist = m_pos - new_pos;
-m_pos = new_pos;
-return std::abs(dist);
-  }
+  auto FindDistanceAndSetPos = [&]() -> int64_t {
+switch (origin) {
+case TraceCursor::SeekType::Set:
+  m_pos = fitPosToBounds(offset);
+  return m_pos;
+case TraceCursor::SeekType::End:
+  m_pos = fitPosToBounds(offset + last_index);
+  return last_index - m_pos;
+case TraceCursor::SeekType::Current:
+  int64_t new_pos = fitPosToBounds(offset + m_pos);
+  int64_t dist = m_pos - new_pos;
+  m_pos = new_pos;
+  return std::abs(dist);
+}
+  };
+  
+	int64_t dist = FindDistanceAndSetPos();
+  

[Lldb-commits] [PATCH] D122859: [trace] Show ideas for the main interfaces for new HTR

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

This diff tries to show the low level structures and the high level APIs for 
interacting with HTR (hierarchical trace representation). Most of the ideas are 
commented in the code. I want to get some feedback with this diff, but the 
actual implementation would be left for other diffs.

I describe in the code two storage strategies: in memory and on disk. Besides 
that, I mention two extensions: a time analyzer and a symbol index. I don't 
intent to implement them right away. I'm thinking about starting with the in 
memory one without adding many abstractions to the code, and also I'd implement 
first only the time analyzer. Later, we could continue implemeting the other 
parts mentioned in this diff, but at least I want to think about them now to 
make sure the basic design doesn't need a complete redesign once we want to add 
the other features.

What I don't include here is the algorithm for creating the call tree. That's 
out of the scope of this diff. I want to focus first on the interfaces, as 
that's the part that should change the least. The algorithm can be implemented 
incrementally.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122859

Files:
  lldb/include/lldb/Target/TraceHTR.h

Index: lldb/include/lldb/Target/TraceHTR.h
===
--- /dev/null
+++ lldb/include/lldb/Target/TraceHTR.h
@@ -0,0 +1,399 @@
+//===-- TraceHTR.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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_TARGET_TRACE_HTR_H
+#define LLDB_TARGET_TRACE_HTR_H
+
+#include "lldb/lldb-types.h"
+
+#include 
+#include 
+#include 
+#include 
+
+/// Hierarchical Trace Representation (HTR):
+///
+/// A high level tracing tool might need to represent a trace in a hierarchical
+/// fashion respecting the chronological structure of the instructions. For
+/// example, a call tree inspector will need to organize the trace instructions
+/// as a tree of blocks, each one representing sequential instructions that
+/// start at a function call and end when that call returns to its parent.
+/// Another tool might just want to display the hierarchy of calls between
+/// modules to understand the flow of data. In summary, these applications need
+/// a way to represent blocks (sequential instructions) that might also have
+/// child blocks.
+///
+/// As the number of instructions in a trace can be in the order of hundreds of
+/// millions, creating copies of the instructions should be avoided, and thus
+/// trace blocks should use instruction identifiers (see \a
+/// TraceCursor::GetId()) to point to some key instructions and then use a
+/// TraceCursor to traverse the trace sequentially from that point.
+///
+/// Creating an HTR of a trace can be a very expensive operation, so the data
+/// storage of the HTR blocks must be efficient and easy to save and load to and
+/// from disk. An additional design consideration is that the underlying data
+/// storage should be replaceable, so that different storage strategies can be
+/// used without having to change the API.
+///
+/// Another design consideration is that this structure should be efficient when
+/// traversing in a recursive fashion, because it might be the most common way
+/// to gather data from it.
+///
+/// Finally, analyzers will be needed. They will compute metrics or indices on
+/// the HTR structures. And not just that, they should be able to be serialized
+/// so that they don't need to be recomputed in future sessions.
+
+namespace lldb_private {
+
+/// This structure contains the underlying metadata of each trace block in
+/// memory. Each block can be referred to with an array index. In order to
+/// facilitate the communication between the analyzers and the storages without
+/// exposing pointers to the underlying data structures, we'll be using these
+/// block indices ubiquitously.
+///
+/// Below this class is another variant of this class when the storage is on
+/// disk.
+class TraceHTRInMemoryStorage {
+  // A block represents all the instructions from GetFirstInstructionId() to
+  // GetLastInstructionId(). A child of a block must be a sequence of
+  // instructions within the above range. Exceptions exist:
+  // - the first child can end before GetFirstInstructionId(), e.g. a function
+  // returning
+  //   to its parent because tracing started late, or because there are gaps in
+  //   the trace
+  // - the last child can start after 

[Lldb-commits] [PATCH] D122523: [lldb] Fix building standalone LLDB on Windows.

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122523

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


[Lldb-commits] [PATCH] D122856: [lldb] Refactor DataBuffer so we can map files as read-only

2022-03-31 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision.
JDevlieghere added reviewers: labath, jasonmolenda, clayborg.
Herald added subscribers: pmatos, asb, atanasyan, jrtc27, kbarton, sbc100, 
nemanjai, sdardis, emaste.
Herald added a project: All.
JDevlieghere requested review of this revision.
Herald added subscribers: MaskRay, aheejin.

Currently, all data buffers are assumed to be writable. This is a problem on 
macOS where it's not allowed to load unsigned binaries in memory as writable. 
To be more precise, `MAP_RESILIENT_CODESIGN` and `MAP_RESILIENT_MEDIA` need to 
be set for mapped (unsigned) binaries on our platform.

Binaries are loaded through `FileSystem::CreateDataBuffer` which returns a 
DataBufferLLVM. The latter is backed by a `llvm::WritableMemoryBuffer` because 
every DataBuffer is considered to be writable. In order to use a read-only 
`llvm::MemoryBuffer` I had to split `DataBuffer` into `DataBuffer` (read-only) 
and `WritableDataBuffer` (read-write).

Differentiating between read-only and read-write buffers worked out everywhere 
except for ObjectFileELF. The latter calls `GetSharedDataBuffer` on the 
`DataExtractor` and applies relocations in place. I understand the purpose of 
making this copy on write for just the affected pages, but I'm not sure how to 
make this work with the new approach:

- Copying the whole object file to the heap when we need to apply the 
relocations is probably too expensive.
- We could try to re-map the file but then we'd need to (1) know that it's a 
DataBufferLLVM and (2) have a way to remap the whole file a writable.

I've ifdef'd out the code to show where the issue is.

rdar://74890607


https://reviews.llvm.org/D122856

Files:
  lldb/include/lldb/Core/ValueObject.h
  lldb/include/lldb/Host/FileSystem.h
  lldb/include/lldb/Symbol/CompactUnwindInfo.h
  lldb/include/lldb/Target/ProcessStructReader.h
  lldb/include/lldb/Target/RegisterCheckpoint.h
  lldb/include/lldb/Target/RegisterContext.h
  lldb/include/lldb/Target/RegisterContextUnwind.h
  lldb/include/lldb/Utility/DataBuffer.h
  lldb/include/lldb/Utility/DataBufferHeap.h
  lldb/include/lldb/Utility/DataBufferLLVM.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Core/SourceManager.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/DataFormatters/StringPrinter.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Host/common/FileSystem.cpp
  lldb/source/Host/common/Host.cpp
  lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp
  lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
  lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
  lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp
  lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
  lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
  lldb/source/Plugins/Language/ObjC/CF.cpp
  lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
  lldb/source/Plugins/Language/ObjC/NSSet.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
  lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm.h
  lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextDarwin_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterContextDarwin_i386.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextDarwin_i386.h
  lldb/source/Plugins/Process/Utility/RegisterContextDarwin_x86_64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextDarwin_x86_64.h
  lldb/source/Plugins/Process/Utility/RegisterContextDummy.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextDummy.h
  lldb/source/Plugins/Process/Utility/RegisterContextHistory.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextHistory.h
  lldb/source/Plugins/Process/Utility/RegisterContextMemory.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextMemory.h
  lldb/source/Plugins/Process/Utility/RegisterContextThreadMemory.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextThreadMemory.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_mips64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_mips64.h
  

[Lldb-commits] [PATCH] D122848: Reduce extraneous temp strings in debugserver, free objects when they're not longer needed

2022-03-31 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

I was chatting with Jonas about this a bit.  The part of this patch which fixes 
all of the `SendPacket(outbuf.c_str())` calls, to avoid a temporary copy of the 
std::string, is clearly necessary and correct.  The changes to clear the 
ostringstreams eagerly after we've escaped the data is noisy, and I'll remove 
those and look at updating JSONGenerator to print escaped string 
representations directly, to eliminate the unescaped copy & the escaped copy 
duplication.  I'll update the patch in a bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122848

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


[Lldb-commits] [lldb] 71ec09b - Revert "[LLDB][NativePDB] Minor fix on inline line table."

2022-03-31 Thread Zequan Wu via lldb-commits

Author: Zequan Wu
Date: 2022-03-31T16:07:49-07:00
New Revision: 71ec09b33ef4f378aaad2431c4b2d1140c7f285a

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

LOG: Revert "[LLDB][NativePDB] Minor fix on inline line table."

This reverts commit 4b2af365b6fadde9e578ca08e6de332388b2f9c2.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index b7e936be4aee6..411c7d902c21e 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -1356,7 +1356,6 @@ void 
SymbolFileNativePDB::ParseInlineSite(PdbCompilandSymId id,
   func_base + code_offset, decl_line + line_offset, 0, decl_file_idx,
   is_start_of_statement, false, is_prologue_end, false, false);
   inline_site_sp->line_entries.push_back(line_entry);
-  has_base = false;
 } else {
   // Add line entry in next call to change_code_offset.
   is_new_line_offset = true;



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


[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

2022-03-31 Thread walter erquinigo via Phabricator via lldb-commits
wallace added inline comments.



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:145
+  private:
+friend class DecodedThread;
+

jj10306 wrote:
> nit: No need to friend the enclosing class since C++11 - 
> https://en.cppreference.com/w/cpp/language/nested_types
TIL!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122603

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


[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

2022-03-31 Thread Jakob Johnson via Phabricator via lldb-commits
jj10306 added inline comments.



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:145
+  private:
+friend class DecodedThread;
+

nit: No need to friend the enclosing class since C++11 - 
https://en.cppreference.com/w/cpp/language/nested_types


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122603

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


[Lldb-commits] [PATCH] D122848: Reduce extraneous temp strings in debugserver, free objects when they're not longer needed

2022-03-31 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added a comment.

In D122848#3420581 , @JDevlieghere 
wrote:

> If debugserver linked against libSupport we could have saved the additional 
> copy altogether by using `llvm::raw_string_ostream`:
>
>   std::string str;
>   llvm::raw_string_ostream stream(str);
>   stream.str() // Flushes and returns a reference to the stack allocated str
>
> Barring that, I wonder if if a little wrapper around `std::ostringstream` 
> could improve readability and avoid bugs where someone forgets to call 
> `stream.str("")`.
>
>   class AggressiveStream {
>   public:
> AggressiveStream() : m_stream(std::make_unique()) {}
>   
> std::ostringstream *() {
>   assert(m_stream && "cannot use stream after having called str()");
>   return *m_stream;
> }
>   
> std::string str() {
>   std::string s = m_stream->str();
>   m_stream.reset();
>   return std::move(s);
> }
>   
>   private:
> std::unique_ptr m_stream;
>   };
>
> WDYT?

That's a cool idea, and certainly less error prone than this by-hand method I 
did, but I think I'd rather fix JSONGenerator::Dump to return quoted strings so 
we don't need to immediately copy the entire string into another string for 
quoting.  I don't feel strongly about it tho.  I might be too optimistic that 
I'm going to fix Dump and it would be good to adopt a safer wrapper around 
ostringsream.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122848

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


[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

2022-03-31 Thread walter erquinigo via Phabricator via lldb-commits
wallace added a comment.

one last nit and good to go




Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:82
+
+  auto dist = FindDistanceAndSetPos();
+  m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos);

don't use auto for simple types


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122603

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


[Lldb-commits] [lldb] 4b2af36 - [LLDB][NativePDB] Minor fix on inline line table.

2022-03-31 Thread Zequan Wu via lldb-commits

Author: Zequan Wu
Date: 2022-03-31T14:57:26-07:00
New Revision: 4b2af365b6fadde9e578ca08e6de332388b2f9c2

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

LOG: [LLDB][NativePDB] Minor fix on inline line table.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index 411c7d902c21e..b7e936be4aee6 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -1356,6 +1356,7 @@ void 
SymbolFileNativePDB::ParseInlineSite(PdbCompilandSymId id,
   func_base + code_offset, decl_line + line_offset, 0, decl_file_idx,
   is_start_of_statement, false, is_prologue_end, false, false);
   inline_site_sp->line_entries.push_back(line_entry);
+  has_base = false;
 } else {
   // Add line entry in next call to change_code_offset.
   is_new_line_offset = true;



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


[Lldb-commits] [PATCH] D122848: Reduce extraneous temp strings in debugserver, free objects when they're not longer needed

2022-03-31 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda added inline comments.



Comment at: lldb/tools/debugserver/source/RNBRemote.cpp:588
+  stream.str(std::string());
+  stream.clear();
   return SendPacket(payload);

JDevlieghere wrote:
> `clear` doesn't do what you think it does, it modifies the state flag which 
> isn't relevant here.
Yeah I know, the common idiom for clearing an ostringstream is to .str("") an 
then .clear() to clear flag state, I was only clear'ing to be proper, even 
though we don't use the object past this point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122848

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


[Lldb-commits] [PATCH] D122848: Reduce extraneous temp strings in debugserver, free objects when they're not longer needed

2022-03-31 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment.

If debugserver linked against libSupport we could have saved the additional 
copy altogether by using `llvm::raw_string_ostream`:

  std::string str;
  llvm::raw_string_ostream stream(str);
  stream.str() // Flushes and returns a reference to the stack allocated str

Barring that, I wonder if if a little wrapper around `std::ostringstream` could 
improve readability and avoid bugs where someone forgets to call 
`stream.str("")`.

  class AggressiveStream {
  public:
AggressiveStream() : m_stream(std::make_unique()) {}
  
std::ostringstream *() {
  assert(m_stream && "cannot use stream after having called str()");
  return *m_stream;
}
  
std::string str() {
  std::string s = m_stream->str();
  m_stream.reset();
  return std::move(s);
}
  
  private:
std::unique_ptr m_stream;
  };

WDYT?




Comment at: lldb/tools/debugserver/source/RNBRemote.cpp:588
+  stream.str(std::string());
+  stream.clear();
   return SendPacket(payload);

`clear` doesn't do what you think it does, it modifies the state flag which 
isn't relevant here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122848

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


[Lldb-commits] [PATCH] D122680: Add a setting to force overwriting commands in "command script add"

2022-03-31 Thread Jim Ingham via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1f7b58f2a504: Add a setting to not require --overwrite to 
overwrite commands. (authored by jingham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122680

Files:
  lldb/include/lldb/Interpreter/CommandInterpreter.h
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/InterpreterProperties.td
  lldb/test/API/commands/command/container/TestContainerCommands.py

Index: lldb/test/API/commands/command/container/TestContainerCommands.py
===
--- lldb/test/API/commands/command/container/TestContainerCommands.py
+++ lldb/test/API/commands/command/container/TestContainerCommands.py
@@ -57,8 +57,9 @@
 self.expect("test-multi test-multi-sub welcome friend", "Test command works",
 substrs=["Hello friend, welcome to LLDB"])
 
-# Make sure overwriting works, first the leaf command:
-# We should not be able to remove extant commands by default:
+# Make sure overwriting works on the leaf command.  First using the
+# explicit option so we should not be able to remove extant commands by default:
+
 self.expect("command script add -c welcome.WelcomeCommand2 test-multi test-multi-sub welcome",
 "overwrite command w/o -o",
 substrs=["cannot add command: sub-command already exists"], error=True)
@@ -67,9 +68,15 @@
 # Make sure we really did overwrite:
 self.expect("test-multi test-multi-sub welcome friend", "Used the new command class",
 substrs=["Hello friend, welcome again to LLDB"])
-
 self.expect("apropos welcome", "welcome should show up in apropos", substrs=["A docstring for the second Welcome"])
 
+# Now switch the default and make sure we can now delete w/o the overwrite option:
+self.runCmd("settings set interpreter.require-overwrite 0")
+self.runCmd("command script add -c welcome.WelcomeCommand test-multi test-multi-sub welcome")
+# Make sure we really did overwrite:
+self.expect("test-multi test-multi-sub welcome friend", "Used the new command class",
+substrs=["Hello friend, welcome to LLDB"])
+
 # Make sure we give good errors when the input is wrong:
 self.expect("command script delete test-mult test-multi-sub welcome", "Delete script command - wrong first path component",
 substrs=["'test-mult' not found"], error=True)
Index: lldb/source/Interpreter/InterpreterProperties.td
===
--- lldb/source/Interpreter/InterpreterProperties.td
+++ lldb/source/Interpreter/InterpreterProperties.td
@@ -36,4 +36,8 @@
 Global,
 DefaultTrue,
 Desc<"If true, LLDB will repeat the previous command if no command was passed to the interpreter. If false, LLDB won't repeat the previous command but only return a new prompt.">;
+  def RequireCommandOverwrite: Property<"require-overwrite", "Boolean">,
+Global,
+DefaultTrue,
+Desc<"If true, require --overwrite in 'command script add' before overwriting existing user commands.">;
 }
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -244,6 +244,12 @@
   nullptr, idx, g_interpreter_properties[idx].default_uint_value != 0);
 }
 
+bool CommandInterpreter::GetRequireCommandOverwrite() const {
+  const uint32_t idx = ePropertyRequireCommandOverwrite;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+  nullptr, idx, g_interpreter_properties[idx].default_uint_value != 0);
+}
+
 void CommandInterpreter::Initialize() {
   LLDB_SCOPED_TIMER();
 
Index: lldb/source/Commands/CommandObjectCommands.cpp
===
--- lldb/source/Commands/CommandObjectCommands.cpp
+++ lldb/source/Commands/CommandObjectCommands.cpp
@@ -1445,7 +1445,7 @@
   m_short_help = std::string(option_arg);
 break;
   case 'o':
-m_overwrite = true;
+m_overwrite_lazy = eLazyBoolYes;
 break;
   case 's':
 m_synchronicity =
@@ -1467,7 +1467,7 @@
   m_class_name.clear();
   m_funct_name.clear();
   m_short_help.clear();
-  m_overwrite = false;
+  m_overwrite_lazy = eLazyBoolCalculate;
   m_synchronicity = eScriptedCommandSynchronicitySynchronous;
 }
 
@@ -1480,7 +1480,7 @@
 std::string m_class_name;
 std::string m_funct_name;
 std::string m_short_help;
-bool m_overwrite = false;
+LazyBool m_overwrite_lazy;
 ScriptedCommandSynchronicity 

[Lldb-commits] [lldb] 1f7b58f - Add a setting to not require --overwrite to overwrite commands.

2022-03-31 Thread Jim Ingham via lldb-commits

Author: Jim Ingham
Date: 2022-03-31T14:15:14-07:00
New Revision: 1f7b58f2a50461493f083b2ed807b25e036286f6

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

LOG: Add a setting to not require --overwrite to overwrite commands.

Protecting against accidental overwriting of commands is good, but
having to pass a flag to overwrite the command when developing your
commands is pretty annoying.  This adds a setting to defeat the protection
so you can do this once at the start of your session and not have to
worry about it again.

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

Added: 


Modified: 
lldb/include/lldb/Interpreter/CommandInterpreter.h
lldb/source/Commands/CommandObjectCommands.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Interpreter/InterpreterProperties.td
lldb/test/API/commands/command/container/TestContainerCommands.py

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h 
b/lldb/include/lldb/Interpreter/CommandInterpreter.h
index 787dfdbcb21f5..641e651e18909 100644
--- a/lldb/include/lldb/Interpreter/CommandInterpreter.h
+++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h
@@ -549,6 +549,8 @@ class CommandInterpreter : public Broadcaster,
   void SetEchoCommentCommands(bool enable);
 
   bool GetRepeatPreviousCommand() const;
+  
+  bool GetRequireCommandOverwrite() const;
 
   const CommandObject::CommandMap () const {
 return m_user_dict;

diff  --git a/lldb/source/Commands/CommandObjectCommands.cpp 
b/lldb/source/Commands/CommandObjectCommands.cpp
index 51385e0d8e58a..1d4687b0650a9 100644
--- a/lldb/source/Commands/CommandObjectCommands.cpp
+++ b/lldb/source/Commands/CommandObjectCommands.cpp
@@ -1445,7 +1445,7 @@ class CommandObjectCommandsScriptAdd : public 
CommandObjectParsed,
   m_short_help = std::string(option_arg);
 break;
   case 'o':
-m_overwrite = true;
+m_overwrite_lazy = eLazyBoolYes;
 break;
   case 's':
 m_synchronicity =
@@ -1467,7 +1467,7 @@ class CommandObjectCommandsScriptAdd : public 
CommandObjectParsed,
   m_class_name.clear();
   m_funct_name.clear();
   m_short_help.clear();
-  m_overwrite = false;
+  m_overwrite_lazy = eLazyBoolCalculate;
   m_synchronicity = eScriptedCommandSynchronicitySynchronous;
 }
 
@@ -1480,7 +1480,7 @@ class CommandObjectCommandsScriptAdd : public 
CommandObjectParsed,
 std::string m_class_name;
 std::string m_funct_name;
 std::string m_short_help;
-bool m_overwrite = false;
+LazyBool m_overwrite_lazy;
 ScriptedCommandSynchronicity m_synchronicity =
 eScriptedCommandSynchronicitySynchronous;
   };
@@ -1499,7 +1499,6 @@ class CommandObjectCommandsScriptAdd : public 
CommandObjectParsed,
 
 ScriptInterpreter *interpreter = GetDebugger().GetScriptInterpreter();
 if (interpreter) {
-
   StringList lines;
   lines.SplitIntoLines(data);
   if (lines.GetSize() > 0) {
@@ -1562,8 +1561,19 @@ class CommandObjectCommandsScriptAdd : public 
CommandObjectParsed,
   result.AppendError("'command script add' requires at least one 
argument");
   return false;
 }
-// Store the options in case we get multi-line input
-m_overwrite = m_options.m_overwrite;
+// Store the options in case we get multi-line input, also figure out the
+// default if not user supplied:
+switch (m_options.m_overwrite_lazy) {
+  case eLazyBoolCalculate:
+m_overwrite = 
!GetDebugger().GetCommandInterpreter().GetRequireCommandOverwrite();
+break;
+  case eLazyBoolYes:
+m_overwrite = true;
+break;
+  case eLazyBoolNo:
+m_overwrite = false;
+}
+
 Status path_error;
 m_container = GetCommandInterpreter().VerifyUserMultiwordCmdPath(
 command, true, path_error);
@@ -1637,7 +1647,7 @@ class CommandObjectCommandsScriptAdd : public 
CommandObjectParsed,
   std::string m_cmd_name;
   CommandObjectMultiword *m_container = nullptr;
   std::string m_short_help;
-  bool m_overwrite = false;
+  bool m_overwrite = eLazyBoolCalculate;
   ScriptedCommandSynchronicity m_synchronicity =
   eScriptedCommandSynchronicitySynchronous;
 };

diff  --git a/lldb/source/Interpreter/CommandInterpreter.cpp 
b/lldb/source/Interpreter/CommandInterpreter.cpp
index a6b7f0e480fc2..f9ba5c3d2a2ea 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -244,6 +244,12 @@ bool CommandInterpreter::GetRepeatPreviousCommand() const {
   nullptr, idx, g_interpreter_properties[idx].default_uint_value != 0);
 }
 
+bool CommandInterpreter::GetRequireCommandOverwrite() const {
+  const uint32_t idx = 

[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

2022-03-31 Thread Alisamar Husain via Phabricator via lldb-commits
zrthxn added inline comments.



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:67-82
   switch (origin) {
   case TraceCursor::SeekType::Set:
 m_pos = fitPosToBounds(offset);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
 return m_pos;
   case TraceCursor::SeekType::End:
 m_pos = fitPosToBounds(offset + last_index);

wallace wrote:
> we can simplify this so that we only invoke CalculateTscRange once
This is incorrect The converted code always returns 0. I've refactored it to 
have CalculateTscRange once but its a side-effect-y function and will need some 
future attention.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122603

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


[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

2022-03-31 Thread Alisamar Husain via Phabricator via lldb-commits
zrthxn updated this revision to Diff 419560.
zrthxn marked 12 inline comments as done.
zrthxn added a comment.

Incorporate feedback and update tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122603

Files:
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  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
@@ -38,8 +38,8 @@
 thread #1: tid = 3842849
   Raw trace size: 4 KiB
   Total number of instructions: 21
-  Total approximate memory usage: 5.31 KiB
-  Average memory usage per instruction: 259 bytes'''])
+  Total approximate memory usage: 0.98 KiB
+  Average memory usage per instruction: 48.00 bytes'''])
 
 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
@@ -40,5 +40,5 @@
 thread #1: tid = 3842849
   Raw trace size: 4 KiB
   Total number of instructions: 21
-  Total approximate memory usage: 5.31 KiB
-  Average memory usage per instruction: 259 bytes'''])
+  Total approximate memory usage: 0.98 KiB
+  Average memory usage per instruction: 48.00 bytes'''])
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
@@ -119,8 +119,9 @@
   s.Printf("  Total number of instructions: %zu\n", insn_len);
   s.Printf("  Total approximate memory usage: %0.2lf KiB\n",
(double)mem_used / 1024);
-  s.Printf("  Average memory usage per instruction: %zu bytes\n",
-   mem_used / insn_len);
+  if (insn_len != 0)
+s.Printf("  Average memory usage per instruction: %0.2lf bytes\n",
+ (double)mem_used / insn_len);
   return;
 }
 
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
@@ -42,6 +42,8 @@
   DecodedThreadSP m_decoded_thread_sp;
   /// Internal instruction index currently pointing at.
   size_t m_pos;
+  /// Tsc range covering the current instruction.
+  llvm::Optional m_tsc_range;
 };
 
 } // namespace trace_intel_pt
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -23,6 +23,7 @@
   assert(!m_decoded_thread_sp->GetInstructions().empty() &&
  "a trace should have at least one instruction or error");
   m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+  m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos);
 }
 
 size_t TraceCursorIntelPT::GetInternalInstructionSize() {
@@ -40,6 +41,10 @@
 
   while (canMoveOne()) {
 m_pos += IsForwards() ? 1 : -1;
+
+if (m_tsc_range && !m_tsc_range->InRange(m_pos))
+  m_tsc_range = IsForwards() ? m_tsc_range->Next() : m_tsc_range->Prev();
+
 if (!m_ignore_errors && IsError())
   return true;
 if (GetInstructionControlFlowType() & m_granularity)
@@ -58,23 +63,29 @@
 return std::min(std::max((int64_t)0, raw_pos), last_index);
   };
 
-  switch (origin) {
-  case TraceCursor::SeekType::Set:
-m_pos = fitPosToBounds(offset);
-return m_pos;
-  case TraceCursor::SeekType::End:
-m_pos = fitPosToBounds(offset + last_index);
-return last_index - m_pos;
-  case TraceCursor::SeekType::Current:
-int64_t new_pos = fitPosToBounds(offset + m_pos);
-int64_t dist = m_pos - new_pos;
-m_pos = new_pos;
-return std::abs(dist);
-  }
+  auto FindDistanceAndSetPos = [&]() -> int64_t {
+switch (origin) {
+case TraceCursor::SeekType::Set:
+  m_pos = fitPosToBounds(offset);
+  return m_pos;
+case TraceCursor::SeekType::End:
+  m_pos = fitPosToBounds(offset + last_index);
+  return last_index - m_pos;
+case TraceCursor::SeekType::Current:
+  int64_t new_pos = fitPosToBounds(offset + m_pos);
+  int64_t dist = m_pos - new_pos;
+  m_pos = new_pos;
+  return std::abs(dist);
+}
+  };
+
+  auto dist = FindDistanceAndSetPos();
+  

[Lldb-commits] [PATCH] D122848: Reduce extraneous temp strings in debugserver, free objects when they're not longer needed

2022-03-31 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda created this revision.
jasonmolenda added reviewers: jingham, JDevlieghere.
jasonmolenda added a project: LLDB.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

Looking at debugserver's memory use a bit, I noticed that the SendPacket method 
which takes a std::string& argument, is often called with a std::string object 
that the caller calls c_str() on, so a temporary copy of the string is created. 
 This patch removes those c_str() method calls.

We also have a common pattern where we have/create a JSONGenerator::ObjectSP, 
dump the string representation of the object into a ostringstream, and then 
binary escape the ostringstream.  We end up with the original ObjectSP 
dictionary, the ostringstream print of it, and the escaped std::string copy of 
it.  Then we pass that to SendPacket which may create a compressed std::string 
of it.  This patch frees those objects as soon as they are no longer needed, so 
our peak memory use is limited to a narrower window.

In the future I want to add a JSONGenerator::DumpEscaped() method that formats 
its output following the binary escape protocol.  Every packet that sends JSON 
responses must escape it before sending, so there's no point in even generating 
the un-escaped version in the first place.  It forces us to always have two 
copies of the string in heap for at least a little while.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122848

Files:
  lldb/tools/debugserver/source/RNBRemote.cpp

Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -584,6 +584,8 @@
   stream << "JSON-async:";
   dictionary.Dump(stream);
   const std::string payload = binary_encode_string(stream.str());
+  stream.str(std::string());
+  stream.clear();
   return SendPacket(payload);
 }
 
@@ -3760,7 +3762,7 @@
   return_message +=
   cstring_to_asciihex_string("debugserver is x86_64 binary running in "
  "translation, attached failed.");
-  SendPacket(return_message.c_str());
+  SendPacket(return_message);
   return rnb_err;
 }
 
@@ -3853,14 +3855,14 @@
   DNBLogError("Tried to attach to pid that doesn't exist");
   std::string return_message = "E96;";
   return_message += cstring_to_asciihex_string("no such process.");
-  return SendPacket(return_message.c_str());
+  return SendPacket(return_message);
 }
 if (process_is_already_being_debugged (pid_attaching_to)) {
   DNBLogError("Tried to attach to process already being debugged");
   std::string return_message = "E96;";
   return_message += cstring_to_asciihex_string("tried to attach to "
"process already being debugged");
-  return SendPacket(return_message.c_str());
+  return SendPacket(return_message);
 }
 uid_t my_uid, process_uid;
 if (attach_failed_due_to_uid_mismatch (pid_attaching_to, 
@@ -3881,7 +3883,7 @@
 + my_username + "' and process is running "
 "as user '" + process_username + "'";
   return_message += cstring_to_asciihex_string(msg.c_str());
-  return SendPacket(return_message.c_str());
+  return SendPacket(return_message);
 }
 if (!login_session_has_gui_access() && !developer_mode_enabled()) {
   DNBLogError("Developer mode is not enabled and this is a "
@@ -3891,7 +3893,7 @@
"not enabled on this machine "
"and this is a non-interactive "
"debug session.");
-  return SendPacket(return_message.c_str());
+  return SendPacket(return_message);
 }
 if (!login_session_has_gui_access()) {
   DNBLogError("This is a non-interactive session");
@@ -3900,7 +3902,7 @@
"non-interactive debug session, "
"cannot get permission to debug "
"processes.");
-  return SendPacket(return_message.c_str());
+  return SendPacket(return_message);
 }
   }
 
@@ -3923,7 +3925,7 @@
   std::string default_return_msg = "E96;";
   default_return_msg += cstring_to_asciihex_string 
   (error_explainer.c_str());
-  SendPacket (default_return_msg.c_str());
+  SendPacket (default_return_msg);
   DNBLogError("Attach failed: \"%s\".", err_str);
   return rnb_err;
 }
@@ -4347,7 +4349,7 @@
 
   std::string data = DNBProcessGetProfileData(pid, scan_type);
   if (!data.empty()) {
-return 

[Lldb-commits] [PATCH] D121844: Applying clang-tidy modernize-use-equals-default over LLDB

2022-03-31 Thread Shafik Yaghmour via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG24f9a2f53db7: [LLDB] Applying clang-tidy 
modernize-use-equals-default over LLDB (authored by shafik).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121844

Files:
  lldb/source/API/SBMemoryRegionInfoList.cpp
  lldb/source/API/SBQueue.cpp
  lldb/source/API/SBValue.cpp
  lldb/source/API/SBValueList.cpp
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Commands/CommandObjectMemoryTag.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectReproducer.cpp
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Commands/CommandObjectSource.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Commands/CommandObjectWatchpointCommand.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Host/macosx/cfcpp/CFCData.cpp
  lldb/source/Host/macosx/cfcpp/CFCMutableArray.cpp
  lldb/source/Host/macosx/cfcpp/CFCMutableDictionary.cpp
  lldb/source/Host/macosx/cfcpp/CFCMutableSet.cpp
  lldb/source/Host/macosx/cfcpp/CFCString.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
  lldb/source/Plugins/REPL/Clang/ClangREPL.cpp
  lldb/source/Target/ExecutionContext.cpp
  lldb/source/Utility/XcodeSDK.cpp
  lldb/tools/debugserver/source/DNBBreakpoint.cpp
  lldb/tools/debugserver/source/DNBDataRef.cpp
  lldb/tools/debugserver/source/MacOSX/CFBundle.cpp
  lldb/tools/debugserver/source/MacOSX/CFString.cpp
  lldb/tools/debugserver/source/MacOSX/MachThreadList.cpp
  lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
  lldb/tools/debugserver/source/StdStringExtractor.cpp
  lldb/tools/debugserver/source/TTYState.cpp
  lldb/tools/lldb-vscode/IOStream.cpp
  lldb/tools/lldb-vscode/VSCode.cpp

Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -58,7 +58,7 @@
 log.reset(new std::ofstream(log_file_path));
 }
 
-VSCode::~VSCode() {}
+VSCode::~VSCode() = default;
 
 int64_t VSCode::GetLineForPC(int64_t sourceReference, lldb::addr_t pc) const {
   auto pos = source_map.find(sourceReference);
Index: lldb/tools/lldb-vscode/IOStream.cpp
===
--- lldb/tools/lldb-vscode/IOStream.cpp
+++ lldb/tools/lldb-vscode/IOStream.cpp
@@ -22,7 +22,7 @@
 
 using namespace lldb_vscode;
 
-StreamDescriptor::StreamDescriptor() {}
+StreamDescriptor::StreamDescriptor() = default;
 
 StreamDescriptor::StreamDescriptor(StreamDescriptor &) {
   *this = std::move(other);
Index: lldb/tools/debugserver/source/TTYState.cpp
===
--- lldb/tools/debugserver/source/TTYState.cpp
+++ lldb/tools/debugserver/source/TTYState.cpp
@@ -18,7 +18,7 @@
 TTYState::TTYState()
 : m_fd(-1), m_tflags(-1), m_ttystateErr(-1), m_processGroup(-1) {}
 
-TTYState::~TTYState() {}
+TTYState::~TTYState() = default;
 
 bool TTYState::GetTTYState(int fd, bool saveProcessGroup) {
   if (fd >= 0 && ::isatty(fd)) {
@@ -62,7 +62,7 @@
 
 TTYStateSwitcher::TTYStateSwitcher() : m_currentState(~0) {}
 
-TTYStateSwitcher::~TTYStateSwitcher() {}
+TTYStateSwitcher::~TTYStateSwitcher() = default;
 
 bool TTYStateSwitcher::GetState(uint32_t idx, int fd, bool saveProcessGroup) {
   if (ValidStateIndex(idx))
Index: lldb/tools/debugserver/source/StdStringExtractor.cpp
===
--- lldb/tools/debugserver/source/StdStringExtractor.cpp
+++ lldb/tools/debugserver/source/StdStringExtractor.cpp
@@ -30,7 +30,7 @@
 }
 
 // Destructor
-StdStringExtractor::~StdStringExtractor() {}
+StdStringExtractor::~StdStringExtractor() = default;
 
 char StdStringExtractor::GetChar(char fail_value) {
   if (m_index < m_packet.size()) {
Index: lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
===
--- lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
+++ lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
@@ -28,7 +28,7 @@
 
 MachVMMemory::MachVMMemory() : m_page_size(kInvalidPageSize), m_err(0) {}
 
-MachVMMemory::~MachVMMemory() {}
+MachVMMemory::~MachVMMemory() = default;
 
 nub_size_t MachVMMemory::PageSize(task_t task) {
   if (m_page_size == kInvalidPageSize) {
Index: 

[Lldb-commits] [lldb] 24f9a2f - [LLDB] Applying clang-tidy modernize-use-equals-default over LLDB

2022-03-31 Thread Shafik Yaghmour via lldb-commits

Author: Shafik Yaghmour
Date: 2022-03-31T13:21:49-07:00
New Revision: 24f9a2f53db78df59761f46ceed3bb5e7aa0d331

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

LOG: [LLDB] Applying clang-tidy modernize-use-equals-default over LLDB

Applied modernize-use-equals-default clang-tidy check over LLDB.

This check is already present in the lldb/.clang-tidy config.

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

Added: 


Modified: 
lldb/source/API/SBMemoryRegionInfoList.cpp
lldb/source/API/SBQueue.cpp
lldb/source/API/SBValue.cpp
lldb/source/API/SBValueList.cpp
lldb/source/Commands/CommandObjectBreakpoint.cpp
lldb/source/Commands/CommandObjectBreakpointCommand.cpp
lldb/source/Commands/CommandObjectCommands.cpp
lldb/source/Commands/CommandObjectExpression.cpp
lldb/source/Commands/CommandObjectFrame.cpp
lldb/source/Commands/CommandObjectLog.cpp
lldb/source/Commands/CommandObjectMemory.cpp
lldb/source/Commands/CommandObjectMemoryTag.cpp
lldb/source/Commands/CommandObjectPlatform.cpp
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Commands/CommandObjectReproducer.cpp
lldb/source/Commands/CommandObjectSettings.cpp
lldb/source/Commands/CommandObjectSource.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Commands/CommandObjectType.cpp
lldb/source/Commands/CommandObjectWatchpoint.cpp
lldb/source/Commands/CommandObjectWatchpointCommand.cpp
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/source/Host/macosx/cfcpp/CFCData.cpp
lldb/source/Host/macosx/cfcpp/CFCMutableArray.cpp
lldb/source/Host/macosx/cfcpp/CFCMutableDictionary.cpp
lldb/source/Host/macosx/cfcpp/CFCMutableSet.cpp
lldb/source/Host/macosx/cfcpp/CFCString.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
lldb/source/Plugins/REPL/Clang/ClangREPL.cpp
lldb/source/Target/ExecutionContext.cpp
lldb/source/Utility/XcodeSDK.cpp
lldb/tools/debugserver/source/DNBBreakpoint.cpp
lldb/tools/debugserver/source/DNBDataRef.cpp
lldb/tools/debugserver/source/MacOSX/CFBundle.cpp
lldb/tools/debugserver/source/MacOSX/CFString.cpp
lldb/tools/debugserver/source/MacOSX/MachThreadList.cpp
lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
lldb/tools/debugserver/source/StdStringExtractor.cpp
lldb/tools/debugserver/source/TTYState.cpp
lldb/tools/lldb-vscode/IOStream.cpp
lldb/tools/lldb-vscode/VSCode.cpp

Removed: 




diff  --git a/lldb/source/API/SBMemoryRegionInfoList.cpp 
b/lldb/source/API/SBMemoryRegionInfoList.cpp
index 39dee86dc3007..f8381d1098a0f 100644
--- a/lldb/source/API/SBMemoryRegionInfoList.cpp
+++ b/lldb/source/API/SBMemoryRegionInfoList.cpp
@@ -21,8 +21,7 @@ class MemoryRegionInfoListImpl {
 public:
   MemoryRegionInfoListImpl() : m_regions() {}
 
-  MemoryRegionInfoListImpl(const MemoryRegionInfoListImpl )
-  : m_regions(rhs.m_regions) {}
+  MemoryRegionInfoListImpl(const MemoryRegionInfoListImpl ) = default;
 
   MemoryRegionInfoListImpl =(const MemoryRegionInfoListImpl ) {
 if (this == )

diff  --git a/lldb/source/API/SBQueue.cpp b/lldb/source/API/SBQueue.cpp
index 1aeede4b1da6f..b97da3ef02edd 100644
--- a/lldb/source/API/SBQueue.cpp
+++ b/lldb/source/API/SBQueue.cpp
@@ -27,7 +27,7 @@ namespace lldb_private {
 
 class QueueImpl {
 public:
-  QueueImpl() {}
+  QueueImpl() = default;
 
   QueueImpl(const lldb::QueueSP _sp) { m_queue_wp = queue_sp; }
 

diff  --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp
index 20581cfabdd66..c39e00d645e27 100644
--- a/lldb/source/API/SBValue.cpp
+++ b/lldb/source/API/SBValue.cpp
@@ -69,9 +69,7 @@ class ValueImpl {
 }
   }
 
-  ValueImpl(const ValueImpl )
-  : m_valobj_sp(rhs.m_valobj_sp), m_use_dynamic(rhs.m_use_dynamic),
-m_use_synthetic(rhs.m_use_synthetic), m_name(rhs.m_name) {}
+  ValueImpl(const ValueImpl ) = default;
 
   ValueImpl =(const ValueImpl ) {
 if (this != ) {

diff  --git a/lldb/source/API/SBValueList.cpp b/lldb/source/API/SBValueList.cpp
index a67030c506f41..4b36b04c8d350 100644
--- a/lldb/source/API/SBValueList.cpp
+++ b/lldb/source/API/SBValueList.cpp
@@ -19,9 +19,9 @@ using namespace lldb_private;
 
 class ValueListImpl {
 public:
-  ValueListImpl() {}
+  ValueListImpl() = default;
 
-  ValueListImpl(const ValueListImpl ) : m_values(rhs.m_values) {}
+  ValueListImpl(const ValueListImpl ) = default;
 
   ValueListImpl =(const ValueListImpl ) {
 if (this == )

diff  --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp 
b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index c4e55fdb3b9c0..4515c7ee58eee 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -179,7 

[Lldb-commits] [PATCH] D121844: Applying clang-tidy modernize-use-equals-default over LLDB

2022-03-31 Thread Shafik Yaghmour via Phabricator via lldb-commits
shafik updated this revision to Diff 419550.
shafik marked 2 inline comments as done.
shafik added a comment.

- Rebased
- Applied clang-format
- Address Pavel's comment


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

https://reviews.llvm.org/D121844

Files:
  lldb/source/API/SBMemoryRegionInfoList.cpp
  lldb/source/API/SBQueue.cpp
  lldb/source/API/SBValue.cpp
  lldb/source/API/SBValueList.cpp
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/CommandObjectBreakpointCommand.cpp
  lldb/source/Commands/CommandObjectCommands.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectLog.cpp
  lldb/source/Commands/CommandObjectMemory.cpp
  lldb/source/Commands/CommandObjectMemoryTag.cpp
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectReproducer.cpp
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/source/Commands/CommandObjectSource.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Commands/CommandObjectWatchpointCommand.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Host/macosx/cfcpp/CFCData.cpp
  lldb/source/Host/macosx/cfcpp/CFCMutableArray.cpp
  lldb/source/Host/macosx/cfcpp/CFCMutableDictionary.cpp
  lldb/source/Host/macosx/cfcpp/CFCMutableSet.cpp
  lldb/source/Host/macosx/cfcpp/CFCString.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
  lldb/source/Plugins/REPL/Clang/ClangREPL.cpp
  lldb/source/Target/ExecutionContext.cpp
  lldb/source/Utility/XcodeSDK.cpp
  lldb/tools/debugserver/source/DNBBreakpoint.cpp
  lldb/tools/debugserver/source/DNBDataRef.cpp
  lldb/tools/debugserver/source/MacOSX/CFBundle.cpp
  lldb/tools/debugserver/source/MacOSX/CFString.cpp
  lldb/tools/debugserver/source/MacOSX/MachThreadList.cpp
  lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
  lldb/tools/debugserver/source/StdStringExtractor.cpp
  lldb/tools/debugserver/source/TTYState.cpp
  lldb/tools/lldb-vscode/IOStream.cpp
  lldb/tools/lldb-vscode/VSCode.cpp

Index: lldb/tools/lldb-vscode/VSCode.cpp
===
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -58,7 +58,7 @@
 log.reset(new std::ofstream(log_file_path));
 }
 
-VSCode::~VSCode() {}
+VSCode::~VSCode() = default;
 
 int64_t VSCode::GetLineForPC(int64_t sourceReference, lldb::addr_t pc) const {
   auto pos = source_map.find(sourceReference);
Index: lldb/tools/lldb-vscode/IOStream.cpp
===
--- lldb/tools/lldb-vscode/IOStream.cpp
+++ lldb/tools/lldb-vscode/IOStream.cpp
@@ -22,7 +22,7 @@
 
 using namespace lldb_vscode;
 
-StreamDescriptor::StreamDescriptor() {}
+StreamDescriptor::StreamDescriptor() = default;
 
 StreamDescriptor::StreamDescriptor(StreamDescriptor &) {
   *this = std::move(other);
Index: lldb/tools/debugserver/source/TTYState.cpp
===
--- lldb/tools/debugserver/source/TTYState.cpp
+++ lldb/tools/debugserver/source/TTYState.cpp
@@ -18,7 +18,7 @@
 TTYState::TTYState()
 : m_fd(-1), m_tflags(-1), m_ttystateErr(-1), m_processGroup(-1) {}
 
-TTYState::~TTYState() {}
+TTYState::~TTYState() = default;
 
 bool TTYState::GetTTYState(int fd, bool saveProcessGroup) {
   if (fd >= 0 && ::isatty(fd)) {
@@ -62,7 +62,7 @@
 
 TTYStateSwitcher::TTYStateSwitcher() : m_currentState(~0) {}
 
-TTYStateSwitcher::~TTYStateSwitcher() {}
+TTYStateSwitcher::~TTYStateSwitcher() = default;
 
 bool TTYStateSwitcher::GetState(uint32_t idx, int fd, bool saveProcessGroup) {
   if (ValidStateIndex(idx))
Index: lldb/tools/debugserver/source/StdStringExtractor.cpp
===
--- lldb/tools/debugserver/source/StdStringExtractor.cpp
+++ lldb/tools/debugserver/source/StdStringExtractor.cpp
@@ -30,7 +30,7 @@
 }
 
 // Destructor
-StdStringExtractor::~StdStringExtractor() {}
+StdStringExtractor::~StdStringExtractor() = default;
 
 char StdStringExtractor::GetChar(char fail_value) {
   if (m_index < m_packet.size()) {
Index: lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
===
--- lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
+++ lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
@@ -28,7 +28,7 @@
 
 MachVMMemory::MachVMMemory() : m_page_size(kInvalidPageSize), m_err(0) {}
 
-MachVMMemory::~MachVMMemory() {}
+MachVMMemory::~MachVMMemory() = default;
 
 nub_size_t MachVMMemory::PageSize(task_t task) {
   if (m_page_size == kInvalidPageSize) {
Index: lldb/tools/debugserver/source/MacOSX/MachThreadList.cpp
===
--- 

[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

2022-03-31 Thread walter erquinigo via Phabricator via lldb-commits
wallace requested changes to this revision.
wallace added a comment.
This revision now requires changes to proceed.

almost there! Mostly cosmetic changes needed




Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:94-98
+  m_instructions.emplace_back(insn);
+  if (!m_last_tsc || *m_last_tsc != tsc) {
+m_instruction_timestamps.emplace(m_instructions.size() - 1, tsc);
+m_last_tsc = tsc;
+  }

We need to handle a special case. It might happen that the first instruction is 
an error, which won't have a TSC, and the second instruction is an actual 
instruction, and from that point on you'll always have TSCs. For this case, we 
can assume that the TSC of the error instruction at the beginning of the trace 
is the same as the first valid TSC.



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:112-113
+DecodedThread::CalculateTscRange(size_t insn_index) const {
+  if (m_instruction_timestamps.empty())
+return None;
+

delete these two lines. The rest of the code will work well without it



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:156-158
+DecodedThread::TscRange::TscRange(std::map::const_iterator 
it,
+  const DecodedThread *decoded_thread)
+: m_it(it), m_decoded_thread(decoded_thread) {





Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:104
   // When adding new members to this class, make sure to update
-  // IntelPTInstruction::GetNonErrorMemoryUsage() if needed.
+  // IntelPTInstruction::GetMemoryUsage() if needed.
   pt_insn m_pt_insn;

+1



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:140
+/// Get the smallest instruction index that has this TSC.
+size_t GetStart() const;
+/// Get the largest instruction index that has this TSC.

Let's be more verbose with the names to improve readability



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:142
+/// Get the largest instruction index that has this TSC.
+size_t GetEnd() const;
+





Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:148
+TscRange(std::map::const_iterator it,
+ const DecodedThread *decoded_thread);
+

let's receive a reference here and then convert it to pointer, so that we 
minimize the number of places with pointers. Also, if later we decide to use a 
shared_ptr instead of a pointer, we can do it inside of the constructor without 
changing this line 



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:150
+
+/// The current range
+std::map::const_iterator m_it;





Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:181-182
 
+  /// Construct the TSC range that covers the given instruction index.
+  /// This operation is O(logn) and should be used sparingly.
+  llvm::Optional CalculateTscRange(size_t insn_index) const;





Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:67-82
   switch (origin) {
   case TraceCursor::SeekType::Set:
 m_pos = fitPosToBounds(offset);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
 return m_pos;
   case TraceCursor::SeekType::End:
 m_pos = fitPosToBounds(offset + last_index);

we can simplify this so that we only invoke CalculateTscRange once



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h:46
+  /// Tsc range covering the current instruction.
+  llvm::Optional m_current_tsc;
 };

rename it to `m_tsc_range`. The word current is very redundant in this case



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:123-124
+  if (insn_len != 0)
+s.Printf("  Average memory usage per instruction: %zu bytes\n",
+ mem_used / insn_len);
   return;

Use doubles, as the average might not be a whole number




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122603

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


[Lldb-commits] [PATCH] D121967: [LLDB][NativePDB] Create inline function decls

2022-03-31 Thread Zequan Wu via Phabricator via lldb-commits
zequanwu added inline comments.



Comment at: lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp:7
+// RUN: %p/Inputs/inline_sites_live.lldbinit 2>&1 | FileCheck %s
+
+inline int bar(int bar_param) {

labath wrote:
> ```
> void __attribute__((optnone)) use(int) {}
> 
> void __attribute__((always_inline)) bar(int param) {
>   use(param); // BP_bar
> }
> 
> void __attribute__((always_inline)) foo(int param) {
>   int local = param+1;
>   bar(local);
>   use(param); // BP_foo
>   use(local);
> }
> 
> int main(int argc) {
>   foo(argc);
> }
> ```
> 
I moved the line `// BP_foo`  to the next line, because there are some 
inaccuracy in the line table with inlined info. I'll fix it later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121967

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


[Lldb-commits] [PATCH] D121967: [LLDB][NativePDB] Create inline function decls

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

Update live debugging test to build with no optimization.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121967

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.h
  lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
  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/Inputs/inline_sites_live.lldbinit
  lldb/test/Shell/SymbolFile/NativePDB/inline_sites.s
  lldb/test/Shell/SymbolFile/NativePDB/inline_sites.test
  lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp
  lldb/test/Shell/SymbolFile/NativePDB/local-variables.cpp

Index: lldb/test/Shell/SymbolFile/NativePDB/local-variables.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/local-variables.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/local-variables.cpp
@@ -157,6 +157,7 @@
 // CHECK-NEXT: | |-ParmVarDecl {{.*}} argc 'int'
 // CHECK-NEXT: | `-ParmVarDecl {{.*}} argv 'char **'
 // CHECK-NEXT: |-FunctionDecl {{.*}} __scrt_common_main_seh 'int ()' static 
+// CHECK-NEXT: |-FunctionDecl {{.*}} invoke_main 'int ()' inline
 // CHECK-NEXT: `-FunctionDecl {{.*}} Function 'int (int, char)'
 // CHECK-NEXT:   |-ParmVarDecl {{.*}} Param1 'int'
 // CHECK-NEXT:   `-ParmVarDecl {{.*}} Param2 'char'
Index: lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp
@@ -0,0 +1,34 @@
+// clang-format off
+// REQUIRES: system-windows
+
+// RUN: %build -o %t.exe -- %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
+// RUN: %p/Inputs/inline_sites_live.lldbinit 2>&1 | FileCheck %s
+
+void use(int) {}
+
+void __attribute__((always_inline)) bar(int param) {
+  use(param); // BP_bar
+}
+
+void __attribute__((always_inline)) foo(int param) {
+  int local = param+1;
+  bar(local);
+  use(param);
+  use(local); // BP_foo
+}
+
+int main(int argc, char** argv) {
+  foo(argc);
+}
+
+// CHECK:  * thread #1, stop reason = breakpoint 1
+// CHECK-NEXT:frame #0: {{.*}}`main [inlined] bar(param=2)
+// CHECK:  (lldb) p param
+// CHECK-NEXT: (int) $0 = 2
+// CHECK:  * thread #1, stop reason = breakpoint 2
+// CHECK-NEXT:frame #0: {{.*}}`main [inlined] foo(param=1)
+// CHECK:  (lldb) p param
+// CHECK-NEXT: (int) $1 = 1
+// CHECK-NEXT: (lldb) p local
+// CHECK-NEXT: (int) $2 = 2
Index: lldb/test/Shell/SymbolFile/NativePDB/inline_sites.test
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/inline_sites.test
@@ -0,0 +1,160 @@
+# clang-format off
+# REQUIRES: lld
+
+# RUN: llvm-mc -triple=x86_64-windows-msvc --filetype=obj %p/Inputs/inline_sites.s > %t.obj
+# RUN: lld-link -debug:full -nodefaultlib -entry:main -base:0x14000 %t.obj -out:%t.exe
+# RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
+# RUN: %p/Inputs/inline_sites.lldbinit 2>&1 | FileCheck %s
+
+# CHECK:  (lldb) image dump line-table a.cpp -v
+# CHECK-NEXT: Line table
+# CHECK-NEXT: 0x000140001000: /tmp/a.cpp:2
+# CHECK-NEXT: 0x000140001004: /tmp/a.h:5, is_start_of_statement = TRUE, is_prologue_end = TRUE
+# CHECK-NEXT: 0x00014000100c: /tmp/a.h:6
+# CHECK-NEXT: 0x000140001010: /tmp/a.h:7
+# CHECK-NEXT: 0x000140001018: /tmp/a.h:9
+# CHECK-NEXT: 0x00014000101c: /tmp/b.h:5, is_start_of_statement = TRUE, is_prologue_end = TRUE
+# CHECK-NEXT: 0x000140001022: /tmp/b.h:6
+# CHECK-NEXT: 0x000140001026: /tmp/b.h:7
+# CHECK-NEXT: 0x00014000102a: /tmp/c.h:5, is_start_of_statement = TRUE, is_prologue_end = TRUE
+# CHECK-NEXT: 0x000140001031: /tmp/c.h:6
+# 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: 0x000140001046: /tmp/a.cpp:4, is_terminal_entry = TRUE
+
+#CHECK: (lldb) b a.h:5
+#CHECK: Breakpoint 1: where = {{.*}}`main + 4 [inlined] Namespace1::foo at a.h:5, address = 0x000140001004
+#CHECK: (lldb) b a.h:6
+#CHECK: Breakpoint 2: where = {{.*}}`main + 12 [inlined] Namespace1::foo + 8 at a.h:6, address = 0x00014000100c
+#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: (lldb) b 

[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

2022-03-31 Thread Alisamar Husain via Phabricator via lldb-commits
zrthxn updated this revision to Diff 419519.
zrthxn added a comment.

Updated tests according to new memory usage calculation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122603

Files:
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  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
@@ -38,8 +38,8 @@
 thread #1: tid = 3842849
   Raw trace size: 4 KiB
   Total number of instructions: 21
-  Total approximate memory usage: 5.31 KiB
-  Average memory usage per instruction: 259 bytes'''])
+  Total approximate memory usage: 0.98 KiB
+  Average memory usage per instruction: 48 bytes'''])
 
 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
@@ -40,5 +40,5 @@
 thread #1: tid = 3842849
   Raw trace size: 4 KiB
   Total number of instructions: 21
-  Total approximate memory usage: 5.31 KiB
-  Average memory usage per instruction: 259 bytes'''])
+  Total approximate memory usage: 0.98 KiB
+  Average memory usage per instruction: 48 bytes'''])
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
@@ -119,8 +119,9 @@
   s.Printf("  Total number of instructions: %zu\n", insn_len);
   s.Printf("  Total approximate memory usage: %0.2lf KiB\n",
(double)mem_used / 1024);
-  s.Printf("  Average memory usage per instruction: %zu bytes\n",
-   mem_used / insn_len);
+  if (insn_len != 0)
+s.Printf("  Average memory usage per instruction: %zu bytes\n",
+ mem_used / insn_len);
   return;
 }
 
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
@@ -42,6 +42,8 @@
   DecodedThreadSP m_decoded_thread_sp;
   /// Internal instruction index currently pointing at.
   size_t m_pos;
+  /// Tsc range covering the current instruction.
+  llvm::Optional m_current_tsc;
 };
 
 } // namespace trace_intel_pt
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -23,6 +23,7 @@
   assert(!m_decoded_thread_sp->GetInstructions().empty() &&
  "a trace should have at least one instruction or error");
   m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+  m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
 }
 
 size_t TraceCursorIntelPT::GetInternalInstructionSize() {
@@ -40,6 +41,11 @@
 
   while (canMoveOne()) {
 m_pos += IsForwards() ? 1 : -1;
+
+if (m_current_tsc && !m_current_tsc->InRange(m_pos))
+  m_current_tsc =
+  IsForwards() ? m_current_tsc->Next() : m_current_tsc->Prev();
+
 if (!m_ignore_errors && IsError())
   return true;
 if (GetInstructionControlFlowType() & m_granularity)
@@ -61,20 +67,23 @@
   switch (origin) {
   case TraceCursor::SeekType::Set:
 m_pos = fitPosToBounds(offset);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
 return m_pos;
   case TraceCursor::SeekType::End:
 m_pos = fitPosToBounds(offset + last_index);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
 return last_index - m_pos;
   case TraceCursor::SeekType::Current:
 int64_t new_pos = fitPosToBounds(offset + m_pos);
 int64_t dist = m_pos - new_pos;
 m_pos = new_pos;
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
 return std::abs(dist);
   }
 }
 
 bool TraceCursorIntelPT::IsError() {
-  return m_decoded_thread_sp->GetInstructions()[m_pos].IsError();
+  return m_decoded_thread_sp->IsInstructionAnError(m_pos);
 }
 
 const char *TraceCursorIntelPT::GetError() {
@@ -85,10 +94,14 @@
   return m_decoded_thread_sp->GetInstructions()[m_pos].GetLoadAddress();
 }
 
-Optional TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+Optional

[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

2022-03-31 Thread Alisamar Husain via Phabricator via lldb-commits
zrthxn updated this revision to Diff 419517.
zrthxn marked an inline comment as done.
zrthxn added a comment.

Fixed issue with TSC becoming invalid midway through trace


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122603

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

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
@@ -119,8 +119,9 @@
   s.Printf("  Total number of instructions: %zu\n", insn_len);
   s.Printf("  Total approximate memory usage: %0.2lf KiB\n",
(double)mem_used / 1024);
-  s.Printf("  Average memory usage per instruction: %zu bytes\n",
-   mem_used / insn_len);
+  if (insn_len != 0)
+s.Printf("  Average memory usage per instruction: %zu bytes\n",
+ mem_used / insn_len);
   return;
 }
 
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
@@ -42,6 +42,8 @@
   DecodedThreadSP m_decoded_thread_sp;
   /// Internal instruction index currently pointing at.
   size_t m_pos;
+  /// Tsc range covering the current instruction.
+  llvm::Optional m_current_tsc;
 };
 
 } // namespace trace_intel_pt
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -23,6 +23,7 @@
   assert(!m_decoded_thread_sp->GetInstructions().empty() &&
  "a trace should have at least one instruction or error");
   m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+  m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
 }
 
 size_t TraceCursorIntelPT::GetInternalInstructionSize() {
@@ -40,6 +41,11 @@
 
   while (canMoveOne()) {
 m_pos += IsForwards() ? 1 : -1;
+
+if (m_current_tsc && !m_current_tsc->InRange(m_pos))
+  m_current_tsc =
+  IsForwards() ? m_current_tsc->Next() : m_current_tsc->Prev();
+
 if (!m_ignore_errors && IsError())
   return true;
 if (GetInstructionControlFlowType() & m_granularity)
@@ -61,20 +67,23 @@
   switch (origin) {
   case TraceCursor::SeekType::Set:
 m_pos = fitPosToBounds(offset);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
 return m_pos;
   case TraceCursor::SeekType::End:
 m_pos = fitPosToBounds(offset + last_index);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
 return last_index - m_pos;
   case TraceCursor::SeekType::Current:
 int64_t new_pos = fitPosToBounds(offset + m_pos);
 int64_t dist = m_pos - new_pos;
 m_pos = new_pos;
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
 return std::abs(dist);
   }
 }
 
 bool TraceCursorIntelPT::IsError() {
-  return m_decoded_thread_sp->GetInstructions()[m_pos].IsError();
+  return m_decoded_thread_sp->IsInstructionAnError(m_pos);
 }
 
 const char *TraceCursorIntelPT::GetError() {
@@ -85,10 +94,14 @@
   return m_decoded_thread_sp->GetInstructions()[m_pos].GetLoadAddress();
 }
 
-Optional TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+Optional
+TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
   switch (counter_type) {
-case lldb::eTraceCounterTSC:
-  return m_decoded_thread_sp->GetInstructions()[m_pos].GetTimestampCounter();
+  case lldb::eTraceCounterTSC:
+if (m_current_tsc)
+  return m_current_tsc->GetTsc();
+else
+  return llvm::None;
   }
 }
 
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
@@ -62,9 +62,6 @@
 /// As mentioned, any gap is represented as an error in this class.
 class IntelPTInstruction {
 public:
-  IntelPTInstruction(const pt_insn _insn, uint64_t timestamp)
-  : m_pt_insn(pt_insn), m_timestamp(timestamp), m_is_error(false) {}
-
   IntelPTInstruction(const pt_insn _insn)
   : m_pt_insn(pt_insn), m_is_error(false) {}
 
@@ -85,13 +82,6 @@
   /// Get the size in bytes of an instance of this class
   static size_t GetMemoryUsage();
 
-  /// Get the timestamp associated with the current instruction. The timestamp
-  /// is similar to what a rdtsc instruction would return.
-  ///
-  /// \return
- 

[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

2022-03-31 Thread Alisamar Husain via Phabricator via lldb-commits
zrthxn updated this revision to Diff 419348.
zrthxn added a comment.

Change tsc check anyway


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122603

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

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
@@ -119,8 +119,9 @@
   s.Printf("  Total number of instructions: %zu\n", insn_len);
   s.Printf("  Total approximate memory usage: %0.2lf KiB\n",
(double)mem_used / 1024);
-  s.Printf("  Average memory usage per instruction: %zu bytes\n",
-   mem_used / insn_len);
+  if (insn_len != 0)
+s.Printf("  Average memory usage per instruction: %zu bytes\n",
+ mem_used / insn_len);
   return;
 }
 
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
@@ -42,6 +42,8 @@
   DecodedThreadSP m_decoded_thread_sp;
   /// Internal instruction index currently pointing at.
   size_t m_pos;
+  /// Tsc range covering the current instruction.
+  llvm::Optional m_current_tsc;
 };
 
 } // namespace trace_intel_pt
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -23,6 +23,7 @@
   assert(!m_decoded_thread_sp->GetInstructions().empty() &&
  "a trace should have at least one instruction or error");
   m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+  m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
 }
 
 size_t TraceCursorIntelPT::GetInternalInstructionSize() {
@@ -40,6 +41,13 @@
 
   while (canMoveOne()) {
 m_pos += IsForwards() ? 1 : -1;
+
+if (!m_current_tsc)
+  m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
+else if (!m_current_tsc->InRange(m_pos))
+  m_current_tsc =
+  IsForwards() ? m_current_tsc->Next() : m_current_tsc->Prev();
+
 if (!m_ignore_errors && IsError())
   return true;
 if (GetInstructionControlFlowType() & m_granularity)
@@ -61,20 +69,23 @@
   switch (origin) {
   case TraceCursor::SeekType::Set:
 m_pos = fitPosToBounds(offset);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
 return m_pos;
   case TraceCursor::SeekType::End:
 m_pos = fitPosToBounds(offset + last_index);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(last_index - m_pos);
 return last_index - m_pos;
   case TraceCursor::SeekType::Current:
 int64_t new_pos = fitPosToBounds(offset + m_pos);
 int64_t dist = m_pos - new_pos;
 m_pos = new_pos;
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
 return std::abs(dist);
   }
 }
 
 bool TraceCursorIntelPT::IsError() {
-  return m_decoded_thread_sp->GetInstructions()[m_pos].IsError();
+  return m_decoded_thread_sp->IsInstructionAnError(m_pos);
 }
 
 const char *TraceCursorIntelPT::GetError() {
@@ -85,10 +96,14 @@
   return m_decoded_thread_sp->GetInstructions()[m_pos].GetLoadAddress();
 }
 
-Optional TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+Optional
+TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
   switch (counter_type) {
-case lldb::eTraceCounterTSC:
-  return m_decoded_thread_sp->GetInstructions()[m_pos].GetTimestampCounter();
+  case lldb::eTraceCounterTSC:
+if (m_current_tsc)
+  return m_current_tsc->GetTsc();
+else
+  return llvm::None;
   }
 }
 
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
@@ -62,9 +62,6 @@
 /// As mentioned, any gap is represented as an error in this class.
 class IntelPTInstruction {
 public:
-  IntelPTInstruction(const pt_insn _insn, uint64_t timestamp)
-  : m_pt_insn(pt_insn), m_timestamp(timestamp), m_is_error(false) {}
-
   IntelPTInstruction(const pt_insn _insn)
   : m_pt_insn(pt_insn), m_is_error(false) {}
 
@@ -85,13 +82,6 @@
   /// Get the size in bytes of an instance of this class
   static size_t GetMemoryUsage();
 
-  /// Get the timestamp associated with the current instruction. The timestamp
-  /// is similar to what a rdtsc instruction would return.
-  

[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

2022-03-31 Thread Alisamar Husain via Phabricator via lldb-commits
zrthxn added inline comments.



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:45-52
+if (!m_current_tsc)
+  m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
+else if (!m_current_tsc->InRange(m_pos)) {
+  if (m_pos > m_current_tsc->GetEnd())
+m_current_tsc = m_current_tsc->Next();
+  if (m_pos < m_current_tsc->GetStart())
+m_current_tsc = m_current_tsc->Prev();

wallace wrote:
> No need to do `m_current_tsc = 
> m_decoded_thread_sp->CalculateTscRange(m_pos);` because its value has already 
> been calculated in the constructor. We can simplify this as well
It is possible that when TraceCursorIntelPT is created the m_current_tsc is 
None, for example when just started the trace and tried to dump instructions... 
But then if a tsc is emitted later, this would cause it to remain None since we 
don't re-calculate it if it was initially None



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:102-103
 
-Optional TraceCursorIntelPT::GetCounter(lldb::TraceCounter 
counter_type) {
+Optional
+TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+  if (!m_current_tsc)

wallace wrote:
> are you using git clang-format? I'm curious why this line changed
Yes  I am. I think its because its longer than 80 chars.



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:104-110
+  if (!m_current_tsc)
+return None;
+
   switch (counter_type) {
-case lldb::eTraceCounterTSC:
-  return 
m_decoded_thread_sp->GetInstructions()[m_pos].GetTimestampCounter();
+  case lldb::eTraceCounterTSC:
+return m_current_tsc->GetTsc();
   }

wallace wrote:
> 
m_current_tsc is already checked at the beginning of this function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122603

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


[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction

2022-03-31 Thread Alisamar Husain via Phabricator via lldb-commits
zrthxn updated this revision to Diff 419347.
zrthxn marked 23 inline comments as done.
zrthxn added a comment.

Included requested changes, removed extra members


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122603

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

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
@@ -119,8 +119,9 @@
   s.Printf("  Total number of instructions: %zu\n", insn_len);
   s.Printf("  Total approximate memory usage: %0.2lf KiB\n",
(double)mem_used / 1024);
-  s.Printf("  Average memory usage per instruction: %zu bytes\n",
-   mem_used / insn_len);
+  if (insn_len != 0)
+s.Printf("  Average memory usage per instruction: %zu bytes\n",
+ mem_used / insn_len);
   return;
 }
 
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
@@ -42,6 +42,8 @@
   DecodedThreadSP m_decoded_thread_sp;
   /// Internal instruction index currently pointing at.
   size_t m_pos;
+  /// Tsc range covering the current instruction.
+  llvm::Optional m_current_tsc;
 };
 
 } // namespace trace_intel_pt
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -23,6 +23,7 @@
   assert(!m_decoded_thread_sp->GetInstructions().empty() &&
  "a trace should have at least one instruction or error");
   m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+  m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
 }
 
 size_t TraceCursorIntelPT::GetInternalInstructionSize() {
@@ -40,6 +41,13 @@
 
   while (canMoveOne()) {
 m_pos += IsForwards() ? 1 : -1;
+
+if (!m_current_tsc)
+  m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
+else if (!m_current_tsc->InRange(m_pos))
+  m_current_tsc =
+  IsForwards() ? m_current_tsc->Next() : m_current_tsc->Prev();
+
 if (!m_ignore_errors && IsError())
   return true;
 if (GetInstructionControlFlowType() & m_granularity)
@@ -61,20 +69,23 @@
   switch (origin) {
   case TraceCursor::SeekType::Set:
 m_pos = fitPosToBounds(offset);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
 return m_pos;
   case TraceCursor::SeekType::End:
 m_pos = fitPosToBounds(offset + last_index);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(last_index - m_pos);
 return last_index - m_pos;
   case TraceCursor::SeekType::Current:
 int64_t new_pos = fitPosToBounds(offset + m_pos);
 int64_t dist = m_pos - new_pos;
 m_pos = new_pos;
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
 return std::abs(dist);
   }
 }
 
 bool TraceCursorIntelPT::IsError() {
-  return m_decoded_thread_sp->GetInstructions()[m_pos].IsError();
+  return m_decoded_thread_sp->IsInstructionAnError(m_pos);
 }
 
 const char *TraceCursorIntelPT::GetError() {
@@ -85,10 +96,14 @@
   return m_decoded_thread_sp->GetInstructions()[m_pos].GetLoadAddress();
 }
 
-Optional TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+Optional
+TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+  if (!m_current_tsc)
+return None;
+
   switch (counter_type) {
-case lldb::eTraceCounterTSC:
-  return m_decoded_thread_sp->GetInstructions()[m_pos].GetTimestampCounter();
+  case lldb::eTraceCounterTSC:
+return m_current_tsc->GetTsc();
   }
 }
 
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
@@ -62,9 +62,6 @@
 /// As mentioned, any gap is represented as an error in this class.
 class IntelPTInstruction {
 public:
-  IntelPTInstruction(const pt_insn _insn, uint64_t timestamp)
-  : m_pt_insn(pt_insn), m_timestamp(timestamp), m_is_error(false) {}
-
   IntelPTInstruction(const pt_insn _insn)
   : m_pt_insn(pt_insn), m_is_error(false) {}
 
@@ -85,13 +82,6 @@
   /// Get the size in bytes of an instance of this class
   static size_t GetMemoryUsage();
 
-  /// Get the timestamp associated with the current instruction. The timestamp
-  /// is 

[Lldb-commits] [PATCH] D121967: [LLDB][NativePDB] Create inline function decls

2022-03-31 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In D121967#3403886 , @zequanwu wrote:

>> Can you say (in english) what are the properties that you are trying to 
>> check there? Maybe we can find a better way to do that...
>
> I'm trying to check for local variables values in inline functions by 
> printing them (see inline comment on `PdbAstBuilder::ParseBlockChildren`). 
> Since this test only runs on x64, I think I will change the live debugging 
> test to a cpp file.

Sorry about the delay. This dropped off my radar.

If the goal is checking for local variables, then I'd say the test is way too 
strict for that. In particular, moving through code (even unoptimized code) 
with step instructions is very sensitive to codegen changes. And the hardcoded 
line numbers everywhere will make life hard for whoever needs to update this 
tests.

Please see inline comments on how I'd approach a local variable test. I use the 
`always_inline` attribute, which allows you to create inline functions in 
unoptimized builds. This greatly reduces our exposure to codegen cleverness. If 
building with -O0, then the optnone attribute is not necessary, but I've left 
it there to demonstrate how that can be used to keep a variable live in 
optimized builds. (It also provides a good anchor to place a breakpoint). I am 
also using breakpoints for moving through the code. Breakpoints are more 
reliable as they're usually not affected by instruction reordering (mainly 
important for optimized code, but a good practice in any case). I set the 
breakpoint's via markers left in the source code, which allows me to avoid 
using any line numbers. I am deliberately not matching the entire block that 
lldb prints when the process stops, as that is not relevant for this test.

I wasn't sure what was the exact setup you wanted to check (one problem with 
overly strict tests), so I've created something simple but still fairly similar 
to your setup. It should be fairly easy to tweak the test to do check other 
things as well.




Comment at: lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp:7
+// RUN: %p/Inputs/inline_sites_live.lldbinit 2>&1 | FileCheck %s
+
+inline int bar(int bar_param) {

```
void __attribute__((optnone)) use(int) {}

void __attribute__((always_inline)) bar(int param) {
  use(param); // BP_bar
}

void __attribute__((always_inline)) foo(int param) {
  int local = param+1;
  bar(local);
  use(param); // BP_foo
  use(local);
}

int main(int argc) {
  foo(argc);
}
```




Comment at: lldb/test/Shell/SymbolFile/NativePDB/inline_sites_live.cpp:24
+
+// CHECK:  (lldb) b main
+// CHECK-NEXT: Breakpoint 1: where = {{.*}}`main + 4 [inlined] foo at {{.*}}:14

```
br set -p BP_bar
br set -p BP_foo
run
(CHECK: stop reason = breakpoint 1)
(CHECK: frame #0: {{.*}}`main [inlined] bar)
p param
continue
(CHECK: stop reason = breakpoint 2)
(CHECK: frame #0: {{.*}}`main [inlined] foo)
p param
p local
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121967

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