[Lldb-commits] [PATCH] D32600: Resurrect pselect MainLoop implementation
labath added inline comments.
Comment at: source/Host/common/MainLoop.cpp:82
+ int queue_id;
+ std::vector events;
+ struct kevent event_list[4];
eugene wrote:
> here and below struct seems to be redundant
One of them holds the input events, and the other the output -- I am assuming
they can't be the same. @beanz?
I am going to rename them to reflect this better though.
Comment at: source/Host/common/MainLoop.cpp:135
+
+template void MainLoop::RunImpl::ForEachReadFD(F &&f) {
+ assert(num_events >= 0);
zturner wrote:
> Why do we need this function? Just have a function that returns an
> `ArrayRef` and let the caller iterate over it themselves.
The logic for detecting whether an event has actually happened is different for
ppoll and kevent based implementations. In case of kevent you already get a
list of events ready, so it may seem that it is redundant, but in case of
ppoll, I have to look through the list of input events, and check that they
have they `POLLIN` flag set. This is basically what is this trying to abstract.
I can believe it is overkill though.
I'll see if I can come up with something cleaner.
Comment at: source/Host/common/MainLoop.cpp:167
+
+#ifdef FORCE_PSELECT
+ fd_set read_fd_set;
zturner wrote:
> How about just moving this `#ifdef` up outside of the Poll function? Given
> that it occurs so many times, i think it makes the logic and the differences
> between the two implementations easier to follow.
OK.
https://reviews.llvm.org/D32600
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32585: Implementation of remote packets for Trace data.
ravitheja added inline comments. Comment at: docs/lldb-gdb-remote.txt:212 //-- +// QTrace:1:type:; +// clayborg wrote: > Should we make all these new packets JSON based to start with? "jTrace"? If > we have any need for JSON at all in this or the other new packets I would say > lets just go with JSON packets. They are currently prefixed with "j". If we > go this route we should specify the mandatory key/value pairs in the header > doc. We should also allow a JSON dictionary from the trace config up at the > SBTrace layer to make it into this packet? I am fine prefixing the packets with "j", I did not understand what you meant by the last sentence. At the moment I am waiting for Pavel or Tamas to reply or anyone else and based on the complete feedback, I will upload a new patch if needed. https://reviews.llvm.org/D32585 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32568: Protect Proces::GetMemoryRegionInfo and ::GetFileLoadAddress with a lock
labath added a comment. Cool, glad that's sorted. I've had some ideas about introducing limited amount of paralellism to the server side, but that's probably is not interesting to you if you're not doing remote debugging. Repository: rL LLVM https://reviews.llvm.org/D32568 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32626: Make the symbol demangling loop order independent
labath added a comment. If you're going to be making drastic changes here, could you please look at the possibility of making a more targeted test, rather than relying on the "run the whole debugger and set a breakpoint" type of tests to verify the finer details of the implementation. I was able to do a unit test for a bug in elf parsing code in https://reviews.llvm.org/D32434, which is not that far from here. That gives me hope it may be possible, but it's hard to say without knowing what kind of changes you would like to make. Repository: rL LLVM https://reviews.llvm.org/D32626 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32600: Resurrect pselect MainLoop implementation
labath updated this revision to Diff 97074.
labath added a comment.
Address review feedback.
https://reviews.llvm.org/D32600
Files:
include/lldb/Host/MainLoop.h
source/Host/common/MainLoop.cpp
Index: source/Host/common/MainLoop.cpp
===
--- source/Host/common/MainLoop.cpp
+++ source/Host/common/MainLoop.cpp
@@ -32,6 +32,10 @@
#define POLL poll
#endif
+#ifdef __ANDROID__
+#define FORCE_PSELECT
+#endif
+
#if SIGNAL_POLLING_UNSUPPORTED
#ifdef LLVM_ON_WIN32
typedef int sigset_t;
@@ -59,6 +63,189 @@
g_signal_flags[signo] = 1;
}
+class MainLoop::RunImpl {
+public:
+ // TODO: Use llvm::Expected
+ static std::unique_ptr Create(MainLoop &loop, Error &error);
+ ~RunImpl();
+
+ Error Poll();
+
+ template void ForEachReadFD(F &&f);
+ template void ForEachSignal(F &&f);
+
+private:
+ MainLoop &loop;
+
+#if HAVE_SYS_EVENT_H
+ int queue_id;
+ std::vector in_events;
+ struct kevent out_events[4];
+ int num_events = -1;
+
+ RunImpl(MainLoop &loop, int queue_id) : loop(loop), queue_id(queue_id) {
+in_events.reserve(loop.m_read_fds.size() + loop.m_signals.size());
+ }
+#else
+ std::vector signals;
+#ifdef FORCE_PSELECT
+ fd_set read_fd_set;
+#else
+ std::vector read_fds;
+#endif
+
+ RunImpl(MainLoop &loop) : loop(loop) {
+signals.reserve(loop.m_signals.size());
+ }
+
+ sigset_t get_sigmask();
+#endif
+};
+
+#if HAVE_SYS_EVENT_H
+MainLoop::RunImpl::~RunImpl() {
+ int r = close(queue_id);
+ assert(r == 0);
+ (void)r;
+}
+std::unique_ptr MainLoop::RunImpl::Create(MainLoop &loop, Error &error)
+{
+ error.Clear();
+ int queue_id = kqueue();
+ if(queue_id < 0) {
+error = Error(errno, eErrorTypePOSIX);
+return nullptr;
+ }
+ return std::unique_ptr(new RunImpl(loop, queue_id));
+}
+
+Error MainLoop::RunImpl::Poll() {
+ in_events.resize(loop.m_read_fds.size() + loop.m_signals.size());
+ unsigned i = 0;
+ for (auto &fd : loop.m_read_fds)
+EV_SET(&in_events[i++], fd.first, EVFILT_READ, EV_ADD, 0, 0, 0);
+
+ for (const auto &sig : loop.m_signals)
+EV_SET(&in_events[i++], sig.first, EVFILT_SIGNAL, EV_ADD, 0, 0, 0);
+
+ num_events = kevent(queue_id, in_events.data(), in_events.size(), out_events,
+ llvm::array_lengthof(out_events), nullptr);
+
+ if (num_events < 0)
+return Error("kevent() failed with error %d\n", num_events);
+ return Error();
+}
+
+template void MainLoop::RunImpl::ForEachReadFD(F &&f) {
+ assert(num_events >= 0);
+ for (int i = 0; i < num_events; ++i) {
+f(out_events[i].ident);
+if (loop.m_terminate_request)
+ return;
+ }
+}
+template void MainLoop::RunImpl::ForEachSignal(F && f) {}
+#else
+MainLoop::RunImpl::~RunImpl() {}
+std::unique_ptr MainLoop::RunImpl::Create(MainLoop &loop, Error &error)
+{
+ error.Clear();
+ return std::unique_ptr(new RunImpl(loop));
+}
+
+sigset_t MainLoop::RunImpl::get_sigmask() {
+#if SIGNAL_POLLING_UNSUPPORTED
+ return 0;
+#else
+ sigset_t sigmask;
+ int ret = pthread_sigmask(SIG_SETMASK, nullptr, &sigmask);
+ assert(ret == 0);
+ (void) ret;
+
+ for (const auto &sig : loop.m_signals) {
+signals.push_back(sig.first);
+sigdelset(&sigmask, sig.first);
+ }
+ return sigmask;
+#endif
+}
+
+#ifdef FORCE_PSELECT
+Error MainLoop::RunImpl::Poll() {
+ signals.clear();
+
+ FD_ZERO(&read_fd_set);
+ int nfds = 0;
+ for (const auto &fd : loop.m_read_fds) {
+FD_SET(fd.first, &read_fd_set);
+nfds = std::max(nfds, fd.first + 1);
+ }
+
+ sigset_t sigmask = get_sigmask();
+ if (pselect(nfds, &read_fd_set, nullptr, nullptr, nullptr, &sigmask) == -1 &&
+ errno != EINTR)
+return Error(errno, eErrorTypePOSIX);
+
+ return Error();
+}
+
+template void MainLoop::RunImpl::ForEachReadFD(F &&f) {
+ for (const auto &fd : loop.m_read_fds) {
+if(!FD_ISSET(fd.first, &read_fd_set))
+ continue;
+
+f(fd.first);
+if (loop.m_terminate_request)
+ return;
+ }
+}
+#else
+Error MainLoop::RunImpl::Poll() {
+ signals.clear();
+ read_fds.clear();
+
+ sigset_t sigmask = get_sigmask();
+
+ for (const auto &fd : loop.m_read_fds) {
+struct pollfd pfd;
+pfd.fd = fd.first;
+pfd.events = POLLIN;
+pfd.revents = 0;
+read_fds.push_back(pfd);
+ }
+
+ if (ppoll(read_fds.data(), read_fds.size(), nullptr, &sigmask) == -1 &&
+ errno != EINTR)
+return Error(errno, eErrorTypePOSIX);
+
+ return Error();
+}
+
+template void MainLoop::RunImpl::ForEachReadFD(F &&f) {
+ for (const auto &fd : read_fds) {
+if ((fd.revents & POLLIN) == 0)
+ continue;
+
+f(fd.fd);
+if (loop.m_terminate_request)
+ return;
+ }
+}
+#endif
+
+template void MainLoop::RunImpl::ForEachSignal(F &&f) {
+ for (int sig : signals) {
+if (g_signal_flags[sig] == 0)
+ continue; // No signal
+g_signal_flags[sig] = 0;
+f(sig);
+
+if (loop.m_terminate_request)
+ return;
+ }
+}
+#endif
+
MainLoop::~MainLoop() {
assert(m_read_fds.size() == 0);
assert(m_
[Lldb-commits] [PATCH] D32600: Resurrect pselect MainLoop implementation
labath added a comment. I am going to check this in to unbork the android bots. I'm happy to address any additional feedback in a followup. I'm also planning to come some unit tests for this class next week. https://reviews.llvm.org/D32600 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r301636 - Resurrect pselect MainLoop implementation
Author: labath
Date: Fri Apr 28 05:26:06 2017
New Revision: 301636
URL: http://llvm.org/viewvc/llvm-project?rev=301636&view=rev
Log:
Resurrect pselect MainLoop implementation
Summary:
It turns out that even though ppoll is available on all the android
devices we support, it does not seem to be working properly on all of
them -- MainLoop just does a busy loop with ppoll returning EINTR and
not making any progress.
This brings back the pselect implementation and makes it available on
android. I could not do any cmake checks for this as the ppoll symbol is
actually avaiable -- it just does not work.
Reviewers: beanz, eugene
Subscribers: srhines, lldb-commits
Differential Revision: https://reviews.llvm.org/D32600
Modified:
lldb/trunk/include/lldb/Host/MainLoop.h
lldb/trunk/source/Host/common/MainLoop.cpp
Modified: lldb/trunk/include/lldb/Host/MainLoop.h
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/MainLoop.h?rev=301636&r1=301635&r2=301636&view=diff
==
--- lldb/trunk/include/lldb/Host/MainLoop.h (original)
+++ lldb/trunk/include/lldb/Host/MainLoop.h Fri Apr 28 05:26:06 2017
@@ -93,6 +93,7 @@ private:
#endif
bool was_blocked : 1;
};
+ class RunImpl;
llvm::DenseMap m_read_fds;
llvm::DenseMap m_signals;
Modified: lldb/trunk/source/Host/common/MainLoop.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/MainLoop.cpp?rev=301636&r1=301635&r2=301636&view=diff
==
--- lldb/trunk/source/Host/common/MainLoop.cpp (original)
+++ lldb/trunk/source/Host/common/MainLoop.cpp Fri Apr 28 05:26:06 2017
@@ -32,6 +32,10 @@
#define POLL poll
#endif
+#ifdef __ANDROID__
+#define FORCE_PSELECT
+#endif
+
#if SIGNAL_POLLING_UNSUPPORTED
#ifdef LLVM_ON_WIN32
typedef int sigset_t;
@@ -59,6 +63,189 @@ static void SignalHandler(int signo, sig
g_signal_flags[signo] = 1;
}
+class MainLoop::RunImpl {
+public:
+ // TODO: Use llvm::Expected
+ static std::unique_ptr Create(MainLoop &loop, Error &error);
+ ~RunImpl();
+
+ Error Poll();
+
+ template void ForEachReadFD(F &&f);
+ template void ForEachSignal(F &&f);
+
+private:
+ MainLoop &loop;
+
+#if HAVE_SYS_EVENT_H
+ int queue_id;
+ std::vector in_events;
+ struct kevent out_events[4];
+ int num_events = -1;
+
+ RunImpl(MainLoop &loop, int queue_id) : loop(loop), queue_id(queue_id) {
+in_events.reserve(loop.m_read_fds.size() + loop.m_signals.size());
+ }
+#else
+ std::vector signals;
+#ifdef FORCE_PSELECT
+ fd_set read_fd_set;
+#else
+ std::vector read_fds;
+#endif
+
+ RunImpl(MainLoop &loop) : loop(loop) {
+signals.reserve(loop.m_signals.size());
+ }
+
+ sigset_t get_sigmask();
+#endif
+};
+
+#if HAVE_SYS_EVENT_H
+MainLoop::RunImpl::~RunImpl() {
+ int r = close(queue_id);
+ assert(r == 0);
+ (void)r;
+}
+std::unique_ptr MainLoop::RunImpl::Create(MainLoop &loop,
Error &error)
+{
+ error.Clear();
+ int queue_id = kqueue();
+ if(queue_id < 0) {
+error = Error(errno, eErrorTypePOSIX);
+return nullptr;
+ }
+ return std::unique_ptr(new RunImpl(loop, queue_id));
+}
+
+Error MainLoop::RunImpl::Poll() {
+ in_events.resize(loop.m_read_fds.size() + loop.m_signals.size());
+ unsigned i = 0;
+ for (auto &fd : loop.m_read_fds)
+EV_SET(&in_events[i++], fd.first, EVFILT_READ, EV_ADD, 0, 0, 0);
+
+ for (const auto &sig : loop.m_signals)
+EV_SET(&in_events[i++], sig.first, EVFILT_SIGNAL, EV_ADD, 0, 0, 0);
+
+ num_events = kevent(queue_id, in_events.data(), in_events.size(), out_events,
+ llvm::array_lengthof(out_events), nullptr);
+
+ if (num_events < 0)
+return Error("kevent() failed with error %d\n", num_events);
+ return Error();
+}
+
+template void MainLoop::RunImpl::ForEachReadFD(F &&f) {
+ assert(num_events >= 0);
+ for (int i = 0; i < num_events; ++i) {
+f(out_events[i].ident);
+if (loop.m_terminate_request)
+ return;
+ }
+}
+template void MainLoop::RunImpl::ForEachSignal(F && f) {}
+#else
+MainLoop::RunImpl::~RunImpl() {}
+std::unique_ptr MainLoop::RunImpl::Create(MainLoop &loop,
Error &error)
+{
+ error.Clear();
+ return std::unique_ptr(new RunImpl(loop));
+}
+
+sigset_t MainLoop::RunImpl::get_sigmask() {
+#if SIGNAL_POLLING_UNSUPPORTED
+ return 0;
+#else
+ sigset_t sigmask;
+ int ret = pthread_sigmask(SIG_SETMASK, nullptr, &sigmask);
+ assert(ret == 0);
+ (void) ret;
+
+ for (const auto &sig : loop.m_signals) {
+signals.push_back(sig.first);
+sigdelset(&sigmask, sig.first);
+ }
+ return sigmask;
+#endif
+}
+
+#ifdef FORCE_PSELECT
+Error MainLoop::RunImpl::Poll() {
+ signals.clear();
+
+ FD_ZERO(&read_fd_set);
+ int nfds = 0;
+ for (const auto &fd : loop.m_read_fds) {
+FD_SET(fd.first, &read_fd_set);
+nfds = std::max(nfds, fd.first + 1);
+ }
+
+ sigset_t sigmask = get_sigmask();
+ if (pselect(nfds, &read_fd_set, nullptr, nullpt
[Lldb-commits] [PATCH] D32600: Resurrect pselect MainLoop implementation
This revision was automatically updated to reflect the committed changes.
Closed by commit rL301636: Resurrect pselect MainLoop implementation (authored
by labath).
Changed prior to commit:
https://reviews.llvm.org/D32600?vs=97074&id=97075#toc
Repository:
rL LLVM
https://reviews.llvm.org/D32600
Files:
lldb/trunk/include/lldb/Host/MainLoop.h
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
@@ -32,6 +32,10 @@
#define POLL poll
#endif
+#ifdef __ANDROID__
+#define FORCE_PSELECT
+#endif
+
#if SIGNAL_POLLING_UNSUPPORTED
#ifdef LLVM_ON_WIN32
typedef int sigset_t;
@@ -59,6 +63,189 @@
g_signal_flags[signo] = 1;
}
+class MainLoop::RunImpl {
+public:
+ // TODO: Use llvm::Expected
+ static std::unique_ptr Create(MainLoop &loop, Error &error);
+ ~RunImpl();
+
+ Error Poll();
+
+ template void ForEachReadFD(F &&f);
+ template void ForEachSignal(F &&f);
+
+private:
+ MainLoop &loop;
+
+#if HAVE_SYS_EVENT_H
+ int queue_id;
+ std::vector in_events;
+ struct kevent out_events[4];
+ int num_events = -1;
+
+ RunImpl(MainLoop &loop, int queue_id) : loop(loop), queue_id(queue_id) {
+in_events.reserve(loop.m_read_fds.size() + loop.m_signals.size());
+ }
+#else
+ std::vector signals;
+#ifdef FORCE_PSELECT
+ fd_set read_fd_set;
+#else
+ std::vector read_fds;
+#endif
+
+ RunImpl(MainLoop &loop) : loop(loop) {
+signals.reserve(loop.m_signals.size());
+ }
+
+ sigset_t get_sigmask();
+#endif
+};
+
+#if HAVE_SYS_EVENT_H
+MainLoop::RunImpl::~RunImpl() {
+ int r = close(queue_id);
+ assert(r == 0);
+ (void)r;
+}
+std::unique_ptr MainLoop::RunImpl::Create(MainLoop &loop, Error &error)
+{
+ error.Clear();
+ int queue_id = kqueue();
+ if(queue_id < 0) {
+error = Error(errno, eErrorTypePOSIX);
+return nullptr;
+ }
+ return std::unique_ptr(new RunImpl(loop, queue_id));
+}
+
+Error MainLoop::RunImpl::Poll() {
+ in_events.resize(loop.m_read_fds.size() + loop.m_signals.size());
+ unsigned i = 0;
+ for (auto &fd : loop.m_read_fds)
+EV_SET(&in_events[i++], fd.first, EVFILT_READ, EV_ADD, 0, 0, 0);
+
+ for (const auto &sig : loop.m_signals)
+EV_SET(&in_events[i++], sig.first, EVFILT_SIGNAL, EV_ADD, 0, 0, 0);
+
+ num_events = kevent(queue_id, in_events.data(), in_events.size(), out_events,
+ llvm::array_lengthof(out_events), nullptr);
+
+ if (num_events < 0)
+return Error("kevent() failed with error %d\n", num_events);
+ return Error();
+}
+
+template void MainLoop::RunImpl::ForEachReadFD(F &&f) {
+ assert(num_events >= 0);
+ for (int i = 0; i < num_events; ++i) {
+f(out_events[i].ident);
+if (loop.m_terminate_request)
+ return;
+ }
+}
+template void MainLoop::RunImpl::ForEachSignal(F && f) {}
+#else
+MainLoop::RunImpl::~RunImpl() {}
+std::unique_ptr MainLoop::RunImpl::Create(MainLoop &loop, Error &error)
+{
+ error.Clear();
+ return std::unique_ptr(new RunImpl(loop));
+}
+
+sigset_t MainLoop::RunImpl::get_sigmask() {
+#if SIGNAL_POLLING_UNSUPPORTED
+ return 0;
+#else
+ sigset_t sigmask;
+ int ret = pthread_sigmask(SIG_SETMASK, nullptr, &sigmask);
+ assert(ret == 0);
+ (void) ret;
+
+ for (const auto &sig : loop.m_signals) {
+signals.push_back(sig.first);
+sigdelset(&sigmask, sig.first);
+ }
+ return sigmask;
+#endif
+}
+
+#ifdef FORCE_PSELECT
+Error MainLoop::RunImpl::Poll() {
+ signals.clear();
+
+ FD_ZERO(&read_fd_set);
+ int nfds = 0;
+ for (const auto &fd : loop.m_read_fds) {
+FD_SET(fd.first, &read_fd_set);
+nfds = std::max(nfds, fd.first + 1);
+ }
+
+ sigset_t sigmask = get_sigmask();
+ if (pselect(nfds, &read_fd_set, nullptr, nullptr, nullptr, &sigmask) == -1 &&
+ errno != EINTR)
+return Error(errno, eErrorTypePOSIX);
+
+ return Error();
+}
+
+template void MainLoop::RunImpl::ForEachReadFD(F &&f) {
+ for (const auto &fd : loop.m_read_fds) {
+if(!FD_ISSET(fd.first, &read_fd_set))
+ continue;
+
+f(fd.first);
+if (loop.m_terminate_request)
+ return;
+ }
+}
+#else
+Error MainLoop::RunImpl::Poll() {
+ signals.clear();
+ read_fds.clear();
+
+ sigset_t sigmask = get_sigmask();
+
+ for (const auto &fd : loop.m_read_fds) {
+struct pollfd pfd;
+pfd.fd = fd.first;
+pfd.events = POLLIN;
+pfd.revents = 0;
+read_fds.push_back(pfd);
+ }
+
+ if (ppoll(read_fds.data(), read_fds.size(), nullptr, &sigmask) == -1 &&
+ errno != EINTR)
+return Error(errno, eErrorTypePOSIX);
+
+ return Error();
+}
+
+template void MainLoop::RunImpl::ForEachReadFD(F &&f) {
+ for (const auto &fd : read_fds) {
+if ((fd.revents & POLLIN) == 0)
+ continue;
+
+f(fd.fd);
+if (loop.m_terminate_request)
+ return;
+ }
+}
+#endif
+
+template void MainLoop::RunImpl::ForEachSignal(F &&f) {
+ for (int sig : signals) {
+if (g_signal_f
[Lldb-commits] [PATCH] D32585: Implementation of remote packets for Trace data.
labath added a reviewer: zturner.
labath requested changes to this revision.
labath added a comment.
This revision now requires changes to proceed.
I am adding Zachary, as he usually has good ideas about APIs.
All in all, it's not a very controversal change, but I have a bunch of inline
comments about the choice of API. They can be summed up as:
- use more ArrayRef
- use less shared pointers
- use LLDB_LOG
I quite like that you have added just the packet plumbing code without an
concrete implementation. However, that is still a significant amount of parsing
code that should be accompanied by a test. The test suite for the client side
of the protocol is ready (TestGdbRemoteCommunicationClient), so I'd like to see
at least that.
Comment at: docs/lldb-gdb-remote.txt:212
//--
+// QTrace:1:type:;
+//
ravitheja wrote:
> clayborg wrote:
> > Should we make all these new packets JSON based to start with? "jTrace"? If
> > we have any need for JSON at all in this or the other new packets I would
> > say lets just go with JSON packets. They are currently prefixed with "j".
> > If we go this route we should specify the mandatory key/value pairs in the
> > header doc. We should also allow a JSON dictionary from the trace config up
> > at the SBTrace layer to make it into this packet?
> I am fine prefixing the packets with "j", I did not understand what you meant
> by the last sentence. At the moment I am waiting for Pavel or Tamas to reply
> or anyone else and based on the complete feedback, I will upload a new patch
> if needed.
I think Greg meant you should also change the packet format to take a json
argument instead of key-value pairs (The packet should be JTrace if it modifies
the state, right ?).
If there is a gdb equivalent which matches this functionality sufficiently
closely, then I would prefer to stick to that. However, given that we already
are attaching a json field to the packet, I think it would make sense to go
json all the way.
Comment at: docs/lldb-gdb-remote.txt:236
+// metabuffersize The size of buffer to hold meta data M
+// used for decoding the trace data.
+// ==
The code seems to be sending some json dictionaries, but I see no mention of
that here.
Comment at: docs/lldb-gdb-remote.txt:239
+//
+// Each tracing instance is identified by a user id which is returned
+// as the reply to this packet. In case the tracing failed to begin an
can we call this "trace id"? I'd like to avoid people confusing this with
*real* UIDs.
Comment at: docs/lldb-gdb-remote.txt:244
+
+send packet: QTrace:1:type:;buffersize:;
+read packet: /E
You seem to be putting "arguments" here, and you also have "mandatory"
arguments down below. This is mostly a style issue, but I propose you either go
with "put all mandatory arguments in the summary and only list optional
arguments below", or with "list all arguments in the table below" approach.
Comment at: docs/lldb-gdb-remote.txt:269
+//
+// An empty response is sent in case of success else an error code is
+// returned.
Empty response means "packet not supported". You should send OK in this case.
Comment at: include/lldb/Host/common/NativeProcessProtocol.h:13
+#include "lldb/Core/TraceOptions.h"
#include "lldb/Host/MainLoop.h"
You are missing the traceOptions file from this patch.
Comment at: include/lldb/Host/common/NativeProcessProtocol.h:333
+ //--
+ virtual lldb::user_id_t StartTracing(lldb::tid_t thread, TraceOptions
&config,
+ Error &error) {
Hard to say without seeing TraceOptions contents, but I would hope this
argument can be const. If you want these functions to modify it, then maybe a
different api would be called for.
Comment at: include/lldb/Host/common/NativeProcessProtocol.h:370
+lldb::tid_t thread = LLDB_INVALID_THREAD_ID) {
+Error error;
+error.SetErrorString("Not implemented");
return Error("Not implemented");
Comment at: include/lldb/Host/common/NativeProcessProtocol.h:396
+ virtual size_t GetTraceData(lldb::user_id_t uid, lldb::tid_t thread,
+ Error &error, void *buf, size_t buf_size,
+ size_t offset = 0) {
llvm::MutableArrayRef instead of buf+size pair. Maybe we could even
pass in a reference to the MutableArrayRef so that the function can truncate it
to the actual size?
Comment at: include/lldb/Host/common/NativeProcessProtocol
[Lldb-commits] [PATCH] D32306: Remove lock from ConstString::GetLength
This revision was automatically updated to reflect the committed changes.
Closed by commit rL301642: Remove lock from ConstString::GetLength (authored by
labath).
Changed prior to commit:
https://reviews.llvm.org/D32306?vs=96841&id=97083#toc
Repository:
rL LLVM
https://reviews.llvm.org/D32306
Files:
lldb/trunk/source/Utility/ConstString.cpp
Index: lldb/trunk/source/Utility/ConstString.cpp
===
--- lldb/trunk/source/Utility/ConstString.cpp
+++ lldb/trunk/source/Utility/ConstString.cpp
@@ -38,14 +38,13 @@
static StringPoolEntryType &
GetStringMapEntryFromKeyData(const char *keyData) {
-char *ptr = const_cast(keyData) - sizeof(StringPoolEntryType);
-return *reinterpret_cast(ptr);
+return StringPoolEntryType::GetStringMapEntryFromKeyData(keyData);
}
- size_t GetConstCStringLength(const char *ccstr) const {
+ static size_t GetConstCStringLength(const char *ccstr) {
if (ccstr != nullptr) {
- const uint8_t h = hash(llvm::StringRef(ccstr));
- llvm::sys::SmartScopedReader rlock(m_string_pools[h].m_mutex);
+ // Since the entry is read only, and we derive the entry entirely from
the
+ // pointer, we don't need the lock.
const StringPoolEntryType &entry = GetStringMapEntryFromKeyData(ccstr);
return entry.getKey().size();
}
@@ -218,10 +217,8 @@
if (m_string == rhs.m_string)
return false;
- llvm::StringRef lhs_string_ref(m_string,
- StringPool().GetConstCStringLength(m_string));
- llvm::StringRef rhs_string_ref(
- rhs.m_string, StringPool().GetConstCStringLength(rhs.m_string));
+ llvm::StringRef lhs_string_ref(GetStringRef());
+ llvm::StringRef rhs_string_ref(rhs.GetStringRef());
// If both have valid C strings, then return the comparison
if (lhs_string_ref.data() && rhs_string_ref.data())
@@ -240,7 +237,7 @@
}
size_t ConstString::GetLength() const {
- return StringPool().GetConstCStringLength(m_string);
+ return Pool::GetConstCStringLength(m_string);
}
bool ConstString::Equals(const ConstString &lhs, const ConstString &rhs,
@@ -255,10 +252,8 @@
return false;
// perform case insensitive equality test
- llvm::StringRef lhs_string_ref(
- lhs.m_string, StringPool().GetConstCStringLength(lhs.m_string));
- llvm::StringRef rhs_string_ref(
- rhs.m_string, StringPool().GetConstCStringLength(rhs.m_string));
+ llvm::StringRef lhs_string_ref(lhs.GetStringRef());
+ llvm::StringRef rhs_string_ref(rhs.GetStringRef());
return lhs_string_ref.equals_lower(rhs_string_ref);
}
@@ -270,10 +265,8 @@
if (lhs_cstr == rhs_cstr)
return 0;
if (lhs_cstr && rhs_cstr) {
-llvm::StringRef lhs_string_ref(
-lhs_cstr, StringPool().GetConstCStringLength(lhs_cstr));
-llvm::StringRef rhs_string_ref(
-rhs_cstr, StringPool().GetConstCStringLength(rhs_cstr));
+llvm::StringRef lhs_string_ref(lhs.GetStringRef());
+llvm::StringRef rhs_string_ref(rhs.GetStringRef());
if (case_sensitive) {
return lhs_string_ref.compare(rhs_string_ref);
Index: lldb/trunk/source/Utility/ConstString.cpp
===
--- lldb/trunk/source/Utility/ConstString.cpp
+++ lldb/trunk/source/Utility/ConstString.cpp
@@ -38,14 +38,13 @@
static StringPoolEntryType &
GetStringMapEntryFromKeyData(const char *keyData) {
-char *ptr = const_cast(keyData) - sizeof(StringPoolEntryType);
-return *reinterpret_cast(ptr);
+return StringPoolEntryType::GetStringMapEntryFromKeyData(keyData);
}
- size_t GetConstCStringLength(const char *ccstr) const {
+ static size_t GetConstCStringLength(const char *ccstr) {
if (ccstr != nullptr) {
- const uint8_t h = hash(llvm::StringRef(ccstr));
- llvm::sys::SmartScopedReader rlock(m_string_pools[h].m_mutex);
+ // Since the entry is read only, and we derive the entry entirely from the
+ // pointer, we don't need the lock.
const StringPoolEntryType &entry = GetStringMapEntryFromKeyData(ccstr);
return entry.getKey().size();
}
@@ -218,10 +217,8 @@
if (m_string == rhs.m_string)
return false;
- llvm::StringRef lhs_string_ref(m_string,
- StringPool().GetConstCStringLength(m_string));
- llvm::StringRef rhs_string_ref(
- rhs.m_string, StringPool().GetConstCStringLength(rhs.m_string));
+ llvm::StringRef lhs_string_ref(GetStringRef());
+ llvm::StringRef rhs_string_ref(rhs.GetStringRef());
// If both have valid C strings, then return the comparison
if (lhs_string_ref.data() && rhs_string_ref.data())
@@ -240,7 +237,7 @@
}
size_t ConstString::GetLength() const {
- return StringPool().GetConstCStringLength(m_string);
+ return Pool::GetConstCStringLength(m_string);
}
bool ConstString::Equals(const ConstString &lhs, const ConstString &rhs,
@@ -255,10 +252,8 @@
return false;
// perform case
[Lldb-commits] [lldb] r301642 - Remove lock from ConstString::GetLength
Author: labath
Date: Fri Apr 28 07:08:28 2017
New Revision: 301642
URL: http://llvm.org/viewvc/llvm-project?rev=301642&view=rev
Log:
Remove lock from ConstString::GetLength
Summary: ConstStrings are immutable, so there is no need to grab even a reader
lock in order to read the length field.
Reviewers: #lldb, labath
Reviewed By: labath
Subscribers: zturner, labath, lldb-commits
Differential Revision: https://reviews.llvm.org/D32306
Patch by Scott Smith
Modified:
lldb/trunk/source/Utility/ConstString.cpp
Modified: lldb/trunk/source/Utility/ConstString.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/ConstString.cpp?rev=301642&r1=301641&r2=301642&view=diff
==
--- lldb/trunk/source/Utility/ConstString.cpp (original)
+++ lldb/trunk/source/Utility/ConstString.cpp Fri Apr 28 07:08:28 2017
@@ -38,14 +38,13 @@ public:
static StringPoolEntryType &
GetStringMapEntryFromKeyData(const char *keyData) {
-char *ptr = const_cast(keyData) - sizeof(StringPoolEntryType);
-return *reinterpret_cast(ptr);
+return StringPoolEntryType::GetStringMapEntryFromKeyData(keyData);
}
- size_t GetConstCStringLength(const char *ccstr) const {
+ static size_t GetConstCStringLength(const char *ccstr) {
if (ccstr != nullptr) {
- const uint8_t h = hash(llvm::StringRef(ccstr));
- llvm::sys::SmartScopedReader rlock(m_string_pools[h].m_mutex);
+ // Since the entry is read only, and we derive the entry entirely from
the
+ // pointer, we don't need the lock.
const StringPoolEntryType &entry = GetStringMapEntryFromKeyData(ccstr);
return entry.getKey().size();
}
@@ -218,10 +217,8 @@ bool ConstString::operator<(const ConstS
if (m_string == rhs.m_string)
return false;
- llvm::StringRef lhs_string_ref(m_string,
- StringPool().GetConstCStringLength(m_string));
- llvm::StringRef rhs_string_ref(
- rhs.m_string, StringPool().GetConstCStringLength(rhs.m_string));
+ llvm::StringRef lhs_string_ref(GetStringRef());
+ llvm::StringRef rhs_string_ref(rhs.GetStringRef());
// If both have valid C strings, then return the comparison
if (lhs_string_ref.data() && rhs_string_ref.data())
@@ -240,7 +237,7 @@ Stream &lldb_private::operator<<(Stream
}
size_t ConstString::GetLength() const {
- return StringPool().GetConstCStringLength(m_string);
+ return Pool::GetConstCStringLength(m_string);
}
bool ConstString::Equals(const ConstString &lhs, const ConstString &rhs,
@@ -255,10 +252,8 @@ bool ConstString::Equals(const ConstStri
return false;
// perform case insensitive equality test
- llvm::StringRef lhs_string_ref(
- lhs.m_string, StringPool().GetConstCStringLength(lhs.m_string));
- llvm::StringRef rhs_string_ref(
- rhs.m_string, StringPool().GetConstCStringLength(rhs.m_string));
+ llvm::StringRef lhs_string_ref(lhs.GetStringRef());
+ llvm::StringRef rhs_string_ref(rhs.GetStringRef());
return lhs_string_ref.equals_lower(rhs_string_ref);
}
@@ -270,10 +265,8 @@ int ConstString::Compare(const ConstStri
if (lhs_cstr == rhs_cstr)
return 0;
if (lhs_cstr && rhs_cstr) {
-llvm::StringRef lhs_string_ref(
-lhs_cstr, StringPool().GetConstCStringLength(lhs_cstr));
-llvm::StringRef rhs_string_ref(
-rhs_cstr, StringPool().GetConstCStringLength(rhs_cstr));
+llvm::StringRef lhs_string_ref(lhs.GetStringRef());
+llvm::StringRef rhs_string_ref(rhs.GetStringRef());
if (case_sensitive) {
return lhs_string_ref.compare(rhs_string_ref);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32306: Remove lock from ConstString::GetLength
labath added inline comments. Comment at: source/Utility/ConstString.cpp:49 + // pointer, we don't need the lock. const StringPoolEntryType &entry = GetStringMapEntryFromKeyData(ccstr); return entry.getKey().size(); scott.smith wrote: > labath wrote: > > labath wrote: > > > scott.smith wrote: > > > > zturner wrote: > > > > > Why do we even have this function which digs into the `StringMap` > > > > > internals rather than just calling existing `StringMap` member > > > > > functions? Can Can we just delete `GetStringMapEntryFromKeyData` > > > > > entirely and use `StringMap::find`? > > > > Probably performance. If we have to call Find, then we have to call > > > > hash, fault in the appropriate bucket, and then finally return the > > > > entry that we already have in hand. Plus we'd need the lock. > > > > > > > > Can we just delete GetStringMapEntryFromKeyData entirely and use > > > > StringMap::find? > > > Unfortunately, I don't think that's possible. `StringMap::find` expects a > > > StringRef. In order to construct that, we need to know the length of the > > > string, and we're back where we started :( > > > > > > In reality, this is doing a very different operation than find (which > > > takes a random string and checks whether it's in the map) -- this takes a > > > string which we know to be in the map and get its size. > > > > > > It will take some rearchitecting of the ConstString class to get rid of > > > this hack. Probably it could be fixed by ConstString storing a > > > StringMap::iterator instead of the raw pointer. In any case, that seems > > > out of scope of this change. > > Cool, I didn't notice that one when looking for it. I guess at this point > > we can just delete our copy of `GetStringMapEntryFromKeyData` completely > > and call the StringPool's version instead. > It will push a lot of lines past the 80 char limit. Do you want me to make > that change? If not, can you submit this one since I do not have commit > access? Thanks! Ok, nevermind, let's leave that as is for now. thanks for the patch. Repository: rL LLVM https://reviews.llvm.org/D32306 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32585: Implementation of remote packets for Trace data.
clayborg added inline comments.
Comment at: docs/lldb-gdb-remote.txt:212
//--
+// QTrace:1:type:;
+//
labath wrote:
> ravitheja wrote:
> > clayborg wrote:
> > > Should we make all these new packets JSON based to start with? "jTrace"?
> > > If we have any need for JSON at all in this or the other new packets I
> > > would say lets just go with JSON packets. They are currently prefixed
> > > with "j". If we go this route we should specify the mandatory key/value
> > > pairs in the header doc. We should also allow a JSON dictionary from the
> > > trace config up at the SBTrace layer to make it into this packet?
> > I am fine prefixing the packets with "j", I did not understand what you
> > meant by the last sentence. At the moment I am waiting for Pavel or Tamas
> > to reply or anyone else and based on the complete feedback, I will upload a
> > new patch if needed.
> I think Greg meant you should also change the packet format to take a json
> argument instead of key-value pairs (The packet should be JTrace if it
> modifies the state, right ?).
>
> If there is a gdb equivalent which matches this functionality sufficiently
> closely, then I would prefer to stick to that. However, given that we already
> are attaching a json field to the packet, I think it would make sense to go
> json all the way.
I think the entire packet should be:
```
$jTrace:
```
where is a JSON dictionary. Ravitheja had talked before about adding an
implementation specific JSON dictionary into the JSON that comes in to
configure the trace at the lldb::SB layer. I was just saying it would be nice
if this implementation specific JSON was sent down as part of this packet so
the GDB server can use it. Maybe we just send the entire thing that came in at
the SB layer?
Comment at: docs/lldb-gdb-remote.txt:239
+//
+// Each tracing instance is identified by a user id which is returned
+// as the reply to this packet. In case the tracing failed to begin an
labath wrote:
> can we call this "trace id"? I'd like to avoid people confusing this with
> *real* UIDs.
trace id would be nice
Comment at: docs/lldb-gdb-remote.txt:269
+//
+// An empty response is sent in case of success else an error code is
+// returned.
labath wrote:
> Empty response means "packet not supported". You should send OK in this case.
Agreed
Comment at: include/lldb/Host/common/NativeProcessProtocol.h:396
+ virtual size_t GetTraceData(lldb::user_id_t uid, lldb::tid_t thread,
+ Error &error, void *buf, size_t buf_size,
+ size_t offset = 0) {
labath wrote:
> llvm::MutableArrayRef instead of buf+size pair. Maybe we could even
> pass in a reference to the MutableArrayRef so that the function can truncate
> it to the actual size?
This is ok for internal functions. If we use a reference to a
llvm::MutableArrayRef<> then we can return Error as the in/out mutable array
ref reference can be updated with the right size right?
https://reviews.llvm.org/D32585
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32585: Implementation of remote packets for Trace data.
zturner added inline comments.
Comment at: docs/lldb-gdb-remote.txt:214
+//
+// BRIEF
+// Packet for starting tracing of type lldb::TraceType. The following
I just noticed that none of our documentation uses doxygen? Weird.
Comment at: include/lldb/Host/common/NativeProcessProtocol.h:396
+ virtual size_t GetTraceData(lldb::user_id_t uid, lldb::tid_t thread,
+ Error &error, void *buf, size_t buf_size,
+ size_t offset = 0) {
clayborg wrote:
> labath wrote:
> > llvm::MutableArrayRef instead of buf+size pair. Maybe we could
> > even pass in a reference to the MutableArrayRef so that the function can
> > truncate it to the actual size?
> This is ok for internal functions. If we use a reference to a
> llvm::MutableArrayRef<> then we can return Error as the in/out mutable array
> ref reference can be updated with the right size right?
Agreed.
```
virtual Error GetTraceData(user_id_t uid, tid_t thread,
MutableArrayRef &Buffer, size_t offset=0);
```
Comment at: include/lldb/Host/common/NativeProcessProtocol.h:429-430
+ //--
+ virtual void GetTraceConfig(lldb::user_id_t uid, lldb::tid_t threadid,
+ Error &error, TraceOptions &config) {
+error.SetErrorString("Not implemented");
`virtual Error GetTraceConfig(user_id_t uid, tid_ threadid, TraceOptions
&config)`
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1110-
+
+ if (log)
+log->Printf("GDBRemoteCommunicationServerLLGS::%s called", __FUNCTION__);
+
Can you use `LLDB_LOG()` macros here (and everywhere else in this patch)?
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1113-1116
+ if (packet.GetStringRef().find("QTrace:1:type:") != 0)
+return SendIllFormedResponse(packet, "QTrace:1: Ill formed packet ");
+
+ packet.SetFilePos(strlen("QTrace:1:type:"));
Can you add a function to `StringExtractor` that looks like this:
```
bool consume_front(StringRef Str) {
StringRef S = GetStringRef();
if (!S.starts_with(Str))
return false;
m_index += Str.size();
return true;
}
```
Then you can change these 3 lines to:
```
if (!packet.consume_front("QTrace:1:type:"))
return SendIllFormedResponse(packet, "QTrace:1: Ill formed packet ");
```
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1138
+
+ StructuredData::DictionarySP custom_params(new StructuredData::Dictionary());
+ llvm::StringRef name, value;
Please use `auto custom_params =
llvm::make_shared();`
as it is more efficient from a memory standpoint.
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1161
+ if (buffersize == std::numeric_limits::max() ||
+ packet.GetBytesLeft() != 0) {
+
Can you use `!packet.empty()`
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1201-1203
+ if (packet.GetStringRef().find("qTrace:conf:read:userid:") != 0)
+return SendIllFormedResponse(packet, "qTrace: Ill formed packet ");
+ packet.SetFilePos(strlen("qTrace:conf:read:userid:"));
Use the `consume_front()` function I proposed earlier.
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1285-1287
+ if (packet.GetStringRef().find("QTrace:0:userid:") != 0)
+return SendIllFormedResponse(packet, "QTrace:0: Ill formed packet ");
+ packet.SetFilePos(strlen("QTrace:0:userid:"));
`consume_front()`
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1344-1352
+ if (packet.GetStringRef().find("qTrace:buffer:read:userid:") == 0) {
+tracetype = BufferData;
+packet.SetFilePos(strlen("qTrace:buffer:read:userid:"));
+ } else if (packet.GetStringRef().find("qTrace:meta:read:userid:") == 0) {
+tracetype = MetaData;
+packet.SetFilePos(strlen("qTrace:meta:read:userid:"));
+ } else {
`consume_front`
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1391
+
+ if (error_encountered || packet.GetBytesLeft() != 0 ||
+ byte_count == std::numeric_limits::max() ||
`!packet.empty()`
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1410-1411
+ if (tracetype == BufferData)
+filled_data = m_debugged_process_sp->GetTraceData(
+uid, tid, error, buf.data(), byte_count, offset);
+ else if (tracetype == MetaData)
If you make the change suggested earlier to convert this function to use array
ref,
[Lldb-commits] [lldb] r301664 - Add remaining SBTrace headers to LLDB framework
Author: penryu
Date: Fri Apr 28 13:10:53 2017
New Revision: 301664
URL: http://llvm.org/viewvc/llvm-project?rev=301664&view=rev
Log:
Add remaining SBTrace headers to LLDB framework
Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj
Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=301664&r1=301663&r2=301664&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Fri Apr 28 13:10:53 2017
@@ -877,6 +877,8 @@
9A357673116E7B6400E8ED2F /* SBStringList.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = 9A357672116E7B6400E8ED2F /* SBStringList.cpp */;
};
9A3576A8116E9AB700E8ED2F /* SBHostOS.h in Headers */ = {isa =
PBXBuildFile; fileRef = 9A3576A7116E9AB700E8ED2F /* SBHostOS.h */; settings =
{ATTRIBUTES = (Public, ); }; };
9A3576AA116E9AC700E8ED2F /* SBHostOS.cpp in Sources */ = {isa =
PBXBuildFile; fileRef = 9A3576A9116E9AC700E8ED2F /* SBHostOS.cpp */; };
+ 9A36D24D1EB3BE7F00AAD9EA /* SBTrace.h in Headers */ = {isa =
PBXBuildFile; fileRef = 9A1E59581EB2B10D002206A5 /* SBTrace.h */; };
+ 9A36D24E1EB3BE7F00AAD9EA /* SBTraceOptions.h in Headers */ =
{isa = PBXBuildFile; fileRef = 9A1E59591EB2B10D002206A5 /* SBTraceOptions.h */;
};
9A4F35101368A51A00823F52 /* StreamAsynchronousIO.cpp in Sources
*/ = {isa = PBXBuildFile; fileRef = 9A4F350F1368A51A00823F52 /*
StreamAsynchronousIO.cpp */; };
9A77AD541E64E2760025CE04 /* RegisterInfoPOSIX_arm.cpp in
Sources */ = {isa = PBXBuildFile; fileRef = 9A77AD501E64E24E0025CE04 /*
RegisterInfoPOSIX_arm.cpp */; };
9AC7038E117674FB0086C050 /* SBInstruction.h in Headers */ =
{isa = PBXBuildFile; fileRef = 9AC7038D117674EB0086C050 /* SBInstruction.h */;
settings = {ATTRIBUTES = (Public, ); }; };
@@ -6429,6 +6431,7 @@
254FBBA31A9166F100BD6378 /* SBAttachInfo.h in
Headers */,
26680221115FD13D008E1FE4 /* SBDefines.h in
Headers */,
8CCB018219BA4E270009FD44 /*
SBThreadCollection.h in Headers */,
+ 9A36D24D1EB3BE7F00AAD9EA /* SBTrace.h in
Headers */,
AF0EBBEC185941360059E52F /* SBQueue.h in
Headers */,
26680222115FD13D008E1FE4 /* SBError.h in
Headers */,
26680223115FD13D008E1FE4 /* SBEvent.h in
Headers */,
@@ -6467,6 +6470,7 @@
941BCC8014E48C4000BB969C /* SBTypeFormat.h in
Headers */,
9475C18F14E5F858001BFC6D /*
SBTypeNameSpecifier.h in Headers */,
941BCC8114E48C4000BB969C /* SBTypeSummary.h in
Headers */,
+ 9A36D24E1EB3BE7F00AAD9EA /* SBTraceOptions.h in
Headers */,
23059A121958B3B2007B8189 /* SBUnixSignals.h in
Headers */,
941BCC8214E48C4000BB969C /* SBTypeSynthetic.h
in Headers */,
9A19A6AF1163BBB200E0D453 /* SBValue.h in
Headers */,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D25886: [Test Suite] Properly respect --framework option
fjricci added inline comments.
Comment at: lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py:1418
"include")),
-'LD_EXTRAS': "-L%s -llldb" % lib_dir}
+'LD_EXTRAS': "-L%s/../lib -llldb -Wl,-rpath,%s/../lib" %
(lib_dir, lib_dir)}
elif sys.platform.startswith('win'):
Why do we need to make this `-L$(lib_dir)/../lib` instead of the original
`-L$(lib_dir)`? This breaks cases where `lib_dir` is `lib64` and not `lib`
Repository:
rL LLVM
https://reviews.llvm.org/D25886
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32149: Correct handling NetBSD core(5) files with threads
krytarowski updated this revision to Diff 97149.
krytarowski added a comment.
Strip non-functional style improvements in unrelated code-parts.
Apply changes to address comments from Joerg.
Repository:
rL LLVM
https://reviews.llvm.org/D32149
Files:
source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
source/Plugins/Process/elf-core/ProcessElfCore.cpp
source/Plugins/Process/elf-core/ProcessElfCore.h
Index: source/Plugins/Process/elf-core/ProcessElfCore.h
===
--- source/Plugins/Process/elf-core/ProcessElfCore.h
+++ source/Plugins/Process/elf-core/ProcessElfCore.h
@@ -125,6 +125,18 @@
lldb_private::ConstString path;
};
+ // Parse thread(s) data structuresNetBSD(prstatus, prpsinfo) from given NOTE
+ // segment
+ lldb_private::Error ParseThreadContextsFromNoteSegmentNetBSD(
+ const elf::ELFProgramHeader *segment_header,
+ lldb_private::DataExtractor segment_data);
+
+ // Parse thread(s) data structuresGeneric(prstatus, prpsinfo) from given NOTE
+ // segment
+ lldb_private::Error ParseThreadContextsFromNoteSegmentGeneric(
+ const elf::ELFProgramHeader *segment_header,
+ lldb_private::DataExtractor segment_data);
+
//--
// For ProcessElfCore only
//--
Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -27,6 +27,7 @@
#include "lldb/Utility/DataBufferLLVM.h"
#include "lldb/Utility/Log.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/ELF.h"
#include "llvm/Support/Threading.h"
@@ -458,9 +459,42 @@
namespace NETBSD {
-enum { NT_PROCINFO = 1, NT_AUXV, NT_AMD64_REGS = 33, NT_AMD64_FPREGS = 35 };
+enum { NT_PROCINFO = 1, NT_AUXV = 2 };
+
+/* Size in bytes */
+enum { NT_PROCINFO_SIZE = 160 };
+
+/* Size in bytes */
+enum {
+ NT_PROCINFO_CPI_VERSION_SIZE = 4,
+ NT_PROCINFO_CPI_CPISIZE_SIZE = 4,
+ NT_PROCINFO_CPI_SIGNO_SIZE = 4,
+ NT_PROCINFO_CPI_SIGCODE_SIZE = 4,
+ NT_PROCINFO_CPI_SIGPEND_SIZE = 16,
+ NT_PROCINFO_CPI_SIGMASK_SIZE = 16,
+ NT_PROCINFO_CPI_SIGIGNORE_SIZE = 16,
+ NT_PROCINFO_CPI_SIGCATCH_SIZE = 16,
+ NT_PROCINFO_CPI_PID_SIZE = 4,
+ NT_PROCINFO_CPI_PPID_SIZE = 4,
+ NT_PROCINFO_CPI_PGRP_SIZE = 4,
+ NT_PROCINFO_CPI_SID_SIZE = 4,
+ NT_PROCINFO_CPI_RUID_SIZE = 4,
+ NT_PROCINFO_CPI_EUID_SIZE = 4,
+ NT_PROCINFO_CPI_SVUID_SIZE = 4,
+ NT_PROCINFO_CPI_RGID_SIZE = 4,
+ NT_PROCINFO_CPI_EGID_SIZE = 4,
+ NT_PROCINFO_CPI_SVGID_SIZE = 4,
+ NT_PROCINFO_CPI_NLWPS_SIZE = 4,
+ NT_PROCINFO_CPI_NAME_SIZE = 32,
+ NT_PROCINFO_CPI_SIGLWP_SIZE = 4,
+};
+
+namespace AMD64 {
+enum { NT_REGS = 33, NT_FPREGS = 35 };
}
+} // namespace NETBSD
+
// Parse a FreeBSD NT_PRSTATUS note - see FreeBSD sys/procfs.h for details.
static void ParseFreeBSDPrStatus(ThreadData &thread_data, DataExtractor &data,
ArchSpec &arch) {
@@ -497,15 +531,43 @@
thread_data.name = data.GetCStr(&offset, 20);
}
-static void ParseNetBSDProcInfo(ThreadData &thread_data, DataExtractor &data) {
+static Error ParseNetBSDProcInfo(DataExtractor &data, uint32_t &cpi_nlwps,
+ uint32_t &cpi_signo, uint32_t &cpi_siglwp) {
lldb::offset_t offset = 0;
- int version = data.GetU32(&offset);
+ uint32_t version = data.GetU32(&offset);
if (version != 1)
-return;
+return Error(
+"Error parsing NetBSD core(5) notes: Unsupported procinfo version");
+
+ uint32_t cpisize = data.GetU32(&offset);
+ if (cpisize != NETBSD::NT_PROCINFO_SIZE)
+return Error(
+"Error parsing NetBSD core(5) notes: Unsupported procinfo size");
+
+ cpi_signo = data.GetU32(&offset); /* killing signal */
+
+ offset += NETBSD::NT_PROCINFO_CPI_SIGCODE_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_SIGPEND_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_SIGMASK_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_SIGIGNORE_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_SIGCATCH_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_PID_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_PPID_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_PGRP_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_SID_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_RUID_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_EUID_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_SVUID_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_RGID_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_EGID_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_SVGID_SIZE;
+ cpi_nlwps = data.GetU32(&offset); /* number of LWPs */
+
+ offset += NETBSD::NT_PROCINFO_CPI_NAME_SIZE;
+ cpi_siglwp = data.GetU32(&offset); /* LWP target of killing signal */
- offset += 4;
- thread_data.signo = data.GetU32(&offset);
+ return Error();
}
static void ParseOpenBSDProcInfo(ThreadData &
[Lldb-commits] [PATCH] D32149: Correct handling NetBSD core(5) files with threads
krytarowski updated this revision to Diff 97150.
krytarowski added a comment.
Remove another unrelated style improvement.
Repository:
rL LLVM
https://reviews.llvm.org/D32149
Files:
source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
source/Plugins/Process/elf-core/ProcessElfCore.cpp
source/Plugins/Process/elf-core/ProcessElfCore.h
Index: source/Plugins/Process/elf-core/ProcessElfCore.h
===
--- source/Plugins/Process/elf-core/ProcessElfCore.h
+++ source/Plugins/Process/elf-core/ProcessElfCore.h
@@ -125,6 +125,18 @@
lldb_private::ConstString path;
};
+ // Parse thread(s) data structuresNetBSD(prstatus, prpsinfo) from given NOTE
+ // segment
+ lldb_private::Error ParseThreadContextsFromNoteSegmentNetBSD(
+ const elf::ELFProgramHeader *segment_header,
+ lldb_private::DataExtractor segment_data);
+
+ // Parse thread(s) data structuresGeneric(prstatus, prpsinfo) from given NOTE
+ // segment
+ lldb_private::Error ParseThreadContextsFromNoteSegmentGeneric(
+ const elf::ELFProgramHeader *segment_header,
+ lldb_private::DataExtractor segment_data);
+
//--
// For ProcessElfCore only
//--
Index: source/Plugins/Process/elf-core/ProcessElfCore.cpp
===
--- source/Plugins/Process/elf-core/ProcessElfCore.cpp
+++ source/Plugins/Process/elf-core/ProcessElfCore.cpp
@@ -27,6 +27,7 @@
#include "lldb/Utility/DataBufferLLVM.h"
#include "lldb/Utility/Log.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/ELF.h"
#include "llvm/Support/Threading.h"
@@ -458,9 +459,42 @@
namespace NETBSD {
-enum { NT_PROCINFO = 1, NT_AUXV, NT_AMD64_REGS = 33, NT_AMD64_FPREGS = 35 };
+enum { NT_PROCINFO = 1, NT_AUXV = 2 };
+
+/* Size in bytes */
+enum { NT_PROCINFO_SIZE = 160 };
+
+/* Size in bytes */
+enum {
+ NT_PROCINFO_CPI_VERSION_SIZE = 4,
+ NT_PROCINFO_CPI_CPISIZE_SIZE = 4,
+ NT_PROCINFO_CPI_SIGNO_SIZE = 4,
+ NT_PROCINFO_CPI_SIGCODE_SIZE = 4,
+ NT_PROCINFO_CPI_SIGPEND_SIZE = 16,
+ NT_PROCINFO_CPI_SIGMASK_SIZE = 16,
+ NT_PROCINFO_CPI_SIGIGNORE_SIZE = 16,
+ NT_PROCINFO_CPI_SIGCATCH_SIZE = 16,
+ NT_PROCINFO_CPI_PID_SIZE = 4,
+ NT_PROCINFO_CPI_PPID_SIZE = 4,
+ NT_PROCINFO_CPI_PGRP_SIZE = 4,
+ NT_PROCINFO_CPI_SID_SIZE = 4,
+ NT_PROCINFO_CPI_RUID_SIZE = 4,
+ NT_PROCINFO_CPI_EUID_SIZE = 4,
+ NT_PROCINFO_CPI_SVUID_SIZE = 4,
+ NT_PROCINFO_CPI_RGID_SIZE = 4,
+ NT_PROCINFO_CPI_EGID_SIZE = 4,
+ NT_PROCINFO_CPI_SVGID_SIZE = 4,
+ NT_PROCINFO_CPI_NLWPS_SIZE = 4,
+ NT_PROCINFO_CPI_NAME_SIZE = 32,
+ NT_PROCINFO_CPI_SIGLWP_SIZE = 4,
+};
+
+namespace AMD64 {
+enum { NT_REGS = 33, NT_FPREGS = 35 };
}
+} // namespace NETBSD
+
// Parse a FreeBSD NT_PRSTATUS note - see FreeBSD sys/procfs.h for details.
static void ParseFreeBSDPrStatus(ThreadData &thread_data, DataExtractor &data,
ArchSpec &arch) {
@@ -497,15 +531,43 @@
thread_data.name = data.GetCStr(&offset, 20);
}
-static void ParseNetBSDProcInfo(ThreadData &thread_data, DataExtractor &data) {
+static Error ParseNetBSDProcInfo(DataExtractor &data, uint32_t &cpi_nlwps,
+ uint32_t &cpi_signo, uint32_t &cpi_siglwp) {
lldb::offset_t offset = 0;
- int version = data.GetU32(&offset);
+ uint32_t version = data.GetU32(&offset);
if (version != 1)
-return;
+return Error(
+"Error parsing NetBSD core(5) notes: Unsupported procinfo version");
+
+ uint32_t cpisize = data.GetU32(&offset);
+ if (cpisize != NETBSD::NT_PROCINFO_SIZE)
+return Error(
+"Error parsing NetBSD core(5) notes: Unsupported procinfo size");
+
+ cpi_signo = data.GetU32(&offset); /* killing signal */
+
+ offset += NETBSD::NT_PROCINFO_CPI_SIGCODE_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_SIGPEND_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_SIGMASK_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_SIGIGNORE_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_SIGCATCH_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_PID_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_PPID_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_PGRP_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_SID_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_RUID_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_EUID_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_SVUID_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_RGID_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_EGID_SIZE;
+ offset += NETBSD::NT_PROCINFO_CPI_SVGID_SIZE;
+ cpi_nlwps = data.GetU32(&offset); /* number of LWPs */
+
+ offset += NETBSD::NT_PROCINFO_CPI_NAME_SIZE;
+ cpi_siglwp = data.GetU32(&offset); /* LWP target of killing signal */
- offset += 4;
- thread_data.signo = data.GetU32(&offset);
+ return Error();
}
static void ParseOpenBSDProcInfo(ThreadData &thread_data, DataExtractor &data) {
@@ -524,12 +586,28 @@
/// 1) A
[Lldb-commits] [PATCH] D32548: Xfail data formatter test cases
penryu abandoned this revision. penryu added a comment. Lang's change in https://reviews.llvm.org/D32554 makes these unnecessary. Thanks, Lang! https://reviews.llvm.org/D32548 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r301686 - Public headers need to be public.
Author: penryu
Date: Fri Apr 28 16:03:18 2017
New Revision: 301686
URL: http://llvm.org/viewvc/llvm-project?rev=301686&view=rev
Log:
Public headers need to be public.
Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj
Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=301686&r1=301685&r2=301686&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Fri Apr 28 16:03:18 2017
@@ -877,8 +877,8 @@
9A357673116E7B6400E8ED2F /* SBStringList.cpp in Sources */ =
{isa = PBXBuildFile; fileRef = 9A357672116E7B6400E8ED2F /* SBStringList.cpp */;
};
9A3576A8116E9AB700E8ED2F /* SBHostOS.h in Headers */ = {isa =
PBXBuildFile; fileRef = 9A3576A7116E9AB700E8ED2F /* SBHostOS.h */; settings =
{ATTRIBUTES = (Public, ); }; };
9A3576AA116E9AC700E8ED2F /* SBHostOS.cpp in Sources */ = {isa =
PBXBuildFile; fileRef = 9A3576A9116E9AC700E8ED2F /* SBHostOS.cpp */; };
- 9A36D24D1EB3BE7F00AAD9EA /* SBTrace.h in Headers */ = {isa =
PBXBuildFile; fileRef = 9A1E59581EB2B10D002206A5 /* SBTrace.h */; };
- 9A36D24E1EB3BE7F00AAD9EA /* SBTraceOptions.h in Headers */ =
{isa = PBXBuildFile; fileRef = 9A1E59591EB2B10D002206A5 /* SBTraceOptions.h */;
};
+ 9A36D24D1EB3BE7F00AAD9EA /* SBTrace.h in Headers */ = {isa =
PBXBuildFile; fileRef = 9A1E59581EB2B10D002206A5 /* SBTrace.h */; settings =
{ATTRIBUTES = (Public, ); }; };
+ 9A36D24E1EB3BE7F00AAD9EA /* SBTraceOptions.h in Headers */ =
{isa = PBXBuildFile; fileRef = 9A1E59591EB2B10D002206A5 /* SBTraceOptions.h */;
settings = {ATTRIBUTES = (Public, ); }; };
9A4F35101368A51A00823F52 /* StreamAsynchronousIO.cpp in Sources
*/ = {isa = PBXBuildFile; fileRef = 9A4F350F1368A51A00823F52 /*
StreamAsynchronousIO.cpp */; };
9A77AD541E64E2760025CE04 /* RegisterInfoPOSIX_arm.cpp in
Sources */ = {isa = PBXBuildFile; fileRef = 9A77AD501E64E24E0025CE04 /*
RegisterInfoPOSIX_arm.cpp */; };
9AC7038E117674FB0086C050 /* SBInstruction.h in Headers */ =
{isa = PBXBuildFile; fileRef = 9AC7038D117674EB0086C050 /* SBInstruction.h */;
settings = {ATTRIBUTES = (Public, ); }; };
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
