[Lldb-commits] [PATCH] D29406: Unify PlatformPOSIX::ResolveExecutable
This revision was automatically updated to reflect the committed changes.
Closed by commit rL293910: Unify PlatformPOSIX::ResolveExecutable (authored by
labath).
Changed prior to commit:
https://reviews.llvm.org/D29406?vs=86672=86846#toc
Repository:
rL LLVM
https://reviews.llvm.org/D29406
Files:
lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
lldb/trunk/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.cpp
lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.h
lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp
lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.h
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
lldb/trunk/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
lldb/trunk/source/Plugins/Platform/NetBSD/PlatformNetBSD.h
lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
lldb/trunk/source/Plugins/Platform/POSIX/PlatformPOSIX.h
Index: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
===
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.h
@@ -31,10 +31,6 @@
//
// lldb_private::Platform functions
//
- lldb_private::Error ResolveExecutable(
- const lldb_private::ModuleSpec _spec, lldb::ModuleSP _sp,
- const lldb_private::FileSpecList *module_search_paths_ptr) override;
-
lldb_private::Error
ResolveSymbolFile(lldb_private::Target ,
const lldb_private::ModuleSpec _spec,
Index: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
===
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -194,120 +194,6 @@
return file_list;
}
-Error PlatformDarwin::ResolveExecutable(
-const ModuleSpec _spec, lldb::ModuleSP _module_sp,
-const FileSpecList *module_search_paths_ptr) {
- Error error;
- // Nothing special to do here, just use the actual file and architecture
-
- char exe_path[PATH_MAX];
- ModuleSpec resolved_module_spec(module_spec);
-
- if (IsHost()) {
-// If we have "ls" as the exe_file, resolve the executable loation based on
-// the current path variables
-if (!resolved_module_spec.GetFileSpec().Exists()) {
- module_spec.GetFileSpec().GetPath(exe_path, sizeof(exe_path));
- resolved_module_spec.GetFileSpec().SetFile(exe_path, true);
-}
-
-if (!resolved_module_spec.GetFileSpec().Exists())
- resolved_module_spec.GetFileSpec().ResolveExecutableLocation();
-
-// Resolve any executable within a bundle on MacOSX
-Host::ResolveExecutableInBundle(resolved_module_spec.GetFileSpec());
-
-if (resolved_module_spec.GetFileSpec().Exists())
- error.Clear();
-else {
- const uint32_t permissions =
- resolved_module_spec.GetFileSpec().GetPermissions();
- if (permissions && (permissions & eFilePermissionsEveryoneR) == 0)
-error.SetErrorStringWithFormat(
-"executable '%s' is not readable",
-resolved_module_spec.GetFileSpec().GetPath().c_str());
- else
-error.SetErrorStringWithFormat(
-"unable to find executable for '%s'",
-resolved_module_spec.GetFileSpec().GetPath().c_str());
-}
- } else {
-if (m_remote_platform_sp) {
- error =
- GetCachedExecutable(resolved_module_spec, exe_module_sp,
- module_search_paths_ptr, *m_remote_platform_sp);
-} else {
- // We may connect to a process and use the provided executable (Don't use
- // local $PATH).
-
- // Resolve any executable within a bundle on MacOSX
- Host::ResolveExecutableInBundle(resolved_module_spec.GetFileSpec());
-
- if (resolved_module_spec.GetFileSpec().Exists())
-error.Clear();
- else
-error.SetErrorStringWithFormat(
-"the platform is not currently connected, and '%s' doesn't exist "
-"in the system root.",
-resolved_module_spec.GetFileSpec().GetFilename().AsCString(""));
-}
- }
-
- if (error.Success()) {
-if (resolved_module_spec.GetArchitecture().IsValid()) {
- error = ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
- module_search_paths_ptr, NULL, NULL);
-
- if (error.Fail() || exe_module_sp.get() == NULL ||
- exe_module_sp->GetObjectFile() == NULL) {
-exe_module_sp.reset();
-error.SetErrorStringWithFormat(
-"'%s' doesn't contain the architecture %s",
-
[Lldb-commits] [PATCH] D29406: Unify PlatformPOSIX::ResolveExecutable
krytarowski added a comment. It looks good. https://reviews.llvm.org/D29406 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29406: Unify PlatformPOSIX::ResolveExecutable
labath added inline comments. Comment at: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:163 + // Resolve any executable within a bundle on MacOSX + Host::ResolveExecutableInBundle(resolved_module_spec.GetFileSpec()); + This is the bit that was only present on darwin. Comment at: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:198 + + error = ModuleList::GetSharedModule(resolved_module_spec, + exe_module_sp, module_search_paths_ptr, nullptr, nullptr); This is the bit that was only present on linux. https://reviews.llvm.org/D29406 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29406: Unify PlatformPOSIX::ResolveExecutable
labath created this revision.
various platforms very using nearly identical code for this method. As far as I
can tell there was nothing platform-specific about the differences, but rather
it looked like it was caused by tiny tweaks being made to individual copies
without affecting the overall functionality. I have taken the superset of all
these tweaks and put it into one method in the base class (incidentaly, nobody
was using the base class implementation of the method, as all classes were
overriding it). From the darwin class I took the slightly improved error
reporting (checking whether the file is readable) and the
ResolveExecutableInBundle call (which has no effect elsewhere as the function
is already a no-op on non-darwin platforms). From the linux class I took the
set-the-triple-vendor-to-host-if-unspecified tweak (present in PlatformKalimba
as well).
PlatformWindows has an identical copy as well. We could resolve that by pushing
this code further down into Platform class, that that would require pushing the
m_remote_platform_sp member as well, which seems like a bad design choice.
https://reviews.llvm.org/D29406
Files:
source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
source/Plugins/Platform/FreeBSD/PlatformFreeBSD.h
source/Plugins/Platform/Kalimba/PlatformKalimba.cpp
source/Plugins/Platform/Kalimba/PlatformKalimba.h
source/Plugins/Platform/Linux/PlatformLinux.cpp
source/Plugins/Platform/Linux/PlatformLinux.h
source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
source/Plugins/Platform/MacOSX/PlatformDarwin.h
source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
source/Plugins/Platform/NetBSD/PlatformNetBSD.h
source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
source/Plugins/Platform/POSIX/PlatformPOSIX.h
Index: source/Plugins/Platform/POSIX/PlatformPOSIX.h
===
--- source/Plugins/Platform/POSIX/PlatformPOSIX.h
+++ source/Plugins/Platform/POSIX/PlatformPOSIX.h
@@ -101,6 +101,10 @@
uint32_t timeout_sec)
override; // Timeout in seconds to wait for shell program to finish
+ lldb_private::Error ResolveExecutable(const lldb_private::ModuleSpec _spec,
+ lldb::ModuleSP _sp,
+ const lldb_private::FileSpecList *module_search_paths_ptr) override;
+
lldb_private::Error MakeDirectory(const lldb_private::FileSpec _spec,
uint32_t mode) override;
Index: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -18,14 +18,16 @@
#include "lldb/Core/Debugger.h"
#include "lldb/Core/Log.h"
#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleSpec.h"
#include "lldb/Core/StreamString.h"
#include "lldb/Core/ValueObject.h"
#include "lldb/Expression/UserExpression.h"
#include "lldb/Host/File.h"
#include "lldb/Host/FileCache.h"
#include "lldb/Host/FileSpec.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Host/Host.h"
+#include "lldb/Host/HostInfo.h"
#include "lldb/Target/DynamicLoader.h"
#include "lldb/Target/ExecutionContext.h"
#include "lldb/Target/Process.h"
@@ -111,6 +113,143 @@
}
}
+Error PlatformPOSIX::ResolveExecutable(const ModuleSpec _spec,
+ lldb::ModuleSP _module_sp,
+ const FileSpecList *module_search_paths_ptr) {
+ Error error;
+ // Nothing special to do here, just use the actual file and architecture
+
+ char exe_path[PATH_MAX];
+ ModuleSpec resolved_module_spec(module_spec);
+
+ if (IsHost()) {
+// If we have "ls" as the exe_file, resolve the executable location based on
+// the current path variables
+if (!resolved_module_spec.GetFileSpec().Exists()) {
+ resolved_module_spec.GetFileSpec().GetPath(exe_path, sizeof(exe_path));
+ resolved_module_spec.GetFileSpec().SetFile(exe_path, true);
+}
+
+if (!resolved_module_spec.GetFileSpec().Exists())
+ resolved_module_spec.GetFileSpec().ResolveExecutableLocation();
+
+// Resolve any executable within a bundle on MacOSX
+Host::ResolveExecutableInBundle(resolved_module_spec.GetFileSpec());
+
+if (resolved_module_spec.GetFileSpec().Exists())
+ error.Clear();
+else {
+ const uint32_t permissions =
+ resolved_module_spec.GetFileSpec().GetPermissions();
+ if (permissions && (permissions & eFilePermissionsEveryoneR) == 0)
+error.SetErrorStringWithFormat(
+"executable '%s' is not readable",
+resolved_module_spec.GetFileSpec().GetPath().c_str());
+ else
+error.SetErrorStringWithFormat(
+"unable to find executable for '%s'",
+resolved_module_spec.GetFileSpec().GetPath().c_str());
+}
+ } else {
+if (m_remote_platform_sp) {
+ error =
+ GetCachedExecutable(resolved_module_spec, exe_module_sp,
+
