Author: Dmitri Gribenko Date: 2024-08-22T13:24:57+02:00 New Revision: 547917aebd1e79a8929b53f0ddf3b5185ee4df74
URL: https://github.com/llvm/llvm-project/commit/547917aebd1e79a8929b53f0ddf3b5185ee4df74 DIFF: https://github.com/llvm/llvm-project/commit/547917aebd1e79a8929b53f0ddf3b5185ee4df74.diff LOG: Revert "[lldb] Extend frame recognizers to hide frames from backtraces (#104523)" This reverts commit f01f80ce6ca7640bb0e267b84b1ed0e89b57e2d9. This commit introduces an msan violation. See the discussion on https://github.com/llvm/llvm-project/pull/104523. Added: Modified: lldb/bindings/python/python-wrapper.swig lldb/include/lldb/API/SBFrame.h lldb/include/lldb/Interpreter/ScriptInterpreter.h lldb/include/lldb/Target/StackFrame.h lldb/include/lldb/Target/StackFrameList.h lldb/include/lldb/Target/StackFrameRecognizer.h lldb/include/lldb/Target/Thread.h lldb/source/API/SBFrame.cpp lldb/source/API/SBThread.cpp lldb/source/Commands/CommandCompletions.cpp lldb/source/Commands/CommandObjectFrame.cpp lldb/source/Commands/CommandObjectMemory.cpp lldb/source/Commands/CommandObjectThread.cpp lldb/source/Commands/Options.td lldb/source/Core/Debugger.cpp lldb/source/Interpreter/CommandInterpreter.cpp lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h lldb/source/Target/Process.cpp lldb/source/Target/StackFrame.cpp lldb/source/Target/StackFrameList.cpp lldb/source/Target/StackFrameRecognizer.cpp lldb/source/Target/Thread.cpp lldb/source/Target/ThreadPlanStepOut.cpp lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py lldb/test/API/commands/frame/recognizer/main.m lldb/test/API/commands/frame/recognizer/recognizer.py Removed: lldb/test/API/lang/cpp/std-function-recognizer/Makefile lldb/test/API/lang/cpp/std-function-recognizer/TestStdFunctionRecognizer.py lldb/test/API/lang/cpp/std-function-recognizer/main.cpp ################################################################################ diff --git a/lldb/bindings/python/python-wrapper.swig b/lldb/bindings/python/python-wrapper.swig index 2ce42e3e017d5b..8f050643fa68b3 100644 --- a/lldb/bindings/python/python-wrapper.swig +++ b/lldb/bindings/python/python-wrapper.swig @@ -813,7 +813,7 @@ PythonObject lldb_private::python::SWIGBridge::LLDBSWIGPython_CreateFrameRecogni } PyObject *lldb_private::python::SWIGBridge::LLDBSwigPython_GetRecognizedArguments( - PyObject *implementor, const lldb::StackFrameSP &frame_sp) { + PyObject * implementor, const lldb::StackFrameSP &frame_sp) { static char callee_name[] = "get_recognized_arguments"; PythonObject arg = SWIGBridge::ToSWIGWrapper(frame_sp); @@ -824,22 +824,6 @@ PyObject *lldb_private::python::SWIGBridge::LLDBSwigPython_GetRecognizedArgument return result; } -bool lldb_private::python::SWIGBridge::LLDBSwigPython_ShouldHide( - PyObject *implementor, const lldb::StackFrameSP &frame_sp) { - static char callee_name[] = "should_hide"; - - PythonObject arg = SWIGBridge::ToSWIGWrapper(frame_sp); - - PythonString str(callee_name); - - PyObject *result = - PyObject_CallMethodObjArgs(implementor, str.get(), arg.get(), NULL); - bool ret_val = result ? PyObject_IsTrue(result) : false; - Py_XDECREF(result); - - return result; -} - void *lldb_private::python::SWIGBridge::LLDBSWIGPython_GetDynamicSetting( void *module, const char *setting, const lldb::TargetSP &target_sp) { if (!module || !setting) diff --git a/lldb/include/lldb/API/SBFrame.h b/lldb/include/lldb/API/SBFrame.h index e0d15c3ecc5b1c..821ff3cf7ce519 100644 --- a/lldb/include/lldb/API/SBFrame.h +++ b/lldb/include/lldb/API/SBFrame.h @@ -104,10 +104,6 @@ class LLDB_API SBFrame { bool IsArtificial() const; - /// Return whether a frame recognizer decided this frame should not - /// be displayes in backtraces etc. - bool IsHidden() const; - /// The version that doesn't supply a 'use_dynamic' value will use the /// target's default. lldb::SBValue EvaluateExpression(const char *expr); diff --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h b/lldb/include/lldb/Interpreter/ScriptInterpreter.h index 89a480a28880aa..05f0d7f0955f3e 100644 --- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h +++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h @@ -252,11 +252,6 @@ class ScriptInterpreter : public PluginInterface { return lldb::ValueObjectListSP(); } - virtual bool ShouldHide(const StructuredData::ObjectSP &implementor, - lldb::StackFrameSP frame_sp) { - return false; - } - virtual StructuredData::GenericSP CreateScriptedBreakpointResolver(const char *class_name, const StructuredDataImpl &args_data, diff --git a/lldb/include/lldb/Target/StackFrame.h b/lldb/include/lldb/Target/StackFrame.h index e4d17847763acf..52f0a1ee662176 100644 --- a/lldb/include/lldb/Target/StackFrame.h +++ b/lldb/include/lldb/Target/StackFrame.h @@ -407,11 +407,6 @@ class StackFrame : public ExecutionContextScope, /// may have limited support for inspecting variables. bool IsArtificial() const; - /// Query whether this frame should be hidden from backtraces. Frame - /// recognizers can customize this behavior and hide distracting - /// system implementation details this way. - bool IsHidden(); - /// Query this frame to find what frame it is in this Thread's /// StackFrameList. /// @@ -523,36 +518,33 @@ class StackFrame : public ExecutionContextScope, bool HasCachedData() const; private: - /// For StackFrame only. - /// \{ + // For StackFrame only lldb::ThreadWP m_thread_wp; uint32_t m_frame_index; uint32_t m_concrete_frame_index; lldb::RegisterContextSP m_reg_context_sp; StackID m_id; - /// \} - - /// The frame code address (might not be the same as the actual PC - /// for inlined frames) as a section/offset address. - Address m_frame_code_addr; + Address m_frame_code_addr; // The frame code address (might not be the same as + // the actual PC for inlined frames) as a + // section/offset address SymbolContext m_sc; Flags m_flags; Scalar m_frame_base; Status m_frame_base_error; - uint16_t m_frame_recognizer_generation; - /// Does this frame have a CFA? Different from CFA == LLDB_INVALID_ADDRESS. - bool m_cfa_is_valid; + bool m_cfa_is_valid; // Does this frame have a CFA? Different from CFA == + // LLDB_INVALID_ADDRESS Kind m_stack_frame_kind; - /// Whether this frame behaves like the zeroth frame, in the sense - /// that its pc value might not immediately follow a call (and thus might - /// be the first address of its function). True for actual frame zero as - /// well as any other frame with the same trait. + // Whether this frame behaves like the zeroth frame, in the sense + // that its pc value might not immediately follow a call (and thus might + // be the first address of its function). True for actual frame zero as + // well as any other frame with the same trait. bool m_behaves_like_zeroth_frame; lldb::VariableListSP m_variable_list_sp; - /// Value objects for each variable in m_variable_list_sp. - ValueObjectList m_variable_list_value_objects; - std::optional<lldb::RecognizedStackFrameSP> m_recognized_frame_sp; + ValueObjectList m_variable_list_value_objects; // Value objects for each + // variable in + // m_variable_list_sp + lldb::RecognizedStackFrameSP m_recognized_frame_sp; StreamString m_disassembly; std::recursive_mutex m_mutex; diff --git a/lldb/include/lldb/Target/StackFrameList.h b/lldb/include/lldb/Target/StackFrameList.h index 7d0e7a5b9a71b2..88e211ff692bd9 100644 --- a/lldb/include/lldb/Target/StackFrameList.h +++ b/lldb/include/lldb/Target/StackFrameList.h @@ -91,7 +91,7 @@ class StackFrameList { size_t GetStatus(Stream &strm, uint32_t first_frame, uint32_t num_frames, bool show_frame_info, uint32_t num_frames_with_source, - bool show_unique = false, bool show_hidden = false, + bool show_unique = false, const char *frame_marker = nullptr); protected: diff --git a/lldb/include/lldb/Target/StackFrameRecognizer.h b/lldb/include/lldb/Target/StackFrameRecognizer.h index 8acebc12c4b1dc..e9ac2750192ef6 100644 --- a/lldb/include/lldb/Target/StackFrameRecognizer.h +++ b/lldb/include/lldb/Target/StackFrameRecognizer.h @@ -17,7 +17,6 @@ #include "lldb/lldb-private-forward.h" #include "lldb/lldb-public.h" -#include <cstdint> #include <deque> #include <optional> #include <vector> @@ -29,23 +28,20 @@ namespace lldb_private { /// This class provides extra information about a stack frame that was /// provided by a specific stack frame recognizer. Right now, this class only /// holds recognized arguments (via GetRecognizedArguments). + class RecognizedStackFrame : public std::enable_shared_from_this<RecognizedStackFrame> { public: - virtual ~RecognizedStackFrame() = default; - virtual lldb::ValueObjectListSP GetRecognizedArguments() { return m_arguments; } virtual lldb::ValueObjectSP GetExceptionObject() { return lldb::ValueObjectSP(); } - virtual lldb::StackFrameSP GetMostRelevantFrame() { return nullptr; } + virtual lldb::StackFrameSP GetMostRelevantFrame() { return nullptr; }; + virtual ~RecognizedStackFrame() = default; std::string GetStopDescription() { return m_stop_desc; } - /// Controls whether this frame should be filtered out when - /// displaying backtraces, for example. - virtual bool ShouldHide() { return false; } protected: lldb::ValueObjectListSP m_arguments; @@ -57,6 +53,7 @@ class RecognizedStackFrame /// A base class for frame recognizers. Subclasses (actual frame recognizers) /// should implement RecognizeFrame to provide a RecognizedStackFrame for a /// given stack frame. + class StackFrameRecognizer : public std::enable_shared_from_this<StackFrameRecognizer> { public: @@ -76,10 +73,10 @@ class StackFrameRecognizer /// Python implementation for frame recognizers. An instance of this class /// tracks a particular Python classobject, which will be asked to recognize /// stack frames. + class ScriptedStackFrameRecognizer : public StackFrameRecognizer { lldb_private::ScriptInterpreter *m_interpreter; lldb_private::StructuredData::ObjectSP m_python_object_sp; - std::string m_python_class; public: @@ -126,14 +123,8 @@ class StackFrameRecognizerManager { lldb::StackFrameRecognizerSP GetRecognizerForFrame(lldb::StackFrameSP frame); lldb::RecognizedStackFrameSP RecognizeFrame(lldb::StackFrameSP frame); - /// Returns a number that changes whenever the list of recognizers - /// has been modified. - uint16_t GetGeneration() const { return m_generation; } private: - /// Increase the generation counter. - void BumpGeneration(); - struct RegisteredEntry { uint32_t recognizer_id; lldb::StackFrameRecognizerSP recognizer; @@ -146,7 +137,6 @@ class StackFrameRecognizerManager { }; std::deque<RegisteredEntry> m_recognizers; - uint16_t m_generation; }; /// \class ValueObjectRecognizerSynthesizedValue @@ -154,6 +144,7 @@ class StackFrameRecognizerManager { /// ValueObject subclass that presents the passed ValueObject as a recognized /// value with the specified ValueType. Frame recognizers should return /// instances of this class as the returned objects in GetRecognizedArguments(). + class ValueObjectRecognizerSynthesizedValue : public ValueObject { public: static lldb::ValueObjectSP Create(ValueObject &parent, lldb::ValueType type) { diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index 38b65b2bc58490..aacc59c292ec79 100644 --- a/lldb/include/lldb/Target/Thread.h +++ b/lldb/include/lldb/Target/Thread.h @@ -1128,11 +1128,11 @@ class Thread : public std::enable_shared_from_this<Thread>, size_t GetStatus(Stream &strm, uint32_t start_frame, uint32_t num_frames, uint32_t num_frames_with_source, bool stop_format, - bool show_hidden, bool only_stacks = false); + bool only_stacks = false); size_t GetStackFrameStatus(Stream &strm, uint32_t first_frame, uint32_t num_frames, bool show_frame_info, - uint32_t num_frames_with_source, bool show_hidden); + uint32_t num_frames_with_source); // We need a way to verify that even though we have a thread in a shared // pointer that the object itself is still valid. Currently this won't be the diff --git a/lldb/source/API/SBFrame.cpp b/lldb/source/API/SBFrame.cpp index 2689ecb2ab7bc7..47fc88625e30c5 100644 --- a/lldb/source/API/SBFrame.cpp +++ b/lldb/source/API/SBFrame.cpp @@ -1195,24 +1195,13 @@ bool SBFrame::IsArtificial() const { std::unique_lock<std::recursive_mutex> lock; ExecutionContext exe_ctx(m_opaque_sp.get(), lock); - if (StackFrame *frame = exe_ctx.GetFramePtr()) + StackFrame *frame = exe_ctx.GetFramePtr(); + if (frame) return frame->IsArtificial(); return false; } -bool SBFrame::IsHidden() const { - LLDB_INSTRUMENT_VA(this); - - std::unique_lock<std::recursive_mutex> lock; - ExecutionContext exe_ctx(m_opaque_sp.get(), lock); - - if (StackFrame *frame = exe_ctx.GetFramePtr()) - return frame->IsHidden(); - - return false; -} - const char *SBFrame::GetFunctionName() { LLDB_INSTRUMENT_VA(this); diff --git a/lldb/source/API/SBThread.cpp b/lldb/source/API/SBThread.cpp index 140a2920f05673..786f62bd66d520 100644 --- a/lldb/source/API/SBThread.cpp +++ b/lldb/source/API/SBThread.cpp @@ -1208,8 +1208,7 @@ bool SBThread::GetStatus(SBStream &status) const { ExecutionContext exe_ctx(m_opaque_sp.get(), lock); if (exe_ctx.HasThreadScope()) { - exe_ctx.GetThreadPtr()->GetStatus(strm, 0, 1, 1, true, - /*show_hidden=*/true); + exe_ctx.GetThreadPtr()->GetStatus(strm, 0, 1, 1, true); } else strm.PutCString("No status"); diff --git a/lldb/source/Commands/CommandCompletions.cpp b/lldb/source/Commands/CommandCompletions.cpp index 216aaf9abce6cf..54f4b368166492 100644 --- a/lldb/source/Commands/CommandCompletions.cpp +++ b/lldb/source/Commands/CommandCompletions.cpp @@ -791,7 +791,7 @@ void CommandCompletions::ThreadIndexes(CommandInterpreter &interpreter, lldb::ThreadSP thread_sp; for (uint32_t idx = 0; (thread_sp = threads.GetThreadAtIndex(idx)); ++idx) { StreamString strm; - thread_sp->GetStatus(strm, 0, 1, 1, true, /*show_hidden*/ true); + thread_sp->GetStatus(strm, 0, 1, 1, true); request.TryCompleteCurrentArg(std::to_string(thread_sp->GetIndexID()), strm.GetString()); } @@ -835,7 +835,7 @@ void CommandCompletions::ThreadIDs(CommandInterpreter &interpreter, lldb::ThreadSP thread_sp; for (uint32_t idx = 0; (thread_sp = threads.GetThreadAtIndex(idx)); ++idx) { StreamString strm; - thread_sp->GetStatus(strm, 0, 1, 1, true, /*show_hidden*/ true); + thread_sp->GetStatus(strm, 0, 1, 1, true); request.TryCompleteCurrentArg(std::to_string(thread_sp->GetID()), strm.GetString()); } diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp index 46c75e3dd159c0..29e460fe3885ff 100644 --- a/lldb/source/Commands/CommandObjectFrame.cpp +++ b/lldb/source/Commands/CommandObjectFrame.cpp @@ -278,30 +278,6 @@ class CommandObjectFrameSelect : public CommandObjectParsed { if (frame_idx == UINT32_MAX) frame_idx = 0; - // If moving up/down by one, skip over hidden frames. - if (*m_options.relative_frame_offset == 1 || - *m_options.relative_frame_offset == -1) { - uint32_t candidate_idx = frame_idx; - const unsigned max_depth = 12; - for (unsigned num_try = 0; num_try < max_depth; ++num_try) { - if (candidate_idx == 0 && *m_options.relative_frame_offset == -1) { - candidate_idx = UINT32_MAX; - break; - } - candidate_idx += *m_options.relative_frame_offset; - if (auto candidate_sp = thread->GetStackFrameAtIndex(candidate_idx)) { - if (candidate_sp->IsHidden()) - continue; - // Now candidate_idx is the first non-hidden frame. - break; - } - candidate_idx = UINT32_MAX; - break; - }; - if (candidate_idx != UINT32_MAX) - m_options.relative_frame_offset = candidate_idx - frame_idx; - } - if (*m_options.relative_frame_offset < 0) { if (static_cast<int32_t>(frame_idx) >= -*m_options.relative_frame_offset) diff --git a/lldb/source/Commands/CommandObjectMemory.cpp b/lldb/source/Commands/CommandObjectMemory.cpp index baf5d9196e553e..137b1ad981073c 100644 --- a/lldb/source/Commands/CommandObjectMemory.cpp +++ b/lldb/source/Commands/CommandObjectMemory.cpp @@ -1570,8 +1570,7 @@ class CommandObjectMemoryHistory : public CommandObjectParsed { const bool stop_format = false; for (auto thread : thread_list) { - thread->GetStatus(*output_stream, 0, UINT32_MAX, 0, stop_format, - /*should_filter*/ false); + thread->GetStatus(*output_stream, 0, UINT32_MAX, 0, stop_format); } result.SetStatus(eReturnStatusSuccessFinishResult); diff --git a/lldb/source/Commands/CommandObjectThread.cpp b/lldb/source/Commands/CommandObjectThread.cpp index 6a89c163f37d51..605f872a9f45e1 100644 --- a/lldb/source/Commands/CommandObjectThread.cpp +++ b/lldb/source/Commands/CommandObjectThread.cpp @@ -89,9 +89,6 @@ class CommandObjectThreadBacktrace : public CommandObjectIterateOverThreads { "invalid boolean value for option '%c': %s", short_option, option_arg.data()); } break; - case 'u': - m_filtered_backtrace = false; - break; default: llvm_unreachable("Unimplemented option"); } @@ -102,7 +99,6 @@ class CommandObjectThreadBacktrace : public CommandObjectIterateOverThreads { m_count = UINT32_MAX; m_start = 0; m_extended_backtrace = false; - m_filtered_backtrace = true; } llvm::ArrayRef<OptionDefinition> GetDefinitions() override { @@ -113,7 +109,6 @@ class CommandObjectThreadBacktrace : public CommandObjectIterateOverThreads { uint32_t m_count; uint32_t m_start; bool m_extended_backtrace; - bool m_filtered_backtrace; }; CommandObjectThreadBacktrace(CommandInterpreter &interpreter) @@ -126,10 +121,7 @@ class CommandObjectThreadBacktrace : public CommandObjectIterateOverThreads { "call stacks.\n" "Use 'settings set frame-format' to customize the printing of " "frames in the backtrace and 'settings set thread-format' to " - "customize the thread header.\n" - "Customizable frame recognizers may filter out less interesting " - "frames, which results in gaps in the numbering. " - "Use '-u' to see all frames.", + "customize the thread header.", nullptr, eCommandRequiresProcess | eCommandRequiresThread | eCommandTryTargetAPILock | eCommandProcessMustBeLaunched | @@ -207,8 +199,7 @@ class CommandObjectThreadBacktrace : public CommandObjectIterateOverThreads { strm.PutChar('\n'); if (ext_thread_sp->GetStatus(strm, m_options.m_start, m_options.m_count, - num_frames_with_source, stop_format, - !m_options.m_filtered_backtrace)) { + num_frames_with_source, stop_format)) { DoExtendedBacktrace(ext_thread_sp.get(), result); } } @@ -237,8 +228,7 @@ class CommandObjectThreadBacktrace : public CommandObjectIterateOverThreads { const uint32_t num_frames_with_source = 0; const bool stop_format = true; if (!thread->GetStatus(strm, m_options.m_start, m_options.m_count, - num_frames_with_source, stop_format, - !m_options.m_filtered_backtrace, only_stacks)) { + num_frames_with_source, stop_format, only_stacks)) { result.AppendErrorWithFormat( "error displaying backtrace for thread: \"0x%4.4x\"\n", thread->GetIndexID()); @@ -1402,8 +1392,7 @@ class CommandObjectThreadException : public CommandObjectIterateOverThreads { const uint32_t num_frames_with_source = 0; const bool stop_format = false; exception_thread_sp->GetStatus(strm, 0, UINT32_MAX, - num_frames_with_source, stop_format, - /*filtered*/ false); + num_frames_with_source, stop_format); } return true; diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index 9c4dbed6939ba9..f050cd2ebb5ae0 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -1048,8 +1048,6 @@ let Command = "thread backtrace" in { Arg<"FrameIndex">, Desc<"Frame in which to start the backtrace">; def thread_backtrace_extended : Option<"extended", "e">, Group<1>, Arg<"Boolean">, Desc<"Show the extended backtrace, if available">; - def thread_backtrace_unfiltered : Option<"unfiltered", "u">, Group<1>, - Desc<"Filter out frames according to installed frame recognizers">; } let Command = "thread step scope" in { diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index 67f01707a2afee..309e01e456580c 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -1869,8 +1869,7 @@ void Debugger::HandleThreadEvent(const EventSP &event_sp) { ThreadSP thread_sp( Thread::ThreadEventData::GetThreadFromEvent(event_sp.get())); if (thread_sp) { - thread_sp->GetStatus(*GetAsyncOutputStream(), 0, 1, 1, stop_format, - /*show_hidden*/ true); + thread_sp->GetStatus(*GetAsyncOutputStream(), 0, 1, 1, stop_format); } } } diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp index 87298803e8415a..e45112530404b8 100644 --- a/lldb/source/Interpreter/CommandInterpreter.cpp +++ b/lldb/source/Interpreter/CommandInterpreter.cpp @@ -835,12 +835,11 @@ void CommandInterpreter::LoadCommandDictionary() { std::unique_ptr<CommandObjectRegexCommand> bt_regex_cmd_up( new CommandObjectRegexCommand( *this, "_regexp-bt", - "Show backtrace of the current thread's call stack. Any numeric " - "argument displays at most that many frames. The argument 'all' " - "displays all threads. Use 'settings set frame-format' to customize " + "Show backtrace of the current thread's call stack. Any numeric " + "argument displays at most that many frames. The argument 'all' " + "displays all threads. Use 'settings set frame-format' to customize " "the printing of individual frames and 'settings set thread-format' " - "to customize the thread header. Frame recognizers may filter the" - "list. Use 'thread backtrace -u (--unfiltered)' to see them all.", + "to customize the thread header.", "bt [<digit> | all]", 0, false)); if (bt_regex_cmd_up) { // accept but don't document "bt -c <number>" -- before bt was a regex diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp index c60200ab186d09..c7202a47d0157e 100644 --- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp +++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp @@ -26,7 +26,6 @@ #include "lldb/Target/RegisterContext.h" #include "lldb/Target/SectionLoadList.h" #include "lldb/Target/StackFrame.h" -#include "lldb/Target/StackFrameRecognizer.h" #include "lldb/Target/ThreadPlanRunToAddress.h" #include "lldb/Target/ThreadPlanStepInRange.h" #include "lldb/Utility/Timer.h" @@ -41,49 +40,8 @@ static ConstString g_coro_frame = ConstString("__coro_frame"); char CPPLanguageRuntime::ID = 0; -/// A frame recognizer that is installed to hide libc++ implementation -/// details from the backtrace. -class LibCXXFrameRecognizer : public StackFrameRecognizer { - RegularExpression m_hidden_function_regex; - RecognizedStackFrameSP m_hidden_frame; - - struct LibCXXHiddenFrame : public RecognizedStackFrame { - bool ShouldHide() override { return true; } - }; - -public: - LibCXXFrameRecognizer() - : m_hidden_function_regex( - R"(^std::__1::(__function.*::operator\(\)|__invoke))" - R"((\[.*\])?)" // ABI tag. - R"(( const)?$)"), // const. - m_hidden_frame(new LibCXXHiddenFrame()) {} - - std::string GetName() override { return "libc++ frame recognizer"; } - - lldb::RecognizedStackFrameSP - RecognizeFrame(lldb::StackFrameSP frame_sp) override { - if (!frame_sp) - return {}; - const auto &sc = frame_sp->GetSymbolContext(lldb::eSymbolContextFunction); - if (!sc.function) - return {}; - - if (m_hidden_function_regex.Execute(sc.function->GetNameNoArguments())) - return m_hidden_frame; - - return {}; - } -}; - CPPLanguageRuntime::CPPLanguageRuntime(Process *process) - : LanguageRuntime(process) { - if (process) - process->GetTarget().GetFrameRecognizerManager().AddRecognizer( - StackFrameRecognizerSP(new LibCXXFrameRecognizer()), {}, - std::make_shared<RegularExpression>("^std::__1::"), - /*first_instruction_only*/ false); -} + : LanguageRuntime(process) {} bool CPPLanguageRuntime::IsAllowedRuntimeValue(ConstString name) { return name == g_this || name == g_promise || name == g_coro_frame; diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h b/lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h index 5351c1a698b4a7..3026b6113ae8f3 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h @@ -227,9 +227,6 @@ class SWIGBridge { LLDBSwigPython_GetRecognizedArguments(PyObject *implementor, const lldb::StackFrameSP &frame_sp); - static bool LLDBSwigPython_ShouldHide(PyObject *implementor, - const lldb::StackFrameSP &frame_sp); - static bool LLDBSWIGPythonRunScriptKeywordProcess( const char *python_function_name, const char *session_dictionary_name, const lldb::ProcessSP &process, std::string &output); diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp index 2a94f110910400..335c482f8495ad 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp @@ -1524,35 +1524,6 @@ lldb::ValueObjectListSP ScriptInterpreterPythonImpl::GetRecognizedArguments( return ValueObjectListSP(); } -bool ScriptInterpreterPythonImpl::ShouldHide( - const StructuredData::ObjectSP &os_plugin_object_sp, - lldb::StackFrameSP frame_sp) { - Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock); - - if (!os_plugin_object_sp) - return false; - - StructuredData::Generic *generic = os_plugin_object_sp->GetAsGeneric(); - if (!generic) - return false; - - PythonObject implementor(PyRefType::Borrowed, - (PyObject *)generic->GetValue()); - - if (!implementor.IsAllocated()) - return false; - - bool result = - SWIGBridge::LLDBSwigPython_ShouldHide(implementor.get(), frame_sp); - - // if it fails, print the error but otherwise go on - if (PyErr_Occurred()) { - PyErr_Print(); - PyErr_Clear(); - } - return result; -} - ScriptedProcessInterfaceUP ScriptInterpreterPythonImpl::CreateScriptedProcessInterface() { return std::make_unique<ScriptedProcessPythonInterface>(*this); diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h index 85d79955e45efc..c2024efb395d70 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h @@ -107,9 +107,6 @@ class ScriptInterpreterPythonImpl : public ScriptInterpreterPython { GetRecognizedArguments(const StructuredData::ObjectSP &implementor, lldb::StackFrameSP frame_sp) override; - bool ShouldHide(const StructuredData::ObjectSP &implementor, - lldb::StackFrameSP frame_sp) override; - lldb::ScriptedProcessInterfaceUP CreateScriptedProcessInterface() override; lldb::ScriptedThreadInterfaceSP CreateScriptedThreadInterface() override; diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index b2a0f13b9a1549..3c9247fdbbbc96 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -5545,8 +5545,7 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx, // Print a backtrace into the log so we can figure out where we are: StreamString s; s.PutCString("Thread state after unsuccessful completion: \n"); - thread->GetStackFrameStatus(s, 0, UINT32_MAX, true, UINT32_MAX, - /*show_hidden*/ true); + thread->GetStackFrameStatus(s, 0, UINT32_MAX, true, UINT32_MAX); log->PutString(s.GetString()); } // Restore the thread state if we are going to discard the plan execution. @@ -5820,8 +5819,8 @@ size_t Process::GetThreadStatus(Stream &strm, continue; } thread_sp->GetStatus(strm, start_frame, num_frames, - num_frames_with_source, stop_format, - /*show_hidden*/ num_frames <= 1); + num_frames_with_source, + stop_format); ++num_thread_infos_dumped; } else { Log *log = GetLog(LLDBLog::Process); diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp index 0ebaf555f86beb..3a2b4d05b28810 100644 --- a/lldb/source/Target/StackFrame.cpp +++ b/lldb/source/Target/StackFrame.cpp @@ -1198,12 +1198,6 @@ bool StackFrame::IsArtificial() const { return m_stack_frame_kind == StackFrame::Kind::Artificial; } -bool StackFrame::IsHidden() { - if (auto recognized_frame_sp = GetRecognizedFrame()) - return recognized_frame_sp->ShouldHide(); - return false; -} - SourceLanguage StackFrame::GetLanguage() { CompileUnit *cu = GetSymbolContext(eSymbolContextCompUnit).comp_unit; if (cu) @@ -1977,16 +1971,12 @@ bool StackFrame::GetStatus(Stream &strm, bool show_frame_info, bool show_source, } RecognizedStackFrameSP StackFrame::GetRecognizedFrame() { - auto process = GetThread()->GetProcess(); - if (!process) - return {}; - // If recognizer list has been modified, discard cache. - auto &manager = process->GetTarget().GetFrameRecognizerManager(); - auto new_generation = manager.GetGeneration(); - if (m_frame_recognizer_generation != new_generation) - m_recognized_frame_sp.reset(); - m_frame_recognizer_generation = new_generation; - if (!m_recognized_frame_sp.has_value()) - m_recognized_frame_sp = manager.RecognizeFrame(CalculateStackFrame()); - return m_recognized_frame_sp.value(); + if (!m_recognized_frame_sp) { + m_recognized_frame_sp = GetThread() + ->GetProcess() + ->GetTarget() + .GetFrameRecognizerManager() + .RecognizeFrame(CalculateStackFrame()); + } + return m_recognized_frame_sp; } diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp index 7808bd3674ab19..0cf9ce1bf043f5 100644 --- a/lldb/source/Target/StackFrameList.cpp +++ b/lldb/source/Target/StackFrameList.cpp @@ -924,7 +924,7 @@ StackFrameList::GetStackFrameSPForStackFramePtr(StackFrame *stack_frame_ptr) { size_t StackFrameList::GetStatus(Stream &strm, uint32_t first_frame, uint32_t num_frames, bool show_frame_info, uint32_t num_frames_with_source, - bool show_unique, bool show_hidden, + bool show_unique, const char *selected_frame_marker) { size_t num_frames_displayed = 0; @@ -951,6 +951,7 @@ size_t StackFrameList::GetStatus(Stream &strm, uint32_t first_frame, unselected_marker = buffer.c_str(); } const char *marker = nullptr; + for (frame_idx = first_frame; frame_idx < last_frame; ++frame_idx) { frame_sp = GetFrameAtIndex(frame_idx); if (!frame_sp) @@ -962,11 +963,6 @@ size_t StackFrameList::GetStatus(Stream &strm, uint32_t first_frame, else marker = unselected_marker; } - - // Hide uninteresting frames unless it's the selected frame. - if (!show_hidden && frame_sp != selected_frame_sp && frame_sp->IsHidden()) - continue; - // Check for interruption here. If we're fetching arguments, this loop // can go slowly: Debugger &dbg = m_thread.GetProcess()->GetTarget().GetDebugger(); diff --git a/lldb/source/Target/StackFrameRecognizer.cpp b/lldb/source/Target/StackFrameRecognizer.cpp index 44411afc65dda9..0ccb1ae9c031e3 100644 --- a/lldb/source/Target/StackFrameRecognizer.cpp +++ b/lldb/source/Target/StackFrameRecognizer.cpp @@ -17,14 +17,10 @@ using namespace lldb; using namespace lldb_private; class ScriptedRecognizedStackFrame : public RecognizedStackFrame { - bool m_hidden; - public: - ScriptedRecognizedStackFrame(ValueObjectListSP args, bool hidden) - : m_hidden(hidden) { - m_arguments = std::move(args); + ScriptedRecognizedStackFrame(ValueObjectListSP args) { + m_arguments = args; } - bool ShouldHide() override { return m_hidden; } }; ScriptedStackFrameRecognizer::ScriptedStackFrameRecognizer( @@ -42,22 +38,13 @@ ScriptedStackFrameRecognizer::RecognizeFrame(lldb::StackFrameSP frame) { ValueObjectListSP args = m_interpreter->GetRecognizedArguments(m_python_object_sp, frame); auto args_synthesized = ValueObjectListSP(new ValueObjectList()); - if (args) { - for (const auto &o : args->GetObjects()) - args_synthesized->Append(ValueObjectRecognizerSynthesizedValue::Create( - *o, eValueTypeVariableArgument)); + for (const auto &o : args->GetObjects()) { + args_synthesized->Append(ValueObjectRecognizerSynthesizedValue::Create( + *o, eValueTypeVariableArgument)); } - bool hidden = m_interpreter->ShouldHide(m_python_object_sp, frame); - return RecognizedStackFrameSP( - new ScriptedRecognizedStackFrame(args_synthesized, hidden)); -} - -void StackFrameRecognizerManager::BumpGeneration() { - uint32_t n = m_generation; - n = (n + 1) & ((1 << 16) - 1); - m_generation = n; + new ScriptedRecognizedStackFrame(args_synthesized)); } void StackFrameRecognizerManager::AddRecognizer( @@ -66,7 +53,6 @@ void StackFrameRecognizerManager::AddRecognizer( m_recognizers.push_front({(uint32_t)m_recognizers.size(), recognizer, false, module, RegularExpressionSP(), symbols, RegularExpressionSP(), first_instruction_only}); - BumpGeneration(); } void StackFrameRecognizerManager::AddRecognizer( @@ -75,7 +61,6 @@ void StackFrameRecognizerManager::AddRecognizer( m_recognizers.push_front({(uint32_t)m_recognizers.size(), recognizer, true, ConstString(), module, std::vector<ConstString>(), symbol, first_instruction_only}); - BumpGeneration(); } void StackFrameRecognizerManager::ForEach( @@ -112,12 +97,10 @@ bool StackFrameRecognizerManager::RemoveRecognizerWithID( if (found == m_recognizers.end()) return false; m_recognizers.erase(found); - BumpGeneration(); return true; } void StackFrameRecognizerManager::RemoveAllRecognizers() { - BumpGeneration(); m_recognizers.clear(); } diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp index fcf0f4e2519085..74d1a268c6dffb 100644 --- a/lldb/source/Target/Thread.cpp +++ b/lldb/source/Target/Thread.cpp @@ -1748,7 +1748,7 @@ std::string Thread::RunModeAsString(lldb::RunMode mode) { size_t Thread::GetStatus(Stream &strm, uint32_t start_frame, uint32_t num_frames, uint32_t num_frames_with_source, - bool stop_format, bool show_hidden, bool only_stacks) { + bool stop_format, bool only_stacks) { if (!only_stacks) { ExecutionContext exe_ctx(shared_from_this()); @@ -1795,7 +1795,7 @@ size_t Thread::GetStatus(Stream &strm, uint32_t start_frame, num_frames_shown = GetStackFrameList()->GetStatus( strm, start_frame, num_frames, show_frame_info, num_frames_with_source, - show_frame_unique, show_hidden, selected_frame_marker); + show_frame_unique, selected_frame_marker); if (num_frames == 1) strm.IndentLess(); strm.IndentLess(); @@ -1893,11 +1893,9 @@ bool Thread::GetDescription(Stream &strm, lldb::DescriptionLevel level, size_t Thread::GetStackFrameStatus(Stream &strm, uint32_t first_frame, uint32_t num_frames, bool show_frame_info, - uint32_t num_frames_with_source, - bool show_hidden) { - return GetStackFrameList()->GetStatus(strm, first_frame, num_frames, - show_frame_info, num_frames_with_source, - /*show_unique*/ false, show_hidden); + uint32_t num_frames_with_source) { + return GetStackFrameList()->GetStatus( + strm, first_frame, num_frames, show_frame_info, num_frames_with_source); } Unwind &Thread::GetUnwinder() { diff --git a/lldb/source/Target/ThreadPlanStepOut.cpp b/lldb/source/Target/ThreadPlanStepOut.cpp index 8ca1dbc2fe4c46..0a1e2ae605efcf 100644 --- a/lldb/source/Target/ThreadPlanStepOut.cpp +++ b/lldb/source/Target/ThreadPlanStepOut.cpp @@ -58,7 +58,7 @@ ThreadPlanStepOut::ThreadPlanStepOut( return; // we can't do anything here. ValidatePlan() will return false. // While stepping out, behave as-if artificial frames are not present. - while (return_frame_sp->IsArtificial() || return_frame_sp->IsHidden()) { + while (return_frame_sp->IsArtificial()) { m_stepped_past_frames.push_back(return_frame_sp); ++return_frame_index; diff --git a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py index 6174ac61a709dd..eea0aafce6e25e 100644 --- a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py +++ b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py @@ -162,46 +162,6 @@ def test_frame_recognizer_1(self): substrs=['*a = 78']) """ - @skipUnlessDarwin - def test_frame_recognizer_hiding(self): - self.build() - - target, process, thread, _ = lldbutil.run_to_name_breakpoint(self, "nested") - frame = thread.GetSelectedFrame() - - # Sanity check. - self.expect( - "thread backtrace", patterns=["frame.*nested", "frame.*baz", "frame.*main"] - ) - - self.expect("frame recognizer clear") - self.expect( - "command script import " - + os.path.join(self.getSourceDir(), "recognizer.py") - ) - - self.expect( - "frame recognizer add -l recognizer.BazFrameRecognizer -f false -s a.out -n baz" - ) - - self.expect( - "frame recognizer list", - substrs=["0: recognizer.BazFrameRecognizer"], - ) - - # Now main should be hidden. - self.expect("thread backtrace", matching=False, patterns=["frame.*baz"]) - self.assertFalse(frame.IsHidden()) - frame = thread.SetSelectedFrame(1) - self.assertIn("baz", frame.name) - self.assertTrue(frame.IsHidden()) - - # Test StepOut. - frame = thread.SetSelectedFrame(0) - thread.StepOut() - frame = thread.GetSelectedFrame() - self.assertIn("main", frame.name) - @skipUnlessDarwin def test_frame_recognizer_multi_symbol(self): self.build() diff --git a/lldb/test/API/commands/frame/recognizer/main.m b/lldb/test/API/commands/frame/recognizer/main.m index 74d219f1fff4c5..6546692bba772e 100644 --- a/lldb/test/API/commands/frame/recognizer/main.m +++ b/lldb/test/API/commands/frame/recognizer/main.m @@ -1,17 +1,16 @@ #import <stdio.h> -void foo(int a, int b) { printf("%d %d\n", a, b); } +void foo(int a, int b) +{ + printf("%d %d\n", a, b); +} void bar(int *ptr) { printf("%d\n", *ptr); } -void nested(int *ptr) { bar(ptr); } - -void baz(int *ptr) { nested(ptr); } - -int main(int argc, const char *argv[]) { - foo(42, 56); - int i = 78; - bar(&i); - baz(&i); - return 0; +int main (int argc, const char * argv[]) +{ + foo(42, 56); + int i = 78; + bar(&i); + return 0; } diff --git a/lldb/test/API/commands/frame/recognizer/recognizer.py b/lldb/test/API/commands/frame/recognizer/recognizer.py index 98666b720b1e2b..1a2a2d5c265070 100644 --- a/lldb/test/API/commands/frame/recognizer/recognizer.py +++ b/lldb/test/API/commands/frame/recognizer/recognizer.py @@ -36,8 +36,3 @@ def get_recognized_arguments(self, frame): class MyOtherFrameRecognizer(object): def get_recognized_arguments(self, frame): return [] - - -class BazFrameRecognizer(object): - def should_hide(self, frame): - return "baz" in frame.name diff --git a/lldb/test/API/lang/cpp/std-function-recognizer/Makefile b/lldb/test/API/lang/cpp/std-function-recognizer/Makefile deleted file mode 100644 index ab034edd121f9f..00000000000000 --- a/lldb/test/API/lang/cpp/std-function-recognizer/Makefile +++ /dev/null @@ -1,4 +0,0 @@ -CXX_SOURCES := main.cpp -USE_LIBCPP := 1 - -include Makefile.rules diff --git a/lldb/test/API/lang/cpp/std-function-recognizer/TestStdFunctionRecognizer.py b/lldb/test/API/lang/cpp/std-function-recognizer/TestStdFunctionRecognizer.py deleted file mode 100644 index 30fe3ecb1e4bf4..00000000000000 --- a/lldb/test/API/lang/cpp/std-function-recognizer/TestStdFunctionRecognizer.py +++ /dev/null @@ -1,84 +0,0 @@ -import lldb -from lldbsuite.test.decorators import * -from lldbsuite.test.lldbtest import * -from lldbsuite.test import lldbutil - - -class LibCxxStdFunctionRecognizerTestCase(TestBase): - NO_DEBUG_INFO_TESTCASE = True - - @add_test_categories(["libc++"]) - def test_backtrace(self): - """Test that std::function implementation details are hidden in bt""" - self.build() - (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint( - self, "// break here", lldb.SBFileSpec("main.cpp") - ) - # Filtered. - self.expect( - "thread backtrace", - ordered=True, - substrs=["frame", "foo", "frame", "main"], - ) - self.expect( - "thread backtrace", matching=False, patterns=["frame.*std::__1::__function"] - ) - # Unfiltered. - self.expect( - "thread backtrace -u", - ordered=True, - patterns=["frame.*foo", "frame.*std::__1::__function", "frame.*main"], - ) - self.expect( - "thread backtrace --unfiltered", - ordered=True, - patterns=["frame.*foo", "frame.*std::__1::__function", "frame.*main"], - ) - - @add_test_categories(["libc++"]) - def test_up_down(self): - """Test that std::function implementation details are skipped""" - self.build() - (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint( - self, "// break here", lldb.SBFileSpec("main.cpp") - ) - frame = thread.GetSelectedFrame() - # up - self.assertIn("foo", frame.GetFunctionName()) - start_idx = frame.GetFrameID() - i = 0 - while i < thread.GetNumFrames(): - self.expect("up") - frame = thread.GetSelectedFrame() - if frame.GetFunctionName() == "main": - break - end_idx = frame.GetFrameID() - self.assertLess(i, end_idx - start_idx, "skipped frames") - - # Back down again. - start_idx = frame.GetFrameID() - for i in range(1, thread.GetNumFrames()): - self.expect("down") - frame = thread.GetSelectedFrame() - if "foo" in frame.GetFunctionName(): - break - end_idx = frame.GetFrameID() - self.assertLess(i, start_idx - end_idx, "skipped frames") - - @add_test_categories(["libc++"]) - def test_api(self): - """Test that std::function implementation details are skipped""" - self.build() - (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint( - self, "// break here", lldb.SBFileSpec("main.cpp") - ) - frame = thread.GetSelectedFrame() - num_hidden = 0 - for i in range(1, thread.GetNumFrames()): - thread.SetSelectedFrame(i) - frame = thread.GetSelectedFrame() - if frame.IsHidden(): - num_hidden += 1 - - self.assertGreater(num_hidden, 0) - self.assertLess(num_hidden, thread.GetNumFrames()) diff --git a/lldb/test/API/lang/cpp/std-function-recognizer/main.cpp b/lldb/test/API/lang/cpp/std-function-recognizer/main.cpp deleted file mode 100644 index 8cf4eaa2e51929..00000000000000 --- a/lldb/test/API/lang/cpp/std-function-recognizer/main.cpp +++ /dev/null @@ -1,10 +0,0 @@ -#include <functional> - -int foo(int x, int y) { - return x * y; // break here -} - -int main(int argc, char *argv[]) { - std::function<int(int, int)> fn = foo; - return fn(argc, 1); -} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits