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;
}
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;
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
