wallace created this revision. wallace added reviewers: jj10306, persona0220. Herald added a project: All. wallace requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits.
The low-level decoder might fall into an infinite decoding loop for various reasons, the simplest being an infinite direct loop reached due to wrong handling of self-modified in the kernel, e.g. 0x0A: pause 0x0C: jump to 0x0A In this case, all the code is sequential and requires no packets to be decoded. The low-level decoder would produce an output like the following 0x0A: pause 0x0C: jump to 0x0A 0x0A: pause 0x0C: jump to 0x0A 0x0A: pause 0x0C: jump to 0x0A ... infinite amount of times These cases are pretty much undecodable, so the least we can do is to identify if we have found a case like this one and show then an error in the trace. - Add a check that breaks decoding of a single PSB once 200k instructions have been decoding after the last packet was processed. - Add a `settings` property for tweaking this number. This is nice because does the basic work needed for future settings. Some notes are added in the code. I haven't been unable to create a test case, but it's found in the thread #12 of the trace 72533820-3eb8-4465-b8e4-4e6bf0ccca99 at Meta. We have to figure out how to artificially create traces with this kind of anomalies. With this change, that anomalous thread now shows: (lldb) thread trace dump instructions 12 -e -i 213100 thread #12: tid = 8 ...missing instructions 213100: (error) decoding truncated: possible infinite decoding loop detected vmlinux-5.12.0-0_fbk8_clang_6656_gc85768aa64da`panic_smp_self_stop + 7 at panic.c:87:2 213099: 0xffffffff81342787 jmp 0xffffffff81342785 ; <+5> [inlined] rep_nop at processor.h:13:2 vmlinux-5.12.0-0_fbk8_clang_6656_gc85768aa64da`panic_smp_self_stop + 5 [inlined] rep_nop at processor.h:13:2 213098: 0xffffffff81342785 pause vmlinux-5.12.0-0_fbk8_clang_6656_gc85768aa64da`panic_smp_self_stop + 7 at panic.c:87:2 213097: 0xffffffff81342787 jmp 0xffffffff81342785 ; <+5> [inlined] rep_nop at processor.h:13:2 vmlinux-5.12.0-0_fbk8_clang_6656_gc85768aa64da`panic_smp_self_stop + 5 [inlined] rep_nop at processor.h:13:2 213096: 0xffffffff81342785 pause vmlinux-5.12.0-0_fbk8_clang_6656_gc85768aa64da`panic_smp_self_stop + 7 at panic.c:87:2 213095: 0xffffffff81342787 jmp 0xffffffff81342785 ; <+5> [inlined] rep_nop at processor.h:13:2 vmlinux-5.12.0-0_fbk8_clang_6656_gc85768aa64da`panic_smp_self_stop + 5 [inlined] rep_nop at processor.h:13:2 213094: 0xffffffff81342785 pause vmlinux-5.12.0-0_fbk8_clang_6656_gc85768aa64da`panic_smp_self_stop + 7 at panic.c:87:2 213093: 0xffffffff81342787 jmp 0xffffffff81342785 ; <+5> [inlined] rep_nop at processor.h:13:2 ... It used to be in an infinite loop. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D136557 Files: lldb/include/lldb/Core/PluginManager.h lldb/source/Core/PluginManager.cpp lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h lldb/source/Plugins/Trace/intel-pt/TraceIntelPTProperties.td
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTProperties.td =================================================================== --- /dev/null +++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTProperties.td @@ -0,0 +1,13 @@ +include "../../../../include/lldb/Core/PropertiesBase.td" + +let Definition = "traceintelpt" in { + def InfiniteDecodingLoopVerificationThreshold: + Property<"infinite-decoding-loop-veritication-threshold", "UInt64">, + Global, + DefaultUnsignedValue<200000>, + Desc<"Specify how many instructions following an individual Intel PT " + "packet must have been decoded before triggering the verification for " + "infinite decoding loops, which might happen if the image is corrupted, " + "such as with self-modifying code. If an infinite loop is detected, an " + "error will be included inside the trace.">; +} Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h =================================================================== --- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h +++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h @@ -22,6 +22,21 @@ class TraceIntelPT : public Trace { public: + /// Properties to be used with the `settings` command. + class PluginProperties : public Properties { + public: + static ConstString GetSettingName(); + + PluginProperties(); + + ~PluginProperties() override = default; + + uint64_t GetInfiniteDecodingLoopVerificationThreshold(); + }; + + /// Return the global properties for this trace plug-in. + static PluginProperties &GetGlobalProperties(); + void Dump(Stream *s) const override; llvm::Expected<FileSpec> SaveToDisk(FileSpec directory, @@ -59,6 +74,8 @@ CreateInstanceForLiveProcess(Process &process); static llvm::StringRef GetPluginNameStatic() { return "intel-pt"; } + + static void DebuggerInitialize(Debugger &debugger); /// \} lldb::CommandObjectSP Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp =================================================================== --- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp +++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp @@ -16,6 +16,7 @@ #include "TraceIntelPTBundleSaver.h" #include "TraceIntelPTConstants.h" #include "lldb/Core/PluginManager.h" +#include "lldb/Interpreter/OptionValueProperties.h" #include "lldb/Target/Process.h" #include "lldb/Target/Target.h" #include "llvm/ADT/None.h" @@ -39,11 +40,51 @@ new CommandObjectThreadTraceStartIntelPT(*this, interpreter)); } +#define LLDB_PROPERTIES_traceintelpt +#include "TraceIntelPTProperties.inc" + +enum { +#define LLDB_PROPERTIES_traceintelpt +#include "TraceIntelPTPropertiesEnum.inc" +}; + +ConstString TraceIntelPT::PluginProperties::GetSettingName() { + return ConstString(TraceIntelPT::GetPluginNameStatic()); +} + +TraceIntelPT::PluginProperties::PluginProperties() : Properties() { + m_collection_sp = std::make_shared<OptionValueProperties>(GetSettingName()); + m_collection_sp->Initialize(g_traceintelpt_properties); +} + +uint64_t +TraceIntelPT::PluginProperties::GetInfiniteDecodingLoopVerificationThreshold() { + const uint32_t idx = ePropertyInfiniteDecodingLoopVerificationThreshold; + return m_collection_sp->GetPropertyAtIndexAsUInt64( + nullptr, idx, g_traceintelpt_properties[idx].default_uint_value); +} + +TraceIntelPT::PluginProperties &TraceIntelPT::GetGlobalProperties() { + static TraceIntelPT::PluginProperties g_settings; + return g_settings; +} + void TraceIntelPT::Initialize() { - PluginManager::RegisterPlugin(GetPluginNameStatic(), "Intel Processor Trace", - CreateInstanceForTraceBundle, - CreateInstanceForLiveProcess, - TraceIntelPTBundleLoader::GetSchema()); + PluginManager::RegisterPlugin( + GetPluginNameStatic(), "Intel Processor Trace", + CreateInstanceForTraceBundle, CreateInstanceForLiveProcess, + TraceIntelPTBundleLoader::GetSchema(), DebuggerInitialize); +} + +void TraceIntelPT::DebuggerInitialize(Debugger &debugger) { + if (!PluginManager::GetSettingForProcessPlugin( + debugger, PluginProperties::GetSettingName())) { + const bool is_global_setting = true; + PluginManager::CreateSettingForTracePlugin( + debugger, GetGlobalProperties().GetValueProperties(), + ConstString("Properties for the intel-pt trace plug-in."), + is_global_setting); + } } void TraceIntelPT::Terminate() { 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,10 @@ /// 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, TraceIntelPT &trace_intel_pt) : 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_trace_intel_pt(trace_intel_pt) {} /// \param[in] trace_intel_pt /// The main Trace object that own the PSB block. @@ -192,7 +193,7 @@ return decoder_up.takeError(); return PSBBlockDecoder(std::move(*decoder_up), psb_block, next_block_ip, - decoded_thread); + decoded_thread, trace_intel_pt); } void DecodePSBBlock() { @@ -213,12 +214,65 @@ } private: - /// Decode all the instructions and events of the given PSB block. - /// - /// \param[in] status - /// The status that was result of synchronizing to the most recent PSB. + /// Decode all the instructions and events of the given PSB block. The + /// decoding loop might stop abruptly if an infinite decoding loop is + /// detected. void DecodeInstructionsAndEvents(int status) { pt_insn insn; + size_t insn_added_since_last_packet = 0; + lldb::addr_t last_packet_offset = LLDB_INVALID_ADDRESS; + const size_t infinite_loop_threshold = + m_trace_intel_pt.GetGlobalProperties() + .GetInfiniteDecodingLoopVerificationThreshold(); + + // Append an instruction and return false if and only if an infinite + // decoding loop has been detected. + auto append_instruction = [&]() -> bool { + m_decoded_thread.AppendInstruction(insn); + + // We want to check if we might have fallen in an infinite loop. We do so + // by checking how many instructions we have added since we processed the + // last Intel PT packet. This number should be low, because at some point + // we should see branches or interrupts that will require a new packet to + // be processed. + lldb::addr_t new_packet_offset; + if (!IsLibiptError( + pt_insn_get_offset(m_decoder_up.get(), &new_packet_offset)) && + new_packet_offset != last_packet_offset) { + last_packet_offset = new_packet_offset; + insn_added_since_last_packet = 0; + } + + // We don't expect a single packet to give, say, 100k instructions. That + // would mean that there are 100k sequential instructions without any + // single branch, which is highly unlikely, or that we found an infinite + // loop using direct jumps, e.g. + // + // 0x0A: nop or pause + // 0x0C: jump to 0x0A + // + // which is indeed code that is found in the kernel. I presume we reach + // this kind of code in the decoder because we don't handle self-modified + // code. + // + // As a note for the future, right now we only check for the + // number of instructions, but if this fails, we need to really identify + // if there's an infinite loop, which is doable in linear time using the + // Knuth-Morris-Pratt algorithm looking for any suffix of the trace that + // repeats itself. + // + // Finally, we are right now only signaling an error, but it would be + // more conservative to also discard all the trace items found in this + // PSB. I prefer not to do that now to give more exposure to this kind + // of anomalies. + if (++insn_added_since_last_packet >= infinite_loop_threshold) { + m_decoded_thread.AppendCustomError( + "decoding truncated: possible infinite decoding loop detected"); + return false; + } + return true; + }; + while (true) { status = ProcessPTEvents(status); @@ -238,7 +292,9 @@ } else if (IsEndOfStream(status)) { break; } - m_decoded_thread.AppendInstruction(insn); + + if (!append_instruction()) + return; } // We need to keep querying non-branching instructions until we hit the @@ -247,7 +303,8 @@ // https://github.com/intel/libipt/blob/master/doc/howto_libipt.md#parallel-decode if (m_next_block_ip && insn.ip != 0) { while (insn.ip != *m_next_block_ip) { - m_decoded_thread.AppendInstruction(insn); + if (!append_instruction()) + return; status = pt_insn_next(m_decoder_up.get(), &insn, sizeof(insn)); @@ -313,6 +370,7 @@ PSBBlock m_psb_block; Optional<lldb::addr_t> m_next_block_ip; DecodedThread &m_decoded_thread; + TraceIntelPT &m_trace_intel_pt; }; Error lldb_private::trace_intel_pt::DecodeSingleTraceForThread( Index: lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt =================================================================== --- lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt +++ lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt @@ -13,6 +13,14 @@ SOURCE TraceIntelPTOptions.td TARGET TraceIntelPTOptionsGen) +lldb_tablegen(TraceIntelPTProperties.inc -gen-lldb-property-defs + SOURCE TraceIntelPTProperties.td + TARGET TraceIntelPTPropertiesGen) + +lldb_tablegen(TraceIntelPTPropertiesEnum.inc -gen-lldb-property-enum-defs + SOURCE TraceIntelPTProperties.td + TARGET TraceIntelPTPropertiesEnumGen) + add_lldb_library(lldbPluginTraceIntelPT PLUGIN CommandObjectTraceStartIntelPT.cpp DecodedThread.cpp @@ -38,4 +46,7 @@ ) -add_dependencies(lldbPluginTraceIntelPT TraceIntelPTOptionsGen) +add_dependencies(lldbPluginTraceIntelPT + TraceIntelPTOptionsGen + TraceIntelPTPropertiesGen + TraceIntelPTPropertiesEnumGen) Index: lldb/source/Core/PluginManager.cpp =================================================================== --- lldb/source/Core/PluginManager.cpp +++ lldb/source/Core/PluginManager.cpp @@ -1051,9 +1051,10 @@ llvm::StringRef name, llvm::StringRef description, CallbackType create_callback_from_bundle, TraceCreateInstanceForLiveProcess create_callback_for_live_process, - llvm::StringRef schema) + llvm::StringRef schema, DebuggerInitializeCallback debugger_init_callback) : PluginInstance<TraceCreateInstanceFromBundle>( - name, description, create_callback_from_bundle), + name, description, create_callback_from_bundle, + debugger_init_callback), schema(schema), create_callback_for_live_process(create_callback_for_live_process) {} @@ -1072,10 +1073,10 @@ llvm::StringRef name, llvm::StringRef description, TraceCreateInstanceFromBundle create_callback_from_bundle, TraceCreateInstanceForLiveProcess create_callback_for_live_process, - llvm::StringRef schema) { + llvm::StringRef schema, DebuggerInitializeCallback debugger_init_callback) { return GetTracePluginInstances().RegisterPlugin( name, description, create_callback_from_bundle, - create_callback_for_live_process, schema); + create_callback_for_live_process, schema, debugger_init_callback); } bool PluginManager::UnregisterPlugin( @@ -1506,6 +1507,7 @@ static const char *kDynamicLoaderPluginName("dynamic-loader"); static const char *kPlatformPluginName("platform"); static const char *kProcessPluginName("process"); +static const char *kTracePluginName("trace"); static const char *kObjectFilePluginName("object-file"); static const char *kSymbolFilePluginName("symbol-file"); static const char *kJITLoaderPluginName("jit-loader"); @@ -1559,6 +1561,14 @@ properties_sp, description, is_global_property); } +bool PluginManager::CreateSettingForTracePlugin( + Debugger &debugger, const lldb::OptionValuePropertiesSP &properties_sp, + ConstString description, bool is_global_property) { + return CreateSettingForPlugin(debugger, ConstString(kTracePluginName), + ConstString("Settings for trace plug-ins"), + properties_sp, description, is_global_property); +} + lldb::OptionValuePropertiesSP PluginManager::GetSettingForObjectFilePlugin(Debugger &debugger, ConstString setting_name) { Index: lldb/include/lldb/Core/PluginManager.h =================================================================== --- lldb/include/lldb/Core/PluginManager.h +++ lldb/include/lldb/Core/PluginManager.h @@ -342,7 +342,8 @@ llvm::StringRef name, llvm::StringRef description, TraceCreateInstanceFromBundle create_callback_from_bundle, TraceCreateInstanceForLiveProcess create_callback_for_live_process, - llvm::StringRef schema); + llvm::StringRef schema, + DebuggerInitializeCallback debugger_init_callback); static bool UnregisterPlugin(TraceCreateInstanceFromBundle create_callback); @@ -487,6 +488,10 @@ Debugger &debugger, const lldb::OptionValuePropertiesSP &properties_sp, ConstString description, bool is_global_property); + static bool CreateSettingForTracePlugin( + Debugger &debugger, const lldb::OptionValuePropertiesSP &properties_sp, + ConstString description, bool is_global_property); + static lldb::OptionValuePropertiesSP GetSettingForObjectFilePlugin(Debugger &debugger, ConstString setting_name);
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits