Skip to content

[lldb] Fix error reporting in SBTarget::ReadMemory #109764

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
Sep 25, 2024
Merged

Conversation

labath
Copy link
Collaborator

@labath labath commented Sep 24, 2024

The function should use the by-ref SBError argument instead of creating a new one. This code has been here since ~forever, and was probably copied from methods which return an SBError result (where one needs to create a local variable).

The function should use the by-ref SBError argument instead of creating
a new one. This code has been here since ~forever, and was probably
copied from methods which return an SBError result (where one needs to
create a local variable).
@llvmbot
Copy link
Member

llvmbot commented Sep 24, 2024

@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)

Changes

The function should use the by-ref SBError argument instead of creating a new one. This code has been here since ~forever, and was probably copied from methods which return an SBError result (where one needs to create a local variable).


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

2 Files Affected:

  • (modified) lldb/source/API/SBTarget.cpp (+2-3)
  • (modified) lldb/test/API/python_api/target/TestTargetAPI.py (+5)
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index 1c1f7e2a03def8..d5017ad6bff166 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -662,15 +662,14 @@ size_t SBTarget::ReadMemory(const SBAddress addr, void *buf, size_t size,
                             lldb::SBError &error) {
   LLDB_INSTRUMENT_VA(this, addr, buf, size, error);
 
-  SBError sb_error;
   size_t bytes_read = 0;
   TargetSP target_sp(GetSP());
   if (target_sp) {
     std::lock_guard<std::recursive_mutex> guard(target_sp->GetAPIMutex());
     bytes_read =
-        target_sp->ReadMemory(addr.ref(), buf, size, sb_error.ref(), true);
+        target_sp->ReadMemory(addr.ref(), buf, size, error.ref(), true);
   } else {
-    sb_error.SetErrorString("invalid target");
+    error.SetErrorString("invalid target");
   }
 
   return bytes_read;
diff --git a/lldb/test/API/python_api/target/TestTargetAPI.py b/lldb/test/API/python_api/target/TestTargetAPI.py
index 2e8d6a5b1e53f6..155a25b576b03a 100644
--- a/lldb/test/API/python_api/target/TestTargetAPI.py
+++ b/lldb/test/API/python_api/target/TestTargetAPI.py
@@ -153,6 +153,11 @@ def test_read_memory(self):
         self.assertSuccess(error, "Make sure memory read succeeded")
         self.assertEqual(len(content), 1)
 
+        # Make sure reading from 0x0 fails
+        sb_addr = lldb.SBAddress(0, target)
+        self.assertIsNone(target.ReadMemory(sb_addr, 1, error))
+        self.assertTrue(error.Fail())
+
     @skipIfWindows  # stdio manipulation unsupported on Windows
     @skipIfRemote  # stdio manipulation unsupported on remote iOS devices<rdar://problem/54581135>
     @skipIf(oslist=["linux"], archs=["arm", "aarch64"])

@labath labath merged commit df6822f into llvm:main Sep 25, 2024
9 checks passed
@labath labath deleted the memory branch November 8, 2024 12:28
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.

5 participants