[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process
labath commandeered this revision. labath edited reviewers, added: zturner; removed: labath. labath added a comment. It looks like this has ground to a halt, but it seems the consensus was to try the Architecture plugin idea. I'm going to hijack this revision to maintain context, and upload a new version implementing that. https://reviews.llvm.org/D31172 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D35298: [MainLoop] Fix possible use of an invalid iterator
petpav01 created this revision.
Store file descriptors from `loop.m_read_fds` (if `FORCE_PSELECT` is defined)
and signals from `loop.m_signals` that need to be processed in
`MainLoop::RunImpl::ProcessEvents()` into a separate vector and then iterate
over this container to invoke the callbacks.
This prevents a problem where when the code iterated directly over
`m_read_fds`/`m_signals`, a callback invoked from within the loop could modify
these variables and invalidate the loop iterator. This would then result in the
following assertion failure:
> llvm/include/llvm/ADT/DenseMap.h:1099: llvm::DenseMapIterator KeyInfoT, Bucket, IsConst>& llvm::DenseMapIterator Bucket, IsConst>::operator++() [with KeyT = int; ValueT =
> std::function; KeyInfoT =
> llvm::DenseMapInfo; Bucket = llvm::detail::DenseMapPair std::function >; bool IsConst = false]:
> assertion "isHandleInSync() && "invalid iterator access!"" failed
https://reviews.llvm.org/D35298
Files:
source/Host/common/MainLoop.cpp
Index: source/Host/common/MainLoop.cpp
===
--- source/Host/common/MainLoop.cpp
+++ source/Host/common/MainLoop.cpp
@@ -193,10 +193,16 @@
void MainLoop::RunImpl::ProcessEvents() {
#ifdef FORCE_PSELECT
- for (const auto &fd : loop.m_read_fds) {
-if (!FD_ISSET(fd.first, &read_fd_set))
- continue;
-IOObject::WaitableHandle handle = fd.first;
+ // Collect first all readable file descriptors into a separate vector and
then
+ // iterate over it to invoke callbacks. Iterating directly over
+ // loop.m_read_fds is not possible because the callbacks can modify the
+ // container which could invalidate the iterator.
+ std::vector fds;
+ for (const auto &fd : loop.m_read_fds)
+if (FD_ISSET(fd.first, &read_fd_set))
+ fds.push_back(fd.first);
+
+ for (const auto &handle : fds) {
#else
for (const auto &fd : read_fds) {
if ((fd.revents & POLLIN) == 0)
@@ -209,13 +215,16 @@
loop.ProcessReadObject(handle);
}
- for (const auto &entry : loop.m_signals) {
+ std::vector signals;
+ for (const auto &entry : loop.m_signals)
+if (g_signal_flags[entry.first] != 0)
+ signals.push_back(entry.first);
+
+ for (const auto &signal : signals) {
if (loop.m_terminate_request)
return;
-if (g_signal_flags[entry.first] == 0)
- continue; // No signal
-g_signal_flags[entry.first] = 0;
-loop.ProcessSignal(entry.first);
+g_signal_flags[signal] = 0;
+loop.ProcessSignal(signal);
}
}
#endif
Index: source/Host/common/MainLoop.cpp
===
--- source/Host/common/MainLoop.cpp
+++ source/Host/common/MainLoop.cpp
@@ -193,10 +193,16 @@
void MainLoop::RunImpl::ProcessEvents() {
#ifdef FORCE_PSELECT
- for (const auto &fd : loop.m_read_fds) {
-if (!FD_ISSET(fd.first, &read_fd_set))
- continue;
-IOObject::WaitableHandle handle = fd.first;
+ // Collect first all readable file descriptors into a separate vector and then
+ // iterate over it to invoke callbacks. Iterating directly over
+ // loop.m_read_fds is not possible because the callbacks can modify the
+ // container which could invalidate the iterator.
+ std::vector fds;
+ for (const auto &fd : loop.m_read_fds)
+if (FD_ISSET(fd.first, &read_fd_set))
+ fds.push_back(fd.first);
+
+ for (const auto &handle : fds) {
#else
for (const auto &fd : read_fds) {
if ((fd.revents & POLLIN) == 0)
@@ -209,13 +215,16 @@
loop.ProcessReadObject(handle);
}
- for (const auto &entry : loop.m_signals) {
+ std::vector signals;
+ for (const auto &entry : loop.m_signals)
+if (g_signal_flags[entry.first] != 0)
+ signals.push_back(entry.first);
+
+ for (const auto &signal : signals) {
if (loop.m_terminate_request)
return;
-if (g_signal_flags[entry.first] == 0)
- continue; // No signal
-g_signal_flags[entry.first] = 0;
-loop.ProcessSignal(entry.first);
+g_signal_flags[signal] = 0;
+loop.ProcessSignal(signal);
}
}
#endif
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process
labath updated this revision to Diff 106165.
labath added a comment.
Herald added subscribers: javed.absar, mgorny.
The architecture plugin idea
https://reviews.llvm.org/D31172
Files:
include/lldb/Core/ArchSpec.h
include/lldb/Core/Architecture.h
include/lldb/Core/PluginManager.h
include/lldb/Target/Process.h
source/API/SystemInitializerFull.cpp
source/Core/ArchSpec.cpp
source/Core/PluginManager.cpp
source/Plugins/Architecture/Arm/ArchitectureArm.cpp
source/Plugins/Architecture/Arm/ArchitectureArm.h
source/Plugins/Architecture/Arm/CMakeLists.txt
source/Plugins/Architecture/CMakeLists.txt
source/Plugins/CMakeLists.txt
source/Target/Process.cpp
source/Target/Thread.cpp
Index: source/Target/Thread.cpp
===
--- source/Target/Thread.cpp
+++ source/Target/Thread.cpp
@@ -442,10 +442,8 @@
if (m_stop_info_override_stop_id != process_stop_id) {
m_stop_info_override_stop_id = process_stop_id;
if (m_stop_info_sp) {
-ArchSpec::StopInfoOverrideCallbackType callback =
-GetProcess()->GetStopInfoOverrideCallback();
-if (callback)
- callback(*this);
+if (Architecture *arch = GetProcess()->GetArchitecturePlugin())
+ arch->OverrideStopInfo(*this);
}
}
}
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -743,8 +743,7 @@
m_profile_data_comm_mutex(), m_profile_data(), m_iohandler_sync(0),
m_memory_cache(*this), m_allocated_memory_cache(*this),
m_should_detach(false), m_next_event_action_ap(), m_public_run_lock(),
- m_private_run_lock(), m_stop_info_override_callback(nullptr),
- m_finalizing(false), m_finalize_called(false),
+ m_private_run_lock(), m_finalizing(false), m_finalize_called(false),
m_clear_thread_plans_on_stop(false), m_force_next_event_delivery(false),
m_last_broadcast_state(eStateInvalid), m_destroy_in_process(false),
m_can_interpret_function_calls(false), m_warnings_issued(),
@@ -855,6 +854,7 @@
m_dynamic_checkers_ap.reset();
m_abi_sp.reset();
m_os_ap.reset();
+ m_architecture_up.reset();
m_system_runtime_ap.reset();
m_dyld_ap.reset();
m_jit_loaders_ap.reset();
@@ -871,7 +871,6 @@
m_language_runtimes.clear();
m_instrumentation_runtimes.clear();
m_next_event_action_ap.reset();
- m_stop_info_override_callback = nullptr;
// Clear the last natural stop ID since it has a strong
// reference to this process
m_mod_id.SetStopEventForLastNaturalStopID(EventSP());
@@ -2711,8 +2710,8 @@
m_jit_loaders_ap.reset();
m_system_runtime_ap.reset();
m_os_ap.reset();
+ m_architecture_up.reset();
m_process_input_reader.reset();
- m_stop_info_override_callback = nullptr;
Module *exe_module = GetTarget().GetExecutableModulePointer();
if (exe_module) {
@@ -2800,8 +2799,8 @@
else
StartPrivateStateThread();
-m_stop_info_override_callback =
-GetTarget().GetArchitecture().GetStopInfoOverrideCallback();
+m_architecture_up = PluginManager::CreateArchitectureInstance(
+GetTarget().GetArchitecture());
// Target was stopped at entry as was intended. Need to notify the
// listeners
@@ -2986,7 +2985,6 @@
m_jit_loaders_ap.reset();
m_system_runtime_ap.reset();
m_os_ap.reset();
- m_stop_info_override_callback = nullptr;
lldb::pid_t attach_pid = attach_info.GetProcessID();
Status error;
@@ -3220,7 +3218,7 @@
}
}
- m_stop_info_override_callback = process_arch.GetStopInfoOverrideCallback();
+ m_architecture_up = PluginManager::CreateArchitectureInstance(process_arch);
}
Status Process::ConnectRemote(Stream *strm, llvm::StringRef remote_url) {
@@ -5849,7 +5847,6 @@
m_instrumentation_runtimes.clear();
m_thread_list.DiscardThreadPlans();
m_memory_cache.Clear(true);
- m_stop_info_override_callback = nullptr;
DoDidExec();
CompleteAttach();
// Flush the process (threads and all stack frames) after running
Index: source/Plugins/CMakeLists.txt
===
--- source/Plugins/CMakeLists.txt
+++ source/Plugins/CMakeLists.txt
@@ -1,4 +1,5 @@
add_subdirectory(ABI)
+add_subdirectory(Architecture)
add_subdirectory(Disassembler)
add_subdirectory(DynamicLoader)
add_subdirectory(ExpressionParser)
Index: source/Plugins/Architecture/CMakeLists.txt
===
--- /dev/null
+++ source/Plugins/Architecture/CMakeLists.txt
@@ -0,0 +1 @@
+add_subdirectory(Arm)
Index: source/Plugins/Architecture/Arm/CMakeLists.txt
===
--- /dev/null
+++ source/Plugins/Architecture/Arm/CMakeLists.txt
@@ -0,0 +1,10 @@
+add_lldb_library(lldbPluginArchitec
[Lldb-commits] [PATCH] D35298: [MainLoop] Fix possible use of an invalid iterator
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks great, thanks for catching this. The code was doing it this way initially, but then this got lost with all the frantic refactors. https://reviews.llvm.org/D35298 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r307768 - Adding Support for Error Strings in Remote Packets
Author: ravitheja
Date: Wed Jul 12 04:15:34 2017
New Revision: 307768
URL: http://llvm.org/viewvc/llvm-project?rev=307768&view=rev
Log:
Adding Support for Error Strings in Remote Packets
Summary:
This patch adds support for sending strings along with
error codes in the reply packets. The implementation is
based on the feedback recieved in the lldb-dev mailing
list. The patch also adds an extra packet for the client
to query if the server has the capability to provide
strings along with error replys.
Reviewers: labath, jingham, sas, lldb-commits, clayborg
Reviewed By: labath, clayborg
Differential Revision: https://reviews.llvm.org/D34945
Modified:
lldb/trunk/docs/lldb-gdb-remote.txt
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/trunk/source/Utility/StringExtractorGDBRemote.cpp
lldb/trunk/source/Utility/StringExtractorGDBRemote.h
Modified: lldb/trunk/docs/lldb-gdb-remote.txt
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/docs/lldb-gdb-remote.txt?rev=307768&r1=307767&r2=307768&view=diff
==
--- lldb/trunk/docs/lldb-gdb-remote.txt (original)
+++ lldb/trunk/docs/lldb-gdb-remote.txt Wed Jul 12 04:15:34 2017
@@ -126,6 +126,32 @@ read packet: $OK#00
This packet can be sent one or more times _prior_ to sending a "A" packet.
//--
+// "QEnableErrorStrings"
+//
+// BRIEF
+// This packet enables reporting of Error strings in remote packet
+// replies from the server to client. If the server supports this
+// feature, it should send an OK response. The client can expect the
+// following error replies if this feature is enabled in the server ->
+//
+// EXX;A
+//
+// where A will be a hex encoded ASCII string.
+// XX is hex encoded byte number.
+//
+// It must be noted that even if the client has enabled reporting
+// strings in error replies, it must not expect error strings to all
+// error replies.
+//
+// PRIORITY TO IMPLEMENT
+// Low. Only needed if the remote target wants to provide strings that
+// are human readable along with an error code.
+//--
+
+send packet: $QErrorStringInPacketSupported
+read packet: $OK#00
+
+//--
// "QSetSTDIN:"
// "QSetSTDOUT:"
// "QSetSTDERR:"
@@ -250,11 +276,12 @@ read packet: OK
//
// Each tracing instance is identified by a trace id which is returned
// as the reply to this packet. In case the tracing failed to begin an
-// error code is returned instead.
+// error code along with a hex encoded ASCII message is returned
+// instead.
//--
send packet: jTraceStart:{"type":,"buffersize":}]
-read packet: /E
+read packet: /E;A
//--
// jTraceStop:
@@ -283,12 +310,12 @@ read packet: /E
// to stop tracing on that thread.
// ==
//
-// An OK response is sent in case of success else an error code is
-// returned.
+// An OK response is sent in case of success else an error code along
+// with a hex encoded ASCII message is returned.
//--
send packet: jTraceStop:{"traceid":}]
-read packet: /E
+read packet: /E;A
//--
// jTraceBufferRead:
@@ -317,11 +344,11 @@ read packet: /E
// ==
//
// The trace data is sent as raw binary data if the read was successful
-// else an error code is sent.
+// else an error code along with a hex encoded ASCII message is sent.
//--
send packet: jTraceBufferRead:{"traceid":,"offset":,"buffersize":}]
-read packet: /E
+read packet: /E;A
//--
// jTraceMetaRead:
@@ -359,11 +386,11 @@ read packet: /E was not found, an
-// error code is returned.
+// error code along with a hex encoded ASCII message is returned.
//--
send packet: jTraceConfigRead:{"traceid":}
-read packet:
{"conf1":,"conf2":,"params":{"paramName":para
[Lldb-commits] [lldb] r307773 - Fixing Android builder
Author: ravitheja
Date: Wed Jul 12 04:54:17 2017
New Revision: 307773
URL: http://llvm.org/viewvc/llvm-project?rev=307773&view=rev
Log:
Fixing Android builder
Modified:
lldb/trunk/source/Utility/StringExtractorGDBRemote.cpp
Modified: lldb/trunk/source/Utility/StringExtractorGDBRemote.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/StringExtractorGDBRemote.cpp?rev=307773&r1=307772&r2=307773&view=diff
==
--- lldb/trunk/source/Utility/StringExtractorGDBRemote.cpp (original)
+++ lldb/trunk/source/Utility/StringExtractorGDBRemote.cpp Wed Jul 12 04:54:17
2017
@@ -469,12 +469,12 @@ lldb_private::Status StringExtractorGDBR
uint8_t errc = GetHexU8(255);
error.SetError(errc, lldb::eErrorTypeGeneric);
-std::string error_messg("Error ");
-error_messg += std::to_string(errc);
-if (GetChar() == ';')
+error.SetErrorStringWithFormat("Error %u", errc);
+std::string error_messg;
+if (GetChar() == ';') {
GetHexByteString(error_messg);
-
-error.SetErrorString(error_messg);
+ error.SetErrorString(error_messg);
+}
}
return error;
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r307782 - [MainLoop] Fix possible use of an invalid iterator
Author: petr.pavlu
Date: Wed Jul 12 05:38:31 2017
New Revision: 307782
URL: http://llvm.org/viewvc/llvm-project?rev=307782&view=rev
Log:
[MainLoop] Fix possible use of an invalid iterator
Store file descriptors from loop.m_read_fds (if FORCE_PSELECT is
defined) and signals from loop.m_signals that need to be processed in
MainLoop::RunImpl::ProcessEvents() into a separate vector and then
iterate over this container to invoke the callbacks.
This prevents a problem where when the code iterated directly over
m_read_fds/m_signals, a callback invoked from within the loop could
modify these variables and invalidate the loop iterator. This would then
result in an assertion failure in llvm::DenseMapIterator::operator++().
Differential Revision: https://reviews.llvm.org/D35298
Modified:
lldb/trunk/source/Host/common/MainLoop.cpp
Modified: lldb/trunk/source/Host/common/MainLoop.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/MainLoop.cpp?rev=307782&r1=307781&r2=307782&view=diff
==
--- lldb/trunk/source/Host/common/MainLoop.cpp (original)
+++ lldb/trunk/source/Host/common/MainLoop.cpp Wed Jul 12 05:38:31 2017
@@ -193,10 +193,16 @@ Status MainLoop::RunImpl::Poll() {
void MainLoop::RunImpl::ProcessEvents() {
#ifdef FORCE_PSELECT
- for (const auto &fd : loop.m_read_fds) {
-if (!FD_ISSET(fd.first, &read_fd_set))
- continue;
-IOObject::WaitableHandle handle = fd.first;
+ // Collect first all readable file descriptors into a separate vector and
then
+ // iterate over it to invoke callbacks. Iterating directly over
+ // loop.m_read_fds is not possible because the callbacks can modify the
+ // container which could invalidate the iterator.
+ std::vector fds;
+ for (const auto &fd : loop.m_read_fds)
+if (FD_ISSET(fd.first, &read_fd_set))
+ fds.push_back(fd.first);
+
+ for (const auto &handle : fds) {
#else
for (const auto &fd : read_fds) {
if ((fd.revents & POLLIN) == 0)
@@ -209,13 +215,16 @@ void MainLoop::RunImpl::ProcessEvents()
loop.ProcessReadObject(handle);
}
- for (const auto &entry : loop.m_signals) {
+ std::vector signals;
+ for (const auto &entry : loop.m_signals)
+if (g_signal_flags[entry.first] != 0)
+ signals.push_back(entry.first);
+
+ for (const auto &signal : signals) {
if (loop.m_terminate_request)
return;
-if (g_signal_flags[entry.first] == 0)
- continue; // No signal
-g_signal_flags[entry.first] = 0;
-loop.ProcessSignal(entry.first);
+g_signal_flags[signal] = 0;
+loop.ProcessSignal(signal);
}
}
#endif
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D35298: [MainLoop] Fix possible use of an invalid iterator
This revision was automatically updated to reflect the committed changes.
Closed by commit rL307782: [MainLoop] Fix possible use of an invalid iterator
(authored by petr.pavlu).
Changed prior to commit:
https://reviews.llvm.org/D35298?vs=106164&id=106187#toc
Repository:
rL LLVM
https://reviews.llvm.org/D35298
Files:
lldb/trunk/source/Host/common/MainLoop.cpp
Index: lldb/trunk/source/Host/common/MainLoop.cpp
===
--- lldb/trunk/source/Host/common/MainLoop.cpp
+++ lldb/trunk/source/Host/common/MainLoop.cpp
@@ -193,10 +193,16 @@
void MainLoop::RunImpl::ProcessEvents() {
#ifdef FORCE_PSELECT
- for (const auto &fd : loop.m_read_fds) {
-if (!FD_ISSET(fd.first, &read_fd_set))
- continue;
-IOObject::WaitableHandle handle = fd.first;
+ // Collect first all readable file descriptors into a separate vector and
then
+ // iterate over it to invoke callbacks. Iterating directly over
+ // loop.m_read_fds is not possible because the callbacks can modify the
+ // container which could invalidate the iterator.
+ std::vector fds;
+ for (const auto &fd : loop.m_read_fds)
+if (FD_ISSET(fd.first, &read_fd_set))
+ fds.push_back(fd.first);
+
+ for (const auto &handle : fds) {
#else
for (const auto &fd : read_fds) {
if ((fd.revents & POLLIN) == 0)
@@ -209,13 +215,16 @@
loop.ProcessReadObject(handle);
}
- for (const auto &entry : loop.m_signals) {
+ std::vector signals;
+ for (const auto &entry : loop.m_signals)
+if (g_signal_flags[entry.first] != 0)
+ signals.push_back(entry.first);
+
+ for (const auto &signal : signals) {
if (loop.m_terminate_request)
return;
-if (g_signal_flags[entry.first] == 0)
- continue; // No signal
-g_signal_flags[entry.first] = 0;
-loop.ProcessSignal(entry.first);
+g_signal_flags[signal] = 0;
+loop.ProcessSignal(signal);
}
}
#endif
Index: lldb/trunk/source/Host/common/MainLoop.cpp
===
--- lldb/trunk/source/Host/common/MainLoop.cpp
+++ lldb/trunk/source/Host/common/MainLoop.cpp
@@ -193,10 +193,16 @@
void MainLoop::RunImpl::ProcessEvents() {
#ifdef FORCE_PSELECT
- for (const auto &fd : loop.m_read_fds) {
-if (!FD_ISSET(fd.first, &read_fd_set))
- continue;
-IOObject::WaitableHandle handle = fd.first;
+ // Collect first all readable file descriptors into a separate vector and then
+ // iterate over it to invoke callbacks. Iterating directly over
+ // loop.m_read_fds is not possible because the callbacks can modify the
+ // container which could invalidate the iterator.
+ std::vector fds;
+ for (const auto &fd : loop.m_read_fds)
+if (FD_ISSET(fd.first, &read_fd_set))
+ fds.push_back(fd.first);
+
+ for (const auto &handle : fds) {
#else
for (const auto &fd : read_fds) {
if ((fd.revents & POLLIN) == 0)
@@ -209,13 +215,16 @@
loop.ProcessReadObject(handle);
}
- for (const auto &entry : loop.m_signals) {
+ std::vector signals;
+ for (const auto &entry : loop.m_signals)
+if (g_signal_flags[entry.first] != 0)
+ signals.push_back(entry.first);
+
+ for (const auto &signal : signals) {
if (loop.m_terminate_request)
return;
-if (g_signal_flags[entry.first] == 0)
- continue; // No signal
-g_signal_flags[entry.first] = 0;
-loop.ProcessSignal(entry.first);
+g_signal_flags[signal] = 0;
+loop.ProcessSignal(signal);
}
}
#endif
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D35305: Remove uint32_t assignment operator from Status
labath created this revision.
It is not presently used, and it's quite dangerous to use -- it assumes the
integer is an osx kern_return_t, but very few of the integers we have lying
around are mach kernel error codes. The error can still be used to a
mach error using a slightly longer (but more explicit) syntax.
https://reviews.llvm.org/D35305
Files:
include/lldb/Utility/Status.h
source/Utility/Status.cpp
Index: source/Utility/Status.cpp
===
--- source/Utility/Status.cpp
+++ source/Utility/Status.cpp
@@ -104,16 +104,6 @@
return *this;
}
-//--
-// Assignment operator
-//--
-const Status &Status::operator=(uint32_t err) {
- m_code = err;
- m_type = eErrorTypeMachKernel;
- m_string.clear();
- return *this;
-}
-
Status::~Status() = default;
//--
Index: include/lldb/Utility/Status.h
===
--- include/lldb/Utility/Status.h
+++ include/lldb/Utility/Status.h
@@ -88,19 +88,6 @@
//--
const Status &operator=(const Status &rhs);
- //--
- /// Assignment operator from a kern_return_t.
- ///
- /// Sets the type to \c MachKernel and the error code to \a err.
- ///
- /// @param[in] err
- /// A mach error code.
- ///
- /// @return
- /// A const reference to this object.
- //--
- const Status &operator=(uint32_t err);
-
~Status();
// llvm::Error support
Index: source/Utility/Status.cpp
===
--- source/Utility/Status.cpp
+++ source/Utility/Status.cpp
@@ -104,16 +104,6 @@
return *this;
}
-//--
-// Assignment operator
-//--
-const Status &Status::operator=(uint32_t err) {
- m_code = err;
- m_type = eErrorTypeMachKernel;
- m_string.clear();
- return *this;
-}
-
Status::~Status() = default;
//--
Index: include/lldb/Utility/Status.h
===
--- include/lldb/Utility/Status.h
+++ include/lldb/Utility/Status.h
@@ -88,19 +88,6 @@
//--
const Status &operator=(const Status &rhs);
- //--
- /// Assignment operator from a kern_return_t.
- ///
- /// Sets the type to \c MachKernel and the error code to \a err.
- ///
- /// @param[in] err
- /// A mach error code.
- ///
- /// @return
- /// A const reference to this object.
- //--
- const Status &operator=(uint32_t err);
-
~Status();
// llvm::Error support
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process
clayborg added a comment.
A few inline fixes, but this looks great. Very close
Comment at: include/lldb/Core/ArchSpec.h:26-30
namespace lldb_private {
class Platform;
-}
-namespace lldb_private {
class Stream;
-}
-namespace lldb_private {
class StringList;
}
This should actually be removed and replaced with a lldb-forward.h file. I know
it was wrong to begin with, but we should fix it as long as we are making
changes.
Comment at: include/lldb/Core/Architecture.h:17
+
+class Architecture {
+public:
This should inherit from PluginInterface so we can extract the plug-in name
from it if needed for logging.
Comment at: include/lldb/Core/Architecture.h:19
+public:
+ Architecture() = default;
+ virtual ~Architecture() = default;
Not sure the constructor is needed?
https://reviews.llvm.org/D31172
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D34945: Adding Support for Error Strings in Remote Packets
clayborg added a comment. Note that if you put "Differential Revision: " followed by the URL for this: Differential Revision: https://reviews.llvm.org/D34945 in your source control commit message it will automatically close this for you and add the SVN revision number into this as a message. Please note the SVN revision in here if possible since you did it manually. https://reviews.llvm.org/D34945 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D35311: lldb-server tests: Add support for testing debugserver
labath created this revision.
Herald added subscribers: mgorny, srhines.
This adds support for running the lldb-server test suite (currently consisting
of only one test) against the debugserver. Currently, the choice which binary
to test is based on the host system. This will need to be replaced by something
more elaborate if/when lldb-server starts supporting debugging on darwin.
I need to make a couple of tweaks to the test client to work with debugserver:
- debugserver has different command-line arguments - launching code adjusted to
handle that
- debugserver sends duplicate "medata" fields in the stop reply packet -
adjusted stop-reply parsing code to handle that
- debugserver replies to the k packet instead of just dropping the connection -
stopping code adjusted, although we should probably consider aligning the
behavior of the two stubs in this case
https://reviews.llvm.org/D35311
Files:
unittests/tools/CMakeLists.txt
unittests/tools/lldb-server/CMakeLists.txt
unittests/tools/lldb-server/tests/MessageObjects.cpp
unittests/tools/lldb-server/tests/MessageObjects.h
unittests/tools/lldb-server/tests/TestClient.cpp
unittests/tools/lldb-server/tests/TestClient.h
Index: unittests/tools/lldb-server/tests/TestClient.h
===
--- unittests/tools/lldb-server/tests/TestClient.h
+++ unittests/tools/lldb-server/tests/TestClient.h
@@ -22,6 +22,9 @@
: public lldb_private::process_gdb_remote::GDBRemoteCommunicationClient {
public:
static void Initialize();
+ static bool IsDebugServer();
+ static bool IsLldbServer();
+
TestClient(const std::string &test_name, const std::string &test_case_name);
virtual ~TestClient();
LLVM_NODISCARD bool StartDebugger();
Index: unittests/tools/lldb-server/tests/TestClient.cpp
===
--- unittests/tools/lldb-server/tests/TestClient.cpp
+++ unittests/tools/lldb-server/tests/TestClient.cpp
@@ -28,6 +28,16 @@
namespace llgs_tests {
void TestClient::Initialize() { HostInfo::Initialize(); }
+bool TestClient::IsDebugServer() {
+#ifdef __APPLE__
+ return true;
+#else
+ return false;
+#endif
+}
+
+bool TestClient::IsLldbServer() { return !IsDebugServer(); }
+
TestClient::TestClient(const std::string &test_name,
const std::string &test_case_name)
: m_test_name(test_name), m_test_case_name(test_case_name),
@@ -39,13 +49,16 @@
const ArchSpec &arch_spec = HostInfo::GetArchitecture();
Args args;
args.AppendArgument(LLDB_SERVER);
- args.AppendArgument("gdbserver");
- args.AppendArgument("--log-channels=gdb-remote packets");
+ if (IsLldbServer()) {
+args.AppendArgument("gdbserver");
+args.AppendArgument("--log-channels=gdb-remote packets");
+ } else {
+args.AppendArgument("--log-flags=0x80");
+ }
args.AppendArgument("--reverse-connect");
std::string log_file_name = GenerateLogFileName(arch_spec);
- if (log_file_name.size()) {
+ if (log_file_name.size())
args.AppendArgument("--log-file=" + log_file_name);
- }
Status error;
TCPSocket listen_socket(true, false);
@@ -83,7 +96,11 @@
bool TestClient::StopDebugger() {
std::string response;
- return SendMessage("k", response, PacketResult::ErrorDisconnected);
+ // Debugserver (non-conformingly?) sends a reply to the k packet instead of
+ // simply closing the connection.
+ PacketResult result =
+ IsDebugServer() ? PacketResult::Success : PacketResult::ErrorDisconnected;
+ return SendMessage("k", response, result);
}
bool TestClient::SetInferior(llvm::ArrayRef inferior_args) {
@@ -192,7 +209,7 @@
return UINT_MAX;
}
-auto elements_or_error = SplitPairList("GetPcRegisterId", response);
+auto elements_or_error = SplitUniquePairList("GetPcRegisterId", response);
if (auto split_error = elements_or_error.takeError()) {
GTEST_LOG_(ERROR) << "GetPcRegisterId: Error splitting response: "
<< response;
Index: unittests/tools/lldb-server/tests/MessageObjects.h
===
--- unittests/tools/lldb-server/tests/MessageObjects.h
+++ unittests/tools/lldb-server/tests/MessageObjects.h
@@ -89,7 +89,10 @@
// Common functions for parsing packet data.
llvm::Expected>
-SplitPairList(llvm::StringRef caller, llvm::StringRef s);
+SplitUniquePairList(llvm::StringRef caller, llvm::StringRef s);
+
+llvm::StringMap>
+SplitPairList(llvm::StringRef s);
template
llvm::Error make_parsing_error(llvm::StringRef format, Args &&... args) {
Index: unittests/tools/lldb-server/tests/MessageObjects.cpp
===
--- unittests/tools/lldb-server/tests/MessageObjects.cpp
+++ unittests/tools/lldb-server/tests/MessageObjects.cpp
@@ -19,7 +19,7 @@
Expected ProcessInfo::Create(StringRef response) {
ProcessInfo process_info;
- auto elements_or_error = Sp
[Lldb-commits] [PATCH] D33213: Use socketpair on all Unix platforms
clayborg added a comment. Looks fine to me, lets make sure Pavel is OK with this. On MacOS socketpair is also much faster. Please run the following command while stopped at a breakpoint with and without your change: (lldb) process plugin packet speed-test It will send and receive packets using a variety of sizes and report the number of packets per second. Repository: rL LLVM https://reviews.llvm.org/D33213 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33213: Use socketpair on all Unix platforms
labath requested changes to this revision. labath added a comment. This revision now requires changes to proceed. Have you tried that this actually works? (It doesn't work for me) As far as I can tell, you will have to teach lldb-server to understand the `--fd` argument before you can flip this switch. Repository: rL LLVM https://reviews.llvm.org/D33213 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process
jingham added a comment. This is a good addition. However, it seems to me that since you only need an ArchSpec to make one of these Architecture plugins, which plugin you get seems fully determined by the Target, not the Process. I understand that the only need for it at present is to do a Process-related task. But that task actually takes a Thread as a parameter, so it doesn't need to know it's containing process to do its job. And it seems like it diminishes the plugin's future utility to have it more limited in scope than it needs to be. What do you think? https://reviews.llvm.org/D31172 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D33213: Use socketpair on all Unix platforms
clayborg added a comment. Ah yes, I thought there was something missing... Repository: rL LLVM https://reviews.llvm.org/D33213 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D35065: [LLDB][ppc64le] Rename enums in AuxVector
brunoalr added a comment. In https://reviews.llvm.org/D35065#803602, @brunoalr wrote: > @labath @joerg @krytarowski thanks for the review. Can anyone commit this, > please? I still don't have commit privileges. thanks, @labath Repository: rL LLVM https://reviews.llvm.org/D35065 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process
labath added a comment. In https://reviews.llvm.org/D31172#806804, @jingham wrote: > This is a good addition. However, it seems to me that since you only need an > ArchSpec to make one of these Architecture plugins, which plugin you get > seems fully determined by the Target, not the Process. I understand that the > only need for it at present is to do a Process-related task. But that task > actually takes a Thread as a parameter, so the Architecture plugin doesn't > need to know it's containing process to do its job. And it seems like it > diminishes the plugin's future utility to have it more limited in scope than > it needs to be. > > What do you think? I'm not sure I understand what you are proposing here. Is it to have the architecture plugin store a Target as a member variable? If that's the case, then I'd say let's wait until a need for that arises. I am not fundamentally against the idea, but I don't see a reason to add it while we don't have a need for it. https://reviews.llvm.org/D31172 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process
clayborg added a comment. I think is saying we should just store the Architecture plug-in in the target instead of in the process. It will also need to be cleared when the target arch is changed. https://reviews.llvm.org/D31172 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process
jingham added a comment. Sorry I wasn't clear. I meant that since the Target knows everything it needs to know to vend the correct Architecture plugin, you should get it from the Target, not the Process. In general, I think that the highest class in the stack that can vend a plugin is the one that should. https://reviews.llvm.org/D31172 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process
jingham added a comment. What Greg said... https://reviews.llvm.org/D31172 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r307870 - The llvm.org bugzilla moved.
Author: jingham Date: Wed Jul 12 17:36:21 2017 New Revision: 307870 URL: http://llvm.org/viewvc/llvm-project?rev=307870&view=rev Log: The llvm.org bugzilla moved. Modified: lldb/trunk/www/sidebar.incl Modified: lldb/trunk/www/sidebar.incl URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/www/sidebar.incl?rev=307870&r1=307869&r2=307870&view=diff == --- lldb/trunk/www/sidebar.incl (original) +++ lldb/trunk/www/sidebar.incl Wed Jul 12 17:36:21 2017 @@ -49,7 +49,7 @@ Build Test SB API Coding Rules - http://llvm.org/bugs";>Bug Reports + http://bugs.llvm.org";>Bug Reports http://llvm.org/svn/llvm-project/lldb/trunk";>Browse SVN http://llvm.org/viewvc/llvm-project/lldb/trunk";>Browse ViewVC ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r307881 - Upstreaming a patch from Github: When evaluation user expressions, ignore InstrumentationRuntime breakpoints. (#235)
Author: kuba.brecka
Date: Wed Jul 12 21:35:27 2017
New Revision: 307881
URL: http://llvm.org/viewvc/llvm-project?rev=307881&view=rev
Log:
Upstreaming a patch from Github: When evaluation user expressions, ignore
InstrumentationRuntime breakpoints. (#235)
Added:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/ubsan/user-expression/
lldb/trunk/packages/Python/lldbsuite/test/functionalities/ubsan/user-expression/Makefile
lldb/trunk/packages/Python/lldbsuite/test/functionalities/ubsan/user-expression/TestUbsanUserExpression.py
lldb/trunk/packages/Python/lldbsuite/test/functionalities/ubsan/user-expression/main.c
Modified:
lldb/trunk/source/Plugins/InstrumentationRuntime/ASan/ASanRuntime.cpp
lldb/trunk/source/Plugins/InstrumentationRuntime/MainThreadChecker/MainThreadCheckerRuntime.cpp
lldb/trunk/source/Plugins/InstrumentationRuntime/TSan/TSanRuntime.cpp
lldb/trunk/source/Plugins/InstrumentationRuntime/UBSan/UBSanRuntime.cpp
Added:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/ubsan/user-expression/Makefile
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/ubsan/user-expression/Makefile?rev=307881&view=auto
==
---
lldb/trunk/packages/Python/lldbsuite/test/functionalities/ubsan/user-expression/Makefile
(added)
+++
lldb/trunk/packages/Python/lldbsuite/test/functionalities/ubsan/user-expression/Makefile
Wed Jul 12 21:35:27 2017
@@ -0,0 +1,6 @@
+LEVEL = ../../../make
+
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -fsanitize=undefined -g
+
+include $(LEVEL)/Makefile.rules
Added:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/ubsan/user-expression/TestUbsanUserExpression.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/ubsan/user-expression/TestUbsanUserExpression.py?rev=307881&view=auto
==
---
lldb/trunk/packages/Python/lldbsuite/test/functionalities/ubsan/user-expression/TestUbsanUserExpression.py
(added)
+++
lldb/trunk/packages/Python/lldbsuite/test/functionalities/ubsan/user-expression/TestUbsanUserExpression.py
Wed Jul 12 21:35:27 2017
@@ -0,0 +1,49 @@
+"""
+Test that hitting a UBSan issue while running user expression doesn't break
the evaluation.
+"""
+
+import os
+import time
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+import json
+
+
+class UbsanUserExpressionTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipUnlessUndefinedBehaviorSanitizer
+def test(self):
+self.build()
+self.ubsan_tests()
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+self.line_breakpoint = line_number('main.c', '// breakpoint line')
+
+def ubsan_tests(self):
+# Load the test
+exe = os.path.join(os.getcwd(), "a.out")
+self.expect(
+"file " + exe,
+patterns=["Current executable set to .*a.out"])
+
+self.runCmd("breakpoint set -f main.c -l %d" % self.line_breakpoint)
+
+self.runCmd("run")
+
+process = self.dbg.GetSelectedTarget().process
+thread = process.GetSelectedThread()
+frame = thread.GetSelectedFrame()
+
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stopped', 'stop reason = breakpoint'])
+
+self.expect("p foo()", substrs=["(int) $0 = 42"])
+
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stopped', 'stop reason = breakpoint'])
Added:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/ubsan/user-expression/main.c
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/ubsan/user-expression/main.c?rev=307881&view=auto
==
---
lldb/trunk/packages/Python/lldbsuite/test/functionalities/ubsan/user-expression/main.c
(added)
+++
lldb/trunk/packages/Python/lldbsuite/test/functionalities/ubsan/user-expression/main.c
Wed Jul 12 21:35:27 2017
@@ -0,0 +1,9 @@
+int foo() {
+ int data[4];
+ int x = *(int *)(((char *)&data[0]) + 2);
+ return 42;
+}
+
+int main() {
+ return 0; // breakpoint line
+}
Modified: lldb/trunk/source/Plugins/InstrumentationRuntime/ASan/ASanRuntime.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/InstrumentationRuntime/ASan/ASanRuntime.cpp?rev=307881&r1=307880&r2=307881&view=diff
==
--- lldb/trunk/source/Plugins/InstrumentationRuntime/ASan/ASanRuntime.cpp
(original)
+++ lldb/trunk/source/Plugins/InstrumentationRuntime/ASan/ASanRuntime.cpp Wed
Jul 12 21:35:27 2017
@@ -247,12 +247,16 @
[Lldb-commits] [PATCH] D34945: Adding Support for Error Strings in Remote Packets
ravitheja added a comment. Uploaded SVN Revision Number - 307773 to fix Android Builder. https://reviews.llvm.org/D34945 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
