[Lldb-commits] [lldb] Revert "Add the ability to break on call-site locations, improve inli… (PR #113947)
via lldb-commits
lldb-commits at lists.llvm.org
Mon Oct 28 11:52:28 PDT 2024
https://github.com/jimingham created https://github.com/llvm/llvm-project/pull/113947
…ne stepping (#112939)"
This was breaking some gdb-remote packet counting tests on the bots. I can't see how this patch could cause that breakage, but I'm reverting to figure that out.
This reverts commit f14743794587db102c6d1b20f9c87a1ac20decfd.
>From f762e521560ebe1140bec2b615d088e8746f030a Mon Sep 17 00:00:00 2001
From: Jim Ingham <jingham at apple.com>
Date: Mon, 28 Oct 2024 11:50:09 -0700
Subject: [PATCH] Revert "Add the ability to break on call-site locations,
improve inline stepping (#112939)"
This was breaking some gdb-remote packet counting tests on the bots. I can't see how this
patch could cause that breakage, but I'm reverting to figure that out.
This reverts commit f14743794587db102c6d1b20f9c87a1ac20decfd.
---
.../lldb/Breakpoint/BreakpointLocation.h | 36 ----
lldb/include/lldb/Breakpoint/BreakpointSite.h | 5 -
lldb/include/lldb/Core/Declaration.h | 6 +-
lldb/include/lldb/Target/StopInfo.h | 12 --
.../lldb/Target/ThreadPlanStepInRange.h | 4 +-
lldb/source/Breakpoint/BreakpointLocation.cpp | 63 +------
lldb/source/Breakpoint/BreakpointResolver.cpp | 15 --
lldb/source/Breakpoint/BreakpointSite.cpp | 17 --
lldb/source/Core/Declaration.cpp | 5 +-
lldb/source/Symbol/Block.cpp | 2 +-
lldb/source/Symbol/CompileUnit.cpp | 111 +-----------
lldb/source/Target/StackFrameList.cpp | 171 ++++++++++++------
lldb/source/Target/StopInfo.cpp | 55 ------
lldb/source/Target/Thread.cpp | 8 -
lldb/source/Target/ThreadPlanStepInRange.cpp | 24 +--
.../source/Target/ThreadPlanStepOverRange.cpp | 2 +-
.../inline-stepping/TestInlineStepping.py | 63 -------
.../inline-stepping/calling.cpp | 25 ---
18 files changed, 131 insertions(+), 493 deletions(-)
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index 3592291bb2d06e..cca00335bc3c67 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/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;
diff --git a/lldb/include/lldb/Breakpoint/BreakpointSite.h b/lldb/include/lldb/Breakpoint/BreakpointSite.h
index 7b3f7be23639f2..17b76d51c1ae53 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointSite.h
+++ b/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
diff --git a/lldb/include/lldb/Core/Declaration.h b/lldb/include/lldb/Core/Declaration.h
index c864b88c6b32a3..4a0e9047b54695 100644
--- a/lldb/include/lldb/Core/Declaration.h
+++ b/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.
///
diff --git a/lldb/include/lldb/Target/StopInfo.h b/lldb/include/lldb/Target/StopInfo.h
index 45beac129e86f7..fae90364deaf0a 100644
--- a/lldb/include/lldb/Target/StopInfo.h
+++ b/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
diff --git a/lldb/include/lldb/Target/ThreadPlanStepInRange.h b/lldb/include/lldb/Target/ThreadPlanStepInRange.h
index 9da8370ef1c925..f9ef87942a7c03 100644
--- a/lldb/include/lldb/Target/ThreadPlanStepInRange.h
+++ b/lldb/include/lldb/Target/ThreadPlanStepInRange.h
@@ -80,8 +80,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;
ThreadPlanStepInRange(const ThreadPlanStepInRange &) = delete;
const ThreadPlanStepInRange &
diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp
index c7ea50407ae1c7..ad9057c8141e99 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/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 =
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp b/lldb/source/Breakpoint/BreakpointResolver.cpp
index 9643602d78c751..8307689c7640cf 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/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);
diff --git a/lldb/source/Breakpoint/BreakpointSite.cpp b/lldb/source/Breakpoint/BreakpointSite.cpp
index 9700a57d3346e0..3ca93f908e30b8 100644
--- a/lldb/source/Breakpoint/BreakpointSite.cpp
+++ b/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]; }
diff --git a/lldb/source/Core/Declaration.cpp b/lldb/source/Core/Declaration.cpp
index a485c4b9ba48a7..579a3999d14ea0 100644
--- a/lldb/source/Core/Declaration.cpp
+++ b/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;
}
diff --git a/lldb/source/Symbol/Block.cpp b/lldb/source/Symbol/Block.cpp
index 5c7772a6db780d..f7d9c0d2d33065 100644
--- a/lldb/source/Symbol/Block.cpp
+++ b/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();
}
diff --git a/lldb/source/Symbol/CompileUnit.cpp b/lldb/source/Symbol/CompileUnit.cpp
index f0f7e40ae70d83..db8f8ce6bcbc92 100644
--- a/lldb/source/Symbol/CompileUnit.cpp
+++ b/lldb/source/Symbol/CompileUnit.cpp
@@ -251,10 +251,7 @@ void CompileUnit::ResolveSymbolContext(
SymbolContextItem resolve_scope, SymbolContextList &sc_list,
RealpathPrefixes *realpath_prefixes) {
const FileSpec file_spec = src_location_spec.GetFileSpec();
- const uint32_t line =
- src_location_spec.GetLine().value_or(LLDB_INVALID_LINE_NUMBER);
- const uint32_t column_num =
- src_location_spec.GetColumn().value_or(LLDB_INVALID_COLUMN_NUMBER);
+ const uint32_t line = src_location_spec.GetLine().value_or(0);
const bool check_inlines = src_location_spec.GetCheckInlines();
// First find all of the file indexes that match our "file_spec". If
@@ -315,112 +312,6 @@ void CompileUnit::ResolveSymbolContext(
0, file_indexes, src_location_spec, &line_entry);
}
- // If we didn't manage to find a breakpoint that matched the line number
- // requested, that might be because it is only an inline call site, and
- // doesn't have a line entry in the line table. Scan for that here.
- //
- // We are making the assumption that if there was an inlined function it will
- // contribute at least 1 non-call-site entry to the line table. That's handy
- // because we don't move line breakpoints over function boundaries, so if we
- // found a hit, and there were also a call site entry, it would have to be in
- // the function containing the PC of the line table match. That way we can
- // limit the call site search to that function.
- // We will miss functions that ONLY exist as a call site entry.
-
- if (line_entry.IsValid() &&
- (line_entry.line != line || line_entry.column != column_num) &&
- resolve_scope & eSymbolContextLineEntry && check_inlines) {
- // We don't move lines over function boundaries, so the address in the
- // line entry will be the in function that contained the line that might
- // be a CallSite, and we can just iterate over that function to find any
- // inline records, and dig up their call sites.
- Address start_addr = line_entry.range.GetBaseAddress();
- Function *function = start_addr.CalculateSymbolContextFunction();
-
- Declaration sought_decl(file_spec, line, column_num);
- // We use this recursive function to descend the block structure looking
- // for a block that has this Declaration as in it's CallSite info.
- // This function recursively scans the sibling blocks of the incoming
- // block parameter.
- std::function<void(Block &)> examine_block =
- [&sought_decl, &sc_list, &src_location_spec, resolve_scope,
- &examine_block](Block &block) -> void {
- // Iterate over the sibling child blocks of the incoming block.
- Block *sibling_block = block.GetFirstChild();
- while (sibling_block) {
- // We only have to descend through the regular blocks, looking for
- // immediate inlines, since those are the only ones that will have this
- // callsite.
- const InlineFunctionInfo *inline_info =
- sibling_block->GetInlinedFunctionInfo();
- if (inline_info) {
- // If this is the call-site we are looking for, record that:
- // We need to be careful because the call site from the debug info
- // will generally have a column, but the user might not have specified
- // it.
- Declaration found_decl = inline_info->GetCallSite();
- uint32_t sought_column = sought_decl.GetColumn();
- if (found_decl.FileAndLineEqual(sought_decl, false) &&
- (sought_column == LLDB_INVALID_COLUMN_NUMBER ||
- sought_column == found_decl.GetColumn())) {
- // If we found a call site, it belongs not in this inlined block,
- // but in the parent block that inlined it.
- Address parent_start_addr;
- if (sibling_block->GetParent()->GetStartAddress(
- parent_start_addr)) {
- SymbolContext sc;
- parent_start_addr.CalculateSymbolContext(&sc, resolve_scope);
- // Now swap out the line entry for the one we found.
- LineEntry call_site_line = sc.line_entry;
- call_site_line.line = found_decl.GetLine();
- call_site_line.column = found_decl.GetColumn();
- bool matches_spec = true;
- // If the user asked for an exact match, we need to make sure the
- // call site we found actually matches the location.
- if (src_location_spec.GetExactMatch()) {
- matches_spec = false;
- if ((src_location_spec.GetFileSpec() ==
- sc.line_entry.GetFile()) &&
- (src_location_spec.GetLine() &&
- *src_location_spec.GetLine() == call_site_line.line) &&
- (src_location_spec.GetColumn() &&
- *src_location_spec.GetColumn() == call_site_line.column))
- matches_spec = true;
- }
- if (matches_spec &&
- sibling_block->GetRangeAtIndex(0, call_site_line.range)) {
- SymbolContext call_site_sc(sc.target_sp, sc.module_sp,
- sc.comp_unit, sc.function, sc.block,
- &call_site_line, sc.symbol);
- sc_list.Append(call_site_sc);
- }
- }
- }
- }
-
- // Descend into the child blocks:
- examine_block(*sibling_block);
- // Now go to the next sibling:
- sibling_block = sibling_block->GetSibling();
- }
- };
-
- if (function) {
- // We don't need to examine the function block, it can't be inlined.
- Block &func_block = function->GetBlock(true);
- examine_block(func_block);
- }
- // If we found entries here, we are done. We only get here because we
- // didn't find an exact line entry for this line & column, but if we found
- // an exact match from the call site info that's strictly better than
- // continuing to look for matches further on in the file.
- // FIXME: Should I also do this for "call site line exists between the
- // given line number and the later line we found in the line table"? That's
- // a closer approximation to our general sliding algorithm.
- if (sc_list.GetSize())
- return;
- }
-
// If "exact == true", then "found_line" will be the same as "line". If
// "exact == false", the "found_line" will be the closest line entry
// with a line number greater than "line" and we will use this for our
diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp
index 94a381edd5e202..3849ec5ed178d9 100644
--- a/lldb/source/Target/StackFrameList.cpp
+++ b/lldb/source/Target/StackFrameList.cpp
@@ -85,32 +85,121 @@ void StackFrameList::ResetCurrentInlinedDepth() {
return;
std::lock_guard<std::recursive_mutex> guard(m_mutex);
+
+ GetFramesUpTo(0, DoNotAllowInterruption);
+ if (m_frames.empty())
+ return;
+ if (!m_frames[0]->IsInlined()) {
+ m_current_inlined_depth = UINT32_MAX;
+ m_current_inlined_pc = LLDB_INVALID_ADDRESS;
+ Log *log = GetLog(LLDBLog::Step);
+ if (log && log->GetVerbose())
+ LLDB_LOGF(
+ log,
+ "ResetCurrentInlinedDepth: Invalidating current inlined depth.\n");
+ return;
+ }
- m_current_inlined_pc = LLDB_INVALID_ADDRESS;
- m_current_inlined_depth = UINT32_MAX;
+ // We only need to do something special about inlined blocks when we are
+ // at the beginning of an inlined function:
+ // FIXME: We probably also have to do something special if the PC is at
+ // the END of an inlined function, which coincides with the end of either
+ // its containing function or another inlined function.
+
+ Block *block_ptr = m_frames[0]->GetFrameBlock();
+ if (!block_ptr)
+ return;
+ Address pc_as_address;
+ lldb::addr_t curr_pc = m_thread.GetRegisterContext()->GetPC();
+ pc_as_address.SetLoadAddress(curr_pc, &(m_thread.GetProcess()->GetTarget()));
+ AddressRange containing_range;
+ if (!block_ptr->GetRangeContainingAddress(pc_as_address, containing_range) ||
+ pc_as_address != containing_range.GetBaseAddress())
+ return;
+
+ // If we got here because of a breakpoint hit, then set the inlined depth
+ // depending on where the breakpoint was set. If we got here because of a
+ // crash, then set the inlined depth to the deepest most block. Otherwise,
+ // we stopped here naturally as the result of a step, so set ourselves in the
+ // containing frame of the whole set of nested inlines, so the user can then
+ // "virtually" step into the frames one by one, or next over the whole mess.
+ // Note: We don't have to handle being somewhere in the middle of the stack
+ // here, since ResetCurrentInlinedDepth doesn't get called if there is a
+ // valid inlined depth set.
StopInfoSP stop_info_sp = m_thread.GetStopInfo();
if (!stop_info_sp)
return;
+ switch (stop_info_sp->GetStopReason()) {
+ case eStopReasonWatchpoint:
+ case eStopReasonException:
+ case eStopReasonExec:
+ case eStopReasonFork:
+ case eStopReasonVFork:
+ case eStopReasonVForkDone:
+ case eStopReasonSignal:
+ // In all these cases we want to stop in the deepest frame.
+ m_current_inlined_pc = curr_pc;
+ m_current_inlined_depth = 0;
+ break;
+ case eStopReasonBreakpoint: {
+ // FIXME: Figure out what this break point is doing, and set the inline
+ // depth appropriately. Be careful to take into account breakpoints that
+ // implement step over prologue, since that should do the default
+ // calculation. For now, if the breakpoints corresponding to this hit are
+ // all internal, I set the stop location to the top of the inlined stack,
+ // since that will make things like stepping over prologues work right.
+ // But if there are any non-internal breakpoints I do to the bottom of the
+ // stack, since that was the old behavior.
+ uint32_t bp_site_id = stop_info_sp->GetValue();
+ BreakpointSiteSP bp_site_sp(
+ m_thread.GetProcess()->GetBreakpointSiteList().FindByID(bp_site_id));
+ bool all_internal = true;
+ if (bp_site_sp) {
+ uint32_t num_owners = bp_site_sp->GetNumberOfConstituents();
+ for (uint32_t i = 0; i < num_owners; i++) {
+ Breakpoint &bp_ref =
+ bp_site_sp->GetConstituentAtIndex(i)->GetBreakpoint();
+ if (!bp_ref.IsInternal()) {
+ all_internal = false;
+ }
+ }
+ }
+ if (!all_internal) {
+ m_current_inlined_pc = curr_pc;
+ m_current_inlined_depth = 0;
+ break;
+ }
+ }
+ [[fallthrough]];
+ default: {
+ // Otherwise, we should set ourselves at the container of the inlining, so
+ // that the user can descend into them. So first we check whether we have
+ // more than one inlined block sharing this PC:
+ int num_inlined_functions = 0;
+
+ for (Block *container_ptr = block_ptr->GetInlinedParent();
+ container_ptr != nullptr;
+ container_ptr = container_ptr->GetInlinedParent()) {
+ if (!container_ptr->GetRangeContainingAddress(pc_as_address,
+ containing_range))
+ break;
+ if (pc_as_address != containing_range.GetBaseAddress())
+ break;
- bool inlined = true;
- auto inline_depth = stop_info_sp->GetSuggestedStackFrameIndex(inlined);
- // We're only adjusting the inlined stack here.
- Log *log = GetLog(LLDBLog::Step);
- if (inline_depth) {
- m_current_inlined_depth = *inline_depth;
- m_current_inlined_pc = m_thread.GetRegisterContext()->GetPC();
-
+ num_inlined_functions++;
+ }
+ m_current_inlined_pc = curr_pc;
+ m_current_inlined_depth = num_inlined_functions + 1;
+ Log *log = GetLog(LLDBLog::Step);
if (log && log->GetVerbose())
LLDB_LOGF(log,
"ResetCurrentInlinedDepth: setting inlined "
"depth: %d 0x%" PRIx64 ".\n",
- m_current_inlined_depth, m_current_inlined_pc);
- } else {
- if (log && log->GetVerbose())
- LLDB_LOGF(
- log,
- "ResetCurrentInlinedDepth: Invalidating current inlined depth.\n");
+ m_current_inlined_depth, curr_pc);
+
+ break;
+ }
}
}
@@ -727,48 +816,19 @@ void StackFrameList::SelectMostRelevantFrame() {
RecognizedStackFrameSP recognized_frame_sp = frame_sp->GetRecognizedFrame();
- if (recognized_frame_sp) {
- if (StackFrameSP most_relevant_frame_sp =
- recognized_frame_sp->GetMostRelevantFrame()) {
- LLDB_LOG(log, "Found most relevant frame at index {0}",
- most_relevant_frame_sp->GetFrameIndex());
- SetSelectedFrame(most_relevant_frame_sp.get());
- return;
- }
- }
- LLDB_LOG(log, "Frame #0 not recognized");
-
- // If this thread has a non-trivial StopInof, then let it suggest
- // a most relevant frame:
- StopInfoSP stop_info_sp = m_thread.GetStopInfo();
- uint32_t stack_idx = 0;
- bool found_relevant = false;
- if (stop_info_sp) {
- // Here we're only asking the stop info if it wants to adjust the real stack
- // index. We have to ask about the m_inlined_stack_depth in
- // Thread::ShouldStop since the plans need to reason with that info.
- bool inlined = false;
- std::optional<uint32_t> stack_opt =
- stop_info_sp->GetSuggestedStackFrameIndex(inlined);
- if (stack_opt) {
- stack_idx = *stack_opt;
- found_relevant = true;
- }
+ if (!recognized_frame_sp) {
+ LLDB_LOG(log, "Frame #0 not recognized");
+ return;
}
- frame_sp = GetFrameAtIndex(stack_idx);
- if (!frame_sp)
- LLDB_LOG(log, "Stop info suggested relevant frame {0} but it didn't exist",
- stack_idx);
- else if (found_relevant)
- LLDB_LOG(log, "Setting selected frame from stop info to {0}", stack_idx);
- // Note, we don't have to worry about "inlined" frames here, because we've
- // already calculated the inlined frame in Thread::ShouldStop, and
- // SetSelectedFrame will take care of that adjustment for us.
- SetSelectedFrame(frame_sp.get());
-
- if (!found_relevant)
+ if (StackFrameSP most_relevant_frame_sp =
+ recognized_frame_sp->GetMostRelevantFrame()) {
+ LLDB_LOG(log, "Found most relevant frame at index {0}",
+ most_relevant_frame_sp->GetFrameIndex());
+ SetSelectedFrame(most_relevant_frame_sp.get());
+ } else {
LLDB_LOG(log, "No relevant frame!");
+ }
}
uint32_t StackFrameList::GetSelectedFrameIndex(
@@ -781,7 +841,6 @@ uint32_t StackFrameList::GetSelectedFrameIndex(
// isn't set, then don't force a selection here, just return 0.
if (!select_most_relevant)
return 0;
- // If the inlined stack frame is set, then use that:
m_selected_frame_idx = 0;
}
return *m_selected_frame_idx;
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index f6387d47504e62..60aa65ed38c749 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -15,7 +15,6 @@
#include "lldb/Breakpoint/WatchpointResource.h"
#include "lldb/Core/Debugger.h"
#include "lldb/Expression/UserExpression.h"
-#include "lldb/Symbol/Block.h"
#include "lldb/Target/Process.h"
#include "lldb/Target/StopInfo.h"
#include "lldb/Target/Target.h"
@@ -247,22 +246,6 @@ class StopInfoBreakpoint : public StopInfo {
return m_description.c_str();
}
- std::optional<uint32_t>
- GetSuggestedStackFrameIndex(bool inlined_stack) override {
- if (!inlined_stack)
- return {};
-
- ThreadSP thread_sp(m_thread_wp.lock());
- if (!thread_sp)
- return {};
- BreakpointSiteSP bp_site_sp(
- thread_sp->GetProcess()->GetBreakpointSiteList().FindByID(m_value));
- if (!bp_site_sp)
- return {};
-
- return bp_site_sp->GetSuggestedStackFrameIndex();
- }
-
protected:
bool ShouldStop(Event *event_ptr) override {
// This just reports the work done by PerformAction or the synchronous
@@ -1181,44 +1164,6 @@ class StopInfoTrace : public StopInfo {
else
return m_description.c_str();
}
-
- std::optional<uint32_t>
- GetSuggestedStackFrameIndex(bool inlined_stack) override {
- // Trace only knows how to adjust inlined stacks:
- if (!inlined_stack)
- return {};
-
- ThreadSP thread_sp = GetThread();
- StackFrameSP frame_0_sp = thread_sp->GetStackFrameAtIndex(0);
- if (!frame_0_sp)
- return {};
- if (!frame_0_sp->IsInlined())
- return {};
- Block *block_ptr = frame_0_sp->GetFrameBlock();
- if (!block_ptr)
- return {};
- Address pc_address = frame_0_sp->GetFrameCodeAddress();
- AddressRange containing_range;
- if (!block_ptr->GetRangeContainingAddress(pc_address, containing_range) ||
- pc_address != containing_range.GetBaseAddress())
- return {};
-
- int num_inlined_functions = 0;
-
- for (Block *container_ptr = block_ptr->GetInlinedParent();
- container_ptr != nullptr;
- container_ptr = container_ptr->GetInlinedParent()) {
- if (!container_ptr->GetRangeContainingAddress(pc_address,
- containing_range))
- break;
- if (pc_address != containing_range.GetBaseAddress())
- break;
-
- num_inlined_functions++;
- }
- inlined_stack = true;
- return num_inlined_functions + 1;
- }
};
// StopInfoException
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index 735295e6f25937..8373cdc36268f8 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -619,14 +619,6 @@ void Thread::WillStop() {
void Thread::SetupForResume() {
if (GetResumeState() != eStateSuspended) {
- // First check whether this thread is going to "actually" resume at all.
- // For instance, if we're stepping from one level to the next of an
- // virtual inlined call stack, we just change the inlined call stack index
- // without actually running this thread. In that case, for this thread we
- // shouldn't push a step over breakpoint plan or do that work.
- if (GetCurrentPlan()->IsVirtualStep())
- return;
-
// If we're at a breakpoint push the step-over breakpoint plan. Do this
// before telling the current plan it will resume, since we might change
// what the current plan is.
diff --git a/lldb/source/Target/ThreadPlanStepInRange.cpp b/lldb/source/Target/ThreadPlanStepInRange.cpp
index 325a70619908b6..567dcc26d0d372 100644
--- a/lldb/source/Target/ThreadPlanStepInRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepInRange.cpp
@@ -41,7 +41,7 @@ ThreadPlanStepInRange::ThreadPlanStepInRange(
"Step Range stepping in", thread, range, addr_context,
stop_others),
ThreadPlanShouldStopHere(this), m_step_past_prologue(true),
- m_virtual_step(eLazyBoolCalculate), m_step_into_target(step_into_target) {
+ m_virtual_step(false), m_step_into_target(step_into_target) {
SetCallbacks();
SetFlagsToDefault();
SetupAvoidNoDebug(step_in_avoids_code_without_debug_info,
@@ -149,7 +149,7 @@ bool ThreadPlanStepInRange::ShouldStop(Event *event_ptr) {
m_sub_plan_sp.reset();
}
- if (m_virtual_step == eLazyBoolYes) {
+ if (m_virtual_step) {
// If we've just completed a virtual step, all we need to do is check for a
// ShouldStopHere plan, and otherwise we're done.
// FIXME - This can be both a step in and a step out. Probably should
@@ -431,7 +431,7 @@ bool ThreadPlanStepInRange::DoPlanExplainsStop(Event *event_ptr) {
bool return_value = false;
- if (m_virtual_step == eLazyBoolYes) {
+ if (m_virtual_step) {
return_value = true;
} else {
StopInfoSP stop_info_sp = GetPrivateStopInfo();
@@ -460,13 +460,10 @@ bool ThreadPlanStepInRange::DoPlanExplainsStop(Event *event_ptr) {
bool ThreadPlanStepInRange::DoWillResume(lldb::StateType resume_state,
bool current_plan) {
- m_virtual_step = eLazyBoolCalculate;
+ m_virtual_step = false;
if (resume_state == eStateStepping && current_plan) {
Thread &thread = GetThread();
// See if we are about to step over a virtual inlined call.
- // But if we already know we're virtual stepping, don't decrement the
- // inlined depth again...
-
bool step_without_resume = thread.DecrementCurrentInlinedDepth();
if (step_without_resume) {
Log *log = GetLog(LLDBLog::Step);
@@ -479,20 +476,11 @@ bool ThreadPlanStepInRange::DoWillResume(lldb::StateType resume_state,
// FIXME: Maybe it would be better to create a InlineStep stop reason, but
// then
// the whole rest of the world would have to handle that stop reason.
- m_virtual_step = eLazyBoolYes;
+ m_virtual_step = true;
}
return !step_without_resume;
}
return true;
}
-bool ThreadPlanStepInRange::IsVirtualStep() {
- if (m_virtual_step == eLazyBoolCalculate) {
- Thread &thread = GetThread();
- if (thread.GetCurrentInlinedDepth() == UINT32_MAX)
- m_virtual_step = eLazyBoolNo;
- else
- m_virtual_step = eLazyBoolYes;
- }
- return m_virtual_step == eLazyBoolYes;
-}
+bool ThreadPlanStepInRange::IsVirtualStep() { return m_virtual_step; }
diff --git a/lldb/source/Target/ThreadPlanStepOverRange.cpp b/lldb/source/Target/ThreadPlanStepOverRange.cpp
index 643ee827c865cb..ef5b4b5c434d16 100644
--- a/lldb/source/Target/ThreadPlanStepOverRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepOverRange.cpp
@@ -402,7 +402,7 @@ bool ThreadPlanStepOverRange::DoWillResume(lldb::StateType resume_state,
if (in_inlined_stack) {
Log *log = GetLog(LLDBLog::Step);
LLDB_LOGF(log,
- "ThreadPlanStepOverRange::DoWillResume: adjusting range to "
+ "ThreadPlanStepInRange::DoWillResume: adjusting range to "
"the frame at inlined depth %d.",
thread.GetCurrentInlinedDepth());
StackFrameSP stack_sp = thread.GetStackFrameAtIndex(0);
diff --git a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
index f52e0f0fd5bcfe..752c3a9cbd286a 100644
--- a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
+++ b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
@@ -32,12 +32,6 @@ def test_step_in_template_with_python_api(self):
self.build()
self.step_in_template()
- @add_test_categories(["pyapi"])
- def test_virtual_inline_stepping(self):
- """Test stepping through a virtual inlined call stack"""
- self.build()
- self.virtual_inline_stepping()
-
def setUp(self):
# Call super's setUp().
TestBase.setUp(self)
@@ -363,60 +357,3 @@ def step_in_template(self):
step_sequence = [["// In max_value specialized", "into"]]
self.run_step_sequence(step_sequence)
-
- def run_to_call_site_and_step(self, source_regex, func_name, start_pos):
- main_spec = lldb.SBFileSpec("calling.cpp")
- # Set the breakpoint by file and line, not sourced regex because
- # we want to make sure we can set breakpoints on call sites:
- call_site_line_num = line_number(self.main_source, source_regex)
- target, process, thread, bkpt = lldbutil.run_to_line_breakpoint(
- self, main_spec, call_site_line_num
- )
-
- # Make sure that the location is at the call site (run_to_line_breakpoint already asserted
- # that there's one location.):
- bkpt_loc = bkpt.location[0]
- strm = lldb.SBStream()
- result = bkpt_loc.GetDescription(strm, lldb.eDescriptionLevelFull)
-
- self.assertTrue(result, "Got a location description")
- desc = strm.GetData()
- self.assertIn(f"calling.cpp:{call_site_line_num}", desc, "Right line listed")
- # We don't get the function name right yet - so we omit it in printing.
- # Turn on this test when that is working.
- # self.assertIn(func_name, desc, "Right function listed")
-
- pc = thread.frame[0].pc
- for i in range(start_pos, 3):
- thread.StepInto()
- frame_0 = thread.frame[0]
-
- trivial_line_num = line_number(
- self.main_source, f"In caller_trivial_inline_{i}."
- )
- self.assertEqual(
- frame_0.line_entry.line,
- trivial_line_num,
- f"Stepped into the caller_trivial_inline_{i}",
- )
- if pc != frame_0.pc:
- # If we get here, we stepped to the expected line number, but
- # the compiler on this system has decided to insert an instruction
- # between the call site of an inlined function with no arguments,
- # returning void, and its immediate call to another void inlined function
- # with no arguments. We aren't going to be testing virtual inline
- # stepping for this function...
- break
-
- process.Kill()
- target.Clear()
-
- def virtual_inline_stepping(self):
- """Use the Python API's to step through a virtual inlined stack"""
- self.run_to_call_site_and_step("At caller_trivial_inline_1", "main", 1)
- self.run_to_call_site_and_step(
- "In caller_trivial_inline_1", "caller_trivial_inline_1", 2
- )
- self.run_to_call_site_and_step(
- "In caller_trivial_inline_2", "caller_trivial_inline_2", 3
- )
diff --git a/lldb/test/API/functionalities/inline-stepping/calling.cpp b/lldb/test/API/functionalities/inline-stepping/calling.cpp
index d7ee56b3c07909..49179ce7c97883 100644
--- a/lldb/test/API/functionalities/inline-stepping/calling.cpp
+++ b/lldb/test/API/functionalities/inline-stepping/calling.cpp
@@ -13,12 +13,6 @@ int called_by_inline_ref (int &value);
inline void inline_trivial_1 () __attribute__((always_inline));
inline void inline_trivial_2 () __attribute__((always_inline));
-// These three should share the same initial pc so we can test
-// virtual inline stepping.
-inline void caller_trivial_inline_1() __attribute__((always_inline));
-inline void caller_trivial_inline_2() __attribute__((always_inline));
-inline void caller_trivial_inline_3() __attribute__((always_inline));
-
void caller_trivial_1 ();
void caller_trivial_2 ();
@@ -85,23 +79,6 @@ caller_trivial_2 ()
inline_value += 1; // At increment in caller_trivial_2.
}
-// When you call caller_trivial_inline_1, the inlined call-site
-// should share a PC with all three of the following inlined
-// functions, so we can exercise "virtual inline stepping".
-void caller_trivial_inline_1() {
- caller_trivial_inline_2(); // In caller_trivial_inline_1.
- inline_value += 1;
-}
-
-void caller_trivial_inline_2() {
- caller_trivial_inline_3(); // In caller_trivial_inline_2.
- inline_value += 1;
-}
-
-void caller_trivial_inline_3() {
- inline_value += 1; // In caller_trivial_inline_3.
-}
-
void
called_by_inline_trivial ()
{
@@ -155,7 +132,5 @@ main (int argc, char **argv)
max_value(123, 456); // Call max_value template
max_value(std::string("abc"), std::string("0022")); // Call max_value specialized
- caller_trivial_inline_1(); // At caller_trivial_inline_1.
-
return 0; // About to return from main.
}
More information about the lldb-commits
mailing list