Revert "[lldb] Extend frame recognizers to hide frames from backtraces (#104523)"

This reverts commit f01f80c.

This commit introduces an msan violation. See the discussion on #104523.

Author: gribozavr

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)

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);

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,

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;
 

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:

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,14 +137,14 @@ class StackFrameRecognizerManager {
   };
 
   std::deque<RegisteredEntry> m_recognizers;
-  uint16_t m_generation;
 };
 
 /// \class ValueObjectRecognizerSynthesizedValue
 ///
 /// 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) {

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

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);
 

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");
 

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());
   }

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)

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);

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;

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 {