https://github.com/jimingham updated
https://github.com/llvm/llvm-project/pull/112939
>From 9c6705b21df14dc911665e1082c9b31ce00d7e7c Mon Sep 17 00:00:00 2001
From: Jim Ingham <jing...@apple.com>
Date: Thu, 3 Oct 2024 18:24:46 -0700
Subject: [PATCH 01/12] Add the ability to break on call-site locations, report
the correct position in the virtual inlined call stack when they are hit, and
step through the inlined stack from there.
---
.../lldb/Breakpoint/BreakpointLocation.h | 31 ++++
lldb/include/lldb/Breakpoint/BreakpointSite.h | 5 +
lldb/include/lldb/Target/StopInfo.h | 11 ++
.../lldb/Target/ThreadPlanStepInRange.h | 4 +-
lldb/source/Breakpoint/BreakpointLocation.cpp | 61 ++++++-
lldb/source/Breakpoint/BreakpointResolver.cpp | 12 ++
lldb/source/Breakpoint/BreakpointSite.cpp | 16 ++
lldb/source/Core/Declaration.cpp | 2 +-
lldb/source/Symbol/CompileUnit.cpp | 104 ++++++++++-
lldb/source/Target/StackFrameList.cpp | 170 ++++++------------
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 | 54 ++++++
.../inline-stepping/calling.cpp | 31 ++++
16 files changed, 462 insertions(+), 128 deletions(-)
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index cca00335bc3c67..f9c258daf137f7 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -11,10 +11,12 @@
#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"
@@ -281,6 +283,18 @@ 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.
+ void SetPreferredLineEntry(const LineEntry &line_entry) {
+ m_preferred_line_entry = line_entry;
+ }
+
+ const std::optional<LineEntry> GetPreferredLineEntry() {
+ return m_preferred_line_entry;
+ }
protected:
friend class BreakpointSite;
@@ -305,6 +319,16 @@ class BreakpointLocation
/// It also takes care of decrementing the ignore counters.
/// 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);
@@ -369,6 +393,13 @@ 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.
+ std::optional<LineEntry> m_preferred_line_entry; // 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.
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 17b76d51c1ae53..30cb5a80b908e0 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointSite.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointSite.h
@@ -169,6 +169,11 @@ 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.
///
diff --git a/lldb/include/lldb/Target/StopInfo.h
b/lldb/include/lldb/Target/StopInfo.h
index fae90364deaf0a..d997e0fd6fc550 100644
--- a/lldb/include/lldb/Target/StopInfo.h
+++ b/lldb/include/lldb/Target/StopInfo.h
@@ -77,6 +77,17 @@ 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 f9ef87942a7c03..6bd79e1d287d35 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.
- bool m_virtual_step; // true if we've just done a "virtual step", i.e. just
- // moved the inline stack depth.
+ LazyBool 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 35058a713aef86..61fe467987228b 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -508,8 +508,19 @@ 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 (GetPreferredLineEntry()) {
+ sc.line_entry = *GetPreferredLineEntry();
+ // 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, true, true, true);
+ false, true, false, show_function_info,
show_function_info, show_function_info);
} else {
if (sc.module_sp) {
s->EOL();
@@ -537,7 +548,10 @@ void BreakpointLocation::GetDescription(Stream *s,
if (sc.line_entry.line > 0) {
s->EOL();
s->Indent("location = ");
- sc.line_entry.DumpStopContext(s, true);
+ if (GetPreferredLineEntry())
+ GetPreferredLineEntry()->DumpStopContext(s, true);
+ else
+ sc.line_entry.DumpStopContext(s, true);
}
} else {
@@ -656,6 +670,49 @@ void
BreakpointLocation::SendBreakpointLocationChangedEvent(
}
}
+std::optional<uint32_t> BreakpointLocation::GetSuggestedStackFrameIndex() {
+ if (!GetPreferredLineEntry())
+ return {};
+ LineEntry preferred = *GetPreferredLineEntry();
+ 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 8307689c7640cf..465fc387aa43e5 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -340,6 +340,18 @@ 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.
+ bp_loc_sp->SetPreferredLineEntry(sc.line_entry);
+ }
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 3ca93f908e30b8..b283952d028305 100644
--- a/lldb/source/Breakpoint/BreakpointSite.cpp
+++ b/lldb/source/Breakpoint/BreakpointSite.cpp
@@ -87,6 +87,22 @@ 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> this_result =
loc_sp->GetSuggestedStackFrameIndex();
+ if (this_result) {
+ if (!result)
+ result = this_result;
+ else
+ result = std::max(*this_result, *result);
+ }
+ }
+ 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 579a3999d14ea0..e7855f3e364376 100644
--- a/lldb/source/Core/Declaration.cpp
+++ b/lldb/source/Core/Declaration.cpp
@@ -71,7 +71,7 @@ int Declaration::Compare(const Declaration &a, const
Declaration &b) {
}
bool Declaration::FileAndLineEqual(const Declaration &declaration) const {
- int file_compare = FileSpec::Compare(this->m_file, declaration.m_file, true);
+ int file_compare = FileSpec::Compare(this->m_file, declaration.m_file,
false);
return file_compare == 0 && this->m_line == declaration.m_line;
}
diff --git a/lldb/source/Symbol/CompileUnit.cpp
b/lldb/source/Symbol/CompileUnit.cpp
index db8f8ce6bcbc92..f4c61d6ec1b196 100644
--- a/lldb/source/Symbol/CompileUnit.cpp
+++ b/lldb/source/Symbol/CompileUnit.cpp
@@ -251,7 +251,8 @@ 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(0);
+ 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 bool check_inlines = src_location_spec.GetCheckInlines();
// First find all of the file indexes that match our "file_spec". If
@@ -311,6 +312,107 @@ void CompileUnit::ResolveSymbolContext(
line_idx = line_table->FindLineEntryIndexByFileIndex(
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)
+ && (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
diff --git a/lldb/source/Target/StackFrameList.cpp
b/lldb/source/Target/StackFrameList.cpp
index 3849ec5ed178d9..cda360baed6491 100644
--- a/lldb/source/Target/StackFrameList.cpp
+++ b/lldb/source/Target/StackFrameList.cpp
@@ -86,120 +86,31 @@ void StackFrameList::ResetCurrentInlinedDepth() {
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, curr_pc);
-
- break;
- }
+ m_current_inlined_depth, m_current_inlined_pc);
+ } else {
+ if (log && log->GetVerbose())
+ LLDB_LOGF(
+ log,
+ "ResetCurrentInlinedDepth: Invalidating current inlined depth.\n");
}
}
@@ -816,19 +727,47 @@ void StackFrameList::SelectMostRelevantFrame() {
RecognizedStackFrameSP recognized_frame_sp = frame_sp->GetRecognizedFrame();
- if (!recognized_frame_sp) {
- LLDB_LOG(log, "Frame #0 not recognized");
- return;
+ 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 (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!");
+ // 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;
+ }
}
+
+ 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)
+ LLDB_LOG(log, "No relevant frame!");
}
uint32_t StackFrameList::GetSelectedFrameIndex(
@@ -841,6 +780,7 @@ 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 bd7032b803df90..8ba46366c8b9d9 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -16,6 +16,7 @@
#include "lldb/Core/Debugger.h"
#include "lldb/Core/ValueObject.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"
@@ -246,6 +247,21 @@ 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
@@ -1164,6 +1180,45 @@ 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 902fbb2b519ef7..d13013c18565c9 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -619,6 +619,14 @@ 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 567dcc26d0d372..5319afb4edc418 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(false), m_step_into_target(step_into_target) {
+ m_virtual_step(eLazyBoolCalculate), 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) {
+ if (m_virtual_step == eLazyBoolYes) {
// 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) {
+ if (m_virtual_step == eLazyBoolYes) {
return_value = true;
} else {
StopInfoSP stop_info_sp = GetPrivateStopInfo();
@@ -460,10 +460,13 @@ bool ThreadPlanStepInRange::DoPlanExplainsStop(Event
*event_ptr) {
bool ThreadPlanStepInRange::DoWillResume(lldb::StateType resume_state,
bool current_plan) {
- m_virtual_step = false;
+ m_virtual_step = eLazyBoolCalculate;
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);
@@ -476,11 +479,20 @@ 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 = true;
+ m_virtual_step = eLazyBoolYes;
}
return !step_without_resume;
}
return true;
}
-bool ThreadPlanStepInRange::IsVirtualStep() { return m_virtual_step; }
+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;
+}
diff --git a/lldb/source/Target/ThreadPlanStepOverRange.cpp
b/lldb/source/Target/ThreadPlanStepOverRange.cpp
index 934f23b3b21a28..3741f51e756870 100644
--- a/lldb/source/Target/ThreadPlanStepOverRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepOverRange.cpp
@@ -397,7 +397,7 @@ bool ThreadPlanStepOverRange::DoWillResume(lldb::StateType
resume_state,
if (in_inlined_stack) {
Log *log = GetLog(LLDBLog::Step);
LLDB_LOGF(log,
- "ThreadPlanStepInRange::DoWillResume: adjusting range to "
+ "ThreadPlanStepOverRange::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 752c3a9cbd286a..92389d5ebfa645 100644
--- a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
+++ b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
@@ -32,6 +32,12 @@ 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)
@@ -357,3 +363,51 @@ 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()
+ print(f"Description:\n{desc}\n")
+ 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 49179ce7c97883..0009388bd80633 100644
--- a/lldb/test/API/functionalities/inline-stepping/calling.cpp
+++ b/lldb/test/API/functionalities/inline-stepping/calling.cpp
@@ -13,6 +13,12 @@ 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 ();
@@ -79,6 +85,29 @@ 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 ()
{
@@ -132,5 +161,7 @@ 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.
}
>From 7a60871262fe9c807930f20833b46c6353860269 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jing...@apple.com>
Date: Fri, 18 Oct 2024 10:55:19 -0700
Subject: [PATCH 02/12] clang-format
---
.../lldb/Breakpoint/BreakpointLocation.h | 21 +++---
lldb/include/lldb/Breakpoint/BreakpointSite.h | 2 +-
lldb/include/lldb/Target/StopInfo.h | 7 +-
.../lldb/Target/ThreadPlanStepInRange.h | 4 +-
lldb/source/Breakpoint/BreakpointLocation.cpp | 17 ++---
lldb/source/Breakpoint/BreakpointResolver.cpp | 12 ++--
lldb/source/Symbol/CompileUnit.cpp | 67 ++++++++++---------
lldb/source/Target/StackFrameList.cpp | 17 ++---
lldb/source/Target/StopInfo.cpp | 16 ++---
lldb/source/Target/Thread.cpp | 2 +-
lldb/source/Target/ThreadPlanStepInRange.cpp | 4 +-
.../inline-stepping/calling.cpp | 30 ++++-----
12 files changed, 102 insertions(+), 97 deletions(-)
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index f9c258daf137f7..552fafab97cb9a 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -283,7 +283,7 @@ 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
@@ -291,7 +291,7 @@ class BreakpointLocation
void SetPreferredLineEntry(const LineEntry &line_entry) {
m_preferred_line_entry = line_entry;
}
-
+
const std::optional<LineEntry> GetPreferredLineEntry() {
return m_preferred_line_entry;
}
@@ -319,7 +319,7 @@ class BreakpointLocation
/// It also takes care of decrementing the ignore counters.
/// 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
@@ -393,13 +393,14 @@ 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.
- std::optional<LineEntry> m_preferred_line_entry; // 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; // 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.
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 30cb5a80b908e0..7b3f7be23639f2 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointSite.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointSite.h
@@ -169,7 +169,7 @@ 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.
diff --git a/lldb/include/lldb/Target/StopInfo.h
b/lldb/include/lldb/Target/StopInfo.h
index d997e0fd6fc550..45beac129e86f7 100644
--- a/lldb/include/lldb/Target/StopInfo.h
+++ b/lldb/include/lldb/Target/StopInfo.h
@@ -79,13 +79,14 @@ class StopInfo : public
std::enable_shared_from_this<StopInfo> {
/// 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
+ /// 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 std::optional<uint32_t>
+ GetSuggestedStackFrameIndex(bool inlined_stack) {
+ return {};
}
virtual bool IsValidForOperatingSystemThread(Thread &thread) { return true; }
diff --git a/lldb/include/lldb/Target/ThreadPlanStepInRange.h
b/lldb/include/lldb/Target/ThreadPlanStepInRange.h
index 6bd79e1d287d35..f2cb1085302318 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.
+ LazyBool 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 61fe467987228b..7e002b9ba4c2e2 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -508,7 +508,7 @@ 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 (GetPreferredLineEntry()) {
@@ -520,7 +520,8 @@ void BreakpointLocation::GetDescription(Stream *s,
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, show_function_info,
+ show_function_info, show_function_info);
} else {
if (sc.module_sp) {
s->EOL();
@@ -670,7 +671,7 @@ void BreakpointLocation::SendBreakpointLocationChangedEvent(
}
}
-std::optional<uint32_t> BreakpointLocation::GetSuggestedStackFrameIndex() {
+std::optional<uint32_t> BreakpointLocation::GetSuggestedStackFrameIndex() {
if (!GetPreferredLineEntry())
return {};
LineEntry preferred = *GetPreferredLineEntry();
@@ -682,12 +683,13 @@ std::optional<uint32_t>
BreakpointLocation::GetSuggestedStackFrameIndex() {
// 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);
+ Declaration preferred_decl(preferred.GetFile(), preferred.line,
+ preferred.column);
uint32_t depth = 0;
Block *inlined_block = sc.block->GetContainingInlinedBlock();
@@ -695,8 +697,8 @@ std::optional<uint32_t>
BreakpointLocation::GetSuggestedStackFrameIndex() {
// 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)
+ if (!inlined_block->GetStartAddress(start_address) ||
+ start_address != m_address)
return {};
const InlineFunctionInfo *info = inlined_block->GetInlinedFunctionInfo();
@@ -710,7 +712,6 @@ std::optional<uint32_t>
BreakpointLocation::GetSuggestedStackFrameIndex() {
depth++;
}
return {};
-
}
void BreakpointLocation::SwapLocation(BreakpointLocationSP swap_from) {
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp
b/lldb/source/Breakpoint/BreakpointResolver.cpp
index 465fc387aa43e5..688dffa02eb162 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -340,17 +340,17 @@ void BreakpointResolver::AddLocation(SearchFilter &filter,
}
BreakpointLocationSP bp_loc_sp(AddLocation(line_start));
- // If the address that we resolved the location to returns a different
+ // 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 &&
+ 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.
- bp_loc_sp->SetPreferredLineEntry(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.
+ bp_loc_sp->SetPreferredLineEntry(sc.line_entry);
}
if (log && bp_loc_sp && !GetBreakpoint()->IsInternal()) {
StreamString s;
diff --git a/lldb/source/Symbol/CompileUnit.cpp
b/lldb/source/Symbol/CompileUnit.cpp
index f4c61d6ec1b196..666e682ff537c4 100644
--- a/lldb/source/Symbol/CompileUnit.cpp
+++ b/lldb/source/Symbol/CompileUnit.cpp
@@ -251,8 +251,10 @@ 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(LLDB_INVALID_LINE_NUMBER);
+ const uint32_t column_num =
+ src_location_spec.GetColumn().value_or(LLDB_INVALID_COLUMN_NUMBER);
const bool check_inlines = src_location_spec.GetCheckInlines();
// First find all of the file indexes that match our "file_spec". If
@@ -312,42 +314,45 @@ void CompileUnit::ResolveSymbolContext(
line_idx = line_table->FindLineEntryIndexByFileIndex(
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
+ // 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
+ // 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) {
+ 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 {
+ 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();
+ 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
@@ -355,13 +360,14 @@ void CompileUnit::ResolveSymbolContext(
// it.
Declaration found_decl = inline_info->GetCallSite();
uint32_t sought_column = sought_decl.GetColumn();
- if (found_decl.FileAndLineEqual(sought_decl)
- && (sought_column == LLDB_INVALID_COLUMN_NUMBER
- || sought_column == found_decl.GetColumn())) {
+ if (found_decl.FileAndLineEqual(sought_decl) &&
+ (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)) {
+ 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.
@@ -373,31 +379,32 @@ void CompileUnit::ResolveSymbolContext(
// 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 ((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 &&
+ 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);
+ 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();
+ 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);
@@ -405,7 +412,7 @@ void CompileUnit::ResolveSymbolContext(
}
// 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
+ // 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
diff --git a/lldb/source/Target/StackFrameList.cpp
b/lldb/source/Target/StackFrameList.cpp
index cda360baed6491..94a381edd5e202 100644
--- a/lldb/source/Target/StackFrameList.cpp
+++ b/lldb/source/Target/StackFrameList.cpp
@@ -85,14 +85,14 @@ void StackFrameList::ResetCurrentInlinedDepth() {
return;
std::lock_guard<std::recursive_mutex> guard(m_mutex);
-
+
m_current_inlined_pc = LLDB_INVALID_ADDRESS;
m_current_inlined_depth = UINT32_MAX;
StopInfoSP stop_info_sp = m_thread.GetStopInfo();
if (!stop_info_sp)
return;
-
+
bool inlined = true;
auto inline_depth = stop_info_sp->GetSuggestedStackFrameIndex(inlined);
// We're only adjusting the inlined stack here.
@@ -745,27 +745,28 @@ void StackFrameList::SelectMostRelevantFrame() {
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
+ // 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);
+ std::optional<uint32_t> stack_opt =
+ stop_info_sp->GetSuggestedStackFrameIndex(inlined);
if (stack_opt) {
stack_idx = *stack_opt;
found_relevant = true;
}
}
-
+
frame_sp = GetFrameAtIndex(stack_idx);
if (!frame_sp)
- LLDB_LOG(log, "Stop info suggested relevant frame {0} but it didn't
exist",
+ 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
+ // 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)
LLDB_LOG(log, "No relevant frame!");
}
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 8ba46366c8b9d9..aa9c97a615b25e 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -247,7 +247,8 @@ class StopInfoBreakpoint : public StopInfo {
return m_description.c_str();
}
- std::optional<uint32_t> GetSuggestedStackFrameIndex(bool inlined_stack)
override {
+ std::optional<uint32_t>
+ GetSuggestedStackFrameIndex(bool inlined_stack) override {
if (!inlined_stack)
return {};
@@ -258,10 +259,10 @@ class StopInfoBreakpoint : public StopInfo {
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
@@ -1180,9 +1181,9 @@ class StopInfoTrace : public StopInfo {
else
return m_description.c_str();
}
-
- std::optional<uint32_t> GetSuggestedStackFrameIndex(bool inlined_stack)
override
- {
+
+ std::optional<uint32_t>
+ GetSuggestedStackFrameIndex(bool inlined_stack) override {
// Trace only knows how to adjust inlined stacks:
if (!inlined_stack)
return {};
@@ -1201,7 +1202,7 @@ class StopInfoTrace : public StopInfo {
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();
@@ -1218,7 +1219,6 @@ class StopInfoTrace : public StopInfo {
inlined_stack = true;
return num_inlined_functions + 1;
}
-
};
// StopInfoException
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index d13013c18565c9..6c22b50d3b6068 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -620,7 +620,7 @@ 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
+ // 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.
diff --git a/lldb/source/Target/ThreadPlanStepInRange.cpp
b/lldb/source/Target/ThreadPlanStepInRange.cpp
index 5319afb4edc418..325a70619908b6 100644
--- a/lldb/source/Target/ThreadPlanStepInRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepInRange.cpp
@@ -486,7 +486,7 @@ bool ThreadPlanStepInRange::DoWillResume(lldb::StateType
resume_state,
return true;
}
-bool ThreadPlanStepInRange::IsVirtualStep() {
+bool ThreadPlanStepInRange::IsVirtualStep() {
if (m_virtual_step == eLazyBoolCalculate) {
Thread &thread = GetThread();
if (thread.GetCurrentInlinedDepth() == UINT32_MAX)
@@ -494,5 +494,5 @@ bool ThreadPlanStepInRange::IsVirtualStep() {
else
m_virtual_step = eLazyBoolYes;
}
- return m_virtual_step == eLazyBoolYes;
+ return m_virtual_step == eLazyBoolYes;
}
diff --git a/lldb/test/API/functionalities/inline-stepping/calling.cpp
b/lldb/test/API/functionalities/inline-stepping/calling.cpp
index 0009388bd80633..d7ee56b3c07909 100644
--- a/lldb/test/API/functionalities/inline-stepping/calling.cpp
+++ b/lldb/test/API/functionalities/inline-stepping/calling.cpp
@@ -15,9 +15,9 @@ 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));
+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 ();
@@ -88,24 +88,18 @@ 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_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_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 caller_trivial_inline_3() {
+ inline_value += 1; // In caller_trivial_inline_3.
}
void
@@ -162,6 +156,6 @@ main (int argc, char **argv)
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.
}
>From a19b3dce64dadccc11559fa1d626ea67c5a9bff7 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jing...@apple.com>
Date: Fri, 18 Oct 2024 10:56:59 -0700
Subject: [PATCH 03/12] Comment -> Doxygen comment.
---
.../lldb/Breakpoint/BreakpointLocation.h | 24 +++++++++----------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index 552fafab97cb9a..fc1de12ae27f92 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -284,10 +284,10 @@ 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.
+ /// 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.
void SetPreferredLineEntry(const LineEntry &line_entry) {
m_preferred_line_entry = line_entry;
}
@@ -320,14 +320,14 @@ 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.
+ /// 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:
>From b35e42b0934db05e9a9fc7cced1989e0f96f7aec Mon Sep 17 00:00:00 2001
From: Jim Ingham <jing...@apple.com>
Date: Fri, 18 Oct 2024 10:59:15 -0700
Subject: [PATCH 04/12] uglification
---
.../inline-stepping/TestInlineStepping.py | 28 +++++++++++++------
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git
a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
index 92389d5ebfa645..631f8d8553220b 100644
--- a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
+++ b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
@@ -369,7 +369,9 @@ def run_to_call_site_and_step(self, source_regex,
func_name, start_pos):
# 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)
+ 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.):
@@ -383,15 +385,21 @@ def run_to_call_site_and_step(self, source_regex,
func_name, start_pos):
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")
+ # 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}")
+
+ 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
@@ -407,7 +415,9 @@ def run_to_call_site_and_step(self, source_regex,
func_name, start_pos):
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)
-
-
+ 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
+ )
>From ecda9b3c304fe7e7ab1b17dee6597cd42254f648 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jing...@apple.com>
Date: Fri, 18 Oct 2024 11:04:57 -0700
Subject: [PATCH 05/12] more doxygenification.
---
lldb/include/lldb/Breakpoint/BreakpointLocation.h | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index fc1de12ae27f92..c09251e7ceea8c 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -393,14 +393,11 @@ 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.
- std::optional<LineEntry>
- m_preferred_line_entry; // 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.
+ /// 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;
>From 8662acfb664bbc9ad84840794f8bc25f8006171a Mon Sep 17 00:00:00 2001
From: Jim Ingham <jing...@apple.com>
Date: Fri, 18 Oct 2024 11:16:06 -0700
Subject: [PATCH 06/12] Remove debugging print's.
---
.../API/functionalities/inline-stepping/TestInlineStepping.py | 1 -
1 file changed, 1 deletion(-)
diff --git
a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
index 631f8d8553220b..f52e0f0fd5bcfe 100644
--- a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
+++ b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
@@ -381,7 +381,6 @@ def run_to_call_site_and_step(self, source_regex,
func_name, start_pos):
self.assertTrue(result, "Got a location description")
desc = strm.GetData()
- print(f"Description:\n{desc}\n")
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.
>From 38f883f3d4222c1374763abddde456c65a47bee1 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jing...@apple.com>
Date: Mon, 21 Oct 2024 10:29:13 -0700
Subject: [PATCH 07/12] More review comments.
---
lldb/source/Breakpoint/BreakpointLocation.cpp | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp
b/lldb/source/Breakpoint/BreakpointLocation.cpp
index 7e002b9ba4c2e2..48ee3b9f96a53d 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -511,8 +511,8 @@ void BreakpointLocation::GetDescription(Stream *s,
// If there's a preferred line entry for printing, use that.
bool show_function_info = true;
- if (GetPreferredLineEntry()) {
- sc.line_entry = *GetPreferredLineEntry();
+ 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
@@ -549,8 +549,8 @@ void BreakpointLocation::GetDescription(Stream *s,
if (sc.line_entry.line > 0) {
s->EOL();
s->Indent("location = ");
- if (GetPreferredLineEntry())
- GetPreferredLineEntry()->DumpStopContext(s, true);
+ if (auto preferred = GetPreferredLineEntry())
+ preferred->DumpStopContext(s, true);
else
sc.line_entry.DumpStopContext(s, true);
}
@@ -672,9 +672,10 @@ void
BreakpointLocation::SendBreakpointLocationChangedEvent(
}
std::optional<uint32_t> BreakpointLocation::GetSuggestedStackFrameIndex() {
- if (!GetPreferredLineEntry())
+ auto preferred_opt = GetPreferredLineEntry();
+ if (!preferred_opt)
return {};
- LineEntry preferred = *GetPreferredLineEntry();
+ LineEntry preferred = *preferred_opt;
SymbolContext sc;
if (!m_address.CalculateSymbolContext(&sc))
return {};
>From 03764f2a50d5899034b0b69455a3175e928c61db Mon Sep 17 00:00:00 2001
From: Jim Ingham <jing...@apple.com>
Date: Mon, 21 Oct 2024 16:16:19 -0700
Subject: [PATCH 08/12] More review comments.
---
lldb/source/Breakpoint/BreakpointSite.cpp | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/lldb/source/Breakpoint/BreakpointSite.cpp
b/lldb/source/Breakpoint/BreakpointSite.cpp
index b283952d028305..40b72aae60be22 100644
--- a/lldb/source/Breakpoint/BreakpointSite.cpp
+++ b/lldb/source/Breakpoint/BreakpointSite.cpp
@@ -92,12 +92,13 @@ 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> this_result =
loc_sp->GetSuggestedStackFrameIndex();
- if (this_result) {
- if (!result)
- result = this_result;
+ 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 = std::max(*this_result, *result);
+ result = this_result;
}
}
return result;
>From 594fb633fbf8acf4c506e5c3ad3cce114b2d8ba4 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jing...@apple.com>
Date: Mon, 21 Oct 2024 17:30:18 -0700
Subject: [PATCH 09/12] Add a `full` parameter to Declaration::Equals since
different code paths need different behaviors.
---
lldb/include/lldb/Core/Declaration.h | 6 +++++-
lldb/source/Breakpoint/BreakpointSite.cpp | 2 +-
lldb/source/Core/Declaration.cpp | 5 +++--
lldb/source/Symbol/Block.cpp | 2 +-
lldb/source/Symbol/CompileUnit.cpp | 2 +-
5 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/lldb/include/lldb/Core/Declaration.h
b/lldb/include/lldb/Core/Declaration.h
index 4a0e9047b54695..c9b4eb9da1d097 100644
--- a/lldb/include/lldb/Core/Declaration.h
+++ b/lldb/include/lldb/Core/Declaration.h
@@ -83,11 +83,15 @@ 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) const;
+ bool FileAndLineEqual(const Declaration &declaration, bool full) const;
/// Dump a description of this object to a Stream.
///
diff --git a/lldb/source/Breakpoint/BreakpointSite.cpp
b/lldb/source/Breakpoint/BreakpointSite.cpp
index 40b72aae60be22..2c07126312b20d 100644
--- a/lldb/source/Breakpoint/BreakpointSite.cpp
+++ b/lldb/source/Breakpoint/BreakpointSite.cpp
@@ -98,7 +98,7 @@ std::optional<uint32_t>
BreakpointSite::GetSuggestedStackFrameIndex() {
if (result)
result = std::max(*loc_frame_index, *result);
else
- result = this_result;
+ result = loc_frame_index;
}
}
return result;
diff --git a/lldb/source/Core/Declaration.cpp b/lldb/source/Core/Declaration.cpp
index e7855f3e364376..36d245e92a0e7d 100644
--- a/lldb/source/Core/Declaration.cpp
+++ b/lldb/source/Core/Declaration.cpp
@@ -70,8 +70,9 @@ int Declaration::Compare(const Declaration &a, const
Declaration &b) {
return 0;
}
-bool Declaration::FileAndLineEqual(const Declaration &declaration) const {
- int file_compare = FileSpec::Compare(this->m_file, declaration.m_file,
false);
+bool Declaration::FileAndLineEqual(const Declaration &declaration, bool full)
+ const {
+ int file_compare = FileSpec::Compare(this->m_file, declaration.m_file, full);
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 f7d9c0d2d33065..5c7772a6db780d 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))
+ function_info->GetCallSite().FileAndLineEqual(find_call_site, true))
return inlined_block;
inlined_block = inlined_block->GetInlinedParent();
}
diff --git a/lldb/source/Symbol/CompileUnit.cpp
b/lldb/source/Symbol/CompileUnit.cpp
index 666e682ff537c4..f0f7e40ae70d83 100644
--- a/lldb/source/Symbol/CompileUnit.cpp
+++ b/lldb/source/Symbol/CompileUnit.cpp
@@ -360,7 +360,7 @@ void CompileUnit::ResolveSymbolContext(
// it.
Declaration found_decl = inline_info->GetCallSite();
uint32_t sought_column = sought_decl.GetColumn();
- if (found_decl.FileAndLineEqual(sought_decl) &&
+ 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,
>From e3c26b000a9f9ffb5b95b69619c2e7c3dbf53b86 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jing...@apple.com>
Date: Tue, 22 Oct 2024 12:18:44 -0700
Subject: [PATCH 10/12] more clang format
---
lldb/include/lldb/Breakpoint/BreakpointLocation.h | 4 ++--
lldb/include/lldb/Core/Declaration.h | 4 ++--
lldb/include/lldb/Target/ThreadPlanStepInRange.h | 2 +-
lldb/source/Breakpoint/BreakpointSite.cpp | 4 ++--
lldb/source/Core/Declaration.cpp | 4 ++--
5 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index c09251e7ceea8c..046ebc34b4e6c9 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -322,8 +322,8 @@ class BreakpointLocation
/// 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
+ /// 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
diff --git a/lldb/include/lldb/Core/Declaration.h
b/lldb/include/lldb/Core/Declaration.h
index c9b4eb9da1d097..c864b88c6b32a3 100644
--- a/lldb/include/lldb/Core/Declaration.h
+++ b/lldb/include/lldb/Core/Declaration.h
@@ -83,9 +83,9 @@ 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
+ /// 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
diff --git a/lldb/include/lldb/Target/ThreadPlanStepInRange.h
b/lldb/include/lldb/Target/ThreadPlanStepInRange.h
index f2cb1085302318..9da8370ef1c925 100644
--- a/lldb/include/lldb/Target/ThreadPlanStepInRange.h
+++ b/lldb/include/lldb/Target/ThreadPlanStepInRange.h
@@ -81,7 +81,7 @@ class ThreadPlanStepInRange : public ThreadPlanStepRange,
// 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.
+ // just moved the inline stack depth.
ConstString m_step_into_target;
ThreadPlanStepInRange(const ThreadPlanStepInRange &) = delete;
const ThreadPlanStepInRange &
diff --git a/lldb/source/Breakpoint/BreakpointSite.cpp
b/lldb/source/Breakpoint/BreakpointSite.cpp
index 2c07126312b20d..9700a57d3346e0 100644
--- a/lldb/source/Breakpoint/BreakpointSite.cpp
+++ b/lldb/source/Breakpoint/BreakpointSite.cpp
@@ -92,8 +92,8 @@ 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();
+ std::optional<uint32_t> loc_frame_index =
+ loc_sp->GetSuggestedStackFrameIndex();
if (loc_frame_index) {
if (result)
result = std::max(*loc_frame_index, *result);
diff --git a/lldb/source/Core/Declaration.cpp b/lldb/source/Core/Declaration.cpp
index 36d245e92a0e7d..a485c4b9ba48a7 100644
--- a/lldb/source/Core/Declaration.cpp
+++ b/lldb/source/Core/Declaration.cpp
@@ -70,8 +70,8 @@ int Declaration::Compare(const Declaration &a, const
Declaration &b) {
return 0;
}
-bool Declaration::FileAndLineEqual(const Declaration &declaration, bool full)
- const {
+bool Declaration::FileAndLineEqual(const Declaration &declaration,
+ bool full) const {
int file_compare = FileSpec::Compare(this->m_file, declaration.m_file, full);
return file_compare == 0 && this->m_line == declaration.m_line;
}
>From 9861c7d8ede4e0dcd22dd4e7f40c0a4d2ad24445 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jing...@apple.com>
Date: Thu, 24 Oct 2024 17:26:15 -0700
Subject: [PATCH 11/12] Enforce that a "PreferredLineEntry" can't change the
location address.
---
lldb/include/lldb/Breakpoint/BreakpointLocation.h | 11 +++++++++--
lldb/source/Breakpoint/BreakpointResolver.cpp | 6 +++++-
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index 046ebc34b4e6c9..32ebc6dab988a1 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -288,8 +288,15 @@ class BreakpointLocation
/// 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.
- void SetPreferredLineEntry(const LineEntry &line_entry) {
- m_preferred_line_entry = line_entry;
+ /// 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() {
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp
b/lldb/source/Breakpoint/BreakpointResolver.cpp
index 688dffa02eb162..166b56e6c55103 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -350,7 +350,11 @@ void BreakpointResolver::AddLocation(SearchFilter &filter,
// 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.
- bp_loc_sp->SetPreferredLineEntry(sc.line_entry);
+ 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;
>From a2aeab6f00319ec46a6835e2fdd318cb22a4d194 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jing...@apple.com>
Date: Mon, 28 Oct 2024 09:46:32 -0700
Subject: [PATCH 12/12] more clang-format
---
lldb/include/lldb/Breakpoint/BreakpointLocation.h | 2 +-
lldb/source/Breakpoint/BreakpointResolver.cpp | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index 32ebc6dab988a1..3592291bb2d06e 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -289,7 +289,7 @@ class BreakpointLocation
/// 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.
+ /// location.
bool SetPreferredLineEntry(const LineEntry &line_entry) {
if (m_address == line_entry.range.GetBaseAddress()) {
m_preferred_line_entry = line_entry;
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp
b/lldb/source/Breakpoint/BreakpointResolver.cpp
index 166b56e6c55103..9643602d78c751 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -353,7 +353,6 @@ void BreakpointResolver::AddLocation(SearchFilter &filter,
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()) {
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
