Merge pull request #9549 from ahoppen/revert-swift-call-site-breakpoints

Revert "Merge pull request #9493 from jimingham/swift-call-site-breakpoints"

Author: ahoppen

lldb/include/lldb/Breakpoint/BreakpointLocation.h

@@ -11,12 +11,10 @@
 
 #include <memory>
 #include <mutex>
-#include <optional>
 
 #include "lldb/Breakpoint/BreakpointOptions.h"
 #include "lldb/Breakpoint/StoppointHitCounter.h"
 #include "lldb/Core/Address.h"
-#include "lldb/Symbol/LineEntry.h"
 #include "lldb/Utility/UserID.h"
 #include "lldb/lldb-private.h"
 
@@ -284,25 +282,6 @@ class BreakpointLocation
   /// Returns the breakpoint location ID.
   lldb::break_id_t GetID() const { return m_loc_id; }
 
-  /// Set the line entry that should be shown to users for this location.
-  /// It is up to the caller to verify that this is a valid entry to show.
-  /// The current use of this is to distinguish among line entries from a
-  /// virtual inlined call stack that all share the same address.
-  /// The line entry must have the same start address as the address for this
-  /// location.
-  bool SetPreferredLineEntry(const LineEntry &line_entry) {
-    if (m_address == line_entry.range.GetBaseAddress()) {
-      m_preferred_line_entry = line_entry;
-      return true;
-    }
-    assert(0 && "Tried to set a preferred line entry with a different address");
-    return false;
-  }
-
-  const std::optional<LineEntry> GetPreferredLineEntry() {
-    return m_preferred_line_entry;
-  }
-
 protected:
   friend class BreakpointSite;
   friend class BreakpointLocationList;
@@ -327,16 +306,6 @@ class BreakpointLocation
   /// If it returns false we should continue, otherwise stop.
   bool IgnoreCountShouldStop();
 
-  /// If this location knows that the virtual stack frame it represents is
-  /// not frame 0, return the suggested stack frame instead.  This will happen
-  /// when the location's address contains a "virtual inlined call stack" and
-  /// the breakpoint was set on a file & line that are not at the bottom of that
-  /// stack.  For now we key off the "preferred line entry" - looking for that
-  /// in the blocks that start with the stop PC.
-  /// This version of the API doesn't take an "inlined" parameter because it
-  /// only changes frames in the inline stack.
-  std::optional<uint32_t> GetSuggestedStackFrameIndex();
-
 private:
   void SwapLocation(lldb::BreakpointLocationSP swap_from);
 
@@ -400,11 +369,6 @@ class BreakpointLocation
   lldb::break_id_t m_loc_id; ///< Breakpoint location ID.
   StoppointHitCounter m_hit_counter; ///< Number of times this breakpoint
                                      /// location has been hit.
-  /// If this exists, use it to print the stop description rather than the
-  /// LineEntry m_address resolves to directly.  Use this for instance when the
-  /// location was given somewhere in the virtual inlined call stack since the
-  /// Address always resolves to the lowest entry in the stack.
-  std::optional<LineEntry> m_preferred_line_entry;
 
   void SetShouldResolveIndirectFunctions(bool do_resolve) {
     m_should_resolve_indirect_functions = do_resolve;

lldb/include/lldb/Breakpoint/BreakpointSite.h

@@ -170,11 +170,6 @@ class BreakpointSite : public std::enable_shared_from_this<BreakpointSite>,
   /// \see lldb::DescriptionLevel
   void GetDescription(Stream *s, lldb::DescriptionLevel level);
 
-  // This runs through all the breakpoint locations owning this site and returns
-  // the greatest of their suggested stack frame indexes.  This only handles
-  // inlined stack changes.
-  std::optional<uint32_t> GetSuggestedStackFrameIndex();
-
   /// Tell whether a breakpoint has a location at this site.
   ///
   /// \param[in] bp_id

lldb/include/lldb/Core/Declaration.h

@@ -84,14 +84,10 @@ class Declaration {
   /// \param[in] declaration
   ///     The const Declaration object to compare with.
   ///
-  /// \param[in] full
-  ///     Same meaning as Full in FileSpec::Equal.  True means an empty
-  ///     directory is not equal to a specified one, false means it is equal.
-  ///
   /// \return
   ///     Returns \b true if \b declaration is at the same file and
   ///     line, \b false otherwise.
-  bool FileAndLineEqual(const Declaration &declaration, bool full) const;
+  bool FileAndLineEqual(const Declaration &declaration) const;
 
   /// Dump a description of this object to a Stream.
   ///

lldb/include/lldb/Target/StopInfo.h

@@ -77,18 +77,6 @@ class StopInfo : public std::enable_shared_from_this<StopInfo> {
       m_description.clear();
   }
 
-  /// This gives the StopInfo a chance to suggest a stack frame to select.
-  /// Passing true for inlined_stack will request changes to the inlined
-  /// call stack.  Passing false will request changes to the real stack
-  /// frame.  The inlined stack gets adjusted before we call into the thread
-  /// plans so they can reason based on the correct values.  The real stack
-  /// adjustment is handled after the frame recognizers get a chance to adjust
-  /// the frame.
-  virtual std::optional<uint32_t>
-  GetSuggestedStackFrameIndex(bool inlined_stack) {
-    return {};
-  }
-
   virtual bool IsValidForOperatingSystemThread(Thread &thread) { return true; }
 
   /// A Continue operation can result in a false stop event

lldb/include/lldb/Target/ThreadPlanStepInRange.h

@@ -100,8 +100,8 @@ class ThreadPlanStepInRange : public ThreadPlanStepRange,
   bool m_step_past_prologue; // FIXME: For now hard-coded to true, we could put
                              // a switch in for this if there's
                              // demand for that.
-  LazyBool m_virtual_step;   // true if we've just done a "virtual step", i.e.
-                             // just moved the inline stack depth.
+  bool m_virtual_step; // true if we've just done a "virtual step", i.e. just
+                       // moved the inline stack depth.
   ConstString m_step_into_target;
   std::vector<lldb::break_id_t> m_step_in_deep_bps; // Places where we might
                                                     // want to stop when we do a

lldb/source/Breakpoint/BreakpointLocation.cpp

@@ -508,20 +508,8 @@ void BreakpointLocation::GetDescription(Stream *s,
         s->PutCString("re-exported target = ");
       else
         s->PutCString("where = ");
-
-      // If there's a preferred line entry for printing, use that.
-      bool show_function_info = true;
-      if (auto preferred = GetPreferredLineEntry()) {
-        sc.line_entry = *preferred;
-        // FIXME: We're going to get the function name wrong when the preferred
-        // line entry is not the lowest one.  For now, just leave the function
-        // out in this case, but we really should also figure out how to easily
-        // fake the function name here.
-        show_function_info = false;
-      }
       sc.DumpStopContext(s, m_owner.GetTarget().GetProcessSP().get(), m_address,
-                         false, true, false, show_function_info,
-                         show_function_info, show_function_info);
+                         false, true, false, true, true, true);
     } else {
       if (sc.module_sp) {
         s->EOL();
@@ -549,10 +537,7 @@ void BreakpointLocation::GetDescription(Stream *s,
         if (sc.line_entry.line > 0) {
           s->EOL();
           s->Indent("location = ");
-          if (auto preferred = GetPreferredLineEntry())
-            preferred->DumpStopContext(s, true);
-          else
-            sc.line_entry.DumpStopContext(s, true);
+          sc.line_entry.DumpStopContext(s, true);
         }
 
       } else {
@@ -671,50 +656,6 @@ void BreakpointLocation::SendBreakpointLocationChangedEvent(
   }
 }
 
-std::optional<uint32_t> BreakpointLocation::GetSuggestedStackFrameIndex() {
-  auto preferred_opt = GetPreferredLineEntry();
-  if (!preferred_opt)
-    return {};
-  LineEntry preferred = *preferred_opt;
-  SymbolContext sc;
-  if (!m_address.CalculateSymbolContext(&sc))
-    return {};
-  // Don't return anything special if frame 0 is the preferred line entry.
-  // We not really telling the stack frame list to do anything special in that
-  // case.
-  if (!LineEntry::Compare(sc.line_entry, preferred))
-    return {};
-
-  if (!sc.block)
-    return {};
-
-  // Blocks have their line info in Declaration form, so make one here:
-  Declaration preferred_decl(preferred.GetFile(), preferred.line,
-                             preferred.column);
-
-  uint32_t depth = 0;
-  Block *inlined_block = sc.block->GetContainingInlinedBlock();
-  while (inlined_block) {
-    // If we've moved to a block that this isn't the start of, that's not
-    // our inlining info or call site, so we can stop here.
-    Address start_address;
-    if (!inlined_block->GetStartAddress(start_address) ||
-        start_address != m_address)
-      return {};
-
-    const InlineFunctionInfo *info = inlined_block->GetInlinedFunctionInfo();
-    if (info) {
-      if (preferred_decl == info->GetDeclaration())
-        return depth;
-      if (preferred_decl == info->GetCallSite())
-        return depth + 1;
-    }
-    inlined_block = inlined_block->GetInlinedParent();
-    depth++;
-  }
-  return {};
-}
-
 void BreakpointLocation::SwapLocation(BreakpointLocationSP swap_from) {
   m_address = swap_from->m_address;
   m_should_resolve_indirect_functions =

lldb/source/Breakpoint/BreakpointResolver.cpp

@@ -340,21 +340,6 @@ void BreakpointResolver::AddLocation(SearchFilter &filter,
   }
 
   BreakpointLocationSP bp_loc_sp(AddLocation(line_start));
-  // If the address that we resolved the location to returns a different
-  // LineEntry from the one in the incoming SC, we're probably dealing with an
-  // inlined call site, so set that as the preferred LineEntry:
-  LineEntry resolved_entry;
-  if (!skipped_prologue && bp_loc_sp &&
-      line_start.CalculateSymbolContextLineEntry(resolved_entry) &&
-      LineEntry::Compare(resolved_entry, sc.line_entry)) {
-    // FIXME: The function name will also be wrong here.  Do we need to record
-    // that as well, or can we figure that out again when we report this
-    // breakpoint location.
-    if (!bp_loc_sp->SetPreferredLineEntry(sc.line_entry)) {
-      LLDB_LOG(log, "Tried to add a preferred line entry that didn't have the "
-                    "same address as this location's address.");
-    }
-  }
   if (log && bp_loc_sp && !GetBreakpoint()->IsInternal()) {
     StreamString s;
     bp_loc_sp->GetDescription(&s, lldb::eDescriptionLevelVerbose);

lldb/source/Breakpoint/BreakpointSite.cpp

@@ -87,23 +87,6 @@ void BreakpointSite::GetDescription(Stream *s, lldb::DescriptionLevel level) {
   m_constituents.GetDescription(s, level);
 }
 
-std::optional<uint32_t> BreakpointSite::GetSuggestedStackFrameIndex() {
-
-  std::optional<uint32_t> result;
-  std::lock_guard<std::recursive_mutex> guard(m_constituents_mutex);
-  for (BreakpointLocationSP loc_sp : m_constituents.BreakpointLocations()) {
-    std::optional<uint32_t> loc_frame_index =
-        loc_sp->GetSuggestedStackFrameIndex();
-    if (loc_frame_index) {
-      if (result)
-        result = std::max(*loc_frame_index, *result);
-      else
-        result = loc_frame_index;
-    }
-  }
-  return result;
-}
-
 bool BreakpointSite::IsInternal() const { return m_constituents.IsInternal(); }
 
 uint8_t *BreakpointSite::GetTrapOpcodeBytes() { return &m_trap_opcode[0]; }

lldb/source/Core/Declaration.cpp

@@ -70,9 +70,8 @@ int Declaration::Compare(const Declaration &a, const Declaration &b) {
   return 0;
 }
 
-bool Declaration::FileAndLineEqual(const Declaration &declaration,
-                                   bool full) const {
-  int file_compare = FileSpec::Compare(this->m_file, declaration.m_file, full);
+bool Declaration::FileAndLineEqual(const Declaration &declaration) const {
+  int file_compare = FileSpec::Compare(this->m_file, declaration.m_file, true);
   return file_compare == 0 && this->m_line == declaration.m_line;
 }
 

lldb/source/Symbol/Block.cpp

@@ -230,7 +230,7 @@ Block *Block::GetContainingInlinedBlockWithCallSite(
     const auto *function_info = inlined_block->GetInlinedFunctionInfo();
 
     if (function_info &&
-        function_info->GetCallSite().FileAndLineEqual(find_call_site, true))
+        function_info->GetCallSite().FileAndLineEqual(find_call_site))
       return inlined_block;
     inlined_block = inlined_block->GetInlinedParent();
   }