Author: Jakob Johnson Date: 2022-08-01T13:53:53-07:00 New Revision: 3bec33b16db11c67d43bda134520a2132ff606c9
URL: https://github.com/llvm/llvm-project/commit/3bec33b16db11c67d43bda134520a2132ff606c9 DIFF: https://github.com/llvm/llvm-project/commit/3bec33b16db11c67d43bda134520a2132ff606c9.diff LOG: [trace] Replace TraceCursorUP with TraceCursorSP The use of `std::unique_ptr` with `TraceCursor` adds unnecessary complexity to adding `SBTraceCursor` bindings Specifically, since `TraceCursor` is an abstract class there's no clean way to provide "deep clone" semantics for `TraceCursorUP` short of creating a pure virtual `clone()` method (afaict). After discussing with @wallace, we decided there is no strong reason to favor wrapping `TraceCursor` with `std::unique_ptr` over `std::shared_ptr`, thus this diff replaces all usages of `std::unique_ptr<TraceCursor>` with `std::shared_ptr<TraceCursor>`. This sets the stage for future diffs to introduce `SBTraceCursor` bindings in a more clean fashion. Test Plan: Differential Revision: https://reviews.llvm.org/D130925 Added: Modified: lldb/include/lldb/Target/Trace.h lldb/include/lldb/Target/TraceCursor.h lldb/include/lldb/Target/TraceDumper.h lldb/include/lldb/lldb-forward.h lldb/source/Commands/CommandObjectThread.cpp lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp lldb/source/Target/TraceDumper.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Target/Trace.h b/lldb/include/lldb/Target/Trace.h index beae9e28417d..917e66992ad1 100644 --- a/lldb/include/lldb/Target/Trace.h +++ b/lldb/include/lldb/Target/Trace.h @@ -169,9 +169,9 @@ class Trace : public PluginInterface, /// Get a \a TraceCursor for the given thread's trace. /// /// \return - /// A \a TraceCursorUP. If the thread is not traced or its trace + /// A \a TraceCursorSP. If the thread is not traced or its trace /// information failed to load, an \a llvm::Error is returned. - virtual llvm::Expected<lldb::TraceCursorUP> + virtual llvm::Expected<lldb::TraceCursorSP> CreateNewCursor(Thread &thread) = 0; /// Dump general info about a given thread's trace. Each Trace plug-in diff --git a/lldb/include/lldb/Target/TraceCursor.h b/lldb/include/lldb/Target/TraceCursor.h index 95b022331634..4e405aeaab7c 100644 --- a/lldb/include/lldb/Target/TraceCursor.h +++ b/lldb/include/lldb/Target/TraceCursor.h @@ -52,7 +52,7 @@ namespace lldb_private { /// /// Sample usage: /// -/// TraceCursorUP cursor = trace.GetTrace(thread); +/// TraceCursorSP cursor = trace.GetTrace(thread); /// /// for (; cursor->HasValue(); cursor->Next()) { /// TraceItemKind kind = cursor->GetItemKind(); diff --git a/lldb/include/lldb/Target/TraceDumper.h b/lldb/include/lldb/Target/TraceDumper.h index ada779990e07..ab3f77916751 100644 --- a/lldb/include/lldb/Target/TraceDumper.h +++ b/lldb/include/lldb/Target/TraceDumper.h @@ -93,7 +93,7 @@ class TraceDumper { /// /// \param[in] options /// Additional options for configuring the dumping. - TraceDumper(lldb::TraceCursorUP &&cursor_up, Stream &s, + TraceDumper(lldb::TraceCursorSP cursor_sp, Stream &s, const TraceDumperOptions &options); /// Dump \a count instructions of the thread trace starting at the current @@ -114,7 +114,7 @@ class TraceDumper { /// Create a trace item for the current position without symbol information. TraceItem CreatRawTraceItem(); - lldb::TraceCursorUP m_cursor_up; + lldb::TraceCursorSP m_cursor_sp; TraceDumperOptions m_options; std::unique_ptr<OutputWriter> m_writer_up; }; diff --git a/lldb/include/lldb/lldb-forward.h b/lldb/include/lldb/lldb-forward.h index c51e1850338f..fd88a45ba06f 100644 --- a/lldb/include/lldb/lldb-forward.h +++ b/lldb/include/lldb/lldb-forward.h @@ -420,7 +420,7 @@ typedef std::weak_ptr<lldb_private::ThreadPlan> ThreadPlanWP; typedef std::shared_ptr<lldb_private::ThreadPlanTracer> ThreadPlanTracerSP; typedef std::shared_ptr<lldb_private::Trace> TraceSP; typedef std::unique_ptr<lldb_private::TraceExporter> TraceExporterUP; -typedef std::unique_ptr<lldb_private::TraceCursor> TraceCursorUP; +typedef std::shared_ptr<lldb_private::TraceCursor> TraceCursorSP; typedef std::shared_ptr<lldb_private::Type> TypeSP; typedef std::weak_ptr<lldb_private::Type> TypeWP; typedef std::shared_ptr<lldb_private::TypeCategoryImpl> TypeCategoryImplSP; diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp index fe0cb0945cde..9aa128aaa288 100644 --- a/lldb/source/Commands/CommandObjectThread.cpp +++ b/lldb/source/Commands/CommandObjectThread.cpp @@ -2269,17 +2269,17 @@ class CommandObjectTraceDumpInstructions : public CommandObjectParsed { m_options.m_dumper_options.id = m_last_id; } - llvm::Expected<TraceCursorUP> cursor_or_error = + llvm::Expected<TraceCursorSP> cursor_or_error = m_exe_ctx.GetTargetSP()->GetTrace()->CreateNewCursor(*thread_sp); if (!cursor_or_error) { result.AppendError(llvm::toString(cursor_or_error.takeError())); return false; } - TraceCursorUP &cursor_up = *cursor_or_error; + TraceCursorSP &cursor_sp = *cursor_or_error; if (m_options.m_dumper_options.id && - !cursor_up->HasId(*m_options.m_dumper_options.id)) { + !cursor_sp->HasId(*m_options.m_dumper_options.id)) { result.AppendError("invalid instruction id\n"); return false; } @@ -2295,10 +2295,10 @@ class CommandObjectTraceDumpInstructions : public CommandObjectParsed { // 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); + cursor_sp->Seek(1, TraceCursor::SeekType::End); } - TraceDumper dumper(std::move(cursor_up), + TraceDumper dumper(std::move(cursor_sp), out_file ? *out_file : result.GetOutputStream(), m_options.m_dumper_options); diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp index f3f0a513e3fa..18b15f25b1ab 100644 --- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp +++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp @@ -176,12 +176,12 @@ Expected<Optional<uint64_t>> TraceIntelPT::FindBeginningOfTimeNanos() { return storage.beginning_of_time_nanos; } -llvm::Expected<lldb::TraceCursorUP> +llvm::Expected<lldb::TraceCursorSP> TraceIntelPT::CreateNewCursor(Thread &thread) { if (Expected<DecodedThreadSP> decoded_thread = Decode(thread)) { if (Expected<Optional<uint64_t>> beginning_of_time = FindBeginningOfTimeNanos()) - return std::make_unique<TraceCursorIntelPT>( + return std::make_shared<TraceCursorIntelPT>( thread.shared_from_this(), *decoded_thread, m_storage.tsc_conversion, *beginning_of_time); else diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h index 7f2c3f8dda5d..b104a8a2581e 100644 --- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h +++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h @@ -71,7 +71,7 @@ class TraceIntelPT : public Trace { llvm::StringRef GetSchema() override; - llvm::Expected<lldb::TraceCursorUP> CreateNewCursor(Thread &thread) override; + llvm::Expected<lldb::TraceCursorSP> CreateNewCursor(Thread &thread) override; void DumpTraceInfo(Thread &thread, Stream &s, bool verbose, bool json) override; diff --git a/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp b/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp index 384abd2166df..df8da3967fc6 100644 --- a/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp +++ b/lldb/source/Plugins/TraceExporter/ctf/CommandObjectThreadTraceExportCTF.cpp @@ -81,7 +81,7 @@ bool CommandObjectThreadTraceExportCTF::DoExecute(Args &command, return false; } else { auto do_work = [&]() -> Error { - Expected<TraceCursorUP> cursor = trace_sp->CreateNewCursor(*thread); + Expected<TraceCursorSP> cursor = trace_sp->CreateNewCursor(*thread); if (!cursor) return cursor.takeError(); TraceHTR htr(*thread, **cursor); diff --git a/lldb/source/Target/TraceDumper.cpp b/lldb/source/Target/TraceDumper.cpp index 872530b657a1..74f6ea0ffa52 100644 --- a/lldb/source/Target/TraceDumper.cpp +++ b/lldb/source/Target/TraceDumper.cpp @@ -295,32 +295,32 @@ CreateWriter(Stream &s, const TraceDumperOptions &options, Thread &thread) { new OutputWriterCLI(s, options, thread)); } -TraceDumper::TraceDumper(lldb::TraceCursorUP &&cursor_up, Stream &s, +TraceDumper::TraceDumper(lldb::TraceCursorSP cursor_sp, Stream &s, const TraceDumperOptions &options) - : m_cursor_up(std::move(cursor_up)), m_options(options), + : m_cursor_sp(std::move(cursor_sp)), m_options(options), m_writer_up(CreateWriter( - s, m_options, *m_cursor_up->GetExecutionContextRef().GetThreadSP())) { + s, m_options, *m_cursor_sp->GetExecutionContextRef().GetThreadSP())) { if (m_options.id) - m_cursor_up->GoToId(*m_options.id); + m_cursor_sp->GoToId(*m_options.id); else if (m_options.forwards) - m_cursor_up->Seek(0, TraceCursor::SeekType::Beginning); + m_cursor_sp->Seek(0, TraceCursor::SeekType::Beginning); else - m_cursor_up->Seek(0, TraceCursor::SeekType::End); + m_cursor_sp->Seek(0, TraceCursor::SeekType::End); - m_cursor_up->SetForwards(m_options.forwards); + m_cursor_sp->SetForwards(m_options.forwards); if (m_options.skip) { - m_cursor_up->Seek((m_options.forwards ? 1 : -1) * *m_options.skip, + m_cursor_sp->Seek((m_options.forwards ? 1 : -1) * *m_options.skip, TraceCursor::SeekType::Current); } } TraceDumper::TraceItem TraceDumper::CreatRawTraceItem() { TraceItem item = {}; - item.id = m_cursor_up->GetId(); + item.id = m_cursor_sp->GetId(); if (m_options.show_timestamps) - item.timestamp = m_cursor_up->GetWallClockTime(); + item.timestamp = m_cursor_sp->GetWallClockTime(); return item; } @@ -378,7 +378,7 @@ CalculateDisass(const TraceDumper::SymbolInfo &symbol_info, } Optional<lldb::user_id_t> TraceDumper::DumpInstructions(size_t count) { - ThreadSP thread_sp = m_cursor_up->GetExecutionContextRef().GetThreadSP(); + ThreadSP thread_sp = m_cursor_sp->GetExecutionContextRef().GetThreadSP(); SymbolInfo prev_symbol_info; Optional<lldb::user_id_t> last_id; @@ -386,32 +386,32 @@ Optional<lldb::user_id_t> TraceDumper::DumpInstructions(size_t count) { ExecutionContext exe_ctx; thread_sp->GetProcess()->GetTarget().CalculateExecutionContext(exe_ctx); - for (size_t insn_seen = 0; insn_seen < count && m_cursor_up->HasValue(); - m_cursor_up->Next()) { + for (size_t insn_seen = 0; insn_seen < count && m_cursor_sp->HasValue(); + m_cursor_sp->Next()) { - last_id = m_cursor_up->GetId(); + last_id = m_cursor_sp->GetId(); TraceItem item = CreatRawTraceItem(); - if (m_cursor_up->IsEvent()) { + if (m_cursor_sp->IsEvent()) { if (!m_options.show_events) continue; - item.event = m_cursor_up->GetEventType(); + item.event = m_cursor_sp->GetEventType(); switch (*item.event) { case eTraceEventCPUChanged: - item.cpu_id = m_cursor_up->GetCPU(); + item.cpu_id = m_cursor_sp->GetCPU(); break; case eTraceEventHWClockTick: - item.hw_clock = m_cursor_up->GetHWClock(); + item.hw_clock = m_cursor_sp->GetHWClock(); break; case eTraceEventDisabledHW: case eTraceEventDisabledSW: break; } - } else if (m_cursor_up->IsError()) { - item.error = m_cursor_up->GetError(); + } else if (m_cursor_sp->IsError()) { + item.error = m_cursor_sp->GetError(); } else { insn_seen++; - item.load_address = m_cursor_up->GetLoadAddress(); + item.load_address = m_cursor_sp->GetLoadAddress(); if (!m_options.raw) { SymbolInfo symbol_info; @@ -429,7 +429,7 @@ Optional<lldb::user_id_t> TraceDumper::DumpInstructions(size_t count) { } m_writer_up->TraceItem(item); } - if (!m_cursor_up->HasValue()) + if (!m_cursor_sp->HasValue()) m_writer_up->NoMoreData(); return last_id; } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
