wallace created this revision. wallace added a reviewer: jj10306. Herald added a project: All. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits.
When tracing on per-core mode, we are tracing all processes, which means that after hitting a breakpoint, our process will stop running (thus producing no more tracing data) but other processes will continue writing to our trace buffers. This causes a big data loss for our trace. As a way to remediate this, I'm adding some logic to pause and unpause tracing based on the target's state. The earlier we do it the better, however, I'm not adding the trigger at the earliest possible point for simplicity of this diff. Later we can improve that part. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D124962 Files: lldb/include/lldb/Host/common/NativeProcessProtocol.h lldb/source/Host/common/NativeProcessProtocol.cpp lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp lldb/source/Plugins/Process/Linux/IntelPTCollector.h lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp lldb/source/Plugins/Process/Linux/NativeProcessLinux.h lldb/source/Plugins/Process/Linux/Perf.cpp lldb/source/Plugins/Process/Linux/Perf.h
Index: lldb/source/Plugins/Process/Linux/Perf.h =================================================================== --- lldb/source/Plugins/Process/Linux/Perf.h +++ lldb/source/Plugins/Process/Linux/Perf.h @@ -208,6 +208,12 @@ /// \a ArrayRef<uint8_t> extending \a aux_size bytes from \a aux_offset. llvm::ArrayRef<uint8_t> GetAuxBuffer() const; + /// Use the ioctl API to disable the perf event. + void DisableWithIoctl() const; + + /// Use the ioctl API to enable the perf event. + void EnableWithIoctl() const; + private: /// Create new \a PerfEvent. /// Index: lldb/source/Plugins/Process/Linux/Perf.cpp =================================================================== --- lldb/source/Plugins/Process/Linux/Perf.cpp +++ lldb/source/Plugins/Process/Linux/Perf.cpp @@ -15,6 +15,7 @@ #include "llvm/Support/MathExtras.h" #include "llvm/Support/MemoryBuffer.h" +#include <sys/ioctl.h> #include <sys/mman.h> #include <sys/syscall.h> #include <unistd.h> @@ -218,3 +219,9 @@ return {reinterpret_cast<uint8_t *>(m_aux_base.get()), static_cast<size_t>(mmap_metadata.aux_size)}; } + +void PerfEvent::DisableWithIoctl() const { + ioctl(*m_fd, PERF_EVENT_IOC_DISABLE); +} + +void PerfEvent::EnableWithIoctl() const { ioctl(*m_fd, PERF_EVENT_IOC_ENABLE); } Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h =================================================================== --- lldb/source/Plugins/Process/Linux/NativeProcessLinux.h +++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.h @@ -209,6 +209,8 @@ /// stopping for threads being destroyed. Status NotifyTracersOfThreadDestroyed(lldb::tid_t tid); + void NotifyTracersProcessStateChanged(lldb::StateType state) override; + /// Writes the raw event message code (vis-a-vis PTRACE_GETEVENTMSG) /// corresponding to the given thread ID to the memory pointed to by @p /// message. Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp =================================================================== --- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp +++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp @@ -1665,6 +1665,11 @@ SignalIfAllThreadsStopped(); } +void NativeProcessLinux::NotifyTracersProcessStateChanged( + lldb::StateType state) { + m_intel_pt_collector.OnProcessStateChanged(state); +} + Status NativeProcessLinux::NotifyTracersOfNewThread(lldb::tid_t tid) { Log *log = GetLog(POSIXLog::Thread); Status error(m_intel_pt_collector.OnThreadCreated(tid)); Index: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h =================================================================== --- lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h +++ lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.h @@ -27,6 +27,11 @@ using IntelPTSingleBufferTraceUP = std::unique_ptr<IntelPTSingleBufferTrace>; +enum class TraceCollectionState { + Running, + Paused, +}; + /// This class wraps a single perf event collecting intel pt data in a single /// buffer. class IntelPTSingleBufferTrace { @@ -43,13 +48,17 @@ /// \param[in] core_id /// The CPU core id where to trace. If \b None, then this traces all CPUs. /// + /// \param[in] initial_state + /// The initial trace collection state. + /// /// \return /// A \a IntelPTSingleBufferTrace instance if tracing was successful, or /// an \a llvm::Error otherwise. static llvm::Expected<IntelPTSingleBufferTraceUP> Start(const TraceIntelPTStartRequest &request, llvm::Optional<lldb::tid_t> tid, - llvm::Optional<lldb::core_id_t> core_id = llvm::None); + llvm::Optional<lldb::core_id_t> core_id, + TraceCollectionState initial_state); /// \return /// The bytes requested by a jLLDBTraceGetBinaryData packet that was routed @@ -65,18 +74,29 @@ /// \param[in] size /// Number of bytes to read. /// + /// \param[in] flush + /// Indicate the CPU to flush out the tracing data available for this + /// buffer. + /// /// \return /// A vector with the requested binary data. The vector will have the /// size of the requested \a size. Non-available positions will be /// filled with zeroes. - llvm::Expected<std::vector<uint8_t>> GetTraceBuffer(size_t offset, - size_t size) const; + std::vector<uint8_t> GetTraceBuffer(size_t offset, size_t size, bool flush); /// \return - /// The total the size in bytes used by the trace buffer managed by this - /// trace instance. + /// The total the size in bytes used by the trace buffer managed by this + /// trace instance. size_t GetTraceBufferSize() const; + /// Change the collection state for this trace. + /// + /// This is a no-op if \p state is the same as the current state. + /// + /// \param[in] state + /// The new state. + void SetCollectionState(TraceCollectionState state); + private: /// Construct new \a IntelPTSingleBufferThreadTrace. Users are supposed to /// create instances of this class via the \a Start() method and not invoke @@ -84,11 +104,20 @@ /// /// \param[in] perf_event /// perf event configured for IntelPT. - IntelPTSingleBufferTrace(PerfEvent &&perf_event) - : m_perf_event(std::move(perf_event)) {} + /// + /// \param[in] collection_state + /// The initial collection state for the provided perf_event. + IntelPTSingleBufferTrace(PerfEvent &&perf_event, + TraceCollectionState collection_state) + : m_perf_event(std::move(perf_event)), + m_collection_state(collection_state) {} /// perf event configured for IntelPT. PerfEvent m_perf_event; + + /// The initial state is stopped because tracing can only start when the + /// process is paused. + TraceCollectionState m_collection_state; }; } // namespace process_linux Index: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp =================================================================== --- lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp +++ lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp @@ -18,7 +18,6 @@ #include <sstream> #include <linux/perf_event.h> -#include <sys/ioctl.h> #include <sys/syscall.h> #include <unistd.h> @@ -211,59 +210,78 @@ return m_perf_event.GetAuxBuffer().size(); } -Expected<std::vector<uint8_t>> -IntelPTSingleBufferTrace::GetTraceBuffer(size_t offset, size_t size) const { - auto fd = m_perf_event.GetFd(); - perf_event_mmap_page &mmap_metadata = m_perf_event.GetMetadataPage(); - // Disable the perf event to force a flush out of the CPU's internal buffer. - // Besides, we can guarantee that the CPU won't override any data as we are - // reading the buffer. - // - // The Intel documentation says: - // - // Packets are first buffered internally and then written out asynchronously. - // To collect packet output for postprocessing, a collector needs first to - // ensure that all packet data has been flushed from internal buffers. - // Software can ensure this by stopping packet generation by clearing - // IA32_RTIT_CTL.TraceEn (see âDisabling Packet Generationâ in - // Section 35.2.7.2). - // - // This is achieved by the PERF_EVENT_IOC_DISABLE ioctl request, as mentioned - // in the man page of perf_event_open. - ioctl(fd, PERF_EVENT_IOC_DISABLE); - - Log *log = GetLog(POSIXLog::Trace); - Status error; - uint64_t head = mmap_metadata.aux_head; - - LLDB_LOG(log, "Aux size -{0} , Head - {1}", mmap_metadata.aux_size, head); - - /** - * When configured as ring buffer, the aux buffer keeps wrapping around - * the buffer and its not possible to detect how many times the buffer - * wrapped. Initially the buffer is filled with zeros,as shown below - * so in order to get complete buffer we first copy firstpartsize, followed - * by any left over part from beginning to aux_head - * - * aux_offset [d,d,d,d,d,d,d,d,0,0,0,0,0,0,0,0,0,0,0] aux_size - * aux_head->||<- firstpartsize ->| - * - * */ +void IntelPTSingleBufferTrace::SetCollectionState( + TraceCollectionState new_state) { + if (new_state == m_collection_state) + return; + m_collection_state = new_state; + switch (new_state) { + case TraceCollectionState::Paused: + m_perf_event.DisableWithIoctl(); + break; + case TraceCollectionState::Running: + m_perf_event.EnableWithIoctl(); + break; + } +} +std::vector<uint8_t> IntelPTSingleBufferTrace::GetTraceBuffer(size_t offset, + size_t size, + bool flush) { std::vector<uint8_t> data(size, 0); - MutableArrayRef<uint8_t> buffer(data); - ReadCyclicBuffer(buffer, m_perf_event.GetAuxBuffer(), - static_cast<size_t>(head), offset); + auto do_read_buffer = [&]() { + perf_event_mmap_page &mmap_metadata = m_perf_event.GetMetadataPage(); + Log *log = GetLog(POSIXLog::Trace); + uint64_t head = mmap_metadata.aux_head; + + LLDB_LOG(log, "Aux size -{0} , Head - {1}", mmap_metadata.aux_size, head); + + /** + * When configured as ring buffer, the aux buffer keeps wrapping around + * the buffer and its not possible to detect how many times the buffer + * wrapped. Initially the buffer is filled with zeros,as shown below + * so in order to get complete buffer we first copy firstpartsize, followed + * by any left over part from beginning to aux_head + * + * aux_offset [d,d,d,d,d,d,d,d,0,0,0,0,0,0,0,0,0,0,0] aux_size + * aux_head->||<- firstpartsize ->| + * + * */ + + MutableArrayRef<uint8_t> buffer(data); + ReadCyclicBuffer(buffer, m_perf_event.GetAuxBuffer(), + static_cast<size_t>(head), offset); + }; + + if (flush) { + // Disable the perf event to force a flush out of the CPU's internal buffer. + // Besides, we can guarantee that the CPU won't override any data as we are + // reading the buffer. + // The Intel documentation says: + // + // Packets are first buffered internally and then written out + // asynchronously. To collect packet output for postprocessing, a collector + // needs first to ensure that all packet data has been flushed from internal + // buffers. Software can ensure this by stopping packet generation by + // clearing IA32_RTIT_CTL.TraceEn (see âDisabling Packet Generationâ in + // Section 35.2.7.2). + // + // This is achieved by the PERF_EVENT_IOC_DISABLE ioctl request, as + // mentioned in the man page of perf_event_open. + TraceCollectionState previous_state = m_collection_state; + SetCollectionState(TraceCollectionState::Paused); + do_read_buffer(); + SetCollectionState(previous_state); + } else { + do_read_buffer(); + } - // Reenable tracing now we have read the buffer - ioctl(fd, PERF_EVENT_IOC_ENABLE); return data; } -Expected<IntelPTSingleBufferTraceUP> -IntelPTSingleBufferTrace::Start(const TraceIntelPTStartRequest &request, - Optional<lldb::tid_t> tid, - Optional<core_id_t> core_id) { +Expected<IntelPTSingleBufferTraceUP> IntelPTSingleBufferTrace::Start( + const TraceIntelPTStartRequest &request, Optional<lldb::tid_t> tid, + Optional<core_id_t> core_id, TraceCollectionState initial_state) { Log *log = GetLog(POSIXLog::Trace); LLDB_LOG(log, "Will start tracing thread id {0} and cpu id {1}", tid, @@ -296,8 +314,10 @@ buffer_numpages)) { return std::move(mmap_err); } - return IntelPTSingleBufferTraceUP( - new IntelPTSingleBufferTrace(std::move(*perf_event))); + IntelPTSingleBufferTraceUP trace_up(new IntelPTSingleBufferTrace( + std::move(*perf_event), TraceCollectionState::Running)); + trace_up->SetCollectionState(initial_state); + return trace_up; } else { return perf_event.takeError(); } Index: lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h =================================================================== --- lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h +++ lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.h @@ -44,10 +44,17 @@ /// /// \param[in] callback.core_trace /// The single-buffer trace instance for the given core. - void - ForEachCore(std::function<void(lldb::core_id_t core_id, - const IntelPTSingleBufferTrace &core_trace)> - callback); + void ForEachCore(std::function<void(lldb::core_id_t core_id, + IntelPTSingleBufferTrace &core_trace)> + callback); + + /// This method should be invoked as early as possible whenever the process + /// resumed or stopped so that intel-pt collection is not enabled whenever + /// the process is not running. This is done to prevent polluting the core + /// traces with executions of unrelated process, which increases the data + /// loss of the target process, given that core traces don't filter by + /// process. + void OnProcessStateChanged(lldb::StateType state); private: IntelPTMultiCoreTrace( @@ -56,6 +63,10 @@ : m_traces_per_core(std::move(traces_per_core)) {} llvm::DenseMap<lldb::core_id_t, IntelPTSingleBufferTraceUP> m_traces_per_core; + + /// The initial state is stopped because tracing can only start when the + /// process is paused. + lldb::StateType m_process_state = lldb::StateType::eStateStopped; }; } // namespace process_linux Index: lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp =================================================================== --- lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp +++ lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp @@ -46,7 +46,8 @@ llvm::DenseMap<core_id_t, IntelPTSingleBufferTraceUP> buffers; for (core_id_t core_id : *core_ids) { if (Expected<IntelPTSingleBufferTraceUP> core_trace = - IntelPTSingleBufferTrace::Start(request, /*tid=*/None, core_id)) + IntelPTSingleBufferTrace::Start(request, /*tid=*/None, core_id, + TraceCollectionState::Paused)) buffers.try_emplace(core_id, std::move(*core_trace)); else return IncludePerfEventParanoidMessageInError(core_trace.takeError()); @@ -56,9 +57,30 @@ } void IntelPTMultiCoreTrace::ForEachCore( - std::function<void(core_id_t core_id, - const IntelPTSingleBufferTrace &core_trace)> + std::function<void(core_id_t core_id, IntelPTSingleBufferTrace &core_trace)> callback) { for (auto &it : m_traces_per_core) callback(it.first, *it.second); } + +void IntelPTMultiCoreTrace::OnProcessStateChanged(lldb::StateType state) { + if (m_process_state == state) + return; + switch (state) { + case eStateStopped: + case eStateExited: { + ForEachCore([&](core_id_t core_id, IntelPTSingleBufferTrace &core_trace) { + core_trace.SetCollectionState(TraceCollectionState::Paused); + }); + break; + } + case eStateRunning: { + ForEachCore([&](core_id_t core_id, IntelPTSingleBufferTrace &core_trace) { + core_trace.SetCollectionState(TraceCollectionState::Running); + }); + break; + } + default: + break; + } +} Index: lldb/source/Plugins/Process/Linux/IntelPTCollector.h =================================================================== --- lldb/source/Plugins/Process/Linux/IntelPTCollector.h +++ lldb/source/Plugins/Process/Linux/IntelPTCollector.h @@ -40,8 +40,7 @@ std::vector<TraceThreadState> GetThreadStates() const; - llvm::Expected<const IntelPTSingleBufferTrace &> - GetTracedThread(lldb::tid_t tid) const; + llvm::Expected<IntelPTSingleBufferTrace &> GetTracedThread(lldb::tid_t tid); llvm::Error TraceStart(lldb::tid_t tid, const TraceIntelPTStartRequest &request); @@ -82,7 +81,7 @@ bool TracesThread(lldb::tid_t tid) const; - const IntelPTThreadTraceCollection &GetThreadTraces() const; + IntelPTThreadTraceCollection &GetThreadTraces(); llvm::Error TraceStart(lldb::tid_t tid); @@ -104,6 +103,9 @@ static bool IsSupported(); + /// To be invoked whenever the state of the target process has changed. + void OnProcessStateChanged(lldb::StateType state); + /// If "process tracing" is enabled, then trace the given thread. llvm::Error OnThreadCreated(lldb::tid_t tid); @@ -125,7 +127,7 @@ /// Implementation of the jLLDBTraceGetBinaryData packet llvm::Expected<std::vector<uint8_t>> - GetBinaryData(const TraceGetBinaryDataRequest &request) const; + GetBinaryData(const TraceGetBinaryDataRequest &request); /// Dispose of all traces void Clear(); @@ -137,8 +139,7 @@ llvm::Error TraceStart(lldb::tid_t tid, const TraceIntelPTStartRequest &request); - llvm::Expected<const IntelPTSingleBufferTrace &> - GetTracedThread(lldb::tid_t tid) const; + llvm::Expected<IntelPTSingleBufferTrace &> GetTracedThread(lldb::tid_t tid); bool IsProcessTracingEnabled() const; Index: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp =================================================================== --- lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp +++ lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp @@ -55,7 +55,8 @@ "Thread %" PRIu64 " already traced", tid); Expected<IntelPTSingleBufferTraceUP> trace_up = - IntelPTSingleBufferTrace::Start(request, tid); + IntelPTSingleBufferTrace::Start(request, tid, /*core_id=*/None, + TraceCollectionState::Running); if (!trace_up) return trace_up.takeError(); @@ -79,8 +80,8 @@ return states; } -Expected<const IntelPTSingleBufferTrace &> -IntelPTThreadTraceCollection::GetTracedThread(lldb::tid_t tid) const { +Expected<IntelPTSingleBufferTrace &> +IntelPTThreadTraceCollection::GetTracedThread(lldb::tid_t tid) { auto it = m_thread_traces.find(tid); if (it == m_thread_traces.end()) return createStringError(inconvertibleErrorCode(), @@ -121,8 +122,7 @@ return m_thread_traces.TraceStart(tid, m_tracing_params); } -const IntelPTThreadTraceCollection & -IntelPTPerThreadProcessTrace::GetThreadTraces() const { +IntelPTThreadTraceCollection &IntelPTPerThreadProcessTrace::GetThreadTraces() { return m_thread_traces; } @@ -222,6 +222,11 @@ } } +void IntelPTCollector::OnProcessStateChanged(lldb::StateType state) { + if (m_per_core_process_trace_up) + m_per_core_process_trace_up->OnProcessStateChanged(state); +} + Error IntelPTCollector::OnThreadCreated(lldb::tid_t tid) { if (!IsProcessTracingEnabled()) return Error::success(); @@ -271,8 +276,8 @@ return toJSON(state); } -Expected<const IntelPTSingleBufferTrace &> -IntelPTCollector::GetTracedThread(lldb::tid_t tid) const { +Expected<IntelPTSingleBufferTrace &> +IntelPTCollector::GetTracedThread(lldb::tid_t tid) { if (IsProcessTracingEnabled() && m_per_thread_process_trace_up->TracesThread(tid)) return m_per_thread_process_trace_up->GetThreadTraces().GetTracedThread( @@ -281,11 +286,12 @@ } Expected<std::vector<uint8_t>> -IntelPTCollector::GetBinaryData(const TraceGetBinaryDataRequest &request) const { +IntelPTCollector::GetBinaryData(const TraceGetBinaryDataRequest &request) { if (request.kind == IntelPTDataKinds::kTraceBuffer) { - if (Expected<const IntelPTSingleBufferTrace &> trace = + if (Expected<IntelPTSingleBufferTrace &> trace = GetTracedThread(*request.tid)) - return trace->GetTraceBuffer(request.offset, request.size); + return trace->GetTraceBuffer(request.offset, request.size, + /*flush=*/true); else return trace.takeError(); } else if (request.kind == IntelPTDataKinds::kProcFsCpuInfo) { Index: lldb/source/Host/common/NativeProcessProtocol.cpp =================================================================== --- lldb/source/Host/common/NativeProcessProtocol.cpp +++ lldb/source/Host/common/NativeProcessProtocol.cpp @@ -312,6 +312,7 @@ Log *log = GetLog(LLDBLog::Process); m_delegate.ProcessStateChanged(this, state); + NotifyTracersProcessStateChanged(state); LLDB_LOG(log, "sent state notification [{0}] from process {1}", state, GetID()); Index: lldb/include/lldb/Host/common/NativeProcessProtocol.h =================================================================== --- lldb/include/lldb/Host/common/NativeProcessProtocol.h +++ lldb/include/lldb/Host/common/NativeProcessProtocol.h @@ -461,6 +461,9 @@ NativeThreadProtocol *GetThreadByIDUnlocked(lldb::tid_t tid); + /// Notify tracers that the state of the target process has changed. + virtual void NotifyTracersProcessStateChanged(lldb::StateType state) {} + private: void SynchronouslyNotifyProcessStateChanged(lldb::StateType state); llvm::Expected<SoftwareBreakpoint>
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits