jj10306 created this revision.
jj10306 added reviewers: wallace, persona0220.
Herald added a project: All.
jj10306 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
Occasionally the assertion that enforces increasing TSC values in
`DecodedThread::NotifyTsc`
would get tripped during large multi CPU trace decoding.
The root cause of this issue was an assumption that all the data of a
PSB will fit within the start,end TSC of the "owning"
`ThreadContinuousExecution`. After investigating, this is not the case
because PSBs can have multiple TSCs.
This diff works around this issue by introducing a TSC upper bound for
each `PSBBlockDecoder`. This fixes the assertion failure by simply
"dropping" the remaining data of PSB whenever the TSC upper bound is
exceeded during decoding.
Future work will do a larger refactor of the multi CPU decoding to
remove the dependencies on this incorrect assumption so that PSB blocks
that span multiple `ThreadContinuousExecutions` are correctly handled.
correctly
Test Plan:
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D136610
Files:
lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
Index: lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
===================================================================
--- lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
+++ lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
@@ -155,9 +155,11 @@
/// appended to. It might have already some instructions.
PSBBlockDecoder(PtInsnDecoderUP &&decoder_up, const PSBBlock &psb_block,
Optional<lldb::addr_t> next_block_ip,
- DecodedThread &decoded_thread)
+ DecodedThread &decoded_thread,
+ llvm::Optional<DecodedThread::TSC> tsc_upper_bound)
: m_decoder_up(std::move(decoder_up)), m_psb_block(psb_block),
- m_next_block_ip(next_block_ip), m_decoded_thread(decoded_thread) {}
+ m_next_block_ip(next_block_ip), m_decoded_thread(decoded_thread),
+ m_tsc_upper_bound(tsc_upper_bound) {}
/// \param[in] trace_intel_pt
/// The main Trace object that own the PSB block.
@@ -185,14 +187,15 @@
static Expected<PSBBlockDecoder>
Create(TraceIntelPT &trace_intel_pt, const PSBBlock &psb_block,
ArrayRef<uint8_t> buffer, Process &process,
- Optional<lldb::addr_t> next_block_ip, DecodedThread &decoded_thread) {
+ Optional<lldb::addr_t> next_block_ip, DecodedThread &decoded_thread,
+ llvm::Optional<DecodedThread::TSC> tsc_upper_bound) {
Expected<PtInsnDecoderUP> decoder_up =
CreateInstructionDecoder(trace_intel_pt, buffer, process);
if (!decoder_up)
return decoder_up.takeError();
return PSBBlockDecoder(std::move(*decoder_up), psb_block, next_block_ip,
- decoded_thread);
+ decoded_thread, tsc_upper_bound);
}
void DecodePSBBlock() {
@@ -279,8 +282,34 @@
return status;
}
- if (event.has_tsc)
- m_decoded_thread.NotifyTsc(event.tsc);
+ if (event.has_tsc) {
+ if (m_tsc_upper_bound && event.tsc > m_tsc_upper_bound.value()) {
+ // This event and all the remaining events of this PSB have a TSC
+ // outside the range of the "owning" ThreadContinuousExecution. For
+ // now we drop all of these events/instructions, future work can
+ // improve upon this by determining the "owning"
+ // ThreadContinuousExecution of the remaining PSB data.
+ std::string err_msg =
+ formatv(
+ "TSC {0} exceeds maximum TSC value {1}. Will skip decoding"
+ " the remining the remaining data of the PSB.",
+ event.tsc, m_tsc_upper_bound.value())
+ .str();
+
+ uint64_t offset;
+ int status = pt_insn_get_offset(m_decoder_up.get(), &offset);
+ if (!IsLibiptError(status)) {
+ err_msg =
+ formatv("At byte offset {0} of PSB with size {1} bytes, {2}",
+ offset, m_psb_block.size, err_msg)
+ .str();
+ }
+ m_decoded_thread.AppendCustomError(err_msg);
+ return -1;
+ } else {
+ m_decoded_thread.NotifyTsc(event.tsc);
+ }
+ }
switch (event.type) {
case ptev_disabled:
@@ -313,6 +342,8 @@
PSBBlock m_psb_block;
Optional<lldb::addr_t> m_next_block_ip;
DecodedThread &m_decoded_thread;
+ // Maximum allowed value of TSCs decoded from m_psb_block.
+ llvm::Optional<DecodedThread::TSC> m_tsc_upper_bound;
};
Error lldb_private::trace_intel_pt::DecodeSingleTraceForThread(
@@ -330,7 +361,7 @@
trace_intel_pt, block, buffer.slice(block.psb_offset, block.size),
*decoded_thread.GetThread()->GetProcess(),
i + 1 < blocks->size() ? blocks->at(i + 1).starting_ip : None,
- decoded_thread);
+ decoded_thread, llvm::None);
if (!decoder)
return decoder.takeError();
@@ -392,13 +423,13 @@
Expected<PSBBlockDecoder> decoder = PSBBlockDecoder::Create(
trace_intel_pt, psb_block,
- buffers.lookup(executions[i].thread_execution.cpu_id)
+ buffers.lookup(execution.thread_execution.cpu_id)
.slice(psb_block.psb_offset, psb_block.size),
*decoded_thread.GetThread()->GetProcess(),
j + 1 < execution.psb_blocks.size()
? execution.psb_blocks[j + 1].starting_ip
: None,
- decoded_thread);
+ decoded_thread, execution.thread_execution.GetEndTSC());
if (!decoder)
return decoder.takeError();
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
