dpu: lldb: Explicitly handle errors when looking up error_storage var…

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',

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;
 }

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)