diff --git a/lldb/include/lldb/Core/AddressRange.h b/lldb/include/lldb/Core/AddressRange.h --- a/lldb/include/lldb/Core/AddressRange.h +++ b/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. /// diff --git a/lldb/include/lldb/Target/Trace.h b/lldb/include/lldb/Target/Trace.h --- a/lldb/include/lldb/Target/Trace.h +++ b/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 GetInstructionCount(Thread &thread) = 0; /// Check if a thread is currently traced by this object. /// diff --git a/lldb/source/Core/AddressRange.cpp b/lldb/source/Core/AddressRange.cpp --- a/lldb/source/Core/AddressRange.cpp +++ b/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 diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h --- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h +++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h @@ -70,7 +70,7 @@ std::function load_addr)> callback) override; - size_t GetInstructionCount(Thread &thread) override; + llvm::Optional GetInstructionCount(Thread &thread) override; size_t GetCursorPosition(Thread &thread) override; diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp --- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp +++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp @@ -122,11 +122,11 @@ break; } -size_t TraceIntelPT::GetInstructionCount(Thread &thread) { +Optional TraceIntelPT::GetInstructionCount(Thread &thread) { if (const DecodedThread *decoded_thread = Decode(thread)) return decoded_thread->GetInstructions().size(); else - return 0; + return None; } Expected TraceIntelPT::GetCPUInfoForLiveProcess() { diff --git a/lldb/source/Target/Trace.cpp b/lldb/source/Target/Trace.cpp --- a/lldb/source/Target/Trace.cpp +++ b/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(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 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 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 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 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 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 insn) -> bool { + if (!insn) { + printInstructionIndex(index); + s << toString(insn.takeError()); - TraverseInstructions( - thread, start_position, TraceDirection::Forwards, - [&](size_t index, Expected 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; }); } diff --git a/lldb/test/API/commands/trace/TestTraceStartStop.py b/lldb/test/API/commands/trace/TestTraceStartStop.py --- a/lldb/test/API/commands/trace/TestTraceStartStop.py +++ b/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'''])