Skip to content

[lldb] Refactor ResolveLoadAddress to return an llvm::Expected (NFC) #94558

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
Jun 6, 2024

Conversation

JDevlieghere
Copy link
Member

No description provided.

@llvmbot llvmbot added the lldb label Jun 6, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 6, 2024

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

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

1 Files Affected:

  • (modified) lldb/source/Expression/DWARFExpression.cpp (+17-31)
diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index 80d03c84fadbd..f0abaa07d7197 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -782,7 +782,6 @@ void UpdateValueTypeFromLocationDescription(Log *log, const DWARFUnit *dwarf_cu,
 ///
 /// \param exe_ctx Pointer to the execution context
 /// \param module_sp shared_ptr contains the module if we have one
-/// \param error_ptr pointer to Status object if we have one
 /// \param dw_op_type C-style string used to vary the error output
 /// \param file_addr the file address we are trying to resolve and turn into a
 ///                  load address
@@ -793,32 +792,22 @@ void UpdateValueTypeFromLocationDescription(Log *log, const DWARFUnit *dwarf_cu,
 ///          the load address succeed or an empty Optinal otherwise. If
 ///          check_sectionoffset is true we consider LLDB_INVALID_ADDRESS a
 ///          success if so_addr.IsSectionOffset() is true.
-static std::optional<lldb::addr_t>
+static llvm::Expected<lldb::addr_t>
 ResolveLoadAddress(ExecutionContext *exe_ctx, lldb::ModuleSP &module_sp,
-                   Status *error_ptr, const char *dw_op_type,
-                   lldb::addr_t file_addr, Address &so_addr,
-                   bool check_sectionoffset = false) {
-  if (!module_sp) {
-    if (error_ptr)
-      error_ptr->SetErrorStringWithFormat(
-          "need module to resolve file address for %s", dw_op_type);
-    return {};
-  }
+                   const char *dw_op_type, lldb::addr_t file_addr,
+                   Address &so_addr, bool check_sectionoffset = false) {
+  if (!module_sp)
+    return llvm::createStringError("need module to resolve file address for %s",
+                                   dw_op_type);
 
-  if (!module_sp->ResolveFileAddress(file_addr, so_addr)) {
-    if (error_ptr)
-      error_ptr->SetErrorString("failed to resolve file address in module");
-    return {};
-  }
+  if (!module_sp->ResolveFileAddress(file_addr, so_addr))
+    return llvm::createStringError("failed to resolve file address in module");
 
-  addr_t load_addr = so_addr.GetLoadAddress(exe_ctx->GetTargetPtr());
+  const addr_t load_addr = so_addr.GetLoadAddress(exe_ctx->GetTargetPtr());
 
   if (load_addr == LLDB_INVALID_ADDRESS &&
-      (check_sectionoffset && !so_addr.IsSectionOffset())) {
-    if (error_ptr)
-      error_ptr->SetErrorString("failed to resolve load address");
-    return {};
-  }
+      (check_sectionoffset && !so_addr.IsSectionOffset()))
+    return llvm::createStringError("failed to resolve load address");
 
   return load_addr;
 }
@@ -988,12 +977,11 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
             LLDB_INVALID_ADDRESS);
 
         Address so_addr;
-        Status load_err;
         auto maybe_load_addr = ResolveLoadAddress(
-            exe_ctx, module_sp, &load_err, "DW_OP_deref", file_addr, so_addr);
+            exe_ctx, module_sp, "DW_OP_deref", file_addr, so_addr);
 
         if (!maybe_load_addr)
-          return load_err.ToError();
+          return maybe_load_addr.takeError();
 
         stack.back().GetScalar() = *maybe_load_addr;
         // Fall through to load address promotion code below.
@@ -1105,14 +1093,12 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
         auto file_addr =
             stack.back().GetScalar().ULongLong(LLDB_INVALID_ADDRESS);
         Address so_addr;
-        Status resolve_err;
-        auto maybe_load_addr =
-            ResolveLoadAddress(exe_ctx, module_sp, &resolve_err,
-                               "DW_OP_deref_size", file_addr, so_addr,
-                               /*check_sectionoffset=*/true);
+        auto maybe_load_addr = ResolveLoadAddress(
+            exe_ctx, module_sp, "DW_OP_deref_size", file_addr, so_addr,
+            /*check_sectionoffset=*/true);
 
         if (!maybe_load_addr)
-          return resolve_err.ToError();
+          return maybe_load_addr.takeError();
 
         addr_t load_addr = *maybe_load_addr;
 

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.

LGTM

@JDevlieghere JDevlieghere merged commit 67aaa9f into llvm:main Jun 6, 2024
7 checks passed
@JDevlieghere JDevlieghere deleted the dwarfexpr-ResolveLoadAddress branch June 6, 2024 15:56
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