wallace updated this revision to Diff 342770.
wallace marked an inline comment as done.
wallace added a comment.
address comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100740/new/
https://reviews.llvm.org/D100740
Files:
lldb/include/lldb/Core/AddressRange.h
lldb/include/lldb/Target/Trace.h
lldb/source/Core/AddressRange.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
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
@@ -13,6 +13,7 @@
#include "lldb/Core/Module.h"
#include "lldb/Core/PluginManager.h"
#include "lldb/Symbol/Function.h"
+#include "lldb/Target/ExecutionContext.h"
#include "lldb/Target/Process.h"
#include "lldb/Target/SectionLoadList.h"
#include "lldb/Target/Thread.h"
@@ -101,189 +102,256 @@
return num == 0 ? 1 : static_cast<int>(log10(num)) + 1;
}
-/// Dump the symbol context of the given instruction address if it's different
-/// from the symbol context of the previous instruction in the trace.
-///
-/// \param[in] prev_sc
-/// The symbol context of the previous instruction in the trace.
-///
-/// \param[in] address
-/// The address whose symbol information will be dumped.
-///
/// \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;
+/// \b true if the provided line entries match line, column and source file.
+/// This function assumes that the line entries are valid.
+static bool FileLineAndColumnMatches(const LineEntry &a, const LineEntry &b) {
+ if (a.line != b.line)
+ return false;
+ if (a.column != b.column)
+ return false;
+
+ return FileSpec::Compare(a.file, b.file, true) == 0;
+}
+
+// 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.
+static bool IsLineEntryValid(const LineEntry &line_entry) {
+ return line_entry.IsValid() && line_entry.line > 0;
+}
+/// Helper structure for \a TraverseInstructionsWithSymbolInfo.
+struct InstructionSymbolInfo {
SymbolContext sc;
- address.CalculateSymbolContext(&sc, eSymbolContextEverything);
+ Address address;
+ lldb::addr_t load_address;
+ lldb::DisassemblerSP disassembler;
+ lldb::InstructionSP instruction;
+ lldb_private::ExecutionContext exe_ctx;
+};
- 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)
+/// 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.
+static void TraverseInstructionsWithSymbolInfo(
+ Trace &trace, Thread &thread, size_t position,
+ Trace::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.Contains(address))
+ return prev_insn.sc;
+
+ SymbolContext sc;
+ address.CalculateSymbolContext(&sc, symbol_scope);
return sc;
+ };
- s.Printf(" ");
+ // 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());
+ 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());
+ };
- if (!sc.module_sp)
- s.Printf("(none)");
- else if (!sc.function && !sc.symbol)
- s.Printf("%s`(none)",
- 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,
- /*show_function_name*/ true);
- s.Printf("\n");
- return sc;
+ trace.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;
+ insn.address.SetLoadAddress(*load_address, &target);
+ 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);
+ });
}
-/// 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.
+/// Compare the symbol contexts of the provided \a InstructionSymbolInfo
+/// objects.
///
/// \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)
+/// \a true if both instructions belong to the same scope level analized
+/// in the following order:
+/// - module
+/// - symbol
+/// - function
+/// - line
+static bool
+IsSameInstructionSymbolContext(const InstructionSymbolInfo &prev_insn,
+ const InstructionSymbolInfo &insn) {
+ // module checks
+ if (insn.sc.module_sp != prev_insn.sc.module_sp)
+ return false;
+
+ // symbol checks
+ if (insn.sc.symbol != prev_insn.sc.symbol)
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);
+ // function checks
+ if (!insn.sc.function && !prev_insn.sc.function)
return true;
- }
+ else if (insn.sc.function != prev_insn.sc.function)
+ return false;
- return false;
+ // line entry checks
+ const bool curr_line_valid = IsLineEntryValid(insn.sc.line_entry);
+ const bool prev_line_valid = IsLineEntryValid(prev_insn.sc.line_entry);
+ if (curr_line_valid && prev_line_valid)
+ return FileLineAndColumnMatches(insn.sc.line_entry,
+ prev_insn.sc.line_entry);
+ return curr_line_valid == prev_line_valid;
}
-/// Dump an instruction instruction given by its address.
+/// Dump the symbol context of the given instruction address if it's different
+/// from the symbol context of the previous instruction in the trace.
///
-/// \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] prev_sc
+/// The symbol context of the previous instruction in the trace.
///
/// \param[in] address
-/// The address of the instruction to dump.
+/// The address whose symbol information will be dumped.
///
/// \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;
- }
+/// The symbol context of the current address, which might differ from the
+/// previous one.
+static void
+DumpInstructionSymbolContext(Stream &s,
+ Optional<InstructionSymbolInfo> prev_insn,
+ InstructionSymbolInfo &insn) {
+ if (prev_insn && IsSameInstructionSymbolContext(*prev_insn, insn))
+ return;
- // 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;
+ s.Printf(" ");
+
+ if (!insn.sc.module_sp)
+ s.Printf("(none)");
+ else if (!insn.sc.function && !insn.sc.symbol)
+ s.Printf("%s`(none)",
+ insn.sc.module_sp->GetFileSpec().GetFilename().AsCString());
+ else
+ 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");
+}
+
+static void DumpInstructionDisassembly(Stream &s, 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();
+ Optional<InstructionSymbolInfo> prev_insn;
- SymbolContext sc;
- DisassemblerSP disassembler;
- ExecutionContext exe_ctx;
- target.CalculateExecutionContext(exe_ctx);
+ TraverseInstructionsWithSymbolInfo(
+ *this, thread, start_position, TraceDirection::Forwards,
+ eSymbolContextEverything, /*disassembler*/ true,
+ [&](size_t index, Expected<InstructionSymbolInfo> insn) -> bool {
+ if (!insn) {
+ printInstructionIndex(index);
+ s << toString(insn.takeError());
- TraverseInstructions(
- 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
+ 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;
});
}
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/source/Core/AddressRange.cpp
===================================================================
--- lldb/source/Core/AddressRange.cpp
+++ lldb/source/Core/AddressRange.cpp
@@ -8,6 +8,7 @@
#include "lldb/Core/AddressRange.h"
#include "lldb/Core/Module.h"
+#include "lldb/Core/Section.h"
#include "lldb/Target/Target.h"
#include "lldb/Utility/ConstString.h"
#include "lldb/Utility/FileSpec.h"
@@ -42,14 +43,22 @@
AddressRange::~AddressRange() {}
-// bool
-// AddressRange::Contains (const Address &addr) const
-//{
-// const addr_t byte_size = GetByteSize();
-// if (byte_size)
-// return addr.GetSection() == m_base_addr.GetSection() &&
-// (addr.GetOffset() - m_base_addr.GetOffset()) < byte_size;
-//}
+bool AddressRange::Contains(const Address &addr) const {
+ SectionSP range_sect_sp = GetBaseAddress().GetSection();
+ SectionSP addr_sect_sp = addr.GetSection();
+ if (range_sect_sp) {
+ if (!addr_sect_sp ||
+ range_sect_sp->GetModule() != addr_sect_sp->GetModule())
+ return false; // Modules do not match.
+ } else if (addr_sect_sp) {
+ return false; // Range has no module but "addr" does because addr has a
+ // section
+ }
+ // Either the modules match, or both have no module, so it is ok to compare
+ // the file addresses in this case only.
+ return ContainsFileAddress(addr);
+}
+
//
// bool
// AddressRange::Contains (const Address *addr) const
Index: lldb/include/lldb/Target/Trace.h
===================================================================
--- lldb/include/lldb/Target/Trace.h
+++ lldb/include/lldb/Target/Trace.h
@@ -210,8 +210,9 @@
/// 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/Core/AddressRange.h
===================================================================
--- lldb/include/lldb/Core/AddressRange.h
+++ lldb/include/lldb/Core/AddressRange.h
@@ -94,8 +94,7 @@
/// \return
/// Returns \b true if \a so_addr is contained in this range,
/// \b false otherwise.
- // bool
- // Contains (const Address &so_addr) const;
+ bool Contains(const Address &so_addr) const;
/// Check if a section offset address is contained in this range.
///
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
