[Lldb-commits] [lldb] r305461 - [swig] Improve the native module import logic
Author: labath Date: Thu Jun 15 06:23:22 2017 New Revision: 305461 URL: http://llvm.org/viewvc/llvm-project?rev=305461&view=rev Log: [swig] Improve the native module import logic The simple module import logic was not sufficient for our distribution model of lldb, which is without the _lldb.pyd file (normally that would be a symlink to the shared library, but symlinks are not really a thing on windows). With the older swigs it worked (loading of the python scripting machinery from within lldb) because the normal swig import logic contained a last-ditch import of a global module _lldb (which is defined when you run python from lldb). Add back the last-ditch import to our custom import logic as well. Modified: lldb/trunk/scripts/lldb.swig Modified: lldb/trunk/scripts/lldb.swig URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/lldb.swig?rev=305461&r1=305460&r2=305461&view=diff == --- lldb/trunk/scripts/lldb.swig (original) +++ lldb/trunk/scripts/lldb.swig Thu Jun 15 06:23:22 2017 @@ -40,7 +40,13 @@ us to override the module import logic t Older swig versions will simply ignore this setting. */ %define MODULEIMPORT -"from . import $module" +"try: +# Try a relative import first +from . import $module +except ImportError: +# Maybe absolute import will work (if we're being loaded from lldb, it +# should). +import $module" %enddef // These versions will not generate working python modules, so error out early. #if SWIG_VERSION >= 0x030009 && SWIG_VERSION < 0x030011 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r305462 - Add llvm::Error assignment operator to Status class
Author: labath
Date: Thu Jun 15 06:23:26 2017
New Revision: 305462
URL: http://llvm.org/viewvc/llvm-project?rev=305462&view=rev
Log:
Add llvm::Error assignment operator to Status class
This enables writing "status = std::move(some_llvm_error)".
Modified:
lldb/trunk/include/lldb/Utility/Status.h
lldb/trunk/source/Utility/Status.cpp
lldb/trunk/unittests/Utility/StatusTest.cpp
Modified: lldb/trunk/include/lldb/Utility/Status.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/Status.h?rev=305462&r1=305461&r2=305462&view=diff
==
--- lldb/trunk/include/lldb/Utility/Status.h (original)
+++ lldb/trunk/include/lldb/Utility/Status.h Thu Jun 15 06:23:26 2017
@@ -104,7 +104,8 @@ public:
~Status();
// llvm::Error support
- explicit Status(llvm::Error error);
+ explicit Status(llvm::Error error) { *this = std::move(error); }
+ const Status &operator=(llvm::Error error);
llvm::Error ToError() const;
//--
Modified: lldb/trunk/source/Utility/Status.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/Status.cpp?rev=305462&r1=305461&r2=305462&view=diff
==
--- lldb/trunk/source/Utility/Status.cpp (original)
+++ lldb/trunk/source/Utility/Status.cpp Thu Jun 15 06:23:26 2017
@@ -56,10 +56,11 @@ Status::Status(const char *format, ...)
va_end(args);
}
-Status::Status(llvm::Error error)
-: m_code(0), m_type(ErrorType::eErrorTypeGeneric) {
- if (!error)
-return;
+const Status &Status::operator=(llvm::Error error) {
+ if (!error) {
+Clear();
+return *this;
+ }
// if the error happens to be a errno error, preserve the error code
error = llvm::handleErrors(
@@ -74,8 +75,12 @@ Status::Status(llvm::Error error)
});
// Otherwise, just preserve the message
- if (error)
+ if (error) {
+SetErrorToGenericError();
SetErrorString(llvm::toString(std::move(error)));
+ }
+
+ return *this;
}
llvm::Error Status::ToError() const {
Modified: lldb/trunk/unittests/Utility/StatusTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/StatusTest.cpp?rev=305462&r1=305461&r2=305462&view=diff
==
--- lldb/trunk/unittests/Utility/StatusTest.cpp (original)
+++ lldb/trunk/unittests/Utility/StatusTest.cpp Thu Jun 15 06:23:26 2017
@@ -33,6 +33,9 @@ TEST(StatusTest, ErrorConstructor) {
EXPECT_TRUE(foo.Fail());
EXPECT_EQ(eErrorTypeGeneric, foo.GetType());
EXPECT_STREQ("foo", foo.AsCString());
+
+ foo = llvm::Error::success();
+ EXPECT_TRUE(foo.Success());
}
TEST(StatusTest, ErrorConversion) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33674: Implementation of Intel(R) Processor Trace support for Linux
labath added a comment.
Second round of comments. I still see a lot of places with redundant checks for
what appear to be operational invariants. I'd like to get those cleared up to
make the code flow better.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:611
+
+if (ProcessorTraceMonitor::GetProcessTraceID() != LLDB_INVALID_UID) {
+ auto traceMonitor = ProcessorTraceMonitor::Create(
Every call to AddThread is followed by this block. Is there a reason we cannot
just move it inside the AddThread function?
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:621
+LLDB_LOG(log, "{0} error in create m_code {1} m_string {2}",
+ __FUNCTION__, error2.GetError(), error2.AsCString());
+ }
LLDB_LOG will already print the function name (same thing everywhere else you
use `__FUNCTION__`), so you don't need to write it out yourself. Also the
AsCString is unnecessary, as Status has a pretty-printer.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2534
+ size_t offset) {
+ TraceOptions trace_options;
+ Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PTRACE));
unused variable
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2627
+ if (!traceMonitor) {
+Status error2(traceMonitor.takeError());
+LLDB_LOG(log, "{0} error in create m_code {1} m_string {2}", __FUNCTION__,
As of r305462 you can write `error = traceMonitor.takeError()`, so you
shouldn't need the extra variable.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2644
+ if (iter != m_processor_trace_monitor.end())
+error = StopTrace(iter->second->GetTraceID(), thread);
+
Should you delete the iterator from the map after this? In fact, the mere act
of deleting the iterator should probably be what triggers the trace stop, in
line with RAII principles.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:2678
+ Status error;
+ if (ProcessorTraceMonitor::GetProcessTraceID() == LLDB_INVALID_UID) {
+error.SetError(ProcessNotBeingTraced, eErrorTypeGeneric);
This is dead code. The caller has already checked this condition. This should
be at most an assertion.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.h:263
+ // is sufficient to obtain the actual ProcessorTrace instance.
+ ProcessorTraceMonitorSP LookupProcessorTraceInstance(lldb::user_id_t traceid,
+ lldb::tid_t thread,
ErrorOr is a better way to return a value *or* an error. Although, in this
case, it sounds like you're really only interested in the found-or-not-found
distinction, so you could just drop the Status argument and let a null return
value signify an error.
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.h:277
+
+ llvm::DenseMap
+ m_processor_trace_monitor;
I'd like to downgrade these to unique pointers to ProcessTraceMonitor. There's
no reason for these to ever outlive or escape the process instance, so it's
natural to say they are strongly owned by it. In other places where you use
ProcessorTraceMonitorSP you can just use regular pointers or references
(depending on whether they can be null or not).
Comment at: source/Plugins/Process/Linux/NativeProcessLinux.h:282
+ // same process user id.
+ llvm::DenseSet m_pt_traced_thread_group;
};
I am confused about the purpose of this member variable. As far as I can tell,
it will always contain *either* all of the threads in the process *or* none of
them. Is there a situation where this is not the case?
Comment at: source/Plugins/Process/Linux/ProcessorTrace.cpp:158
+LLDB_LOG(log, "ProcessorTrace failed to open Config file");
+error.SetError(FileNotFound, eErrorTypePOSIX);
+return error;
eErrorTypePOSIX is used for errno error values. Please don't try to pass your
invented error codes as these.
Comment at: source/Plugins/Process/Linux/ProcessorTrace.cpp:321
+ if (!pt_monitor_sp) {
+LLDB_LOG(log, "NativeProcessLinux {0}", "Alloc failed");
+error.SetError(AllocFailed, eErrorTypeGeneric);
dead code
Comment at: source/Plugins/Process/Linux/ProcessorTrace.cpp:412
+#ifndef PERF_ATTR_SIZE_VER5
+ LLDB_LOG(log, "ProcessorTrace {0} Not supported ", __FUNCTION__);
+ error.SetError(PerfEventNotSupported, eErrorTypeGeneric);
Just put llvm_unreachable here. There shouldn't be any way to construct this
object if tracing is not supported.
Comment at: source/Plugins/Process/Linux/ProcessorTrace.cpp:480
+ if (base == n
[Lldb-commits] [PATCH] D34236: Delete ProcessLauncherPosix
labath created this revision.
Herald added subscribers: mgorny, srhines.
ProcessLauncherPosix was using posix_spawn for launching the process,
but this function is not available on all platforms we support, and even
where it was avaialable, it did not support the full range of options we
require for launching (most importantly, launching in stop-on-entry
mode). For these reasons, the set of ifdefs around these functions has
grown untractably large, and we were forced to implement our own
launcher from more basic primitives anyway (ProcessLauncherPosixFork --
used on Linux, Android, and NetBSD).
Therefore, I remove this class, and move the relevant parts of the code
to the darwin-specific Host.mm file. This is the platform that code was
originally written for anyway, and it's the only platform where this
implementation makes sense (e.g. the lack of the "thread-specific
working directory" concept makes these functions racy on all other
platforms). This allows us to remove a lot of ifdefs and simplify the
code.
Effectively, the only change this introduces is that FreeBSD will now
use the fork-based launcher instead of posix_spawnp. That sholdn't be a
problem as this approach works at least on one other BSD-based system
already.
https://reviews.llvm.org/D34236
Files:
include/lldb/Host/Host.h
include/lldb/Host/posix/ProcessLauncherPosix.h
source/Host/CMakeLists.txt
source/Host/common/Host.cpp
source/Host/macosx/Host.mm
source/Host/posix/ProcessLauncherPosix.cpp
unittests/tools/lldb-server/tests/TestClient.cpp
Index: unittests/tools/lldb-server/tests/TestClient.cpp
===
--- unittests/tools/lldb-server/tests/TestClient.cpp
+++ unittests/tools/lldb-server/tests/TestClient.cpp
@@ -12,7 +12,6 @@
#include "lldb/Host/HostInfo.h"
#include "lldb/Host/common/TCPSocket.h"
#include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
-#include "lldb/Host/posix/ProcessLauncherPosix.h"
#include "lldb/Interpreter/Args.h"
#include "lldb/Target/ProcessLaunchInfo.h"
#include "llvm/ADT/StringExtras.h"
Index: source/Host/posix/ProcessLauncherPosix.cpp
===
--- source/Host/posix/ProcessLauncherPosix.cpp
+++ /dev/null
@@ -1,34 +0,0 @@
-//===-- ProcessLauncherPosix.cpp *- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "lldb/Host/posix/ProcessLauncherPosix.h"
-#include "lldb/Host/Host.h"
-#include "lldb/Host/HostProcess.h"
-
-#include "lldb/Target/ProcessLaunchInfo.h"
-
-#include
-
-using namespace lldb;
-using namespace lldb_private;
-
-HostProcess
-ProcessLauncherPosix::LaunchProcess(const ProcessLaunchInfo &launch_info,
-Status &error) {
- lldb::pid_t pid;
- char exe_path[PATH_MAX];
-
- launch_info.GetExecutableFile().GetPath(exe_path, sizeof(exe_path));
-
- // TODO(zturner): Move the code from LaunchProcessPosixSpawn to here, and make
- // MacOSX re-use this
- // ProcessLauncher when it wants a posix_spawn launch.
- error = Host::LaunchProcessPosixSpawn(exe_path, launch_info, pid);
- return HostProcess(pid);
-}
Index: source/Host/macosx/Host.mm
===
--- source/Host/macosx/Host.mm
+++ source/Host/macosx/Host.mm
@@ -1024,6 +1024,47 @@
}
#endif
+static short GetPosixspawnFlags(const ProcessLaunchInfo &launch_info) {
+ short flags = POSIX_SPAWN_SETSIGDEF | POSIX_SPAWN_SETSIGMASK;
+
+ if (launch_info.GetFlags().Test(eLaunchFlagExec))
+flags |= POSIX_SPAWN_SETEXEC; // Darwin specific posix_spawn flag
+
+ if (launch_info.GetFlags().Test(eLaunchFlagDebug))
+flags |= POSIX_SPAWN_START_SUSPENDED; // Darwin specific posix_spawn flag
+
+ if (launch_info.GetFlags().Test(eLaunchFlagDisableASLR))
+flags |= _POSIX_SPAWN_DISABLE_ASLR; // Darwin specific posix_spawn flag
+
+ if (launch_info.GetLaunchInSeparateProcessGroup())
+flags |= POSIX_SPAWN_SETPGROUP;
+
+#ifdef POSIX_SPAWN_CLOEXEC_DEFAULT
+#if defined(__x86_64__) || defined(__i386__)
+ static LazyBool g_use_close_on_exec_flag = eLazyBoolCalculate;
+ if (g_use_close_on_exec_flag == eLazyBoolCalculate) {
+g_use_close_on_exec_flag = eLazyBoolNo;
+
+uint32_t major, minor, update;
+if (HostInfo::GetOSVersion(major, minor, update)) {
+ // Kernel panic if we use the POSIX_SPAWN_CLOEXEC_DEFAULT on 10.7 or
+ // earlier
+ if (major > 10 || (major == 10 && minor > 7)) {
+// Only enable for 10.8 and later OS versions
+g_use_close_on_exec_flag = eLazyBoolYes;
+ }
+}
+ }
+#else
+ static LazyBool g_use_close_on_exec_flag = eLazyBoolYes;
+#endif // defined(__x86_64__) || defined(__i386__)
+ // Close all
[Lldb-commits] [PATCH] D34199: Tweak SysV_arm64 function entry unwind plan
tberghammer accepted this revision. tberghammer added a comment. This revision is now accepted and ready to land. Looks good https://reviews.llvm.org/D34199 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D34236: Delete ProcessLauncherPosix
krytarowski accepted this revision. krytarowski added a comment. This revision is now accepted and ready to land. NetBSD part looks good. In next step we can rename ProcessLauncherPosixFork to ProcessLauncherPosix. https://reviews.llvm.org/D34236 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D34236: Delete ProcessLauncherPosix
rnk added a comment. `posix_spawn` is considered the preferred, efficient, canonical way to launch processes on Mac. Are you sure you want to remove support for it? https://reviews.llvm.org/D34236 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D34236: Delete ProcessLauncherPosix
jingham added a comment. This patch doesn't remove support for posix_spawn on OS X - it is moving to the macOS Host.mm. Kind of sad as posix_spawn was supposed to remove a lot of the platform-dependent and error-prone parts of process launching from the system. But you have to live with what you have... https://reviews.llvm.org/D34236 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D34199: Tweak SysV_arm64 function entry unwind plan
jasonmolenda accepted this revision. jasonmolenda added a comment. Looks good to me. https://reviews.llvm.org/D34199 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D34199: Tweak SysV_arm64 function entry unwind plan
omjavaid accepted this revision. omjavaid added a comment. LGTM https://reviews.llvm.org/D34199 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r305547 - Change the code I added to LocateDSYMInVincinityOfExecutable to use
Author: jmolenda
Date: Thu Jun 15 23:19:36 2017
New Revision: 305547
URL: http://llvm.org/viewvc/llvm-project?rev=305547&view=rev
Log:
Change the code I added to LocateDSYMInVincinityOfExecutable to use
the FileSpec methods for adding/removing file path components instead
of using std::strings; feedback from Sean on the change I added in
r305441.
Modified:
lldb/trunk/source/Host/common/Symbols.cpp
Modified: lldb/trunk/source/Host/common/Symbols.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/Symbols.cpp?rev=305547&r1=305546&r2=305547&view=diff
==
--- lldb/trunk/source/Host/common/Symbols.cpp (original)
+++ lldb/trunk/source/Host/common/Symbols.cpp Thu Jun 15 23:19:36 2017
@@ -104,9 +104,10 @@ static bool LocateDSYMInVincinityOfExecu
}
return true;
} else {
- // Get a clean copy of the executable path, without the final
filename
- FileSpec path_dir_fspec (exec_fspec->GetDirectory().AsCString(),
true);
- std::string path_dir_str = path_dir_fspec.GetPath();
+ FileSpec parent_dirs = exec_fspec;
+
+ // Remove the binary name from the FileSpec
+ parent_dirs.RemoveLastPathComponent();
// Add a ".dSYM" name to each directory component of the path,
stripping
// off components. e.g. we may have a binary like
@@ -121,24 +122,37 @@ static bool LocateDSYMInVincinityOfExecu
// need to continue.
for (int i = 0; i < 4; i++) {
-std::string path_dir_plus_dsym (path_dir_str);
-path_dir_plus_dsym += ".dSYM/Contents/Resources/DWARF/";
-path_dir_plus_dsym += exec_fspec->GetFilename().AsCString();
-dsym_fspec.SetFile (path_dir_plus_dsym, true);
-if (dsym_fspec.Exists() &&
-FileAtPathContainsArchAndUUID(
-dsym_fspec, module_spec.GetArchitecturePtr(),
-module_spec.GetUUIDPtr())) {
- if (log) {
-log->Printf("dSYM with matching UUID & arch found at %s",
-path_dir_plus_dsym.c_str());
- }
- return true;
-}
-auto const last_slash = path_dir_str.rfind('/');
-if (last_slash == std::string::npos)
+// Does this part of the path have a "." character - could it be a
bundle's
+// top level directory?
+const char *fn = parent_dirs.GetFilename().AsCString();
+if (fn == nullptr)
break;
-path_dir_str.resize(last_slash);
+if (::strchr (fn, '.') != nullptr) {
+ dsym_fspec = parent_dirs;
+ dsym_fspec.RemoveLastPathComponent();
+
+ // If the current directory name is "Foundation.framework", see
if
+ //
"Foundation.framework.dSYM/Contents/Resources/DWARF/Foundation"
+ // exists & has the right uuid.
+ std::string dsym_fn = fn;
+ dsym_fn += ".dSYM";
+ dsym_fspec.AppendPathComponent(dsym_fn.c_str());
+ dsym_fspec.AppendPathComponent("Contents");
+ dsym_fspec.AppendPathComponent("Resources");
+ dsym_fspec.AppendPathComponent("DWARF");
+
dsym_fspec.AppendPathComponent(exec_fspec->GetFilename().AsCString());
+ if (dsym_fspec.Exists() &&
+ FileAtPathContainsArchAndUUID(
+ dsym_fspec, module_spec.GetArchitecturePtr(),
+ module_spec.GetUUIDPtr())) {
+if (log) {
+ log->Printf("dSYM with matching UUID & arch found at %s",
+ dsym_fspec.GetPath().c_str());
+}
+return true;
+ }
+}
+parent_dirs.RemoveLastPathComponent();
}
}
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
