wallace updated this revision to Diff 339044.
wallace retitled this revision from "[trace] Dedup different source lines when
dumping instructions + refactor" to "json".
wallace added a comment.
ready for review
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100740/new/
https://reviews.llvm.org/D100740
Files:
lldb/include/lldb/Symbol/LineEntry.h
lldb/include/lldb/Target/Trace.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
lldb/source/Symbol/LineEntry.cpp
lldb/source/Target/Trace.cpp
lldb/test/API/commands/trace/TestTraceStartStop.py
Index: lldb/test/API/commands/trace/TestTraceStartStop.py
===================================================================
--- lldb/test/API/commands/trace/TestTraceStartStop.py
+++ lldb/test/API/commands/trace/TestTraceStartStop.py
@@ -100,7 +100,6 @@
a.out`main \+ 11 at main.cpp:4
\[1\] {ADDRESS_REGEX} movl .*
\[2\] {ADDRESS_REGEX} jmp .* ; <\+28> at main.cpp:4
- a.out`main \+ 28 at main.cpp:4
\[3\] {ADDRESS_REGEX} cmpl .*
\[4\] {ADDRESS_REGEX} jle .* ; <\+20> at main.cpp:5'''])
Index: lldb/source/Target/Trace.cpp
===================================================================
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -101,6 +101,53 @@
return num == 0 ? 1 : static_cast<int>(log10(num)) + 1;
}
+/// Compare the symbol contexts of the provided \a InstructionSymbolInfo objects.
+///
+/// \return
+/// \a true if both instructions belong to the same scope level analized
+/// in the following order:
+/// - line
+/// - function
+/// - symbol
+/// - module
+static bool
+IsSameInstructionSymbolContext(Optional<Trace::InstructionSymbolInfo> prev_insn,
+ Trace::InstructionSymbolInfo &insn) {
+ if (!prev_insn)
+ return false;
+
+ // This custom LineEntry validator is neded because some line_entries have
+ // 0 as line, which is meaningless. Notice that LineEntry::IsValid only
+ // checks that line is not LLDB_INVALID_LINE_NUMBER, i.e. UINT32_MAX.
+ auto is_line_entry_valid = [](const LineEntry &line_entry) {
+ return line_entry.IsValid() && line_entry.line > 0;
+ };
+
+ // line entry checks
+ if (is_line_entry_valid(insn.sc.line_entry) ^
+ is_line_entry_valid(prev_insn->sc.line_entry))
+ return false;
+ if (is_line_entry_valid(insn.sc.line_entry) &&
+ is_line_entry_valid(prev_insn->sc.line_entry))
+ return LineEntry::Compare(insn.sc.line_entry, prev_insn->sc.line_entry,
+ /*compare_addresses*/ false) == 0;
+
+ // function checks
+ if ((insn.sc.function != nullptr) ^ (prev_insn->sc.function != nullptr))
+ return false;
+ if (insn.sc.function != nullptr && prev_insn->sc.function != nullptr)
+ return insn.sc.function == prev_insn->sc.function;
+
+ // symbol checks
+ if ((insn.sc.symbol != nullptr) ^ (prev_insn->sc.symbol != nullptr))
+ return false;
+ if (insn.sc.symbol != nullptr && prev_insn->sc.symbol != nullptr)
+ return insn.sc.symbol == prev_insn->sc.symbol;
+
+ // module checks
+ return insn.sc.module_sp == prev_insn->sc.module_sp;
+}
+
/// Dump the symbol context of the given instruction address if it's different
/// from the symbol context of the previous instruction in the trace.
///
@@ -113,181 +160,174 @@
/// \return
/// The symbol context of the current address, which might differ from the
/// previous one.
-static SymbolContext DumpSymbolContext(Stream &s, const SymbolContext &prev_sc,
- Target &target, const Address &address) {
- AddressRange range;
- if (prev_sc.GetAddressRange(eSymbolContextEverything, 0,
- /*inline_block_range*/ false, range) &&
- range.ContainsFileAddress(address))
- return prev_sc;
-
- SymbolContext sc;
- address.CalculateSymbolContext(&sc, eSymbolContextEverything);
-
- if (!prev_sc.module_sp && !sc.module_sp)
- return sc;
- if (prev_sc.module_sp == sc.module_sp && !sc.function && !sc.symbol &&
- !prev_sc.function && !prev_sc.symbol)
- return sc;
+static void
+DumpInstructionSymbolContext(Stream &s,
+ Optional<Trace::InstructionSymbolInfo> prev_insn,
+ Trace::InstructionSymbolInfo &insn) {
+ if (IsSameInstructionSymbolContext(prev_insn, insn))
+ return;
s.Printf(" ");
- if (!sc.module_sp)
+ if (!insn.sc.module_sp)
s.Printf("(none)");
- else if (!sc.function && !sc.symbol)
+ else if (!insn.sc.function && !insn.sc.symbol)
s.Printf("%s`(none)",
- sc.module_sp->GetFileSpec().GetFilename().AsCString());
+ insn.sc.module_sp->GetFileSpec().GetFilename().AsCString());
else
- sc.DumpStopContext(&s, &target, address, /*show_fullpath*/ false,
- /*show_module*/ true, /*show_inlined_frames*/ false,
- /*show_function_arguments*/ true,
+ insn.sc.DumpStopContext(&s, insn.exe_ctx.GetTargetPtr(), insn.address,
+ /*show_fullpath*/ false,
+ /*show_module*/ true, /*show_inlined_frames*/ false,
+ /*show_function_arguments*/ true,
/*show_function_name*/ true);
s.Printf("\n");
- return sc;
}
-/// Dump an instruction given by its address using a given disassembler, unless
-/// the instruction is not present in the disassembler.
-///
-/// \param[in] disassembler
-/// A disassembler containing a certain instruction list.
-///
-/// \param[in] address
-/// The address of the instruction to dump.
-///
-/// \return
-/// \b true if the information could be dumped, \b false otherwise.
-static bool TryDumpInstructionInfo(Stream &s,
- const DisassemblerSP &disassembler,
- const ExecutionContext &exe_ctx,
- const Address &address) {
- if (!disassembler)
- return false;
-
- if (InstructionSP instruction =
- disassembler->GetInstructionList().GetInstructionAtAddress(address)) {
- instruction->Dump(&s, /*show_address*/ false, /*show_bytes*/ false,
- /*max_opcode_byte_size*/ 0, &exe_ctx,
- /*sym_ctx*/ nullptr, /*prev_sym_ctx*/ nullptr,
- /*disassembly_addr_format*/ nullptr,
- /*max_address_text_size*/ 0);
- return true;
- }
-
- return false;
-}
-
-/// Dump an instruction instruction given by its address.
-///
-/// \param[in] prev_disassembler
-/// The disassembler that was used to dump the previous instruction in the
-/// trace. It is useful to avoid recomputations.
-///
-/// \param[in] address
-/// The address of the instruction to dump.
-///
-/// \return
-/// A disassembler that contains the given instruction, which might differ
-/// from the previous disassembler.
-static DisassemblerSP
-DumpInstructionInfo(Stream &s, const SymbolContext &sc,
- const DisassemblerSP &prev_disassembler,
- ExecutionContext &exe_ctx, const Address &address) {
- // We first try to use the previous disassembler
- if (TryDumpInstructionInfo(s, prev_disassembler, exe_ctx, address))
- return prev_disassembler;
-
- // Now we try using the current function's disassembler
- if (sc.function) {
- DisassemblerSP disassembler =
- sc.function->GetInstructions(exe_ctx, nullptr, true);
- if (TryDumpInstructionInfo(s, disassembler, exe_ctx, address))
- return disassembler;
- }
-
- // We fallback to disassembly one instruction
- Target &target = exe_ctx.GetTargetRef();
- const ArchSpec &arch = target.GetArchitecture();
- AddressRange range(address, arch.GetMaximumOpcodeByteSize() * 1);
- DisassemblerSP disassembler = Disassembler::DisassembleRange(
- arch, /*plugin_name*/ nullptr,
- /*flavor*/ nullptr, target, range, /*prefer_file_cache*/ true);
- if (TryDumpInstructionInfo(s, disassembler, exe_ctx, address))
- return disassembler;
- return nullptr;
+static void DumpInstructionDisassembly(Stream &s,
+ Trace::InstructionSymbolInfo &insn) {
+ if (!insn.instruction)
+ return;
+ insn.instruction->Dump(&s, /*show_address*/ false, /*show_bytes*/ false,
+ /*max_opcode_byte_size*/ 0, &insn.exe_ctx, &insn.sc,
+ /*prev_sym_ctx*/ nullptr,
+ /*disassembly_addr_format*/ nullptr,
+ /*max_address_text_size*/ 0);
}
void Trace::DumpTraceInstructions(Thread &thread, Stream &s, size_t count,
size_t end_position, bool raw) {
- if (!IsTraced(thread)) {
+ Optional<size_t> instructions_count = GetInstructionCount(thread);
+ if (!instructions_count) {
s.Printf("thread #%u: tid = %" PRIu64 ", not traced\n", thread.GetIndexID(),
thread.GetID());
return;
}
- size_t instructions_count = GetInstructionCount(thread);
s.Printf("thread #%u: tid = %" PRIu64 ", total instructions = %zu\n",
- thread.GetIndexID(), thread.GetID(), instructions_count);
+ thread.GetIndexID(), thread.GetID(), *instructions_count);
- if (count == 0 || end_position >= instructions_count)
+ if (count == 0 || end_position >= *instructions_count)
return;
+ int digits_count = GetNumberOfDigits(end_position);
size_t start_position =
end_position + 1 < count ? 0 : end_position + 1 - count;
-
- int digits_count = GetNumberOfDigits(end_position);
auto printInstructionIndex = [&](size_t index) {
s.Printf(" [%*zu] ", digits_count, index);
};
bool was_prev_instruction_an_error = false;
- Target &target = thread.GetProcess()->GetTarget();
-
- SymbolContext sc;
- DisassemblerSP disassembler;
- ExecutionContext exe_ctx;
- target.CalculateExecutionContext(exe_ctx);
+ Optional<InstructionSymbolInfo> prev_insn;
- TraverseInstructions(
+ TraverseInstructionsWithSymbolInfo(
thread, start_position, TraceDirection::Forwards,
- [&](size_t index, Expected<lldb::addr_t> load_address) -> bool {
- if (load_address) {
- // We print an empty line after a sequence of errors to show more
- // clearly that there's a gap in the trace
+ eSymbolContextEverything, /*disassembler*/ true,
+ [&](size_t index, Expected<InstructionSymbolInfo> insn) -> bool {
+ if (!insn) {
+ printInstructionIndex(index);
+ s << toString(insn.takeError());
+
+ prev_insn = None;
+ was_prev_instruction_an_error = true;
+ } else {
if (was_prev_instruction_an_error)
s.Printf(" ...missing instructions\n");
- Address address;
- if (!raw) {
- target.GetSectionLoadList().ResolveLoadAddress(*load_address,
- address);
-
- sc = DumpSymbolContext(s, sc, target, address);
- }
+ if (!raw)
+ DumpInstructionSymbolContext(s, prev_insn, *insn);
printInstructionIndex(index);
- s.Printf("0x%016" PRIx64 " ", *load_address);
+ s.Printf("0x%016" PRIx64 " ", insn->load_address);
- if (!raw) {
- disassembler =
- DumpInstructionInfo(s, sc, disassembler, exe_ctx, address);
- }
+ if (!raw)
+ DumpInstructionDisassembly(s, *insn);
+ prev_insn = *insn;
was_prev_instruction_an_error = false;
- } else {
- printInstructionIndex(index);
- s << toString(load_address.takeError());
- was_prev_instruction_an_error = true;
- if (!raw)
- sc = SymbolContext();
}
s.Printf("\n");
-
return index < end_position;
});
}
+void Trace::TraverseInstructionsWithSymbolInfo(
+ Thread &thread, size_t position, TraceDirection direction,
+ SymbolContextItem symbol_scope, bool include_disassembler,
+ std::function<bool(size_t index, Expected<InstructionSymbolInfo> insn)>
+ callback) {
+ InstructionSymbolInfo prev_insn;
+
+ Target &target = thread.GetProcess()->GetTarget();
+ ExecutionContext exe_ctx;
+ target.CalculateExecutionContext(exe_ctx);
+ const ArchSpec &arch = target.GetArchitecture();
+
+ // Find the symbol context for the given address reusing the previous
+ // instruction's symbol context when possible.
+ auto calculate_symbol_context = [&](const Address &address) {
+ AddressRange range;
+ if (prev_insn.sc.GetAddressRange(symbol_scope, 0,
+ /*inline_block_range*/ false, range) &&
+ range.ContainsFileAddress(address))
+ return prev_insn.sc;
+
+ SymbolContext sc;
+ address.CalculateSymbolContext(&sc, symbol_scope);
+ return sc;
+ };
+
+ // Find the disassembler for the given address reusing the previous
+ // instruction's disassembler when possible.
+ auto calculate_disass = [&](const Address &address, const SymbolContext &sc) {
+ if (prev_insn.disassembler) {
+ if (InstructionSP instruction =
+ prev_insn.disassembler->GetInstructionList()
+ .GetInstructionAtAddress(address))
+ return std::make_tuple(prev_insn.disassembler, instruction);
+ }
+
+ if (sc.function) {
+ if (DisassemblerSP disassembler =
+ sc.function->GetInstructions(exe_ctx, nullptr, true)) {
+ if (InstructionSP instruction =
+ disassembler->GetInstructionList().GetInstructionAtAddress(
+ address))
+ return std::make_tuple(disassembler, instruction);
+ }
+ }
+ // We fallback to a single instruction disassembler
+ AddressRange range(address, arch.GetMaximumOpcodeByteSize() * 1);
+ DisassemblerSP disassembler = Disassembler::DisassembleRange(
+ arch, /*plugin_name*/ nullptr,
+ /*flavor*/ nullptr, target, range, /*prefer_file_cache*/ true);
+ return std::make_tuple(disassembler,
+ disassembler ? disassembler->GetInstructionList()
+ .GetInstructionAtAddress(address)
+ : InstructionSP());
+ };
+
+ TraverseInstructions(
+ thread, position, direction,
+ [&](size_t index, Expected<lldb::addr_t> load_address) -> bool {
+ if (!load_address)
+ return callback(index, load_address.takeError());
+
+ InstructionSymbolInfo insn;
+ insn.load_address = *load_address;
+ insn.exe_ctx = exe_ctx;
+ target.GetSectionLoadList().ResolveLoadAddress(*load_address,
+ insn.address);
+ if (symbol_scope != 0)
+ insn.sc = calculate_symbol_context(insn.address);
+ if (include_disassembler)
+ std::tie(insn.disassembler, insn.instruction) =
+ calculate_disass(insn.address, insn.sc);
+ prev_insn = insn;
+ return callback(index, insn);
+ });
+}
+
Error Trace::Start(const llvm::json::Value &request) {
if (!m_live_process)
return createStringError(inconvertibleErrorCode(),
Index: lldb/source/Symbol/LineEntry.cpp
===================================================================
--- lldb/source/Symbol/LineEntry.cpp
+++ lldb/source/Symbol/LineEntry.cpp
@@ -155,27 +155,30 @@
return LineEntry::Compare(a, b) < 0;
}
-int LineEntry::Compare(const LineEntry &a, const LineEntry &b) {
- int result = Address::CompareFileAddress(a.range.GetBaseAddress(),
- b.range.GetBaseAddress());
- if (result != 0)
- return result;
-
- const lldb::addr_t a_byte_size = a.range.GetByteSize();
- const lldb::addr_t b_byte_size = b.range.GetByteSize();
-
- if (a_byte_size < b_byte_size)
- return -1;
- if (a_byte_size > b_byte_size)
- return +1;
-
- // Check for an end sequence entry mismatch after we have determined that the
- // address values are equal. If one of the items is an end sequence, we don't
- // care about the line, file, or column info.
- if (a.is_terminal_entry > b.is_terminal_entry)
- return -1;
- if (a.is_terminal_entry < b.is_terminal_entry)
- return +1;
+int LineEntry::Compare(const LineEntry &a, const LineEntry &b,
+ bool compare_address_ranges) {
+ if (compare_address_ranges) {
+ int result = Address::CompareFileAddress(a.range.GetBaseAddress(),
+ b.range.GetBaseAddress());
+ if (result != 0)
+ return result;
+
+ const lldb::addr_t a_byte_size = a.range.GetByteSize();
+ const lldb::addr_t b_byte_size = b.range.GetByteSize();
+
+ if (a_byte_size < b_byte_size)
+ return -1;
+ if (a_byte_size > b_byte_size)
+ return +1;
+
+ // Check for an end sequence entry mismatch after we have determined that
+ // the address values are equal. If one of the items is an end sequence, we
+ // don't care about the line, file, or column info.
+ if (a.is_terminal_entry > b.is_terminal_entry)
+ return -1;
+ if (a.is_terminal_entry < b.is_terminal_entry)
+ return +1;
+ }
if (a.line < b.line)
return -1;
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
@@ -70,7 +70,7 @@
std::function<bool(size_t index, llvm::Expected<lldb::addr_t> load_addr)>
callback) override;
- size_t GetInstructionCount(Thread &thread) override;
+ llvm::Optional<size_t> GetInstructionCount(Thread &thread) override;
size_t GetCursorPosition(Thread &thread) override;
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -122,11 +122,11 @@
break;
}
-size_t TraceIntelPT::GetInstructionCount(Thread &thread) {
+Optional<size_t> TraceIntelPT::GetInstructionCount(Thread &thread) {
if (const DecodedThread *decoded_thread = Decode(thread))
return decoded_thread->GetInstructions().size();
else
- return 0;
+ return None;
}
Expected<pt_cpu> TraceIntelPT::GetCPUInfoForLiveProcess() {
Index: lldb/include/lldb/Target/Trace.h
===================================================================
--- lldb/include/lldb/Target/Trace.h
+++ lldb/include/lldb/Target/Trace.h
@@ -14,6 +14,7 @@
#include "llvm/Support/JSON.h"
#include "lldb/Core/PluginInterface.h"
+#include "lldb/Target/ExecutionContext.h"
#include "lldb/Target/Thread.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/TraceGDBRemotePackets.h"
@@ -204,14 +205,43 @@
std::function<bool(size_t index, llvm::Expected<lldb::addr_t> load_addr)>
callback) = 0;
+ /// Helper structure for \a TraverseInstructionsWithSymbolInfo.
+ struct InstructionSymbolInfo {
+ SymbolContext sc;
+ Address address;
+ lldb::addr_t load_address;
+ lldb::DisassemblerSP disassembler;
+ lldb::InstructionSP instruction;
+ lldb_private::ExecutionContext exe_ctx;
+ };
+
+ /// Similar to \a TraverseInstructions byt the callback receives an \a
+ /// InstructionSymbolInfo object with symbol information for the given
+ /// instruction, calculated efficiently.
+ ///
+ /// \param[in] symbol_scope
+ /// If not \b 0, then the \a InstructionSymbolInfo will have its
+ /// SymbolContext calculated up to that level.
+ ///
+ /// \param[in] include_disassembler
+ /// If \b true, then the \a InstructionSymbolInfo will have the
+ /// \a disassembler and \a instruction objects calculated.
+ void TraverseInstructionsWithSymbolInfo(
+ Thread &thread, size_t position, TraceDirection direction,
+ lldb::SymbolContextItem symbol_scope, bool include_disassembler,
+ std::function<bool(size_t index,
+ llvm::Expected<InstructionSymbolInfo> insn)>
+ callback);
+
/// Get the number of available instructions in the trace of the given thread.
///
/// \param[in] thread
/// The thread whose trace will be inspected.
///
/// \return
- /// The total number of instructions in the trace.
- virtual size_t GetInstructionCount(Thread &thread) = 0;
+ /// The total number of instructions in the trace, or \a llvm::None if the
+ /// thread is not being traced.
+ virtual llvm::Optional<size_t> GetInstructionCount(Thread &thread) = 0;
/// Check if a thread is currently traced by this object.
///
Index: lldb/include/lldb/Symbol/LineEntry.h
===================================================================
--- lldb/include/lldb/Symbol/LineEntry.h
+++ lldb/include/lldb/Symbol/LineEntry.h
@@ -91,11 +91,16 @@
/// \param[in] rhs
/// The Right Hand Side const LineEntry object reference.
///
+ /// \param[in] compare_addresses
+ /// If \b true, then the addresses of the LineEntry objects will be
+ /// compared, otherwise only the line, column and file names must match.
+ ///
/// \return
/// -1 if lhs < rhs
/// 0 if lhs == rhs
/// 1 if lhs > rhs
- static int Compare(const LineEntry &lhs, const LineEntry &rhs);
+ static int Compare(const LineEntry &lhs, const LineEntry &rhs,
+ bool compare_addresses = true);
/// Give the range for this LineEntry + any additional LineEntries for this
/// same source line that are contiguous.
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
