tberghammer retitled this revision from "Change Platform::LoadImage to copy the
file to the remote platform" to "Add a new option to Platform::LoadImage to
install the image".
tberghammer updated the summary for this revision.
tberghammer updated this revision to Diff 41877.
tberghammer added a comment.
Address review comments
Note: PlatformAndroid::DoLoadImage isn't necessary because of the cleanup done
in http://reviews.llvm.org/rL254608
http://reviews.llvm.org/D15152
Files:
include/lldb/API/SBProcess.h
include/lldb/Target/Platform.h
packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py
source/API/SBProcess.cpp
source/Commands/CommandObjectProcess.cpp
source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
source/Plugins/Platform/POSIX/PlatformPOSIX.h
source/Target/Platform.cpp
Index: source/Target/Platform.cpp
===================================================================
--- source/Target/Platform.cpp
+++ source/Target/Platform.cpp
@@ -1985,14 +1985,52 @@
}
uint32_t
-Platform::LoadImage(lldb_private::Process* process, const FileSpec& image_spec, Error& error)
+Platform::LoadImage(lldb_private::Process* process,
+ const lldb_private::FileSpec& local_file,
+ const lldb_private::FileSpec& remote_file,
+ lldb_private::Error& error)
+{
+ if (local_file && remote_file)
+ {
+ // Both local and remote file was specified. Install the local file to the given location.
+ error = Install(local_file, remote_file);
+ if (error.Fail())
+ return LLDB_INVALID_IMAGE_TOKEN;
+ return DoLoadImage(process, remote_file, error);
+ }
+
+ if (local_file)
+ {
+ // Only local file was specified. Install it to the current working directory.
+ FileSpec target_file = GetWorkingDirectory();
+ target_file.AppendPathComponent(local_file.GetFilename().AsCString());
+ error = Install(local_file, target_file);
+ if (error.Fail())
+ return LLDB_INVALID_IMAGE_TOKEN;
+ return DoLoadImage(process, target_file, error);
+ }
+
+ if (remote_file)
+ {
+ // Only remote file was specified so we don't have to do any copying
+ return DoLoadImage(process, remote_file, error);
+ }
+
+ error.SetErrorString("Neither local nor remote file was specified");
+ return LLDB_INVALID_IMAGE_TOKEN;
+}
+
+uint32_t
+Platform::DoLoadImage (lldb_private::Process* process,
+ const lldb_private::FileSpec& remote_file,
+ lldb_private::Error& error)
{
error.SetErrorString("LoadImage is not supported on the current platform");
return LLDB_INVALID_IMAGE_TOKEN;
}
Error
Platform::UnloadImage(lldb_private::Process* process, uint32_t image_token)
{
- return Error("UnLoadImage is not supported on the current platform");
+ return Error("UnloadImage is not supported on the current platform");
}
Index: source/Plugins/Platform/POSIX/PlatformPOSIX.h
===================================================================
--- source/Plugins/Platform/POSIX/PlatformPOSIX.h
+++ source/Plugins/Platform/POSIX/PlatformPOSIX.h
@@ -174,9 +174,9 @@
DisconnectRemote () override;
uint32_t
- LoadImage (lldb_private::Process* process,
- const lldb_private::FileSpec& image_spec,
- lldb_private::Error& error) override;
+ DoLoadImage (lldb_private::Process* process,
+ const lldb_private::FileSpec& remote_file,
+ lldb_private::Error& error) override;
lldb_private::Error
UnloadImage (lldb_private::Process* process, uint32_t image_token) override;
Index: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===================================================================
--- source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -898,10 +898,12 @@
}
uint32_t
-PlatformPOSIX::LoadImage(lldb_private::Process* process, const FileSpec& image_spec, Error& error)
+PlatformPOSIX::DoLoadImage(lldb_private::Process* process,
+ const lldb_private::FileSpec& remote_file,
+ lldb_private::Error& error)
{
char path[PATH_MAX];
- image_spec.GetPath(path, sizeof(path));
+ remote_file.GetPath(path, sizeof(path));
StreamString expr;
expr.Printf(R"(
Index: source/Commands/CommandObjectProcess.cpp
===================================================================
--- source/Commands/CommandObjectProcess.cpp
+++ source/Commands/CommandObjectProcess.cpp
@@ -1174,6 +1174,57 @@
class CommandObjectProcessLoad : public CommandObjectParsed
{
public:
+ class CommandOptions : public Options
+ {
+ public:
+ CommandOptions (CommandInterpreter &interpreter) :
+ Options(interpreter)
+ {
+ // Keep default values of all options in one place: OptionParsingStarting ()
+ OptionParsingStarting ();
+ }
+
+ ~CommandOptions () override = default;
+
+ Error
+ SetOptionValue (uint32_t option_idx, const char *option_arg) override
+ {
+ Error error;
+ const int short_option = m_getopt_table[option_idx].val;
+ switch (short_option)
+ {
+ case 'i':
+ do_install = true;
+ if (option_arg && option_arg[0])
+ install_path.SetFile(option_arg, false);
+ break;
+ default:
+ error.SetErrorStringWithFormat("invalid short option character '%c'", short_option);
+ break;
+ }
+ return error;
+ }
+
+ void
+ OptionParsingStarting () override
+ {
+ do_install = false;
+ install_path.Clear();
+ }
+
+ const OptionDefinition*
+ GetDefinitions () override
+ {
+ return g_option_table;
+ }
+
+ // Options table: Required for subclasses of Options.
+ static OptionDefinition g_option_table[];
+
+ // Instance variables to hold the values for command options.
+ bool do_install;
+ FileSpec install_path;
+ };
CommandObjectProcessLoad (CommandInterpreter &interpreter) :
CommandObjectParsed (interpreter,
@@ -1183,12 +1234,17 @@
eCommandRequiresProcess |
eCommandTryTargetAPILock |
eCommandProcessMustBeLaunched |
- eCommandProcessMustBePaused )
+ eCommandProcessMustBePaused ),
+ m_options (interpreter)
{
}
- ~CommandObjectProcessLoad () override
+ ~CommandObjectProcessLoad () override = default;
+
+ Options *
+ GetOptions () override
{
+ return &m_options;
}
protected:
@@ -1198,18 +1254,34 @@
Process *process = m_exe_ctx.GetProcessPtr();
const size_t argc = command.GetArgumentCount();
-
for (uint32_t i=0; i<argc; ++i)
{
Error error;
- const char *image_path = command.GetArgumentAtIndex(i);
- FileSpec image_spec (image_path, false);
PlatformSP platform = process->GetTarget().GetPlatform();
- platform->ResolveRemotePath(image_spec, image_spec);
- uint32_t image_token = platform->LoadImage(process, image_spec, error);
+ const char *image_path = command.GetArgumentAtIndex(i);
+ uint32_t image_token = LLDB_INVALID_IMAGE_TOKEN;
+
+ if (!m_options.do_install)
+ {
+ FileSpec image_spec (image_path, false);
+ platform->ResolveRemotePath(image_spec, image_spec);
+ image_token = platform->LoadImage(process, FileSpec(), image_spec, error);
+ }
+ else if (m_options.install_path)
+ {
+ FileSpec image_spec (image_path, true);
+ platform->ResolveRemotePath(m_options.install_path, m_options.install_path);
+ image_token = platform->LoadImage(process, image_spec, m_options.install_path, error);
+ }
+ else
+ {
+ FileSpec image_spec (image_path, true);
+ image_token = platform->LoadImage(process, image_spec, FileSpec(), error);
+ }
+
if (image_token != LLDB_INVALID_IMAGE_TOKEN)
{
- result.AppendMessageWithFormat ("Loading \"%s\"...ok\nImage %u loaded.\n", image_path, image_token);
+ result.AppendMessageWithFormat ("Loading \"%s\"...ok\nImage %u loaded.\n", image_path, image_token);
result.SetStatus (eReturnStatusSuccessFinishResult);
}
else
@@ -1220,8 +1292,16 @@
}
return result.Succeeded();
}
+
+ CommandOptions m_options;
};
+OptionDefinition
+CommandObjectProcessLoad::CommandOptions::g_option_table[] =
+{
+ { LLDB_OPT_SET_ALL, false, "install", 'i', OptionParser::eOptionalArgument, nullptr, nullptr, 0, eArgTypePath, "Install the shared library to the target. If specified without an argument then the library will installed in the current working directory."},
+ { 0, false, nullptr, 0 , 0, nullptr, nullptr, 0, eArgTypeNone, nullptr }
+};
//-------------------------------------------------------------------------
// CommandObjectProcessUnload
Index: source/API/SBProcess.cpp
===================================================================
--- source/API/SBProcess.cpp
+++ source/API/SBProcess.cpp
@@ -1288,7 +1288,15 @@
}
uint32_t
-SBProcess::LoadImage (lldb::SBFileSpec &sb_image_spec, lldb::SBError &sb_error)
+SBProcess::LoadImage (lldb::SBFileSpec &sb_remote_image_spec, lldb::SBError &sb_error)
+{
+ return LoadImage(SBFileSpec(), sb_remote_image_spec, sb_error);
+}
+
+uint32_t
+SBProcess::LoadImage (const lldb::SBFileSpec &sb_local_image_spec,
+ const lldb::SBFileSpec &sb_remote_image_spec,
+ lldb::SBError &sb_error)
{
ProcessSP process_sp(GetSP());
if (process_sp)
@@ -1298,7 +1306,10 @@
{
Mutex::Locker api_locker (process_sp->GetTarget().GetAPIMutex());
PlatformSP platform_sp = process_sp->GetTarget().GetPlatform();
- return platform_sp->LoadImage (process_sp.get(), *sb_image_spec, sb_error.ref());
+ return platform_sp->LoadImage (process_sp.get(),
+ *sb_local_image_spec,
+ *sb_remote_image_spec,
+ sb_error.ref());
}
else
{
Index: packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py
===================================================================
--- packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py
+++ packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py
@@ -210,19 +210,13 @@
else:
dylibName = 'libloadunload_a.so'
- if lldb.remote_platform:
- # Don't use os.path.join as we have to use the path separator for the target
- dylibPath = shlib_dir + '/' + dylibName
- else:
- dylibPath = dylibName
-
# Make sure that a_function does not exist at this point.
self.expect("image lookup -n a_function", "a_function should not exist yet",
error=True, matching=False, patterns = ["1 match found"])
# Use lldb 'process load' to load the dylib.
- self.expect("process load %s" % dylibPath, "%s loaded correctly" % dylibPath,
- patterns = ['Loading "%s".*ok' % dylibPath,
+ self.expect("process load %s --install" % dylibName, "%s loaded correctly" % dylibName,
+ patterns = ['Loading "%s".*ok' % dylibName,
'Image [0-9]+ loaded'])
# Search for and match the "Image ([0-9]+) loaded" pattern.
Index: include/lldb/Target/Platform.h
===================================================================
--- include/lldb/Target/Platform.h
+++ include/lldb/Target/Platform.h
@@ -997,9 +997,18 @@
/// @param[in] process
/// The process to load the image.
///
- /// @param[in] image_spec
- /// The image file spec that points to the shared library that
- /// you want to load.
+ /// @param[in] local_file
+ /// The file spec that points to the shared library that you want
+ /// to load if the library is located on the host. The library will
+ /// be copied over to the location specified by remote_file or into
+ /// the current working directory with the same filename if the
+ /// remote_file isn't specified.
+ ///
+ /// @param[in] remote_file
+ /// If local_file is specified then the location where the library
+ /// should be copied over from the host. If local_file isn't
+ /// specified, then the path for the shared library on the target
+ /// what you want to load.
///
/// @param[out] error
/// An error object that gets filled in with any errors that
@@ -1011,11 +1020,17 @@
/// LLDB_INVALID_IMAGE_TOKEN will be returned if the shared
/// library can't be opened.
//------------------------------------------------------------------
- virtual uint32_t
+ uint32_t
LoadImage (lldb_private::Process* process,
- const lldb_private::FileSpec& image_spec,
+ const lldb_private::FileSpec& local_file,
+ const lldb_private::FileSpec& remote_file,
lldb_private::Error& error);
+ virtual uint32_t
+ DoLoadImage (lldb_private::Process* process,
+ const lldb_private::FileSpec& remote_file,
+ lldb_private::Error& error);
+
virtual Error
UnloadImage (lldb_private::Process* process, uint32_t image_token);
Index: include/lldb/API/SBProcess.h
===================================================================
--- include/lldb/API/SBProcess.h
+++ include/lldb/API/SBProcess.h
@@ -294,8 +294,21 @@
uint32_t
GetNumSupportedHardwareWatchpoints (lldb::SBError &error) const;
+ // Load an image from the target to the currently running process.
uint32_t
- LoadImage (lldb::SBFileSpec &image_spec, lldb::SBError &error);
+ LoadImage (lldb::SBFileSpec &remote_image_spec, lldb::SBError &error);
+
+ // Load an image to the currently running process.
+ // If both local and remote image is specified then copy the local image to the remote image and
+ // then load it from thew remote path.
+ // If only the local image is specified then the image will be copied to the current working
+ // directory and opened from there.
+ // If only the remote image is specified then open the image from the target (no compying done
+ // in this case)
+ uint32_t
+ LoadImage (const lldb::SBFileSpec &local_image_spec,
+ const lldb::SBFileSpec &remote_image_spec,
+ lldb::SBError &error);
lldb::SBError
UnloadImage (uint32_t image_token);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
