[Lldb-commits] [lldb] r307870 - The llvm.org bugzilla moved.

2017-07-12 Thread Jim Ingham via lldb-commits 
Author: jingham
Date: Wed Jul 12 17:36:21 2017
New Revision: 307870

URL: http://llvm.org/viewvc/llvm-project?rev=307870=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=307869=307870=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] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

2017-07-12 Thread Jim Ingham via Phabricator via lldb-commits 
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] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

2017-07-12 Thread Jim Ingham via Phabricator via lldb-commits 
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

2017-07-12 Thread Greg Clayton via Phabricator via lldb-commits 
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

2017-07-12 Thread Pavel Labath via Phabricator via lldb-commits 
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] D35065: [LLDB][ppc64le] Rename enums in AuxVector

2017-07-12 Thread Bruno Rosa via Phabricator via lldb-commits 
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] D33213: Use socketpair on all Unix platforms

2017-07-12 Thread Greg Clayton via Phabricator via lldb-commits 
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] D31172: Move stop info override callback code from ArchSpec into Process

2017-07-12 Thread Jim Ingham via Phabricator via lldb-commits 
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

2017-07-12 Thread Pavel Labath via Phabricator via lldb-commits 
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] D33213: Use socketpair on all Unix platforms

2017-07-12 Thread Greg Clayton via Phabricator via lldb-commits 
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] D35311: lldb-server tests: Add support for testing debugserver

2017-07-12 Thread Pavel Labath via Phabricator via lldb-commits 
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 _name, const std::string _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 _name,
const std::string _case_name)
 : m_test_name(test_name), m_test_case_name(test_case_name),
@@ -39,13 +49,16 @@
   const ArchSpec _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;

[Lldb-commits] [PATCH] D34945: Adding Support for Error Strings in Remote Packets

2017-07-12 Thread Greg Clayton via Phabricator via lldb-commits 
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] D31172: Move stop info override callback code from ArchSpec into Process

2017-07-12 Thread Greg Clayton via Phabricator via lldb-commits 
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] D35305: Remove uint32_t assignment operator from Status

2017-07-12 Thread Pavel Labath via Phabricator via lldb-commits 
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 ::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 =(const Status );
 
-  //--
-  /// 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 =(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 ::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 =(const Status );
 
-  //--
-  /// 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 =(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] D35298: [MainLoop] Fix possible use of an invalid iterator

2017-07-12 Thread Phabricator via Phabricator via lldb-commits 
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=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  : loop.m_read_fds) {
-if (!FD_ISSET(fd.first, _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  : loop.m_read_fds)
+if (FD_ISSET(fd.first, _fd_set))
+  fds.push_back(fd.first);
+
+  for (const auto  : fds) {
 #else
   for (const auto  : read_fds) {
 if ((fd.revents & POLLIN) == 0)
@@ -209,13 +215,16 @@
 loop.ProcessReadObject(handle);
   }
 
-  for (const auto  : loop.m_signals) {
+  std::vector signals;
+  for (const auto  : loop.m_signals)
+if (g_signal_flags[entry.first] != 0)
+  signals.push_back(entry.first);
+
+  for (const auto  : 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  : loop.m_read_fds) {
-if (!FD_ISSET(fd.first, _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  : loop.m_read_fds)
+if (FD_ISSET(fd.first, _fd_set))
+  fds.push_back(fd.first);
+
+  for (const auto  : fds) {
 #else
   for (const auto  : read_fds) {
 if ((fd.revents & POLLIN) == 0)
@@ -209,13 +215,16 @@
 loop.ProcessReadObject(handle);
   }
 
-  for (const auto  : loop.m_signals) {
+  std::vector signals;
+  for (const auto  : loop.m_signals)
+if (g_signal_flags[entry.first] != 0)
+  signals.push_back(entry.first);
+
+  for (const auto  : 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] [lldb] r307782 - [MainLoop] Fix possible use of an invalid iterator

2017-07-12 Thread Petr Pavlu via lldb-commits 
Author: petr.pavlu
Date: Wed Jul 12 05:38:31 2017
New Revision: 307782

URL: http://llvm.org/viewvc/llvm-project?rev=307782=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=307781=307782=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  : loop.m_read_fds) {
-if (!FD_ISSET(fd.first, _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  : loop.m_read_fds)
+if (FD_ISSET(fd.first, _fd_set))
+  fds.push_back(fd.first);
+
+  for (const auto  : fds) {
 #else
   for (const auto  : read_fds) {
 if ((fd.revents & POLLIN) == 0)
@@ -209,13 +215,16 @@ void MainLoop::RunImpl::ProcessEvents()
 loop.ProcessReadObject(handle);
   }
 
-  for (const auto  : loop.m_signals) {
+  std::vector signals;
+  for (const auto  : loop.m_signals)
+if (g_signal_flags[entry.first] != 0)
+  signals.push_back(entry.first);
+
+  for (const auto  : 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] [lldb] r307773 - Fixing Android builder

2017-07-12 Thread Ravitheja Addepally via lldb-commits 
Author: ravitheja
Date: Wed Jul 12 04:54:17 2017
New Revision: 307773

URL: http://llvm.org/viewvc/llvm-project?rev=307773=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=307772=307773=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] r307768 - Adding Support for Error Strings in Remote Packets

2017-07-12 Thread Ravitheja Addepally via lldb-commits 
Author: ravitheja
Date: Wed Jul 12 04:15:34 2017
New Revision: 307768

URL: http://llvm.org/viewvc/llvm-project?rev=307768=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=307767=307768=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":paramValue}]}];/E

[Lldb-commits] [PATCH] D35298: [MainLoop] Fix possible use of an invalid iterator

2017-07-12 Thread Pavel Labath via Phabricator via lldb-commits 
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] [PATCH] D31172: Move stop info override callback code from ArchSpec into Process

2017-07-12 Thread Pavel Labath via Phabricator via lldb-commits 
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 @@

[Lldb-commits] [PATCH] D35298: [MainLoop] Fix possible use of an invalid iterator

2017-07-12 Thread Petr Pavlu via Phabricator via lldb-commits 
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  : loop.m_read_fds) {
-if (!FD_ISSET(fd.first, _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  : loop.m_read_fds)
+if (FD_ISSET(fd.first, _fd_set))
+  fds.push_back(fd.first);
+
+  for (const auto  : fds) {
 #else
   for (const auto  : read_fds) {
 if ((fd.revents & POLLIN) == 0)
@@ -209,13 +215,16 @@
 loop.ProcessReadObject(handle);
   }
 
-  for (const auto  : loop.m_signals) {
+  std::vector signals;
+  for (const auto  : loop.m_signals)
+if (g_signal_flags[entry.first] != 0)
+  signals.push_back(entry.first);
+
+  for (const auto  : 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  : loop.m_read_fds) {
-if (!FD_ISSET(fd.first, _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  : loop.m_read_fds)
+if (FD_ISSET(fd.first, _fd_set))
+  fds.push_back(fd.first);
+
+  for (const auto  : fds) {
 #else
   for (const auto  : read_fds) {
 if ((fd.revents & POLLIN) == 0)
@@ -209,13 +215,16 @@
 loop.ProcessReadObject(handle);
   }
 
-  for (const auto  : loop.m_signals) {
+  std::vector signals;
+  for (const auto  : loop.m_signals)
+if (g_signal_flags[entry.first] != 0)
+  signals.push_back(entry.first);
+
+  for (const auto  : 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

2017-07-12 Thread Pavel Labath via Phabricator via lldb-commits 
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