wallace updated this revision to Diff 427448. wallace added a comment. add better error handling
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124962/new/ 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,19 @@ /// \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. This doesn't terminate the + /// perf event. + /// + /// \return + /// An Error if the perf event couldn't be disabled. + llvm::Error DisableWithIoctl() const; + + /// Use the ioctl API to enable the perf event. + /// + /// \return + /// An Error if the perf event couldn't be enabled. + llvm::Error 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,19 @@ return {reinterpret_cast<uint8_t *>(m_aux_base.get()), static_cast<size_t>(mmap_metadata.aux_size)}; } + +Error PerfEvent::DisableWithIoctl() const { + if (ioctl(*m_fd, PERF_EVENT_IOC_DISABLE) < 0) + return createStringError(inconvertibleErrorCode(), + "Can't disable perf event. %s", + std::strerror(errno)); + return Error::success(); +} + +Error PerfEvent::EnableWithIoctl() const { + if (ioctl(*m_fd, PERF_EVENT_IOC_ENABLE) < 0) + return createStringError(inconvertibleErrorCode(), + "Can't disable perf event. %s", + std::strerror(errno)); + return Error::success(); +} 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,33 @@ /// \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; + 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. + /// + /// \return + /// An error if the state couldn't be changed. + llvm::Error 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 +108,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,84 @@ 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); +Error IntelPTSingleBufferTrace::SetCollectionState( + TraceCollectionState new_state) { + if (new_state == m_collection_state) + return Error::success(); - 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 ->| - * - * */ + switch (new_state) { + case TraceCollectionState::Paused: + if (Error err = m_perf_event.DisableWithIoctl()) + return err; + break; + case TraceCollectionState::Running: + if (Error err = m_perf_event.EnableWithIoctl()) + return err; + break; + } + m_collection_state = new_state; + return Error::success(); +} +Expected<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; + if (Error err = SetCollectionState(TraceCollectionState::Paused)) + return std::move(err); + do_read_buffer(); + if (Error err = SetCollectionState(previous_state)) + return std::move(err); + } 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 +320,11 @@ 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)); + if (Error err = trace_up->SetCollectionState(initial_state)) + return std::move(err); + 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,19 @@ /// /// \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 + /// resumes or stops so that intel-pt collection is not enabled when + /// the process is not running. This is done to prevent polluting the core + /// traces with executions of unrelated processes, which increases the data + /// loss of the target process, given that core traces don't filter by + /// process. + /// A possible way to avoid this is to use CR3 filtering, which is equivalent + /// to process filtering, but the perf_event API doesn't support it. + void OnProcessStateChanged(lldb::StateType state); private: IntelPTMultiCoreTrace( @@ -56,6 +65,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 @@ -10,6 +10,8 @@ #include "Procfs.h" +#include "Plugins/Process/POSIX/ProcessPOSIXLog.h" + using namespace lldb; using namespace lldb_private; using namespace process_linux; @@ -46,7 +48,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 +59,38 @@ } 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) { + if (Error err = + core_trace.SetCollectionState(TraceCollectionState::Paused)) { + LLDB_LOG_ERROR(GetLog(POSIXLog::Trace), std::move(err), + "Unable to pause the core trace for core {0}", core_id); + } + }); + break; + } + case eStateRunning: { + ForEachCore([](core_id_t core_id, IntelPTSingleBufferTrace &core_trace) { + if (Error err = + core_trace.SetCollectionState(TraceCollectionState::Running)) { + LLDB_LOG_ERROR(GetLog(POSIXLog::Trace), std::move(err), + "Unable to resume the core trace for core {0}", core_id); + } + }); + 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