Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[lldb] Use Function::GetAddressRange*s* in "frame diagnose" #130949

Merged
merged 1 commit into from
Mar 13, 2025

Conversation

labath
Copy link
Collaborator

@labath labath commented Mar 12, 2025

No test because generating discontinous functions is tedious and there's nothing particularly interesting happening in here. As long as the analyzer stays within a single basic block. it doesn't really care whether the function is discontinous or not. I could create cases where the algorithm breaks when going across basic blocks, but that's more of inherent limitation of the algorithm (the inability to follow jumps "backwards") than something specific to discontinous functions.

At this point, I'm more interested in cleaning up the last few remaining uses of the deprecated function that I'm about improving "frame diagnose".

No test because generating discontinous functions is tedious and there's
nothing particularly interesting happening in here. As long as the
analyzer stays within a single basic block. it doesn't really care
whether the function is discontinous or not. I could create cases the
algorithm breaks when going across basic blocks, but that's more of
inherent limitation of the algorithm (the inability to follow jumps
"backwards") than something specific to discontinous functions.

At this point, I'm more interested in cleaning up the last few remaining
uses of the deprecated function that I'm about improving "frame
diagnose".
@labath labath requested a review from DavidSpickett March 12, 2025 12:17
@labath labath requested a review from JDevlieghere as a code owner March 12, 2025 12:17
@llvmbot llvmbot added the lldb label Mar 12, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 12, 2025

@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)

Changes

No test because generating discontinous functions is tedious and there's nothing particularly interesting happening in here. As long as the analyzer stays within a single basic block. it doesn't really care whether the function is discontinous or not. I could create cases where the algorithm breaks when going across basic blocks, but that's more of inherent limitation of the algorithm (the inability to follow jumps "backwards") than something specific to discontinous functions.

At this point, I'm more interested in cleaning up the last few remaining uses of the deprecated function that I'm about improving "frame diagnose".


Full diff: https://github.com/llvm/llvm-project/pull/130949.diff

1 Files Affected:

  • (modified) lldb/source/Target/StackFrame.cpp (+6-10)
diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp
index d92b7d8b9d899..bab36e9aa1033 100644
--- a/lldb/source/Target/StackFrame.cpp
+++ b/lldb/source/Target/StackFrame.cpp
@@ -1783,15 +1783,11 @@ lldb::ValueObjectSP StackFrame::GuessValueForRegisterAndOffset(ConstString reg,
     return ValueObjectSP();
   }
 
-  AddressRange pc_range = function->GetAddressRange();
-
-  if (GetFrameCodeAddress().GetFileAddress() <
-          pc_range.GetBaseAddress().GetFileAddress() ||
-      GetFrameCodeAddress().GetFileAddress() -
-              pc_range.GetBaseAddress().GetFileAddress() >=
-          pc_range.GetByteSize()) {
+  AddressRange unused_range;
+  if (!function->GetRangeContainingLoadAddress(
+          GetFrameCodeAddress().GetLoadAddress(target_sp.get()), *target_sp,
+          unused_range))
     return ValueObjectSP();
-  }
 
   const char *plugin_name = nullptr;
   const char *flavor = nullptr;
@@ -1799,8 +1795,8 @@ lldb::ValueObjectSP StackFrame::GuessValueForRegisterAndOffset(ConstString reg,
   const char *features = nullptr;
   const bool force_live_memory = true;
   DisassemblerSP disassembler_sp = Disassembler::DisassembleRange(
-      target_arch, plugin_name, flavor, cpu, features, *target_sp, pc_range,
-      force_live_memory);
+      target_arch, plugin_name, flavor, cpu, features, *target_sp,
+      function->GetAddressRanges(), force_live_memory);
 
   if (!disassembler_sp || !disassembler_sp->GetInstructionList().GetSize()) {
     return ValueObjectSP();

Copy link
Collaborator

@DavidSpickett DavidSpickett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating the API, not trying to add anything new to frame diagnose, sounds good to me.

@labath labath merged commit a413ef8 into llvm:main Mar 13, 2025
12 checks passed
@labath labath deleted the diagnose branch March 13, 2025 12:35
frederik-h pushed a commit to frederik-h/llvm-project that referenced this pull request Mar 18, 2025
)

No test because generating discontinous functions is tedious and there's
nothing particularly interesting happening in here. As long as the
analyzer stays within a single basic block. it doesn't really care
whether the function is discontinous or not. I could create cases where
the algorithm breaks when going across basic blocks, but that's more of
inherent limitation of the algorithm (the inability to follow jumps
"backwards") than something specific to discontinous functions.

At this point, I'm more interested in cleaning up the last few remaining
uses of the deprecated function that I'm about improving "frame
diagnose".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants