Skip to content

[lldb] Return an llvm::Error from GetFrameBaseValue #111882

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

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

JDevlieghere
Copy link
Member

@JDevlieghere JDevlieghere commented Oct 10, 2024

This fixes the following assertion: "Cannot create Expected from Error success value." The problem was that GetFrameBaseValue return false without updating the Status argument. This patch eliminates this possibility by returning an llvm:Error.

@llvmbot
Copy link
Member

llvmbot commented Oct 10, 2024

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

This fixes the following assertion: "Cannot create Expected<T> from Error success value." The problem was that GetFrameBaseValue returned false without updating the Status argument. This patch makes sure that every return path updates the Status if the pointer is valid.


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

1 Files Affected:

  • (modified) lldb/source/Target/StackFrame.cpp (+3-1)
diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp
index fe0d4c93c50627..730ec0387917f8 100644
--- a/lldb/source/Target/StackFrame.cpp
+++ b/lldb/source/Target/StackFrame.cpp
@@ -1084,7 +1084,9 @@ bool StackFrame::GetFrameBaseValue(Scalar &frame_base, Status *error_ptr) {
   if (!m_cfa_is_valid) {
     m_frame_base_error = Status::FromErrorString(
         "No frame base available for this historical stack frame.");
-    return false;
+    if (error_ptr)
+      *error_ptr = m_frame_base_error.Clone();
+    return m_frame_base_error.Success();
   }
 
   if (m_flags.IsClear(GOT_FRAME_BASE)) {

This fixes the following assertion: "Cannot create Expected<T> from
Error success value." The problem was that GetFrameBaseValue return
false without updating the Status argument. This patch eliminates this
possibility by returning an llvm:Error.
@JDevlieghere JDevlieghere force-pushed the GetFrameBaseValueStatus branch from e00c696 to c3c1126 Compare October 10, 2024 18:41
@JDevlieghere JDevlieghere changed the title [lldb] Make sure Status is updated in GetFrameBaseValue [lldb] Return an llvm::Error from GetFrameBaseValue Oct 10, 2024
Copy link
Collaborator

@adrian-prantl adrian-prantl left a comment

Choose a reason for hiding this comment

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

Thanks!

@JDevlieghere JDevlieghere merged commit 69b0b7e into llvm:main Oct 10, 2024
5 of 6 checks passed
@JDevlieghere JDevlieghere deleted the GetFrameBaseValueStatus branch October 10, 2024 20:11
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
This fixes the following assertion: "Cannot create Expected<T> from
Error success value." The problem was that GetFrameBaseValue return
false without updating the Status argument. This patch eliminates the 
opportunity for mistakes by returning an llvm:Error.
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