From 6275365467a3dbfdd69f749f55cced9a7ea6573d Mon Sep 17 00:00:00 2001
From: Adam Brouwers-Harries <aharries@upmem.com>
Date: Fri, 12 Aug 2022 16:24:49 +0200
Subject: [PATCH 1/2] dpu: lldb: Explicitly handle errors when looking up
 error_storage variable

---
 lldb/scripts/dpu/dpu_commands.py               |  4 ++++
 lldb/source/Plugins/Process/Dpu/Dpu.cpp        | 14 +++++++++-----
 lldb/source/Plugins/Process/Dpu/ProcessDpu.cpp | 16 +++++++++++++---
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/lldb/scripts/dpu/dpu_commands.py b/lldb/scripts/dpu/dpu_commands.py
index 60fd5243e2cb8..68c83e37add16 100644
--- a/lldb/scripts/dpu/dpu_commands.py
+++ b/lldb/scripts/dpu/dpu_commands.py
@@ -317,9 +317,13 @@ def dpu_attach(debugger, command, result, internal_dict):
     # normal lldb GDBRemote packet. Instead, we set an environment variable with the value that we
     # have looked up from the loaded bianry. If we cannot find one, then this environment variable
     # remains unset, and we cannot detach and re-attach within different processes.
+    # NOTE: The value of `storage.location` is a *HEX ENCODED* address.
     storage = target_dpu.FindFirstGlobalVariable("error_storage")
     if storage.IsValid():
         lldb_server_dpu_env["UPMEM_LLDB_ERROR_STORE_ADDR"] = str(storage.location)
+    else
+        print("Could not find valid storage location for `error_storage` variable in `target_dpu`")
+        return None
 
     subprocess.Popen(['lldb-server-dpu',
                       'gdbserver',
diff --git a/lldb/source/Plugins/Process/Dpu/Dpu.cpp b/lldb/source/Plugins/Process/Dpu/Dpu.cpp
index 1abb5714cb877..e669d8395fc8c 100644
--- a/lldb/source/Plugins/Process/Dpu/Dpu.cpp
+++ b/lldb/source/Plugins/Process/Dpu/Dpu.cpp
@@ -118,8 +118,12 @@ bool Dpu::SetPrintfSequenceAddrsFromRuntimeInfo(dpu_program_t *runtime) {
 }
 
 bool Dpu::SetErrorStoreAddr(const uint32_t _error_store_addr) {
-  error_store_addr = _error_store_addr;
-  return true;
+  if (_error_store_addr != 0 /*nullptr*/) {
+    error_store_addr = _error_store_addr;
+    return true;
+  } else {
+    return false;
+  }
 }
 
 bool Dpu::SetErrorStoreAddrFromRuntimeInfo(dpu_program_t *runtime) {
@@ -155,9 +159,9 @@ bool Dpu::LoadElf(const FileSpec &elf_file_path) {
   // we *don't* want to fail. This needs to be refactored so that we either a)
   // warn the user, or b) integrate the failure in the same way as the
   // SetPrintfSequence... does.
-  // if (!SetErrorStoreAddrFromRuntimeInfo(runtime))
-  //   return false;
-  SetErrorStoreAddrFromRuntimeInfo(runtime);
+  // NOTE: For now we are disregarding this, in order to fail more effectively.
+  if (!SetErrorStoreAddrFromRuntimeInfo(runtime))
+    return false;
 
   return true;
 }
diff --git a/lldb/source/Plugins/Process/Dpu/ProcessDpu.cpp b/lldb/source/Plugins/Process/Dpu/ProcessDpu.cpp
index 4a33880b36d53..112f1d4bef11b 100644
--- a/lldb/source/Plugins/Process/Dpu/ProcessDpu.cpp
+++ b/lldb/source/Plugins/Process/Dpu/ProcessDpu.cpp
@@ -214,9 +214,19 @@ ProcessDpu::Factory::Attach(
     dpu->SetNrThreads(::strtoll(nr_tasklets_ptr, NULL, 10));
 
   char *error_store_addr_ptr = std::getenv("UPMEM_LLDB_ERROR_STORE_ADDR");
-
-  if (error_store_addr_ptr != NULL)
-    dpu->SetErrorStoreAddr(::strtol(error_store_addr_ptr, NULL, 16));
+  if (error_store_addr_ptr != NULL) {
+    // Decode the error store address ptr string to an integer, and set the
+    // error store address to this value. Note that this is a hex number, due to
+    // how LLDB returns the string value of a pointers location.
+    if (!dpu->SetErrorStoreAddr(::strtol(error_store_addr_ptr, NULL, 16)))
+      return Status("Decoded (%s) to a null pointer as the error store address",
+                    error_store_addr_ptr)
+          .ToError();
+  } else {
+    return Status("Recieved null pointer from UPMEM_LLDB_ERROR_STORE_ADDR "
+                  "environment variable (not set?)")
+        .ToError();
+  }
 
   success = rank->SaveContext();
   if (!success)

From 26b6c836a9d13c4c109f6427f6e2def363f7c82e Mon Sep 17 00:00:00 2001
From: Adam Brouwers-Harries <aharries@upmem.com>
Date: Tue, 16 Aug 2022 16:41:30 +0200
Subject: [PATCH 2/2] Correct syntax of if-else

---
 lldb/scripts/dpu/dpu_commands.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/scripts/dpu/dpu_commands.py b/lldb/scripts/dpu/dpu_commands.py
index 68c83e37add16..99f57c9238c16 100644
--- a/lldb/scripts/dpu/dpu_commands.py
+++ b/lldb/scripts/dpu/dpu_commands.py
@@ -321,7 +321,7 @@ def dpu_attach(debugger, command, result, internal_dict):
     storage = target_dpu.FindFirstGlobalVariable("error_storage")
     if storage.IsValid():
         lldb_server_dpu_env["UPMEM_LLDB_ERROR_STORE_ADDR"] = str(storage.location)
-    else
+    else:
         print("Could not find valid storage location for `error_storage` variable in `target_dpu`")
         return None