Skip to content

Commit f6eb089

Browse files
committed
[trace][intelpt] Fix multi CPU decoding TSC assertion error
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: Differential Revision: https://reviews.llvm.org/D136610
1 parent fccff3f commit f6eb089

File tree

1 file changed

+56
-9
lines changed

1 file changed

+56
-9
lines changed

lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp

Lines changed: 56 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -329,12 +329,18 @@ class PSBBlockDecoder {
329329
/// \param[in] decoded_thread
330330
/// A \a DecodedThread object where the decoded instructions will be
331331
/// appended to. It might have already some instructions.
332+
///
333+
/// \param[in] tsc_upper_bound
334+
/// Maximum allowed value of TSCs decoded from this PSB block.
335+
/// Any of this PSB's data occurring after this TSC will be excluded.
332336
PSBBlockDecoder(PtInsnDecoderUP &&decoder_up, const PSBBlock &psb_block,
333337
Optional<lldb::addr_t> next_block_ip,
334-
DecodedThread &decoded_thread, TraceIntelPT &trace_intel_pt)
338+
DecodedThread &decoded_thread, TraceIntelPT &trace_intel_pt,
339+
llvm::Optional<DecodedThread::TSC> tsc_upper_bound)
335340
: m_decoder_up(std::move(decoder_up)), m_psb_block(psb_block),
336341
m_next_block_ip(next_block_ip), m_decoded_thread(decoded_thread),
337-
m_anomaly_detector(*m_decoder_up, trace_intel_pt, decoded_thread) {}
342+
m_anomaly_detector(*m_decoder_up, trace_intel_pt, decoded_thread),
343+
m_tsc_upper_bound(tsc_upper_bound) {}
338344

339345
/// \param[in] trace_intel_pt
340346
/// The main Trace object that own the PSB block.
@@ -362,14 +368,15 @@ class PSBBlockDecoder {
362368
static Expected<PSBBlockDecoder>
363369
Create(TraceIntelPT &trace_intel_pt, const PSBBlock &psb_block,
364370
ArrayRef<uint8_t> buffer, Process &process,
365-
Optional<lldb::addr_t> next_block_ip, DecodedThread &decoded_thread) {
371+
Optional<lldb::addr_t> next_block_ip, DecodedThread &decoded_thread,
372+
llvm::Optional<DecodedThread::TSC> tsc_upper_bound) {
366373
Expected<PtInsnDecoderUP> decoder_up =
367374
CreateInstructionDecoder(trace_intel_pt, buffer, process);
368375
if (!decoder_up)
369376
return decoder_up.takeError();
370377

371378
return PSBBlockDecoder(std::move(*decoder_up), psb_block, next_block_ip,
372-
decoded_thread, trace_intel_pt);
379+
decoded_thread, trace_intel_pt, tsc_upper_bound);
373380
}
374381

375382
void DecodePSBBlock() {
@@ -451,6 +458,41 @@ class PSBBlockDecoder {
451458
}
452459
}
453460

461+
/// Process the TSC of a decoded PT event. Specifically, check if this TSC
462+
/// is below the TSC upper bound for this PSB. If the TSC exceeds the upper
463+
/// bound, return an error to abort decoding. Otherwise add the it to the
464+
/// underlying DecodedThread and decoding should continue as expected.
465+
///
466+
/// \param[in] tsc
467+
/// The TSC of the a decoded event.
468+
Error ProcessPTEventTSC(DecodedThread::TSC tsc) {
469+
if (m_tsc_upper_bound && tsc >= *m_tsc_upper_bound) {
470+
// This event and all the remaining events of this PSB have a TSC
471+
// outside the range of the "owning" ThreadContinuousExecution. For
472+
// now we drop all of these events/instructions, future work can
473+
// improve upon this by determining the "owning"
474+
// ThreadContinuousExecution of the remaining PSB data.
475+
std::string err_msg = formatv("decoding truncated: TSC {0} exceeds "
476+
"maximum TSC value {1}, will skip decoding"
477+
" the remaining data of the PSB",
478+
tsc, *m_tsc_upper_bound)
479+
.str();
480+
481+
uint64_t offset;
482+
int status = pt_insn_get_offset(m_decoder_up.get(), &offset);
483+
if (!IsLibiptError(status)) {
484+
err_msg = formatv("{2} (skipping {0} of {1} bytes)", offset,
485+
m_psb_block.size, err_msg)
486+
.str();
487+
}
488+
m_decoded_thread.AppendCustomError(err_msg);
489+
return createStringError(inconvertibleErrorCode(), err_msg);
490+
} else {
491+
m_decoded_thread.NotifyTsc(tsc);
492+
return Error::success();
493+
}
494+
}
495+
454496
/// Before querying instructions, we need to query the events associated with
455497
/// that instruction, e.g. timing and trace disablement events.
456498
///
@@ -471,8 +513,12 @@ class PSBBlockDecoder {
471513
return status;
472514
}
473515

474-
if (event.has_tsc)
475-
m_decoded_thread.NotifyTsc(event.tsc);
516+
if (event.has_tsc) {
517+
if (Error err = ProcessPTEventTSC(event.tsc)) {
518+
consumeError(std::move(err));
519+
return -pte_internal;
520+
}
521+
}
476522

477523
switch (event.type) {
478524
case ptev_disabled:
@@ -506,6 +552,7 @@ class PSBBlockDecoder {
506552
Optional<lldb::addr_t> m_next_block_ip;
507553
DecodedThread &m_decoded_thread;
508554
PSBBlockAnomalyDetector m_anomaly_detector;
555+
llvm::Optional<DecodedThread::TSC> m_tsc_upper_bound;
509556
};
510557

511558
Error lldb_private::trace_intel_pt::DecodeSingleTraceForThread(
@@ -523,7 +570,7 @@ Error lldb_private::trace_intel_pt::DecodeSingleTraceForThread(
523570
trace_intel_pt, block, buffer.slice(block.psb_offset, block.size),
524571
*decoded_thread.GetThread()->GetProcess(),
525572
i + 1 < blocks->size() ? blocks->at(i + 1).starting_ip : None,
526-
decoded_thread);
573+
decoded_thread, llvm::None);
527574
if (!decoder)
528575
return decoder.takeError();
529576

@@ -585,13 +632,13 @@ Error lldb_private::trace_intel_pt::DecodeSystemWideTraceForThread(
585632

586633
Expected<PSBBlockDecoder> decoder = PSBBlockDecoder::Create(
587634
trace_intel_pt, psb_block,
588-
buffers.lookup(executions[i].thread_execution.cpu_id)
635+
buffers.lookup(execution.thread_execution.cpu_id)
589636
.slice(psb_block.psb_offset, psb_block.size),
590637
*decoded_thread.GetThread()->GetProcess(),
591638
j + 1 < execution.psb_blocks.size()
592639
? execution.psb_blocks[j + 1].starting_ip
593640
: None,
594-
decoded_thread);
641+
decoded_thread, execution.thread_execution.GetEndTSC());
595642
if (!decoder)
596643
return decoder.takeError();
597644

0 commit comments

Comments
 (0)