[Lldb-commits] [PATCH] D32585: Implementation of remote packets for Trace data.

[Ravitheja Addepally via Phabricator via lldb-commits]

ravitheja added a comment.

>   TraceOptions opt;
>   opt.setType(...);
>   opt.setBufferSize(...);
>   opt.setMetaBufferSize(); // with an appropriate TraceOptions constructor, 
> this could be a one-liner
> std::future result = std::async(std::launch::async, [&] {
>   return client.StartTrace(opt, error);
> });
>   HandlePacket(server, 
> "JTrace:start:type:dead:buffersize:beef:metabuffersize:baad:threadid:f00d:jparams:...",
>  "47");
>   ASSERT_EQ(47, result.get());
>   ASSERT_TRUE(error.Success());
> 
> 
> This depends on the packet code being in GdbRemoteCommunicationClient and not 
> ProcessGdbRemote as it is now, but moving it there is a good idea anyway -- 
> ProcessGdbRemote should do higher level stuff, packet parsing should be left 
> in the GDBRCClient.
> 
> The server side code unfortunately cannot be tested this way, but I believe I 
> will get around to enabling that soon as well.

@labath Can I do the unit tests in a seperate patch after this patch ? It would 
be easier for me and perhaps also easier to review.


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.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

llvm policy is to commit tests alongside the code under test. I also think it's 
easier to review as you have the code and the test on the same screen.

What's the reason that prevents you from doing that?


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.

[Ravitheja Addepally via Phabricator via lldb-commits]

ravitheja added a comment.

Well nothings preventing me from doing so, I have the changes for that were 
suggested to me for this revision. I thought I would first upload them, so that 
people can look at the parallel while I upload the linux server code and Unit 
tests.


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.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D32585#758391, @ravitheja wrote:

> Well nothings preventing me from doing so, I have the changes for that were 
> suggested to me for this revision. I thought I would first upload them, so 
> that people can look at the parallel while I upload the linux server code and 
> Unit tests.
>
> Ok so I will upload what I have right now, and let the people review it in 
> parallel, while I  work on the unit tests and I will add them here in the 
> next differential to this patch. I won't merge this patch until then.


Uploading them to the phabricator is fine, of course. I would even say 
recommended. Feel free to do that.


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] D32930: New framework for lldb client-server communication tests.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I think we're getting close, but I see a couple more issues here.




Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:24
+  if (elements["pid"].getAsInteger(16, process_info->pid))
+return make_parsing_error("ProcessInfo: pid");
+  if (elements["parent-pid"].getAsInteger(16, process_info->parent_pid))

I like how clean this ended up looking :)



Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:82
+if (!thread_info)
+  return make_parsing_error(
+  formatv("JThreadsInfo: JSON obj at {0}", i).str());

What do you think hiding the `formatv` call inside `make_parsing_error` so we 
can write `return make_parsing_error("JSON object at {0}", i);`



Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:10
+
+#include 
+#include 

This still looks wrong. Did you run clang-format on the full patch (`git 
clang-format origin/master` should do the trick. you may need to set the 
clangFormat.binary git setting to point to a clang-format executable) ?



Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:55
+ private:
+  llvm::StringRef name;
+  llvm::StringRef reason;

As far as I can tell, this StringRef points to the JSON object created in 
JThreadsInfo::Create, which is ephemeral to that function (I.e. it's a dangling 
reference). StringRefs are great for function arguments, but you have to be 
careful if you want to persist them. It's usually best to convert them back to 
a string at that point. Either std::string or llvm::SmallString if you want 
to optimize (I guess N=16 would be a good value for thread names).

Same goes for other StringRefs persisted as member variables.



Comment at: unittests/tools/lldb-server/tests/TestClient.cpp:62
+.str();
+GTEST_LOG_(ERROR) << error;
+return false;

This won't actually fail the test (i'm not sure whether you intended that or 
not). I think it would be better to bubble the error one more level and do the 
assert in the TEST. After zachary is done with D33059, we will have a nice way 
to assert on llvm::Error types. After I commit D33241, you can easily convert 
Status into llvm::Error.

Also the whole `Error` class is marked as nodiscard, so you won't need to 
annotate all functions with the macro explicitly.



Comment at: unittests/tools/lldb-server/tests/TestClient.cpp:115
+
+  if (thread_id == 0) thread_id = process_info->GetPid();
+

This is a linux-ism. Other targets don't have the "pid == main thread id" 
concept. What is the semantics you intended for the thread_id = 0 case? If you 
wanted to resume the whole process (all threads) you can send `vCont;c` or just 
`c`. We also have the LLDB_INVALID_THREAD_ID symbolic constant to signify 
invalid thread.



Comment at: unittests/tools/lldb-server/tests/TestClient.cpp:148
+
+bool TestClient::SendMessage(const std::string& message) {
+  std::string dummy_string;

The message argument could be a StringRef to give more flexibility to the 
callers. Here we don't have to worry about lifetime, as the function does not 
persist the message.



Comment at: unittests/tools/lldb-server/tests/TestClient.cpp:223
+   << '-' << arch.GetArchitectureName() << ".log";
+  log_file.str();
+  return log_file_name;

return log_file.str()


https://reviews.llvm.org/D32930



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r303348 - Add Status -- llvm::Error glue

[Pavel Labath via lldb-commits]

Author: labath
Date: Thu May 18 07:46:50 2017
New Revision: 303348

URL: http://llvm.org/viewvc/llvm-project?rev=303348&view=rev
Log:
Add Status -- llvm::Error glue

Summary:
This adds functions to convert between llvm::Error and Status classes.
Posix errors in Status are represented as llvm::ECError, and the rest as
llvm::StringError.

For the conversion from Error to Status, ECError is again represented as
a posix error in Status, while other errors are stored as generic errors
and only the string value is preserved.

Reviewers: zturner, jingham

Subscribers: lldb-commits

Differential Revision: https://reviews.llvm.org/D33241

Modified:
lldb/trunk/include/lldb/Utility/Status.h
lldb/trunk/source/Utility/Status.cpp
lldb/trunk/unittests/Utility/StatusTest.cpp

Modified: lldb/trunk/include/lldb/Utility/Status.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/Status.h?rev=303348&r1=303347&r2=303348&view=diff
==
--- lldb/trunk/include/lldb/Utility/Status.h (original)
+++ lldb/trunk/include/lldb/Utility/Status.h Thu May 18 07:46:50 2017
@@ -1,5 +1,4 @@
-//===-- Status.h -*- C++
-//-*-===//
+//===-- Status.h *- C++ 
-*-===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -8,22 +7,20 @@
 //
 
//===--===//
 
-#ifndef __DCError_h__
-#define __DCError_h__
-#if defined(__cplusplus)
+#ifndef LLDB_UTILITY_STATUS_H
+#define LLDB_UTILITY_STATUS_H
 
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h" // for ErrorType, ErrorType...
 #include "llvm/ADT/StringRef.h" // for StringRef
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
-
 #include 
+#include  // for uint32_t
 #include 
 #include  // for error_code
 #include   // for forward
 
-#include  // for uint32_t
-
 namespace llvm {
 class raw_ostream;
 }
@@ -106,6 +103,10 @@ public:
 
   ~Status();
 
+  // llvm::Error support
+  explicit Status(llvm::Error error);
+  llvm::Error ToError() const;
+
   //--
   /// Get the error string associated with the current error.
   //
@@ -274,5 +275,4 @@ template <> struct format_providerhttp://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/Status.cpp?rev=303348&r1=303347&r2=303348&view=diff
==
--- lldb/trunk/source/Utility/Status.cpp (original)
+++ lldb/trunk/source/Utility/Status.cpp Thu May 18 07:46:50 2017
@@ -56,6 +56,37 @@ Status::Status(const char *format, ...)
   va_end(args);
 }
 
+Status::Status(llvm::Error error)
+: m_code(0), m_type(ErrorType::eErrorTypeGeneric) {
+  if (!error)
+return;
+
+  // if the error happens to be a errno error, preserve the error code
+  error = llvm::handleErrors(
+  std::move(error), [&](std::unique_ptr e) -> llvm::Error {
+std::error_code ec = e->convertToErrorCode();
+if (ec.category() == std::generic_category()) {
+  m_code = ec.value();
+  m_type = ErrorType::eErrorTypePOSIX;
+  return llvm::Error::success();
+}
+return llvm::Error(std::move(e));
+  });
+
+  // Otherwise, just preserve the message
+  if (error)
+SetErrorString(llvm::toString(std::move(error)));
+}
+
+llvm::Error Status::ToError() const {
+  if (Success())
+return llvm::Error::success();
+  if (m_type == ErrorType::eErrorTypePOSIX)
+return llvm::errorCodeToError(std::error_code(m_code, 
std::generic_category()));
+  return llvm::make_error(AsCString(),
+ llvm::inconvertibleErrorCode());
+}
+
 //--
 // Assignment operator
 //--

Modified: lldb/trunk/unittests/Utility/StatusTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/StatusTest.cpp?rev=303348&r1=303347&r2=303348&view=diff
==
--- lldb/trunk/unittests/Utility/StatusTest.cpp (original)
+++ lldb/trunk/unittests/Utility/StatusTest.cpp Thu May 18 07:46:50 2017
@@ -11,9 +11,40 @@
 #include "gtest/gtest.h"
 
 using namespace lldb_private;
+using namespace lldb;
 
 TEST(StatusTest, Formatv) {
   EXPECT_EQ("", llvm::formatv("{0}", Status()).str());
   EXPECT_EQ("Hello Status", llvm::formatv("{0}", Status("Hello 
Status")).str());
   EXPECT_EQ("Hello", llvm::formatv("{0:5}", Status("Hello Error")).str());
 }
+
+TEST(StatusTest, ErrorConstructor) {
+  EXPECT_TRUE(Status(llvm::Error::success()).Success());
+
+  Status eagain(
+  llvm::errorCodeToError(std::error_code(EAGAIN, 
std::generic_category(;
+  EXPECT_TRUE(eagain.Fail())

[Lldb-commits] [PATCH] D33241: Add Status -- llvm::Error glue

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL303348: Add Status -- llvm::Error glue (authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D33241?vs=99283&id=99429#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33241

Files:
  lldb/trunk/include/lldb/Utility/Status.h
  lldb/trunk/source/Utility/Status.cpp
  lldb/trunk/unittests/Utility/StatusTest.cpp

Index: lldb/trunk/include/lldb/Utility/Status.h
===
--- lldb/trunk/include/lldb/Utility/Status.h
+++ lldb/trunk/include/lldb/Utility/Status.h
@@ -1,29 +1,26 @@
-//===-- Status.h -*- C++
-//-*-===//
+//===-- Status.h *- C++ -*-===//
 //
 // The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===--===//
 
-#ifndef __DCError_h__
-#define __DCError_h__
-#if defined(__cplusplus)
+#ifndef LLDB_UTILITY_STATUS_H
+#define LLDB_UTILITY_STATUS_H
 
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h" // for ErrorType, ErrorType...
 #include "llvm/ADT/StringRef.h" // for StringRef
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
-
 #include 
+#include  // for uint32_t
 #include 
 #include  // for error_code
 #include   // for forward
 
-#include  // for uint32_t
-
 namespace llvm {
 class raw_ostream;
 }
@@ -106,6 +103,10 @@
 
   ~Status();
 
+  // llvm::Error support
+  explicit Status(llvm::Error error);
+  llvm::Error ToError() const;
+
   //--
   /// Get the error string associated with the current error.
   //
@@ -274,5 +275,4 @@
 };
 }
 
-#endif // #if defined(__cplusplus)
-#endif // #ifndef __DCError_h__
+#endif // #ifndef LLDB_UTILITY_STATUS_H
Index: lldb/trunk/unittests/Utility/StatusTest.cpp
===
--- lldb/trunk/unittests/Utility/StatusTest.cpp
+++ lldb/trunk/unittests/Utility/StatusTest.cpp
@@ -11,9 +11,40 @@
 #include "gtest/gtest.h"
 
 using namespace lldb_private;
+using namespace lldb;
 
 TEST(StatusTest, Formatv) {
   EXPECT_EQ("", llvm::formatv("{0}", Status()).str());
   EXPECT_EQ("Hello Status", llvm::formatv("{0}", Status("Hello Status")).str());
   EXPECT_EQ("Hello", llvm::formatv("{0:5}", Status("Hello Error")).str());
 }
+
+TEST(StatusTest, ErrorConstructor) {
+  EXPECT_TRUE(Status(llvm::Error::success()).Success());
+
+  Status eagain(
+  llvm::errorCodeToError(std::error_code(EAGAIN, std::generic_category(;
+  EXPECT_TRUE(eagain.Fail());
+  EXPECT_EQ(eErrorTypePOSIX, eagain.GetType());
+  EXPECT_EQ(Status::ValueType(EAGAIN), eagain.GetError());
+
+  Status foo(llvm::make_error(
+  "foo", llvm::inconvertibleErrorCode()));
+  EXPECT_TRUE(foo.Fail());
+  EXPECT_EQ(eErrorTypeGeneric, foo.GetType());
+  EXPECT_STREQ("foo", foo.AsCString());
+}
+
+TEST(StatusTest, ErrorConversion) {
+  EXPECT_FALSE(bool(Status().ToError()));
+
+  llvm::Error eagain = Status(EAGAIN, ErrorType::eErrorTypePOSIX).ToError();
+  EXPECT_TRUE(bool(eagain));
+  std::error_code ec = llvm::errorToErrorCode(std::move(eagain));
+  EXPECT_EQ(EAGAIN, ec.value());
+  EXPECT_EQ(std::generic_category(), ec.category());
+
+  llvm::Error foo = Status("foo").ToError();
+  EXPECT_TRUE(bool(foo));
+  EXPECT_EQ("foo", llvm::toString(std::move(foo)));
+}
Index: lldb/trunk/source/Utility/Status.cpp
===
--- lldb/trunk/source/Utility/Status.cpp
+++ lldb/trunk/source/Utility/Status.cpp
@@ -56,6 +56,37 @@
   va_end(args);
 }
 
+Status::Status(llvm::Error error)
+: m_code(0), m_type(ErrorType::eErrorTypeGeneric) {
+  if (!error)
+return;
+
+  // if the error happens to be a errno error, preserve the error code
+  error = llvm::handleErrors(
+  std::move(error), [&](std::unique_ptr e) -> llvm::Error {
+std::error_code ec = e->convertToErrorCode();
+if (ec.category() == std::generic_category()) {
+  m_code = ec.value();
+  m_type = ErrorType::eErrorTypePOSIX;
+  return llvm::Error::success();
+}
+return llvm::Error(std::move(e));
+  });
+
+  // Otherwise, just preserve the message
+  if (error)
+SetErrorString(llvm::toString(std::move(error)));
+}
+
+llvm::Error Status::ToError() const {
+  if (Success())
+return llvm::Error::success();
+  if (m_type == ErrorType::eErrorTypePOSIX)
+return llvm::errorCodeToError(std::error_code(m_code, std::generic_category()));
+  return llvm::make_error(AsCString(),
+ llvm::inconvertibleErrorCode());
+}
+
 //-

[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.

[Tamas Berghammer via Phabricator via lldb-commits]

tberghammer added inline comments.



Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:83
+// Common functions for parsing packet data.
+std::unordered_map SplitPairList(const std::string& 
s);
+std::vector SplitList(const std::string& s, char delimeter);

jmajors wrote:
> zturner wrote:
> > tberghammer wrote:
> > > What if the key isn't unique?
> > Return an `llvm::StringMap` here.  Also the argument should be a 
> > `StringRef`.
> I was working on the assumption that the keys in lldb response packets would 
> be unique, and that it would map a key to a list of values if it needed to 
> have a one:many relationship. Are there any that have duplicate keys?
I think in an lldb-server test we should make as little assumption about 
lldb-server as possible. I am not aware of any packet where duplicated keys are 
allowed but if we want to rely on it I think we should add a test expectation 
verifying this as having repeated keys would mean lldb-server is buggy (what is 
exactly what we want to catch here)



Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:41
+private:
+  std::string source;
+};

The LLDB coding convention is to prefix member variables with "m_". Do we want 
to follow that one here or should we follow the LLVM one what is CapitalCase 
(you are following neither of them at the moment)?



Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:52
+
+  unsigned long ReadRegister(unsigned long register_id) const;
+

"unsigned long" is only 32 bit on some systems but a register can be arbitrary 
large so the return value should be something more generic. This is true for 
every location you are working with registers.



Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:63
+ public:
+  static llvm::Expected> Create(
+  llvm::StringRef response, llvm::support::endianness endian);

Why do we need the unique_ptr here? Can it return llvm::Expected 
instead? Same question for the other Create functions.


https://reviews.llvm.org/D32930



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D33283: RunThreadPlan: Fix halting logic in IgnoreBreakpoints = false

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Ok, I've missed the distinction between plan completing (aka being "done") and 
completing **sucessfully**. Things make a bit more sense after that.

With that in mind, let me try to explain how I understand it the code now, and 
then you can tell me if it's correct :)

For our purposes we can get a Stopped event due to one of these things 
happening:

- plan_success
- breakpoint_hit
- interrupt
- plan_failure (I'm not really sure when can that happen)
- other (this can include the process stopping due to a signal)

What the old code did in the non-halt case was:

  if(plan_success) handle_and_stop()
  else if(breakpoint_hit) handle_and_stop()
  else {
// "interrupt" (which we can still get even though we haven't sent it) , 
"plan_failure" or "other"
handle_and_stop()
  }

The old code in the halt case did:

  if (plan_failure || plan_success) handle_and_stop()
  else {
// "interrupt", "breakpoint_hit" or "other"
resume_with_all_threads()
  }

Here, the else part is wrong because we treat two other events the same way as 
the interrupt.

In my mind, the two versions of the code should behave the same way for all 
cases except for the "interrupt" case. In the first one, we should stop because 
the interrupt must have been initiated externally, while in the second one, we 
should resume because it was (probably) our halt event.

I'll upload a new version which attempts to to just that. The logic in the 
first case should remain unchanged, while the "halt" logic becomes:

  if(plan_success) handle_and_stop()
  else if(breakpoint_hit) handle_and_stop()
  else if(interrupt) resume_with_all_threads()
  else {
// "plan_failure" or "other"
handle_and_stop()
  }

Let me know what you think.

PS: I'm not in a hurry with this, so I can wait a couple of days if you're busy 
right now.


https://reviews.llvm.org/D33283



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D33283: RunThreadPlan: Fix halting logic in IgnoreBreakpoints = false

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 99435.
labath added a comment.

New version


https://reviews.llvm.org/D33283

Files:
  
packages/Python/lldbsuite/test/expression_command/unwind_expression/TestUnwindExpression.py
  source/Target/Process.cpp

Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -4823,6 +4823,47 @@
 return *options.GetTimeout() - GetOneThreadExpressionTimeout(options);
 }
 
+static llvm::Optional
+HandleStoppedEvent(Thread &thread, const ThreadPlanSP &thread_plan_sp,
+   RestorePlanState &restorer, const EventSP &event_sp,
+   EventSP &event_to_broadcast_sp,
+   const EvaluateExpressionOptions &options, bool handle_interrupts) {
+  Log *log = GetLogIfAnyCategoriesSet(LIBLLDB_LOG_STEP | LIBLLDB_LOG_PROCESS);
+
+  ThreadPlanSP plan = thread.GetCompletedPlan();
+  if (plan == thread_plan_sp && plan->PlanSucceeded()) {
+LLDB_LOG(log, "execution completed successfully");
+
+// Restore the plan state so it will get reported as intended when we are
+// done.
+restorer.Clean();
+return eExpressionCompleted;
+  }
+
+  StopInfoSP stop_info_sp = thread.GetStopInfo();
+  if (stop_info_sp && stop_info_sp->GetStopReason() == eStopReasonBreakpoint) {
+LLDB_LOG(log, "stopped for breakpoint: {0}.", stop_info_sp->GetDescription());
+if (!options.DoesIgnoreBreakpoints()) {
+  // Restore the plan state and then force Private to false.  We are going
+  // to stop because of this plan so we need it to become a public plan or
+  // it won't report correctly when we continue to its termination later on.
+  restorer.Clean();
+  thread_plan_sp->SetPrivate(false);
+  event_to_broadcast_sp = event_sp;
+}
+return eExpressionHitBreakpoint;
+  }
+
+  if (!handle_interrupts &&
+  Process::ProcessEventData::GetInterruptedFromEvent(event_sp.get()))
+return llvm::None;
+
+  LLDB_LOG(log, "thread plan did not successfully complete");
+  if (!options.DoesUnwindOnError())
+event_to_broadcast_sp = event_sp;
+  return eExpressionInterrupted;
+}
+
 ExpressionResults
 Process::RunThreadPlan(ExecutionContext &exe_ctx,
lldb::ThreadPlanSP &thread_plan_sp,
@@ -5228,65 +5269,22 @@
   "but our thread (index-id=%u) has vanished.",
   thread_idx_id);
 return_value = eExpressionInterrupted;
-  } else {
+  } else if (Process::ProcessEventData::GetRestartedFromEvent(
+ event_sp.get())) {
 // If we were restarted, we just need to go back up to fetch
 // another event.
-if (Process::ProcessEventData::GetRestartedFromEvent(
-event_sp.get())) {
-  if (log) {
-log->Printf("Process::RunThreadPlan(): Got a stop and "
-"restart, so we'll continue waiting.");
-  }
-  keep_going = true;
-  do_resume = false;
-  handle_running_event = true;
-} else {
-  ThreadPlanSP plan = thread->GetCompletedPlan();
-  if (plan == thread_plan_sp && plan->PlanSucceeded()) {
-
-if (log)
-  log->PutCString("Process::RunThreadPlan(): execution "
-  "completed successfully.");
-
-// Restore the plan state so it will get reported as
-// intended when we are done.
-thread_plan_restorer.Clean();
-
-return_value = eExpressionCompleted;
-  } else {
-StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
-// Something restarted the target, so just wait for it to
-// stop for real.
-if (stop_info_sp &&
-stop_info_sp->GetStopReason() == eStopReasonBreakpoint) {
-  if (log)
-log->Printf("Process::RunThreadPlan() stopped for "
-"breakpoint: %s.",
-stop_info_sp->GetDescription());
-  return_value = eExpressionHitBreakpoint;
-  if (!options.DoesIgnoreBreakpoints()) {
-// Restore the plan state and then force Private to
-// false.  We are
-// going to stop because of this plan so we need it to
-// become a public
-// plan or it won't report correctly when we continue to
-// its termination
-// later on.
-thread_plan_restorer.Clean();
-

[Lldb-commits] Patch for fixing FDE indexing when scan debug_info section

[Tatyana Krasnukha via lldb-commits]

Fix FDE indexing while scan debug_info section.

There are some differences between eh_frame and debug_frame formats that are 
not considered by DWARFCallFrameInfo::GetFDEIndex.
An FDE entry contains CIE_pointer in debug_frame in same place as cie_id in 
eh_frame. As described in dwarf 
standard (section 6.4.1),
CIE_pointer is an "offset into the .debug_frame section". So, variable 
cie_offset should be equal cie_id for debug_frame.
FDE entries with zeroth CIE pointer (which is actually placed in cie_id 
variable) shouldn't be ignored also.

I had same issue as described here 
http://lists.llvm.org/pipermail/lldb-dev/2014-October/005520.html , and these 
changes have fixed it for me (with "m_is_eh_frame" set to false, of course).

Tatyana Krasnukha
Software Engineer, Sr. I, Solutions Group, Synopsys Inc.
w +7.812.408.7463 | m +7 981 757-4491 | 
taty...@synopsys.com



DWARFCallFrameInfo.cpp.patch
Description: DWARFCallFrameInfo.cpp.patch
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.

[Jason Majors via Phabricator via lldb-commits]

jmajors marked 7 inline comments as done.
jmajors added inline comments.



Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:10
+
+#include 
+#include 

labath wrote:
> Do you still need these includes?
Yes. I'm using a stringstream to convert integer register IDs to two nibble hex 
strings.



Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:26-27
+Expected> ProcessInfo::Create(StringRef response) 
{
+  std::unique_ptr process_info =
+std::unique_ptr(new ProcessInfo);
+  auto elements = SplitPairList(response);

zturner wrote:
> How about `std::unique_ptr process_info(new ProcessInfo);`
Java brainwashing. :)



Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:10
+
+#include 
+#include 

labath wrote:
> This still looks wrong. Did you run clang-format on the full patch (`git 
> clang-format origin/master` should do the trick. you may need to set the 
> clangFormat.binary git setting to point to a clang-format executable) ?
I ran clang-format -i *h *cpp. It reordered the includes.
I just ran it as a git subcommand, and it didn't change these lines.
 I even tried scrambling the includes around, and that's the order it seems to 
want them in.



Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:63
+ public:
+  static llvm::Expected> Create(
+  llvm::StringRef response, llvm::support::endianness endian);

tberghammer wrote:
> Why do we need the unique_ptr here? Can it return llvm::Expected 
> instead? Same question for the other Create functions.
Since I don't build the JThreadsInfo, ProcessInfo, and StopReply members of 
TestClient in the constructor, I'd need a default constructor, which I hid to 
force use of Create(). Also, it allows me to have an uninitialized value for 
these members, so I can verify some things happen in the correct order.



Comment at: unittests/tools/lldb-server/tests/TestClient.cpp:62
+.str();
+GTEST_LOG_(ERROR) << error;
+return false;

labath wrote:
> This won't actually fail the test (i'm not sure whether you intended that or 
> not). I think it would be better to bubble the error one more level and do 
> the assert in the TEST. After zachary is done with D33059, we will have a 
> nice way to assert on llvm::Error types. After I commit D33241, you can 
> easily convert Status into llvm::Error.
> 
> Also the whole `Error` class is marked as nodiscard, so you won't need to 
> annotate all functions with the macro explicitly.
The false return/no discard combo causes the test to fail.
I didn't want to return an Error object, because it adds a lot of overhead.
If Zachary's assert change reduces this overhead, I can switch it.



Comment at: unittests/tools/lldb-server/tests/TestClient.cpp:115
+
+  if (thread_id == 0) thread_id = process_info->GetPid();
+

labath wrote:
> This is a linux-ism. Other targets don't have the "pid == main thread id" 
> concept. What is the semantics you intended for the thread_id = 0 case? If 
> you wanted to resume the whole process (all threads) you can send `vCont;c` 
> or just `c`. We also have the LLDB_INVALID_THREAD_ID symbolic constant to 
> signify invalid thread.
I was using 0 so the caller didn't have to know what the main thread id was.


https://reviews.llvm.org/D32930



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.

[Jason Majors via Phabricator via lldb-commits]

jmajors updated this revision to Diff 99491.
jmajors marked an inline comment as done.
jmajors added a comment.

More CL feedback.


https://reviews.llvm.org/D32930

Files:
  unittests/CMakeLists.txt
  unittests/tools/CMakeLists.txt
  unittests/tools/lldb-server/CMakeLists.txt
  unittests/tools/lldb-server/inferior/thread_inferior.cpp
  unittests/tools/lldb-server/tests/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
  unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp

Index: unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
===
--- /dev/null
+++ unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
@@ -0,0 +1,55 @@
+//===-- ThreadsInJstopinfoTest.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include 
+#include "TestClient.h"
+#include "gtest/gtest.h"
+
+using namespace llgs_tests;
+
+class ThreadsInJstopinfoTest : public ::testing::Test {
+ protected:
+  virtual void SetUp() { TestClient::Initialize(); }
+};
+
+TEST_F(ThreadsInJstopinfoTest, TestStopReplyContainsThreadPcsLlgs) {
+  std::vector inferior_args;
+  // This inferior spawns N threads, then forces a break.
+  inferior_args.push_back(THREAD_INFERIOR);
+  inferior_args.push_back("4");
+
+  auto test_info = ::testing::UnitTest::GetInstance()->current_test_info();
+
+  TestClient client(test_info->name(), test_info->test_case_name());
+  ASSERT_TRUE(client.StartDebugger());
+  ASSERT_TRUE(client.SetInferior(inferior_args));
+  ASSERT_TRUE(client.ListThreadsInStopReply());
+  ASSERT_TRUE(client.Continue());
+  unsigned int pc_reg = client.GetPcRegisterId();
+  ASSERT_NE(pc_reg, UINT_MAX);
+
+  auto jthreads_info = client.GetJThreadsInfo();
+  ASSERT_TRUE(jthreads_info);
+
+  auto stop_reply = client.GetLatestStopReply();
+  auto stop_reply_pcs = stop_reply.GetThreadPcs();
+  auto thread_infos = jthreads_info->GetThreadInfos();
+  ASSERT_EQ(stop_reply_pcs.size(), thread_infos.size())
+  << "Thread count mismatch.";
+
+  for (auto stop_reply_pc : stop_reply_pcs) {
+unsigned long tid = stop_reply_pc.first;
+ASSERT_TRUE(thread_infos.find(tid) != thread_infos.end())
+<< "Thread ID: " << tid << " not in JThreadsInfo.";
+ASSERT_EQ(stop_reply_pcs[tid], thread_infos[tid].ReadRegister(pc_reg))
+<< "Mismatched PC for thread: " << tid;
+  }
+
+  ASSERT_TRUE(client.StopDebugger());
+}
Index: unittests/tools/lldb-server/tests/TestClient.h
===
--- /dev/null
+++ unittests/tools/lldb-server/tests/TestClient.h
@@ -0,0 +1,58 @@
+//===-- TestClient.h *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include 
+#include 
+#include "MessageObjects.h"
+#include "Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h"
+#include "lldb/Core/ArchSpec.h"
+#include "lldb/Target/ProcessLaunchInfo.h"
+
+namespace llgs_tests {
+// TODO: Make the test client an abstract base class, with different children
+// for different types of connections: llgs v. debugserver
+class TestClient
+: public lldb_private::process_gdb_remote::GDBRemoteCommunicationClient {
+ public:
+  static void Initialize();
+  TestClient(const std::string& test_name, const std::string& test_case_name);
+  virtual ~TestClient();
+  LLVM_NODISCARD bool StartDebugger();
+  LLVM_NODISCARD bool StopDebugger();
+  LLVM_NODISCARD bool SetInferior(llvm::ArrayRef inferior_args);
+  LLVM_NODISCARD bool ListThreadsInStopReply();
+  LLVM_NODISCARD bool SetBreakpoint(unsigned long address);
+  LLVM_NODISCARD bool Continue(unsigned long thread_id = 0);
+  const ProcessInfo& GetProcessInfo();
+  std::unique_ptr GetJThreadsInfo();
+  const StopReply& GetLatestStopReply();
+  LLVM_NODISCARD bool SendMessage(llvm::StringRef message);
+  LLVM_NODISCARD bool SendMessage(llvm::StringRef message,
+  std::string& response_string);
+  LLVM_NODISCARD bool SendMessage(llvm::StringRef message,
+  std::string& response_string,
+  PacketResult expected_result);
+  unsigned int GetPcRegisterId();
+
+ private:
+  LLVM_NODISCARD bool GenerateConnectionAddress(std::string& address);
+  std::string GenerateLogFileName(const lldb_private::

[Lldb-commits] [PATCH] D32820: Parallelize demangling

[Ed Maste via Phabricator via lldb-commits]

emaste added a comment.

> Without tcmalloc, on Ubuntu 14.04, 40 core VM: 13%
>  With tcmalloc, on Ubuntu 14.04, 40 core VM: 24% (built using cmake ... 
> -DCMAKE_EXE_LINKER_FLAGS=-ltcmalloc_minimal, which amazingly only works when 
> building with clang, not gcc...)

Do you have a brief set of steps you use for benchmarking? I'd like to compare 
on FreeBSD using a similar test.


Repository:
  rL LLVM

https://reviews.llvm.org/D32820



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D33347: Fix incorrect Status -> Error rename in IOHandler

[Brian Gianforcaro via Phabricator via lldb-commits]

bgianfo created this revision.

Change 302872 was a massive rename of the Error class to Status.

The change included an incorrect rename of the "Status" window
in the LLDB GUI from "Status to "Error".  This patch undoes this incorrect
rename and restores the status window's correct name.


Repository:
  rL LLVM

https://reviews.llvm.org/D33347

Files:
  source/Core/IOHandler.cpp


Index: source/Core/IOHandler.cpp
===
--- source/Core/IOHandler.cpp
+++ source/Core/IOHandler.cpp
@@ -4640,7 +4640,7 @@
 WindowSP threads_window_sp(
 main_window_sp->CreateSubWindow("Threads", threads_bounds, false));
 WindowSP status_window_sp(
-main_window_sp->CreateSubWindow("Error", status_bounds, false));
+main_window_sp->CreateSubWindow("Status", status_bounds, false));
 status_window_sp->SetCanBeActive(
 false); // Don't let the status bar become the active window
 main_window_sp->SetDelegate(


Index: source/Core/IOHandler.cpp
===
--- source/Core/IOHandler.cpp
+++ source/Core/IOHandler.cpp
@@ -4640,7 +4640,7 @@
 WindowSP threads_window_sp(
 main_window_sp->CreateSubWindow("Threads", threads_bounds, false));
 WindowSP status_window_sp(
-main_window_sp->CreateSubWindow("Error", status_bounds, false));
+main_window_sp->CreateSubWindow("Status", status_bounds, false));
 status_window_sp->SetCanBeActive(
 false); // Don't let the status bar become the active window
 main_window_sp->SetDelegate(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D33347: Fix incorrect Status -> Error rename in IOHandler

[Brian Gianforcaro via Phabricator via lldb-commits]

bgianfo updated this revision to Diff 99525.
bgianfo added a comment.

Fixing source path


https://reviews.llvm.org/D33347

Files:
  lldb/trunk/source/Core/IOHandler.cpp


Index: lldb/trunk/source/Core/IOHandler.cpp
===
--- lldb/trunk/source/Core/IOHandler.cpp
+++ lldb/trunk/source/Core/IOHandler.cpp
@@ -4640,7 +4640,7 @@
 WindowSP threads_window_sp(
 main_window_sp->CreateSubWindow("Threads", threads_bounds, false));
 WindowSP status_window_sp(
-main_window_sp->CreateSubWindow("Error", status_bounds, false));
+main_window_sp->CreateSubWindow("Status", status_bounds, false));
 status_window_sp->SetCanBeActive(
 false); // Don't let the status bar become the active window
 main_window_sp->SetDelegate(


Index: lldb/trunk/source/Core/IOHandler.cpp
===
--- lldb/trunk/source/Core/IOHandler.cpp
+++ lldb/trunk/source/Core/IOHandler.cpp
@@ -4640,7 +4640,7 @@
 WindowSP threads_window_sp(
 main_window_sp->CreateSubWindow("Threads", threads_bounds, false));
 WindowSP status_window_sp(
-main_window_sp->CreateSubWindow("Error", status_bounds, false));
+main_window_sp->CreateSubWindow("Status", status_bounds, false));
 status_window_sp->SetCanBeActive(
 false); // Don't let the status bar become the active window
 main_window_sp->SetDelegate(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits