[Lldb-commits] [PATCH] D115017: Fix error reporting for "process load" and add a test for it.

[Jim Ingham via Phabricator via lldb-commits]

jingham created this revision.
jingham added a reviewer: JDevlieghere.
Herald added a subscriber: emaste.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

We weren't reporting errors (like file not found) from "process load", because when you just passed one string and no paths, the utility function didn't actually gather the error.  Fix that and add a test.

The only weak part of the patch is that the error string comes from the platform plugin so I don't actually know what it is going to look like.  So I tested that we don't produce the "unknown error" string instead.  That's not really API, it's just what the only current implementation produces when it couldn't get the error.  If we get a second implementation, we'll either just ensure it produces the same string or extract that somewhere and set it as the generic error.  But that seemed overkill for this patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115017

Files:
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/test/API/functionalities/load_unload/TestLoadUnload.py
  lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py


Index: lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
===================================================================
--- lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
+++ lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
@@ -65,7 +65,12 @@
         # First try with no correct directories on the path, and make sure that doesn't blow up:
         token = process.LoadImageUsingPaths(lib_spec, paths, out_spec, error)
         self.assertEqual(token, lldb.LLDB_INVALID_IMAGE_TOKEN, "Only looked on the provided path.")
-
+        # Make sure we got some error back in this case.  Since we don't actually know what
+        # the error will look like, let's look for the absence of "unknown reasons".
+        error_str = error.description
+        self.assertNotEqual(len(error_str), 0, "Got an empty error string")
+        self.assertNotIn("unknown reasons", error_str, "Error string had unknown reasons")
+        
         # Now add the correct dir to the paths list and try again:
         paths.AppendString(self.hidden_dir)
         token = process.LoadImageUsingPaths(lib_spec, paths, out_spec, error)
@@ -121,8 +126,6 @@
 
         process.UnloadImage(token)
 
-
-
         # Finally, passing in an absolute path should work like the basename:
         # This should NOT work because we've taken hidden_dir off the paths:
         abs_spec = lldb.SBFileSpec(os.path.join(self.hidden_dir, self.lib_name))
@@ -137,5 +140,3 @@
 
         self.assertNotEqual(token, lldb.LLDB_INVALID_IMAGE_TOKEN, "Got a valid token")
         self.assertEqual(out_spec, lldb.SBFileSpec(self.hidden_lib), "Found the expected library")
-
-
Index: lldb/test/API/functionalities/load_unload/TestLoadUnload.py
===================================================================
--- lldb/test/API/functionalities/load_unload/TestLoadUnload.py
+++ lldb/test/API/functionalities/load_unload/TestLoadUnload.py
@@ -240,6 +240,15 @@
         else:
             remoteDylibPath = localDylibPath
 
+        # First make sure that we get some kind of error if process load fails.
+        # We print some error even if the load fails, which isn't formalized.
+        # The only plugin at present (Posix) that supports this says "unknown reasons".
+        # If another plugin shows up, let's require it uses "unknown error" as well.
+        non_existant_shlib = "/NoSuchDir/NoSuchSubdir/ReallyNo/NotAFile"
+        self.expect("process load %s"%(non_existant_shlib), error=True, matching=False,
+                    patterns=["unknown reasons"])
+        
+
         # Make sure that a_function does not exist at this point.
         self.expect(
             "image lookup -n a_function",
Index: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===================================================================
--- lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -589,6 +589,8 @@
       result_ptr->image_ptr = dlopen(name, RTLD_LAZY);
       if (result_ptr->image_ptr)
         result_ptr->error_str = nullptr;
+      else
+        result_ptr->error_str = dlerror();
       return nullptr;
     }
     


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D115017.391519.patch
Type: text/x-patch
Size: 3221 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20211203/0abd1965/attachment.bin>