diff --git a/lldb/include/lldb/Target/TraceCursor.h b/lldb/include/lldb/Target/TraceCursor.h --- a/lldb/include/lldb/Target/TraceCursor.h +++ b/lldb/include/lldb/Target/TraceCursor.h @@ -25,51 +25,66 @@ /// Live processes: /// In the case of a live process trace, an instance of a \a TraceCursor should /// point to the trace at the moment it was collected. If the process is later -/// resumed and new trace data is collected, that should leave that old cursor -/// unaffected. +/// resumed and new trace data is collected, then it's up to each trace plug-in +/// to decide whether to leave the old cursor unaffected or not. /// /// Errors in the trace: /// As there could be errors when reconstructing the instructions of a trace, /// these errors are represented as failed instructions, and the cursor can -/// point at them. The consumer should invoke \a TraceCursor::GetError() to -/// check if the cursor is pointing to either a valid instruction or an error. +/// point at them. The consumer should invoke \a TraceCursor::IsError() to +/// check if the cursor is pointing to either a valid instruction or an error, +/// and then \a TraceCursor::GetError() can return the actual error message. /// /// Instructions: /// A \a TraceCursor always points to a specific instruction or error in the /// trace. /// /// Defaults: -/// By default, the cursor points at the end item of the trace, moves -/// backwards, has a move granularity of \a -/// eTraceInstructionControlFlowTypeInstruction (i.e. visit every instruction) -/// and stops at every error (the "ignore errors" flag is \b false). See the -/// \a TraceCursor::Next() method for more documentation. +/// By default, the cursor points at the most recent item in the trace and is +/// set up to iterate backwards. See the \a TraceCursor::Next() method for +/// more documentation. /// /// Sample usage: /// /// TraceCursorUP cursor = trace.GetTrace(thread); /// -/// cursor->SetGranularity(eTraceInstructionControlFlowTypeCall | -/// eTraceInstructionControlFlowTypeReturn); +/// for (; cursor->HasValue(); cursor->Next()) { +/// if (cursor->IsError()) { +/// cout << "error found at: " << cursor->GetError() << endl; +/// continue; +/// } /// -/// do { -/// if (llvm::Error error = cursor->GetError()) -/// cout << "error found at: " << llvm::toString(error) << endl; -/// else if (cursor->GetInstructionControlFlowType() & -/// eTraceInstructionControlFlowTypeCall) -/// std::cout << "call found at " << cursor->GetLoadAddress() << -/// std::endl; -/// else if (cursor->GetInstructionControlFlowType() & -/// eTraceInstructionControlFlowTypeReturn) -/// std::cout << "return found at " << cursor->GetLoadAddress() << -/// std::endl; -/// } while(cursor->Next()); +/// switch (cursor->GetInstructionControlFlowType()) { +/// eTraceInstructionControlFlowTypeCall: +/// std::cout << "CALL found at " << cursor->GetLoadAddress() << +/// std::endl; break; +/// eTraceInstructionControlFlowTypeReturn: +/// std::cout << "RETURN found at " << cursor->GetLoadAddress() << +/// std::endl; break; +/// } +/// } /// -/// Low level traversal: -/// Unlike the \a TraceCursor::Next() API, which uses a given granularity and -/// direction to advance the cursor, the \a TraceCursor::Seek() method can be -/// used to reposition the cursor to an offset of the end, beginning, or -/// current position of the trace. +/// As the trace might be empty or the cursor might have reached the end of the +/// trace, you should always invoke \a HasValue() to make sure you don't access +/// invalid memory. +/// +/// Random accesses: +/// +/// The Trace Cursor offer random acesses in the trace via two APIs: +/// +/// TraceCursor::Seek(): +/// Unlike the \a TraceCursor::Next() API, which moves instruction by +/// instruction, the \a TraceCursor::Seek() method can be used to +/// reposition the cursor to an offset of the end, beginning, or current +/// position of the trace. +/// +/// TraceCursor::GetId() / TraceCursor::SetId(id): +/// Each item (error or instruction) in the trace has a numeric identifier +/// which is defined by the trace plug-in. It's possible to access the id +/// of the current item using GetId(), and to reposition the cursor to a +/// given id using SetId(id). +/// +/// You can read more in the documentation of these methods. class TraceCursor { public: /// Helper enum to indicate the reference point when invoking @@ -90,12 +105,6 @@ virtual ~TraceCursor() = default; - /// Set the granularity to use in the \a TraceCursor::Next() method. - void SetGranularity(lldb::TraceInstructionControlFlowType granularity); - - /// Set the "ignore errors" flag to use in the \a TraceCursor::Next() method. - void SetIgnoreErrors(bool ignore_errors); - /// Set the direction to use in the \a TraceCursor::Next() method. /// /// \param[in] forwards @@ -109,8 +118,7 @@ /// \b true if the current direction is forwards, \b false if backwards. bool IsForwards() const; - /// Move the cursor to the next instruction that matches the current - /// granularity. + /// Move the cursor to the next item (instruction or error). /// /// Direction: /// The traversal is done following the current direction of the trace. If @@ -118,21 +126,12 @@ /// chronologically. Otherwise, the traversal is done in /// the opposite direction. By default, a cursor moves backwards unless /// changed with \a TraceCursor::SetForwards(). - /// - /// Granularity: - /// The cursor will traverse the trace looking for the first instruction - /// that matches the current granularity. If there aren't any matching - /// instructions, the cursor won't move, to give the opportunity of - /// changing granularities. - /// - /// Ignore errors: - /// If the "ignore errors" flags is \b false, the traversal will stop as - /// soon as it finds an error in the trace and the cursor will point at - /// it. - /// + virtual void Next() = 0; + /// \return - /// \b true if the cursor effectively moved, \b false otherwise. - virtual bool Next() = 0; + /// \b true if the cursor is pointing to a valid item. \b false if the + /// cursor has reached the end of the trace. + virtual bool HasValue() const = 0; /// Instruction identifiers: /// @@ -169,8 +168,8 @@ /// /// \return /// \b true if the given identifier exists and the cursor effectively - /// moved. Otherwise, \b false is returned and the cursor doesn't change - /// its position. + /// moved to it. Otherwise, \b false is returned and the cursor now points + /// to an invalid item, i.e. calling \a HasValue() will return \b false. virtual bool GoToId(lldb::user_id_t id) = 0; /// \return @@ -185,25 +184,26 @@ /// \} /// Make the cursor point to an item in the trace based on an origin point and - /// an offset. This API doesn't distinguishes instruction types nor errors in - /// the trace, unlike the \a TraceCursor::Next() method. + /// an offset. /// /// The resulting position of the trace is /// origin + offset /// - /// If this resulting position would be out of bounds, it will be adjusted to - /// the last or first item in the trace correspondingly. + /// If this resulting position would be out of bounds, the trace then points + /// to an invalid item, i.e. calling \a HasValue() returns \b false. /// /// \param[in] offset /// How many items to move forwards (if positive) or backwards (if - /// negative) from the given origin point. + /// negative) from the given origin point. For example, if origin is \b + /// End, then a negative offset would move backward in the trace, but a + /// positive offset would move past the trace to an invalid item. /// /// \param[in] origin /// The reference point to use when moving the cursor. /// /// \return - /// The number of trace items moved from the origin. - virtual uint64_t Seek(int64_t offset, SeekType origin) = 0; + /// \b true if and only if the cursor ends up pointing to a valid item. + virtual bool Seek(int64_t offset, SeekType origin) = 0; /// \return /// The \a ExecutionContextRef of the backing thread from the creation time @@ -260,10 +260,6 @@ protected: ExecutionContextRef m_exe_ctx_ref; - - lldb::TraceInstructionControlFlowType m_granularity = - lldb::eTraceInstructionControlFlowTypeInstruction; - bool m_ignore_errors = false; bool m_forwards = false; }; diff --git a/lldb/include/lldb/Target/TraceInstructionDumper.h b/lldb/include/lldb/Target/TraceInstructionDumper.h --- a/lldb/include/lldb/Target/TraceInstructionDumper.h +++ b/lldb/include/lldb/Target/TraceInstructionDumper.h @@ -108,14 +108,6 @@ /// if no instructions were visited. llvm::Optional DumpInstructions(size_t count); - /// \return - /// \b true if there's still more data to traverse in the trace. - bool HasMoreData(); - - /// Indicate to the dumper that no more data is available in the trace. - /// This will prevent further iterations. - void SetNoMoreData(); - private: /// Create an instruction entry for the current position without symbol /// information. @@ -125,8 +117,6 @@ lldb::TraceCursorUP m_cursor_up; TraceInstructionDumperOptions m_options; - /// If \b true, all the instructions have been traversed. - bool m_no_more_data = false; std::unique_ptr m_writer_up; }; diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp --- a/lldb/source/Commands/CommandObjectThread.cpp +++ b/lldb/source/Commands/CommandObjectThread.cpp @@ -2285,16 +2285,17 @@ lldb::eFilePermissionsFileDefault); } + if (m_options.m_continue && !m_last_id) { + // We need to stop processing data when we already ran out of instructions + // in a previous command. We can fake this by setting the cursor past the + // end of the trace. + cursor_up->Seek(1, TraceCursor::SeekType::End); + } + TraceInstructionDumper dumper( std::move(cursor_up), out_file ? *out_file : result.GetOutputStream(), m_options.m_dumper_options); - if (m_options.m_continue && !m_last_id) { - // We need to tell the dumper to stop processing data when - // we already ran out of instructions in a previous command - dumper.SetNoMoreData(); - } - m_last_id = dumper.DumpInstructions(m_options.m_count); return true; } diff --git a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp --- a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp +++ b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp @@ -225,10 +225,6 @@ } lldb::TraceCursorUP DecodedThread::GetCursor() { - // We insert a fake error signaling an empty trace if needed becasue the - // TraceCursor requires non-empty traces. - if (m_instruction_ips.empty()) - AppendError(createStringError(inconvertibleErrorCode(), "empty trace")); return std::make_unique(m_thread_sp, shared_from_this()); } diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h b/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h --- a/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h +++ b/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h @@ -19,9 +19,11 @@ TraceCursorIntelPT(lldb::ThreadSP thread_sp, DecodedThreadSP decoded_thread_sp); - uint64_t Seek(int64_t offset, SeekType origin) override; + bool Seek(int64_t offset, SeekType origin) override; - virtual bool Next() override; + void Next() override; + + bool HasValue() const override; const char *GetError() override; @@ -43,12 +45,17 @@ bool HasId(lldb::user_id_t id) const override; private: - size_t GetInternalInstructionSize(); + /// \return + /// The number of instructions and errors in the trace. + int64_t GetItemsCount() const; + + /// Calculate the tsc range for the current position if needed. + void CalculateTscRange(); /// Storage of the actual instructions DecodedThreadSP m_decoded_thread_sp; /// Internal instruction index currently pointing at. - size_t m_pos; + int64_t m_pos; /// Tsc range covering the current instruction. llvm::Optional m_tsc_range; }; diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp --- a/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp +++ b/lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp @@ -20,69 +20,49 @@ TraceCursorIntelPT::TraceCursorIntelPT(ThreadSP thread_sp, DecodedThreadSP decoded_thread_sp) : TraceCursor(thread_sp), m_decoded_thread_sp(decoded_thread_sp) { - assert(m_decoded_thread_sp->GetInstructionsCount() > 0 && - "a trace should have at least one instruction or error"); - m_pos = m_decoded_thread_sp->GetInstructionsCount() - 1; - m_tsc_range = - m_decoded_thread_sp->CalculateTscRange(m_pos, /*hint_range*/ None); + Seek(0, SeekType::End); } -size_t TraceCursorIntelPT::GetInternalInstructionSize() { +int64_t TraceCursorIntelPT::GetItemsCount() const { return m_decoded_thread_sp->GetInstructionsCount(); } -bool TraceCursorIntelPT::Next() { - auto canMoveOne = [&]() { - if (IsForwards()) - return m_pos + 1 < GetInternalInstructionSize(); - return m_pos > 0; - }; +void TraceCursorIntelPT::CalculateTscRange() { + // If we failed, then we look for the exact range + if (!m_tsc_range || !m_tsc_range->InRange(m_pos)) + m_tsc_range = m_decoded_thread_sp->CalculateTscRange( + m_pos, /*hit_range=*/m_tsc_range); +} - size_t initial_pos = m_pos; +void TraceCursorIntelPT::Next() { + m_pos += IsForwards() ? 1 : -1; - while (canMoveOne()) { - m_pos += IsForwards() ? 1 : -1; + // We try to go to a neighbor tsc range that might contain the current pos + if (m_tsc_range && !m_tsc_range->InRange(m_pos)) + m_tsc_range = IsForwards() ? m_tsc_range->Next() : m_tsc_range->Prev(); - if (m_tsc_range && !m_tsc_range->InRange(m_pos)) - m_tsc_range = IsForwards() ? m_tsc_range->Next() : m_tsc_range->Prev(); + // If we failed, this call will fix it + CalculateTscRange(); +} - if (!m_ignore_errors && IsError()) - return true; - if (GetInstructionControlFlowType() & m_granularity) - return true; +bool TraceCursorIntelPT::Seek(int64_t offset, SeekType origin) { + switch (origin) { + case TraceCursor::SeekType::Beginning: + m_pos = offset; + break; + case TraceCursor::SeekType::End: + m_pos = GetItemsCount() - 1 + offset; + break; + case TraceCursor::SeekType::Current: + m_pos += offset; } + CalculateTscRange(); - // Didn't find any matching instructions - m_pos = initial_pos; - return false; + return HasValue(); } -uint64_t TraceCursorIntelPT::Seek(int64_t offset, SeekType origin) { - int64_t last_index = GetInternalInstructionSize() - 1; - - auto fitPosToBounds = [&](int64_t raw_pos) -> int64_t { - return std::min(std::max((int64_t)0, raw_pos), last_index); - }; - - auto FindDistanceAndSetPos = [&]() -> int64_t { - switch (origin) { - case TraceCursor::SeekType::Beginning: - 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(); - m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos, m_tsc_range); - return dist; +bool TraceCursorIntelPT::HasValue() const { + return m_pos >= 0 && m_pos < GetItemsCount(); } bool TraceCursorIntelPT::IsError() { diff --git a/lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp b/lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp --- a/lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp +++ b/lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp @@ -154,7 +154,8 @@ // TODO: Make distinction between errors by storing the error messages. // Currently, all errors are treated the same. m_instruction_layer_up->AppendInstruction(0); - more_data_in_trace = cursor.Next(); + cursor.Next(); + more_data_in_trace = cursor.HasValue(); } else { lldb::addr_t current_instruction_load_address = cursor.GetLoadAddress(); lldb::TraceInstructionControlFlowType current_instruction_type = @@ -162,7 +163,8 @@ m_instruction_layer_up->AppendInstruction( current_instruction_load_address); - more_data_in_trace = cursor.Next(); + cursor.Next(); + more_data_in_trace = cursor.HasValue(); if (current_instruction_type & lldb::eTraceInstructionControlFlowTypeCall) { if (more_data_in_trace && !cursor.IsError()) { diff --git a/lldb/source/Target/TraceCursor.cpp b/lldb/source/Target/TraceCursor.cpp --- a/lldb/source/Target/TraceCursor.cpp +++ b/lldb/source/Target/TraceCursor.cpp @@ -21,15 +21,6 @@ return m_exe_ctx_ref; } -void TraceCursor::SetGranularity( - lldb::TraceInstructionControlFlowType granularity) { - m_granularity = granularity; -} - -void TraceCursor::SetIgnoreErrors(bool ignore_errors) { - m_ignore_errors = ignore_errors; -} - void TraceCursor::SetForwards(bool forwards) { m_forwards = forwards; } bool TraceCursor::IsForwards() const { return m_forwards; } diff --git a/lldb/source/Target/TraceInstructionDumper.cpp b/lldb/source/Target/TraceInstructionDumper.cpp --- a/lldb/source/Target/TraceInstructionDumper.cpp +++ b/lldb/source/Target/TraceInstructionDumper.cpp @@ -261,33 +261,20 @@ : m_cursor_up(std::move(cursor_up)), m_options(options), m_writer_up(CreateWriter(s, m_options)) { - if (m_options.id) { - if (!m_cursor_up->GoToId(*m_options.id)) { - m_writer_up->InfoMessage("invalid instruction id"); - SetNoMoreData(); - } - } else if (m_options.forwards) { + if (m_options.id) + m_cursor_up->GoToId(*m_options.id); + else if (m_options.forwards) m_cursor_up->Seek(0, TraceCursor::SeekType::Beginning); - } else { + else m_cursor_up->Seek(0, TraceCursor::SeekType::End); - } m_cursor_up->SetForwards(m_options.forwards); if (m_options.skip) { - uint64_t to_skip = *m_options.skip; - if (m_cursor_up->Seek((m_options.forwards ? 1 : -1) * to_skip, - TraceCursor::SeekType::Current) < to_skip) { - // This happens when the skip value was more than the number of - // available instructions. - SetNoMoreData(); - } + m_cursor_up->Seek((m_options.forwards ? 1 : -1) * *m_options.skip, + TraceCursor::SeekType::Current); } } -void TraceInstructionDumper::SetNoMoreData() { m_no_more_data = true; } - -bool TraceInstructionDumper::HasMoreData() { return !m_no_more_data; } - TraceInstructionDumper::InstructionEntry TraceInstructionDumper::CreatRawInstructionEntry() { InstructionEntry insn; @@ -375,11 +362,8 @@ ExecutionContext exe_ctx; thread_sp->GetProcess()->GetTarget().CalculateExecutionContext(exe_ctx); - for (size_t i = 0; i < count; i++) { - if (!HasMoreData()) { - m_writer_up->InfoMessage("no more data"); - break; - } + for (size_t i = 0; i < count && m_cursor_up->HasValue(); + m_cursor_up->Next(), i++) { last_id = m_cursor_up->GetId(); if (m_options.forwards) { @@ -418,9 +402,8 @@ // makes sense. PrintEvents(); } - - if (!m_cursor_up->Next()) - SetNoMoreData(); } + if (!m_cursor_up->HasValue()) + m_writer_up->InfoMessage("no more data"); return last_id; }