[Lldb-commits] [PATCH] D104413: Fixed use of -o and -k in LLDB under Windows when statically compiled with vcruntime.

2021-06-23 Thread Levon Ter-Grigoryan via Phabricator via lldb-commits 
PatriosTheGreat updated this revision to Diff 353875.
PatriosTheGreat added a comment.

Hi everyone.
Thanks for review.
In order to move new method code to Debugger class I also had to move there 
SetInputFile(lldb::FileSP file) method.
After I did that -- the reproducer tests started to fail. 
If I understand correctly there is a problem to deserialize SBStream object 
during the reply session and in previous patch it the replay tests was working 
since I was calling SetInputFile API method from SetStreamInput.
To fix that in this patch I replaced SetStreamInput method with SetInputData 
(const char* data, size_t size); which takes a raw chat array instead of 
SBStream.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104413/new/

https://reviews.llvm.org/D104413

Files:
  lldb/bindings/interface/SBDebugger.i
  lldb/include/lldb/API/SBDebugger.h
  lldb/include/lldb/Core/Debugger.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Core/Debugger.cpp
  lldb/test/API/python_api/default-constructor/sb_debugger.py
  lldb/test/API/python_api/file_handle/TestFileHandle.py
  lldb/tools/driver/Driver.cpp

Index: lldb/tools/driver/Driver.cpp
===
--- lldb/tools/driver/Driver.cpp
+++ lldb/tools/driver/Driver.cpp
@@ -23,7 +23,6 @@
 #include "llvm/Support/Format.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
@@ -42,14 +41,6 @@
 #include 
 #include 
 
-// Includes for pipe()
-#if defined(_WIN32)
-#include 
-#include 
-#else
-#include 
-#endif
-
 #if !defined(__APPLE__)
 #include "llvm/Support/DataTypes.h"
 #endif
@@ -400,60 +391,6 @@
   return error;
 }
 
-static inline int OpenPipe(int fds[2], std::size_t size) {
-#ifdef _WIN32
-  return _pipe(fds, size, O_BINARY);
-#else
-  (void)size;
-  return pipe(fds);
-#endif
-}
-
-static ::FILE *PrepareCommandsForSourcing(const char *commands_data,
-  size_t commands_size) {
-  enum PIPES { READ, WRITE }; // Indexes for the read and write fds
-  int fds[2] = {-1, -1};
-
-  if (OpenPipe(fds, commands_size) != 0) {
-WithColor::error()
-<< "can't create pipe file descriptors for LLDB commands\n";
-return nullptr;
-  }
-
-  ssize_t nrwr = write(fds[WRITE], commands_data, commands_size);
-  if (size_t(nrwr) != commands_size) {
-WithColor::error()
-<< format(
-   "write(%i, %p, %" PRIu64
-   ") failed (errno = %i) when trying to open LLDB commands pipe",
-   fds[WRITE], static_cast(commands_data),
-   static_cast(commands_size), errno)
-<< '\n';
-llvm::sys::Process::SafelyCloseFileDescriptor(fds[READ]);
-llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
-return nullptr;
-  }
-
-  // Close the write end of the pipe, so that the command interpreter will exit
-  // when it consumes all the data.
-  llvm::sys::Process::SafelyCloseFileDescriptor(fds[WRITE]);
-
-  // Open the read file descriptor as a FILE * that we can return as an input
-  // handle.
-  ::FILE *commands_file = fdopen(fds[READ], "rb");
-  if (commands_file == nullptr) {
-WithColor::error() << format("fdopen(%i, \"rb\") failed (errno = %i) "
- "when trying to open LLDB commands pipe",
- fds[READ], errno)
-   << '\n';
-llvm::sys::Process::SafelyCloseFileDescriptor(fds[READ]);
-return nullptr;
-  }
-
-  // 'commands_file' now owns the read descriptor.
-  return commands_file;
-}
-
 std::string EscapeString(std::string arg) {
   std::string::size_type pos = 0;
   while ((pos = arg.find_first_of("\"\\", pos)) != std::string::npos) {
@@ -583,21 +520,16 @@
   // Check if we have any data in the commands stream, and if so, save it to a
   // temp file
   // so we can then run the command interpreter using the file contents.
-  const char *commands_data = commands_stream.GetData();
-  const size_t commands_size = commands_stream.GetSize();
-
   bool go_interactive = true;
-  if ((commands_data != nullptr) && (commands_size != 0u)) {
-FILE *commands_file =
-PrepareCommandsForSourcing(commands_data, commands_size);
-
-if (commands_file == nullptr) {
-  // We should have already printed an error in PrepareCommandsForSourcing.
+  if ((commands_stream.GetData() != nullptr) &&
+  (commands_stream.GetSize() != 0u)) {
+SBError error = m_debugger.SetInputData(commands_stream.GetData(),
+commands_stream.GetSize());
+if (error.Fail()) {
+  WithColor::error() << error.GetCString() << '\n';
   return 1;
 }
 
-m_debugger.SetInputFileHandle(commands_file, true);
-
 // Set the debugger into Sync mode when running the command file. Otherwise
 // command files that run the target wo

[Lldb-commits] [PATCH] D104768: [lldb] Remove CommandReturnObject's SetError(StringRef)

2021-06-23 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett created this revision.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Replacing existing uses with AppendError.

SetError is also part of the SBI API. Instead of removing
it outright, deprecate it and have it call AppendError.

AppendError is added to the API so users can move to that
instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104768

Files:
  lldb/include/lldb/API/SBCommandReturnObject.h
  lldb/include/lldb/Interpreter/CommandReturnObject.h
  lldb/source/API/SBCommandReturnObject.cpp
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/CommandObjectMultiword.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectReproducer.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/CommandObjectTrace.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Interpreter/CommandObject.cpp
  lldb/source/Interpreter/CommandReturnObject.cpp

Index: lldb/source/Interpreter/CommandReturnObject.cpp
===
--- lldb/source/Interpreter/CommandReturnObject.cpp
+++ lldb/source/Interpreter/CommandReturnObject.cpp
@@ -106,12 +106,7 @@
 void CommandReturnObject::SetError(const Status &error,
const char *fallback_error_cstr) {
   assert(error.Fail() && "Expected a failed Status");
-  SetError(error.AsCString(fallback_error_cstr));
-}
-
-void CommandReturnObject::SetError(llvm::StringRef error_str) {
-  SetStatus(eReturnStatusFailed);
-  AppendError(error_str);
+  AppendError(error.AsCString(fallback_error_cstr));
 }
 
 // Similar to AppendError, but do not prepend 'Status: ' to message, and don't
Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -256,7 +256,7 @@
   if (GetFlags().Test(eCommandProcessMustBeTraced)) {
 Target *target = m_exe_ctx.GetTargetPtr();
 if (target && !target->GetTrace()) {
-  result.SetError("Process is not being traced.");
+  result.AppendError("Process is not being traced.");
   return false;
 }
   }
Index: lldb/source/Commands/CommandObjectType.cpp
===
--- lldb/source/Commands/CommandObjectType.cpp
+++ lldb/source/Commands/CommandObjectType.cpp
@@ -2744,7 +2744,7 @@
   bool DoExecute(llvm::StringRef raw_command_line,
  CommandReturnObject &result) override {
 if (raw_command_line.empty()) {
-  result.SetError(
+  result.AppendError(
   "type lookup cannot be invoked without a type name as argument");
   return false;
 }
Index: lldb/source/Commands/CommandObjectTrace.cpp
===
--- lldb/source/Commands/CommandObjectTrace.cpp
+++ lldb/source/Commands/CommandObjectTrace.cpp
@@ -251,7 +251,7 @@
   bool DoExecute(Args &command, CommandReturnObject &result) override {
 Status error;
 if (command.empty()) {
-  result.SetError(
+  result.AppendError(
   "trace schema cannot be invoked without a plug-in as argument");
   return false;
 }
Index: lldb/source/Commands/CommandObjectThread.cpp
===
--- lldb/source/Commands/CommandObjectThread.cpp
+++ lldb/source/Commands/CommandObjectThread.cpp
@@ -1728,7 +1728,7 @@
   true /* condense_trivial */, m_options.m_unreported);
   // If we didn't find a TID, stop here and return an error.
   if (!success) {
-result.SetError("Error dumping plans:");
+result.AppendError("Error dumping plans:");
 result.AppendError(tmp_strm.GetString());
 return false;
   }
@@ -1966,7 +1966,7 @@
 TraceSP trace_sp = process_sp->GetTarget().GetTrace();
 
 if (llvm::Error err = trace_sp->Stop(tids))
-  result.SetError(toString(std::move(err)));
+  result.AppendError(toString(std::move(err)));
 else
   result.SetStatus(eReturnStatusSuccessFinishResult);
 
@@ -2091,7 +2091,7 @@
trace_sp->GetCursorPosition(*thread_sp)) -
m_consecutive_repetitions * count;
 if (position < 0)
-  result.SetError("error: no more data");
+  result.AppendError("error: no more data");
 else
   trace_sp->DumpTraceInstructions(*thread_sp, result.GetOutputStream(),
   count, position, m_options.m_raw);
Index: lldb/source/Commands/CommandObjectReproducer.cpp
===
--- lldb/source/Commands/CommandObjectReproducer.cpp
+++ lldb/source/Commands/CommandObjectReproducer.cpp
@@ -162,7 +162,8 @@
 return loader;
 
   // Thi

[Lldb-commits] [PATCH] D104768: [lldb] Remove CommandReturnObject's SetError(StringRef)

2021-06-23 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.
Herald added a subscriber: JDevlieghere.

I'm not totally sure about adding AppendError to the API.

Should we (and where should I) document that AppendError expects a non empty 
message? On the other hand if you're using a release build the assert to check 
that isn't a factor, you'll just set eReturnStatusFailed and not add any error 
text. So it's not going to come up for a lot of users.

Or I could just leave the API as is, with SetError calling AppendError 
internally.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104768/new/

https://reviews.llvm.org/D104768

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

[Lldb-commits] [PATCH] D104768: [lldb] Remove CommandReturnObject's SetError(StringRef)

2021-06-23 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

We should never assert unless we have to within SB API calls as that takes down 
the whole debugging session and all this is directly available to users via the 
script interpreter. I didn't realize we exposed that function via the SB API 
when I looked at the callers of those functions in D104525 
, so that's a regression we should fix. I can 
make a patch for that.

Anyway, this LGTM but please drop the addition of `AppendError` from the SB 
API. Adding a permanent new function to the SB API requires a bit more review 
than NFC refactoring. So that's the way to go:

> Or I could just leave the API as is, with SetError calling AppendError 
> internally.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104768/new/

https://reviews.llvm.org/D104768

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

[Lldb-commits] [PATCH] D104525: [lldb] Assert that CommandResultObject error messages are not empty

2021-06-23 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.

I was going to say we only would need to remove the assert for this one:

  void CommandReturnObject::SetError(const Status &error,
 const char *fallback_error_cstr) {

Since SetError checks that the `char *` is not null in the API layer before 
calling SetError proper. However, the assert is that the string is not empty. 
You could pass in a non null ptr, to an empty string. So you're right, those 
need to be removed.

(I'll update this patch shortly)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104525/new/

https://reviews.llvm.org/D104525

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

[Lldb-commits] [PATCH] D104768: [lldb] Remove CommandReturnObject's SetError(StringRef)

2021-06-23 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.

I was going to say we only would need to remove the assert for this one:

  void CommandReturnObject::SetError(const Status &error,
 const char *fallback_error_cstr) {

Since SetError checks that the char * is not null in the API layer before 
calling SetError proper. However, the assert is that the string is not empty. 
You could pass in a non null ptr, to an empty string. So you're right, those 
need to be removed.

(I'll update this patch shortly)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104768/new/

https://reviews.llvm.org/D104768

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

[Lldb-commits] [PATCH] D104768: [lldb] Remove CommandReturnObject's SetError(StringRef)

2021-06-23 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett updated this revision to Diff 353919.
DavidSpickett added a comment.

Don't change the API, just have SetError call underlying
AppendError.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104768/new/

https://reviews.llvm.org/D104768

Files:
  lldb/include/lldb/Interpreter/CommandReturnObject.h
  lldb/source/API/SBCommandReturnObject.cpp
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/CommandObjectMultiword.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectReproducer.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/CommandObjectTrace.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Interpreter/CommandObject.cpp
  lldb/source/Interpreter/CommandReturnObject.cpp

Index: lldb/source/Interpreter/CommandReturnObject.cpp
===
--- lldb/source/Interpreter/CommandReturnObject.cpp
+++ lldb/source/Interpreter/CommandReturnObject.cpp
@@ -106,12 +106,7 @@
 void CommandReturnObject::SetError(const Status &error,
const char *fallback_error_cstr) {
   assert(error.Fail() && "Expected a failed Status");
-  SetError(error.AsCString(fallback_error_cstr));
-}
-
-void CommandReturnObject::SetError(llvm::StringRef error_str) {
-  SetStatus(eReturnStatusFailed);
-  AppendError(error_str);
+  AppendError(error.AsCString(fallback_error_cstr));
 }
 
 // Similar to AppendError, but do not prepend 'Status: ' to message, and don't
Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -256,7 +256,7 @@
   if (GetFlags().Test(eCommandProcessMustBeTraced)) {
 Target *target = m_exe_ctx.GetTargetPtr();
 if (target && !target->GetTrace()) {
-  result.SetError("Process is not being traced.");
+  result.AppendError("Process is not being traced.");
   return false;
 }
   }
Index: lldb/source/Commands/CommandObjectType.cpp
===
--- lldb/source/Commands/CommandObjectType.cpp
+++ lldb/source/Commands/CommandObjectType.cpp
@@ -2744,7 +2744,7 @@
   bool DoExecute(llvm::StringRef raw_command_line,
  CommandReturnObject &result) override {
 if (raw_command_line.empty()) {
-  result.SetError(
+  result.AppendError(
   "type lookup cannot be invoked without a type name as argument");
   return false;
 }
Index: lldb/source/Commands/CommandObjectTrace.cpp
===
--- lldb/source/Commands/CommandObjectTrace.cpp
+++ lldb/source/Commands/CommandObjectTrace.cpp
@@ -251,7 +251,7 @@
   bool DoExecute(Args &command, CommandReturnObject &result) override {
 Status error;
 if (command.empty()) {
-  result.SetError(
+  result.AppendError(
   "trace schema cannot be invoked without a plug-in as argument");
   return false;
 }
Index: lldb/source/Commands/CommandObjectThread.cpp
===
--- lldb/source/Commands/CommandObjectThread.cpp
+++ lldb/source/Commands/CommandObjectThread.cpp
@@ -1728,7 +1728,7 @@
   true /* condense_trivial */, m_options.m_unreported);
   // If we didn't find a TID, stop here and return an error.
   if (!success) {
-result.SetError("Error dumping plans:");
+result.AppendError("Error dumping plans:");
 result.AppendError(tmp_strm.GetString());
 return false;
   }
@@ -1966,7 +1966,7 @@
 TraceSP trace_sp = process_sp->GetTarget().GetTrace();
 
 if (llvm::Error err = trace_sp->Stop(tids))
-  result.SetError(toString(std::move(err)));
+  result.AppendError(toString(std::move(err)));
 else
   result.SetStatus(eReturnStatusSuccessFinishResult);
 
@@ -2091,7 +2091,7 @@
trace_sp->GetCursorPosition(*thread_sp)) -
m_consecutive_repetitions * count;
 if (position < 0)
-  result.SetError("error: no more data");
+  result.AppendError("error: no more data");
 else
   trace_sp->DumpTraceInstructions(*thread_sp, result.GetOutputStream(),
   count, position, m_options.m_raw);
Index: lldb/source/Commands/CommandObjectReproducer.cpp
===
--- lldb/source/Commands/CommandObjectReproducer.cpp
+++ lldb/source/Commands/CommandObjectReproducer.cpp
@@ -162,7 +162,8 @@
 return loader;
 
   // This is a soft error because this is expected to fail during capture.
-  result.SetError("Not specifying a reproducer is only support during replay.");
+  result.AppendError(
+  "Not specifying a reproducer is

[Lldb-commits] [PATCH] D104768: [lldb] Remove CommandReturnObject's SetError(StringRef)

2021-06-23 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104768/new/

https://reviews.llvm.org/D104768

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

[Lldb-commits] [lldb] 1b1c8e4 - [lldb] Remove CommandReturnObject's SetError(StringRef)

2021-06-23 Thread David Spickett via lldb-commits 

Author: David Spickett
Date: 2021-06-23T11:25:10Z
New Revision: 1b1c8e4a984c07a3c985408d3d80c8e24e60d3d1

URL: 
https://github.com/llvm/llvm-project/commit/1b1c8e4a984c07a3c985408d3d80c8e24e60d3d1
DIFF: 
https://github.com/llvm/llvm-project/commit/1b1c8e4a984c07a3c985408d3d80c8e24e60d3d1.diff

LOG: [lldb] Remove CommandReturnObject's SetError(StringRef)

Replacing existing uses with AppendError.

SetError is also part of the SBI API. This remains
but instead of calling the underlying SetError it
will call AppendError.

Reviewed By: teemperor

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

Added: 


Modified: 
lldb/include/lldb/Interpreter/CommandReturnObject.h
lldb/source/API/SBCommandReturnObject.cpp
lldb/source/Commands/CommandObjectBreakpoint.cpp
lldb/source/Commands/CommandObjectMultiword.cpp
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Commands/CommandObjectReproducer.cpp
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Commands/CommandObjectTrace.cpp
lldb/source/Commands/CommandObjectType.cpp
lldb/source/Interpreter/CommandObject.cpp
lldb/source/Interpreter/CommandReturnObject.cpp

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h 
b/lldb/include/lldb/Interpreter/CommandReturnObject.h
index e2b237c6d0f1c..0c995b73c4631 100644
--- a/lldb/include/lldb/Interpreter/CommandReturnObject.h
+++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h
@@ -132,8 +132,6 @@ class CommandReturnObject {
 
   void SetError(const Status &error, const char *fallback_error_cstr = 
nullptr);
 
-  void SetError(llvm::StringRef error_cstr);
-
   lldb::ReturnStatus GetStatus();
 
   void SetStatus(lldb::ReturnStatus status);

diff  --git a/lldb/source/API/SBCommandReturnObject.cpp 
b/lldb/source/API/SBCommandReturnObject.cpp
index 9ebdbfc6080ce..00150d198fca5 100644
--- a/lldb/source/API/SBCommandReturnObject.cpp
+++ b/lldb/source/API/SBCommandReturnObject.cpp
@@ -363,7 +363,7 @@ void SBCommandReturnObject::SetError(const char 
*error_cstr) {
  error_cstr);
 
   if (error_cstr)
-ref().SetError(error_cstr);
+ref().AppendError(error_cstr);
 }
 
 namespace lldb_private {

diff  --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp 
b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index 9460ffdaa2f27..ba66a12267191 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -1798,7 +1798,7 @@ class CommandObjectBreakpointNameAdd : public 
CommandObjectParsed {
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
 if (!m_name_options.m_name.OptionWasSet()) {
-  result.SetError("No name option provided.");
+  result.AppendError("No name option provided.");
   return false;
 }
 
@@ -1812,7 +1812,7 @@ class CommandObjectBreakpointNameAdd : public 
CommandObjectParsed {
 
 size_t num_breakpoints = breakpoints.GetSize();
 if (num_breakpoints == 0) {
-  result.SetError("No breakpoints, cannot add names.");
+  result.AppendError("No breakpoints, cannot add names.");
   return false;
 }
 
@@ -1824,7 +1824,7 @@ class CommandObjectBreakpointNameAdd : public 
CommandObjectParsed {
 
 if (result.Succeeded()) {
   if (valid_bp_ids.GetSize() == 0) {
-result.SetError("No breakpoints specified, cannot add names.");
+result.AppendError("No breakpoints specified, cannot add names.");
 return false;
   }
   size_t num_valid_ids = valid_bp_ids.GetSize();
@@ -1883,7 +1883,7 @@ class CommandObjectBreakpointNameDelete : public 
CommandObjectParsed {
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
 if (!m_name_options.m_name.OptionWasSet()) {
-  result.SetError("No name option provided.");
+  result.AppendError("No name option provided.");
   return false;
 }
 
@@ -1897,7 +1897,7 @@ class CommandObjectBreakpointNameDelete : public 
CommandObjectParsed {
 
 size_t num_breakpoints = breakpoints.GetSize();
 if (num_breakpoints == 0) {
-  result.SetError("No breakpoints, cannot delete names.");
+  result.AppendError("No breakpoints, cannot delete names.");
   return false;
 }
 
@@ -1909,7 +1909,7 @@ class CommandObjectBreakpointNameDelete : public 
CommandObjectParsed {
 
 if (result.Succeeded()) {
   if (valid_bp_ids.GetSize() == 0) {
-result.SetError("No breakpoints specified, cannot delete names.");
+result.AppendError("No breakpoints specified, cannot delete names.");
 return false;
   }
   ConstString bp_name(m_name_options.m_name.GetCurrentValue());

diff  --git a/lldb/source/Commands/CommandObjectMultiword.cpp 
b/lldb/source/Commands/CommandObjectMultiword.cpp
index 9aca787d3036d..3eafd00832446 100644
--- a/lldb/source/Commands/CommandO

[Lldb-commits] [PATCH] D104768: [lldb] Remove CommandReturnObject's SetError(StringRef)

2021-06-23 Thread David Spickett via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1b1c8e4a984c: [lldb] Remove CommandReturnObject's 
SetError(StringRef) (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104768/new/

https://reviews.llvm.org/D104768

Files:
  lldb/include/lldb/Interpreter/CommandReturnObject.h
  lldb/source/API/SBCommandReturnObject.cpp
  lldb/source/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Commands/CommandObjectMultiword.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  lldb/source/Commands/CommandObjectReproducer.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/CommandObjectTrace.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Interpreter/CommandObject.cpp
  lldb/source/Interpreter/CommandReturnObject.cpp

Index: lldb/source/Interpreter/CommandReturnObject.cpp
===
--- lldb/source/Interpreter/CommandReturnObject.cpp
+++ lldb/source/Interpreter/CommandReturnObject.cpp
@@ -106,12 +106,7 @@
 void CommandReturnObject::SetError(const Status &error,
const char *fallback_error_cstr) {
   assert(error.Fail() && "Expected a failed Status");
-  SetError(error.AsCString(fallback_error_cstr));
-}
-
-void CommandReturnObject::SetError(llvm::StringRef error_str) {
-  SetStatus(eReturnStatusFailed);
-  AppendError(error_str);
+  AppendError(error.AsCString(fallback_error_cstr));
 }
 
 // Similar to AppendError, but do not prepend 'Status: ' to message, and don't
Index: lldb/source/Interpreter/CommandObject.cpp
===
--- lldb/source/Interpreter/CommandObject.cpp
+++ lldb/source/Interpreter/CommandObject.cpp
@@ -256,7 +256,7 @@
   if (GetFlags().Test(eCommandProcessMustBeTraced)) {
 Target *target = m_exe_ctx.GetTargetPtr();
 if (target && !target->GetTrace()) {
-  result.SetError("Process is not being traced.");
+  result.AppendError("Process is not being traced.");
   return false;
 }
   }
Index: lldb/source/Commands/CommandObjectType.cpp
===
--- lldb/source/Commands/CommandObjectType.cpp
+++ lldb/source/Commands/CommandObjectType.cpp
@@ -2744,7 +2744,7 @@
   bool DoExecute(llvm::StringRef raw_command_line,
  CommandReturnObject &result) override {
 if (raw_command_line.empty()) {
-  result.SetError(
+  result.AppendError(
   "type lookup cannot be invoked without a type name as argument");
   return false;
 }
Index: lldb/source/Commands/CommandObjectTrace.cpp
===
--- lldb/source/Commands/CommandObjectTrace.cpp
+++ lldb/source/Commands/CommandObjectTrace.cpp
@@ -251,7 +251,7 @@
   bool DoExecute(Args &command, CommandReturnObject &result) override {
 Status error;
 if (command.empty()) {
-  result.SetError(
+  result.AppendError(
   "trace schema cannot be invoked without a plug-in as argument");
   return false;
 }
Index: lldb/source/Commands/CommandObjectThread.cpp
===
--- lldb/source/Commands/CommandObjectThread.cpp
+++ lldb/source/Commands/CommandObjectThread.cpp
@@ -1728,7 +1728,7 @@
   true /* condense_trivial */, m_options.m_unreported);
   // If we didn't find a TID, stop here and return an error.
   if (!success) {
-result.SetError("Error dumping plans:");
+result.AppendError("Error dumping plans:");
 result.AppendError(tmp_strm.GetString());
 return false;
   }
@@ -1966,7 +1966,7 @@
 TraceSP trace_sp = process_sp->GetTarget().GetTrace();
 
 if (llvm::Error err = trace_sp->Stop(tids))
-  result.SetError(toString(std::move(err)));
+  result.AppendError(toString(std::move(err)));
 else
   result.SetStatus(eReturnStatusSuccessFinishResult);
 
@@ -2091,7 +2091,7 @@
trace_sp->GetCursorPosition(*thread_sp)) -
m_consecutive_repetitions * count;
 if (position < 0)
-  result.SetError("error: no more data");
+  result.AppendError("error: no more data");
 else
   trace_sp->DumpTraceInstructions(*thread_sp, result.GetOutputStream(),
   count, position, m_options.m_raw);
Index: lldb/source/Commands/CommandObjectReproducer.cpp
===
--- lldb/source/Commands/CommandObjectReproducer.cpp
+++ lldb/source/Commands/CommandObjectReproducer.cpp
@@ -162,7 +162,8 @@
 return loader;
 
   // This is a soft error because this is expected to fail during capture.
-  result.SetError("Not specifying a reproducer i

[Lldb-commits] [PATCH] D104778: [lldb] Remove asserts in CommandReturnObject SetError and AppendError

2021-06-23 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett created this revision.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

I added asserts to these in https://reviews.llvm.org/D104525.
They are available (directly or otherwise) via the API so we
should not assert.

Restore the previous behaviour. If the message
is empty, we return early before printing anything.
For SetError don't assert that the error is a failure.

The remaining assert is in AppendRawError which
is not part of the API.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104778

Files:
  lldb/source/Interpreter/CommandReturnObject.cpp


Index: lldb/source/Interpreter/CommandReturnObject.cpp
===
--- lldb/source/Interpreter/CommandReturnObject.cpp
+++ lldb/source/Interpreter/CommandReturnObject.cpp
@@ -99,13 +99,13 @@
 
 void CommandReturnObject::AppendError(llvm::StringRef in_string) {
   SetStatus(eReturnStatusFailed);
-  assert(!in_string.empty() && "Expected a non-empty error message");
+  if (in_string.empty())
+return;
   error(GetErrorStream()) << in_string.rtrim() << '\n';
 }
 
 void CommandReturnObject::SetError(const Status &error,
const char *fallback_error_cstr) {
-  assert(error.Fail() && "Expected a failed Status");
   AppendError(error.AsCString(fallback_error_cstr));
 }
 


Index: lldb/source/Interpreter/CommandReturnObject.cpp
===
--- lldb/source/Interpreter/CommandReturnObject.cpp
+++ lldb/source/Interpreter/CommandReturnObject.cpp
@@ -99,13 +99,13 @@
 
 void CommandReturnObject::AppendError(llvm::StringRef in_string) {
   SetStatus(eReturnStatusFailed);
-  assert(!in_string.empty() && "Expected a non-empty error message");
+  if (in_string.empty())
+return;
   error(GetErrorStream()) << in_string.rtrim() << '\n';
 }
 
 void CommandReturnObject::SetError(const Status &error,
const char *fallback_error_cstr) {
-  assert(error.Fail() && "Expected a failed Status");
   AppendError(error.AsCString(fallback_error_cstr));
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D104778: [lldb] Remove asserts in CommandReturnObject SetError and AppendError

2021-06-23 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

LGTM, I'll add them in my patch. Thanks for the quick turnaround!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104778/new/

https://reviews.llvm.org/D104778

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

[Lldb-commits] [PATCH] D104778: [lldb] Remove asserts in CommandReturnObject SetError and AppendError

2021-06-23 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.

As noted, the assert in AppendRawError remains. It has two internal callers and 
no API definition.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104778/new/

https://reviews.llvm.org/D104778

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

[Lldb-commits] [PATCH] D104778: [lldb] Remove asserts in CommandReturnObject SetError and AppendError

2021-06-23 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.

> LGTM, I'll add them in my patch. Thanks for the quick turnaround!

To clarify, is there still a regression with the API?

- This removes the asserts so you won't crash
- No functions are changed in `lldb/include/lldb/API/SBCommandReturnObject.h`

The one thing that is different is that calling the API SetError with an empty 
string will now set eReturnStatusFailed. So:

  s = do_thing_and_return_err_string_if_err(...)
  return_object.SetError(s)

Would now set it to failed even if s is empty.

Perhaps the SetError(...) functions should retain the empty message == nop 
behaviour and AppendError can set the status unconditionally?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104778/new/

https://reviews.llvm.org/D104778

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

[Lldb-commits] [PATCH] D104778: [lldb] Remove asserts in CommandReturnObject SetError and AppendError

2021-06-23 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor added a comment.

In D104778#2835701 , @DavidSpickett 
wrote:

>> LGTM, I'll add them in my patch. Thanks for the quick turnaround!
>
> To clarify, is there still a regression with the API?

No regression anymore from what I can see. My point was that I'll fix the usage 
from the SB API and then I can just re-add the asserts in the same patch (the 
asserts themselves are a good idea, the SB API just should have been updated to 
prevent hitting them from user scripts)

> - This removes the asserts so you won't crash
> - No functions are changed in `lldb/include/lldb/API/SBCommandReturnObject.h`
>
> The one thing that is different is that calling the API SetError with an 
> empty string will now set eReturnStatusFailed. So:
>
>   s = do_thing_and_return_err_string_if_err(...)
>   return_object.SetError(s)
>
> Would now set it to failed even if s is empty.
>
> Perhaps the SetError(...) functions should retain the empty message == nop 
> behaviour and AppendError can set the status unconditionally?

I think the assert behaviour is fine for the internal API, but the SB API 
behaviour is a bit more tricky. But let's discuss the expected behaviour in a 
dedicated review for that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104778/new/

https://reviews.llvm.org/D104778

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

[Lldb-commits] [lldb] 8a5165b - [lldb][NFC] Remove some redundant semicolons on HostInfoMacOSX

2021-06-23 Thread Raphael Isemann via lldb-commits 

Author: Raphael Isemann
Date: 2021-06-23T15:06:12+02:00
New Revision: 8a5165b3b9f20017ad28e28d7bc519f1855f0bfa

URL: 
https://github.com/llvm/llvm-project/commit/8a5165b3b9f20017ad28e28d7bc519f1855f0bfa
DIFF: 
https://github.com/llvm/llvm-project/commit/8a5165b3b9f20017ad28e28d7bc519f1855f0bfa.diff

LOG: [lldb][NFC] Remove some redundant semicolons on HostInfoMacOSX

Added: 


Modified: 
lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm

Removed: 




diff  --git a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm 
b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
index 4c396c3fe016d..f822533f1b41a 100644
--- a/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ b/lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -513,10 +513,10 @@ static void ParseOSVersion(llvm::VersionTuple &version, 
NSString *Key) {
 namespace {
 class SharedCacheInfo {
 public:
-  const UUID &GetUUID() const { return m_uuid; };
+  const UUID &GetUUID() const { return m_uuid; }
   const llvm::StringMap &GetImages() const {
 return m_images;
-  };
+  }
 
   SharedCacheInfo();
 



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

[Lldb-commits] [lldb] fe63db2 - [lldb] Remove asserts in CommandReturnObject SetError and AppendError

2021-06-23 Thread David Spickett via lldb-commits 

Author: David Spickett
Date: 2021-06-23T13:11:14Z
New Revision: fe63db25bcc0dd31fc471f60591a0f24ee8dbf57

URL: 
https://github.com/llvm/llvm-project/commit/fe63db25bcc0dd31fc471f60591a0f24ee8dbf57
DIFF: 
https://github.com/llvm/llvm-project/commit/fe63db25bcc0dd31fc471f60591a0f24ee8dbf57.diff

LOG: [lldb] Remove asserts in CommandReturnObject SetError and AppendError

I added asserts to these in https://reviews.llvm.org/D104525.
They are available (directly or otherwise) via the API so we
should not assert.

Restore the previous behaviour. If the message
is empty, we return early before printing anything.
For SetError don't assert that the error is a failure.

The remaining assert is in AppendRawError which
is not part of the API.

Reviewed By: teemperor

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

Added: 


Modified: 
lldb/source/Interpreter/CommandReturnObject.cpp

Removed: 




diff  --git a/lldb/source/Interpreter/CommandReturnObject.cpp 
b/lldb/source/Interpreter/CommandReturnObject.cpp
index 85cfd9ab5cf5..506b0d6e7cde 100644
--- a/lldb/source/Interpreter/CommandReturnObject.cpp
+++ b/lldb/source/Interpreter/CommandReturnObject.cpp
@@ -99,13 +99,13 @@ void CommandReturnObject::AppendWarning(llvm::StringRef 
in_string) {
 
 void CommandReturnObject::AppendError(llvm::StringRef in_string) {
   SetStatus(eReturnStatusFailed);
-  assert(!in_string.empty() && "Expected a non-empty error message");
+  if (in_string.empty())
+return;
   error(GetErrorStream()) << in_string.rtrim() << '\n';
 }
 
 void CommandReturnObject::SetError(const Status &error,
const char *fallback_error_cstr) {
-  assert(error.Fail() && "Expected a failed Status");
   AppendError(error.AsCString(fallback_error_cstr));
 }
 



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

[Lldb-commits] [PATCH] D104778: [lldb] Remove asserts in CommandReturnObject SetError and AppendError

2021-06-23 Thread David Spickett via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfe63db25bcc0: [lldb] Remove asserts in CommandReturnObject 
SetError and AppendError (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104778/new/

https://reviews.llvm.org/D104778

Files:
  lldb/source/Interpreter/CommandReturnObject.cpp


Index: lldb/source/Interpreter/CommandReturnObject.cpp
===
--- lldb/source/Interpreter/CommandReturnObject.cpp
+++ lldb/source/Interpreter/CommandReturnObject.cpp
@@ -99,13 +99,13 @@
 
 void CommandReturnObject::AppendError(llvm::StringRef in_string) {
   SetStatus(eReturnStatusFailed);
-  assert(!in_string.empty() && "Expected a non-empty error message");
+  if (in_string.empty())
+return;
   error(GetErrorStream()) << in_string.rtrim() << '\n';
 }
 
 void CommandReturnObject::SetError(const Status &error,
const char *fallback_error_cstr) {
-  assert(error.Fail() && "Expected a failed Status");
   AppendError(error.AsCString(fallback_error_cstr));
 }
 


Index: lldb/source/Interpreter/CommandReturnObject.cpp
===
--- lldb/source/Interpreter/CommandReturnObject.cpp
+++ lldb/source/Interpreter/CommandReturnObject.cpp
@@ -99,13 +99,13 @@
 
 void CommandReturnObject::AppendError(llvm::StringRef in_string) {
   SetStatus(eReturnStatusFailed);
-  assert(!in_string.empty() && "Expected a non-empty error message");
+  if (in_string.empty())
+return;
   error(GetErrorStream()) << in_string.rtrim() << '\n';
 }
 
 void CommandReturnObject::SetError(const Status &error,
const char *fallback_error_cstr) {
-  assert(error.Fail() && "Expected a failed Status");
   AppendError(error.AsCString(fallback_error_cstr));
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D104395: [LLDB][GUI] Add initial forms support

2021-06-23 Thread Omar Emara via Phabricator via lldb-commits 
OmarEmaraDev added a comment.

Since we are still working on this diff. I will add the other form 
functionality I have been working on here as well if you don't mind.




Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1396-1405
+  // 
+  // | |
+  // | |
+  // | Form elements here. |
+  // | |
+  // |...  |
+  // |-|

clayborg wrote:
> Should we put a cancel button and "Submit" button in the window's bottom line 
> to save space? Any errors could be popped up with a modal dialog when the 
> user tries to submit. The help for a form could specify that "Esc" will 
> cancel the dialog and we wouldn't have to write it into the bottom of the 
> window.
Moving the buttons to the border seems like a good idea to me, so I will do 
that.

However, I think we should generally avoid double modals, as it is frowned upon 
in the UX world. So showing errors in a modal is probably not a good idea.  

I am currently reworking some aspects of forms to be more dynamic. The new 
forms supports full vertical scrolling, field validation, per field errors, and 
list of fields. So errors can be incorporated nicely in the new design.



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1450
+// Draw the centered submit button.
+const char *button_text = "[Submit]";
+int x = (window.GetWidth() - sizeof(button_text) - 1) / 2;

clayborg wrote:
> Maybe we want to add a ButtonDelegate class and each window can add a list of 
> button delegates. Then the window draw code would know to draw them?
I guess it depends if we really need this flexibility. I can't think of a form 
that would need (or should have) more than confirmation and cancellation. Can 
you give an example where a different configuration would be needed?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104395/new/

https://reviews.llvm.org/D104395

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

[Lldb-commits] [PATCH] D104395: [LLDB][GUI] Add initial forms support

2021-06-23 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

In D104395#2836661 , @OmarEmaraDev 
wrote:

> Since we are still working on this diff. I will add the other form 
> functionality I have been working on here as well if you don't mind.

Sure thing!




Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1405
+  // | Error message if it exists. |
+  // |__[Press Esc to cancel]__|
+  //

Sounds good! Just want to make sure we do the right thing. I look forward to 
seeing what you come up with



Comment at: lldb/source/Core/IOHandlerCursesGUI.cpp:1450
+// Draw the centered submit button.
+const char *button_text = "[Submit]";
+int x = (window.GetWidth() - sizeof(button_text) - 1) / 2;

OmarEmaraDev wrote:
> clayborg wrote:
> > Maybe we want to add a ButtonDelegate class and each window can add a list 
> > of button delegates. Then the window draw code would know to draw them?
> I guess it depends if we really need this flexibility. I can't think of a 
> form that would need (or should have) more than confirmation and 
> cancellation. Can you give an example where a different configuration would 
> be needed?
Just thinking of any window that requires buttons. Forms will have "Cancel" and 
"OK" or "Submit". Maybe the "Submit" should be a constructor argument so that 
the target create form can say "Create Target" instead of "Submit"?

We might end up doing modal dialog windows for some settings or validations of 
fields that pop up a OK/Cancel dialog, or Abort/Retry/Ingore, or a progress 
dialog that might have a Cancel button. Each of these is a window. Just trying 
to think ahead of what we will need. 

A file picker window would be pretty neat as well, so that when a user selects 
the "Path" field, maybe a window pops up and allows selecting the file using a 
nice dialog box, and that maybe have buttons too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104395/new/

https://reviews.llvm.org/D104395

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

[Lldb-commits] [lldb] 5bebc0b - [lldb] Decouple ObjCLanguage from Symtab

2021-06-23 Thread Alex Langford via lldb-commits 

Author: Alex Langford
Date: 2021-06-23T13:49:46-07:00
New Revision: 5bebc0b177d0f02af3d8d2b02d182c04763ee468

URL: 
https://github.com/llvm/llvm-project/commit/5bebc0b177d0f02af3d8d2b02d182c04763ee468
DIFF: 
https://github.com/llvm/llvm-project/commit/5bebc0b177d0f02af3d8d2b02d182c04763ee468.diff

LOG: [lldb] Decouple ObjCLanguage from Symtab

We can extend/modify `GetMethodNameVariants` to suit our purposes here.
What symtab is looking for is alternate names we may want to use to
search for a specific symbol, and asking for variants of a name makes
the most sense here.

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

Added: 


Modified: 
lldb/include/lldb/Target/Language.h
lldb/source/Breakpoint/BreakpointResolverName.cpp
lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
lldb/source/Symbol/CMakeLists.txt
lldb/source/Symbol/Symtab.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index 0639a40020db..11b9daa38945 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -184,12 +184,22 @@ class Language : public PluginInterface {
 
   virtual const char *GetLanguageSpecificTypeLookupHelp();
 
+  class MethodNameVariant {
+ConstString m_name;
+lldb::FunctionNameType m_type;
+
+  public:
+MethodNameVariant(ConstString name, lldb::FunctionNameType type)
+: m_name(name), m_type(type) {}
+ConstString GetName() const { return m_name; }
+lldb::FunctionNameType GetType() const { return m_type; }
+  };
   // If a language can have more than one possible name for a method, this
   // function can be used to enumerate them. This is useful when doing name
   // lookups.
-  virtual std::vector
+  virtual std::vector
   GetMethodNameVariants(ConstString method_name) const {
-return std::vector();
+return std::vector();
   };
 
   /// Returns true iff the given symbol name is compatible with the mangling

diff  --git a/lldb/source/Breakpoint/BreakpointResolverName.cpp 
b/lldb/source/Breakpoint/BreakpointResolverName.cpp
index b314135bc208..121ac5690d70 100644
--- a/lldb/source/Breakpoint/BreakpointResolverName.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolverName.cpp
@@ -220,11 +220,15 @@ void BreakpointResolverName::AddNameLookup(ConstString 
name,
   m_lookups.emplace_back(lookup);
 
   auto add_variant_funcs = [&](Language *lang) {
-for (ConstString variant_name : lang->GetMethodNameVariants(name)) {
-  Module::LookupInfo variant_lookup(name, name_type_mask,
-lang->GetLanguageType());
-  variant_lookup.SetLookupName(variant_name);
-  m_lookups.emplace_back(variant_lookup);
+for (Language::MethodNameVariant variant :
+ lang->GetMethodNameVariants(name)) {
+  // FIXME: Should we be adding variants that aren't of type Full?
+  if (variant.GetType() & lldb::eFunctionNameTypeFull) {
+Module::LookupInfo variant_lookup(name, variant.GetType(),
+  lang->GetLanguageType());
+variant_lookup.SetLookupName(variant.GetName());
+m_lookups.emplace_back(variant_lookup);
+  }
 }
 return true;
   };

diff  --git a/lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp 
b/lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
index 785b1fd3b546..3fb0c9e28124 100644
--- a/lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ b/lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -225,14 +225,17 @@ ConstString 
ObjCLanguage::MethodName::GetFullNameWithoutCategory(
   return ConstString();
 }
 
-std::vector
+std::vector
 ObjCLanguage::GetMethodNameVariants(ConstString method_name) const {
-  std::vector variant_names;
+  std::vector variant_names;
   ObjCLanguage::MethodName objc_method(method_name.GetCString(), false);
   if (!objc_method.IsValid(false)) {
 return variant_names;
   }
 
+  variant_names.emplace_back(objc_method.GetSelector(),
+ lldb::eFunctionNameTypeSelector);
+
   const bool is_class_method =
   objc_method.GetType() == MethodName::eTypeClassMethod;
   const bool is_instance_method =
@@ -242,25 +245,30 @@ ObjCLanguage::GetMethodNameVariants(ConstString 
method_name) const {
 
   if (is_class_method || is_instance_method) {
 if (name_sans_category)
-  variant_names.emplace_back(name_sans_category);
+  variant_names.emplace_back(name_sans_category,
+ lldb::eFunctionNameTypeFull);
   } else {
 StreamString strm;
 
 strm.Printf("+%s", objc_method.GetFullName().GetCString());
-variant_names.emplace_back(strm.GetString());
+variant_names.emplace_back(ConstString(strm.GetString()),
+   lldb::eFunctionNameTypeFull);
 strm.Clear();
 
 strm.Printf("-%s", objc_m

[Lldb-commits] [PATCH] D104067: [lldb] Decouple ObjCLanguage from Symtab

2021-06-23 Thread Alex Langford via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5bebc0b177d0: [lldb] Decouple ObjCLanguage from Symtab 
(authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104067/new/

https://reviews.llvm.org/D104067

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
  lldb/source/Symbol/CMakeLists.txt
  lldb/source/Symbol/Symtab.cpp

Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -9,8 +9,6 @@
 #include 
 #include 
 
-#include "Plugins/Language/ObjC/ObjCLanguage.h"
-
 #include "lldb/Core/Module.h"
 #include "lldb/Core/RichManglingContext.h"
 #include "lldb/Core/Section.h"
@@ -18,6 +16,7 @@
 #include "lldb/Symbol/Symbol.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Symbol/Symtab.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/Timer.h"
@@ -268,6 +267,13 @@
 m_name_indexes_computed = true;
 LLDB_SCOPED_TIMER();
 
+// Collect all loaded language plugins.
+std::vector languages;
+Language::ForEach([&languages](Language *l) {
+  languages.push_back(l);
+  return true;
+});
+
 auto &name_to_index = GetNameToSymbolIndexMap(lldb::eFunctionNameTypeNone);
 auto &basename_to_index =
 GetNameToSymbolIndexMap(lldb::eFunctionNameTypeBase);
@@ -334,13 +340,17 @@
 
 // If the demangled name turns out to be an ObjC name, and is a category
 // name, add the version without categories to the index too.
-ObjCLanguage::MethodName objc_method(name.GetStringRef(), true);
-if (objc_method.IsValid(true)) {
-  selector_to_index.Append(objc_method.GetSelector(), value);
-
-  if (ConstString objc_method_no_category =
-  objc_method.GetFullNameWithoutCategory(true))
-name_to_index.Append(objc_method_no_category, value);
+for (Language *lang : languages) {
+  for (auto variant : lang->GetMethodNameVariants(name)) {
+if (variant.GetType() & lldb::eFunctionNameTypeSelector)
+  selector_to_index.Append(variant.GetName(), value);
+else if (variant.GetType() & lldb::eFunctionNameTypeFull)
+  name_to_index.Append(variant.GetName(), value);
+else if (variant.GetType() & lldb::eFunctionNameTypeMethod)
+  method_to_index.Append(variant.GetName(), value);
+else if (variant.GetType() & lldb::eFunctionNameTypeBase)
+  basename_to_index.Append(variant.GetName(), value);
+  }
 }
   }
 }
Index: lldb/source/Symbol/CMakeLists.txt
===
--- lldb/source/Symbol/CMakeLists.txt
+++ lldb/source/Symbol/CMakeLists.txt
@@ -44,7 +44,6 @@
 lldbHost
 lldbTarget
 lldbUtility
-lldbPluginObjCLanguage
 
   LINK_COMPONENTS
 Support
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
@@ -100,7 +100,8 @@
   //  variant_names[1] => "-[NSString(my_additions) myStringWithCString:]"
   //  variant_names[2] => "+[NSString myStringWithCString:]"
   //  variant_names[3] => "-[NSString myStringWithCString:]"
-  std::vector
+  // Also returns the FunctionNameType of each possible name.
+  std::vector
   GetMethodNameVariants(ConstString method_name) const override;
 
   bool SymbolNameFitsToLanguage(Mangled mangled) const override;
Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -225,14 +225,17 @@
   return ConstString();
 }
 
-std::vector
+std::vector
 ObjCLanguage::GetMethodNameVariants(ConstString method_name) const {
-  std::vector variant_names;
+  std::vector variant_names;
   ObjCLanguage::MethodName objc_method(method_name.GetCString(), false);
   if (!objc_method.IsValid(false)) {
 return variant_names;
   }
 
+  variant_names.emplace_back(objc_method.GetSelector(),
+ lldb::eFunctionNameTypeSelector);
+
   const bool is_class_method =
   objc_method.GetType() == MethodName::eTypeClassMethod;
   const bool is_instance_method =
@@ -242,25 +245,30 @@
 
   if (is_class_method || is_instance_method) {
 if (name_sans_category)
-  variant_names.emplace_back(name_sans_category);
+  variant_names.emplace_bac

[Lldb-commits] [PATCH] D104488: Create synthetic symbol names on demand to improve memory consumption and startup times.

2021-06-23 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

Anyone have any objections now?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104488/new/

https://reviews.llvm.org/D104488

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

[Lldb-commits] [PATCH] D104422: [trace] Add a TraceCursor class

2021-06-23 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Just need to initialize the m_stop_id in the cursor to the invalid value and 
this is good to go.




Comment at: lldb/include/lldb/Target/TraceCursor.h:132
+  /// The stop ID when the cursor was created.
+  uint32_t m_stop_id;
+  /// The trace that owns this cursor.

This needs to be initialized right?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104422/new/

https://reviews.llvm.org/D104422

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

[Lldb-commits] [PATCH] D104488: Create synthetic symbol names on demand to improve memory consumption and startup times.

2021-06-23 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added a comment.

I'm fine with the change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104488/new/

https://reviews.llvm.org/D104488

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

[Lldb-commits] [PATCH] D104488: Create synthetic symbol names on demand to improve memory consumption and startup times.

2021-06-23 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

Sounds good, can someone accept the patch?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104488/new/

https://reviews.llvm.org/D104488

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

[Lldb-commits] [PATCH] D104488: Create synthetic symbol names on demand to improve memory consumption and startup times.

2021-06-23 Thread walter erquinigo via Phabricator via lldb-commits 
wallace added a comment.

good to go


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104488/new/

https://reviews.llvm.org/D104488

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

[Lldb-commits] [lldb] 2aa1dd1 - [trace] Add a TraceCursor class

2021-06-23 Thread Walter Erquinigo via lldb-commits 

Author: Walter Erquinigo
Date: 2021-06-23T22:28:01-07:00
New Revision: 2aa1dd1c66dc3b1f6253eec9fc68c081a945b74d

URL: 
https://github.com/llvm/llvm-project/commit/2aa1dd1c66dc3b1f6253eec9fc68c081a945b74d
DIFF: 
https://github.com/llvm/llvm-project/commit/2aa1dd1c66dc3b1f6253eec9fc68c081a945b74d.diff

LOG: [trace] Add a TraceCursor class

As a follow up of D103588, I'm reinitiating the discussion with a new proposal 
for traversing instructions in a trace which uses the feedback gotten in that 
diff.

See the embedded documentation in TraceCursor for more information. The idea is 
to offer an OOP way to traverse instructions exposing a minimal interface that 
makes no assumptions on:

- the number of instructions in the trace (i.e. having indices for instructions 
might be impractical for gigantic intel-pt traces, as it would require to 
decode the entire trace). This renders the use of indices to point to 
instructions impractical. Traces are big and expensive, and the consumer should 
try to do look linear lookups (forwards and/or backwards) and avoid random 
accesses (the API could be extended though, but for now I want to dicard that 
funcionality and leave the API extensible if needed).
- the way the instructions are represented internally by each Trace plug-in. 
They could be mmap'ed from a file, exist in plain vector or generated on the 
fly as the user requests the data.
- the actual data structure used internally for each plug-in. Ideas like having 
a struct TraceInstruction have been discarded because that would make the 
plug-in follow a certain data type, which might be costly. Instead, the user 
can ask the cursor for each independent property of the instruction it's 
pointing at.

The way to get a cursor is to ask Trace.h for the end or being cursor or a 
thread's trace.

There are some benefits of this approach:
- there's little cost to create a cursor, and this allows for lazily decoding a 
trace as the user requests data.
- each trace plug-in could decide how to cache the instructions it generates. 
For example, if a trace is small, it might decide to keep everything in memory, 
or if the trace is massive, it might decide to keep around the last thousands 
of instructions to speed up local searches.
- a cursor can outlive a stop point, which makes trace comparison for live 
processes feasible. An application of this is to compare profiling data of two 
runs of the same function, which should be doable with intel pt.

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

Added: 
lldb/include/lldb/Target/TraceCursor.h
lldb/source/Target/TraceCursor.cpp

Modified: 
lldb/include/lldb/Target/Trace.h
lldb/include/lldb/lldb-defines.h
lldb/include/lldb/lldb-enumerations.h
lldb/include/lldb/lldb-forward.h
lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
lldb/source/Target/CMakeLists.txt
lldb/source/Target/Trace.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Trace.h 
b/lldb/include/lldb/Target/Trace.h
index 5ff7a149b01ee..bb2b93514994c 100644
--- a/lldb/include/lldb/Target/Trace.h
+++ b/lldb/include/lldb/Target/Trace.h
@@ -15,6 +15,7 @@
 
 #include "lldb/Core/PluginInterface.h"
 #include "lldb/Target/Thread.h"
+#include "lldb/Target/TraceCursor.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/TraceGDBRemotePackets.h"
 #include "lldb/Utility/UnimplementedError.h"
@@ -204,6 +205,14 @@ class Trace : public PluginInterface,
   std::function load_addr)>
   callback) = 0;
 
+  /// Get a \a TraceCursor for the given thread's trace.
+  ///
+  /// \return
+  /// A \a TraceCursorUP. If the thread is not traced or its trace
+  /// information failed to load, the corresponding error is embedded in 
the
+  /// trace.
+  virtual lldb::TraceCursorUP GetCursor(Thread &thread) = 0;
+
   /// Get the number of available instructions in the trace of the given 
thread.
   ///
   /// \param[in] thread
@@ -279,6 +288,11 @@ class Trace : public PluginInterface,
   /// Get the trace file of the given post mortem thread.
   llvm::Expected GetPostMortemTraceFile(lldb::tid_t tid);
 
+  /// \return
+  /// The stop ID of the live process being traced, or an invalid stop ID
+  /// if the trace is in an error or invalid state.
+  uint32_t GetStopID();
+
 protected:
   /// Get binary data of a live thread given a data identifier.
   ///
@@ -350,8 +364,8 @@ class Trace : public PluginInterface,
   /// The result is cached through the same process stop.
   void RefreshLiveProcessState();
 
+  uint32_t m_stop_id = LLDB_INVALID_STOP_ID;
   /// Process traced by this object if doing live tracing. Otherwise it's null.
-  int64_t m_stop_id = -1;
   Process *m_live_process = nullptr;
   /// tid -> data kind -> size
   std::map>

diff

[Lldb-commits] [PATCH] D104422: [trace] Add a TraceCursor class

2021-06-23 Thread Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2aa1dd1c66dc: [trace] Add a TraceCursor class (authored by 
Walter Erquinigo ).

Changed prior to commit:
  https://reviews.llvm.org/D104422?vs=353135&id=354157#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104422/new/

https://reviews.llvm.org/D104422

Files:
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/lldb-defines.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/include/lldb/lldb-forward.h
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Trace.cpp
  lldb/source/Target/TraceCursor.cpp

Index: lldb/source/Target/TraceCursor.cpp
===
--- /dev/null
+++ lldb/source/Target/TraceCursor.cpp
@@ -0,0 +1,15 @@
+//===-- TraceCursor.cpp -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Target/TraceCursor.h"
+
+#include "lldb/Target/Trace.h"
+
+using namespace lldb_private;
+
+bool TraceCursor::IsStale() { return m_stop_id != m_trace_sp->GetStopID(); }
Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -473,3 +473,8 @@
 
   DoRefreshLiveProcessState(std::move(live_process_state));
 }
+
+uint32_t Trace::GetStopID() {
+  RefreshLiveProcessState();
+  return m_stop_id;
+}
Index: lldb/source/Target/CMakeLists.txt
===
--- lldb/source/Target/CMakeLists.txt
+++ lldb/source/Target/CMakeLists.txt
@@ -68,6 +68,7 @@
   ThreadSpec.cpp
   ThreadPostMortemTrace.cpp
   Trace.cpp
+  TraceCursor.cpp
   TraceSessionFileParser.cpp
   UnixSignals.cpp
   UnwindAssembly.cpp
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
@@ -74,6 +74,8 @@
 
   size_t GetCursorPosition(Thread &thread) override;
 
+  lldb::TraceCursorUP GetCursor(Thread &thread) override;
+
   void DoRefreshLiveProcessState(
   llvm::Expected state) override;
 
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -106,6 +106,11 @@
   return decoded_thread->GetCursorPosition();
 }
 
+lldb::TraceCursorUP TraceIntelPT::GetCursor(Thread &thread) {
+  // TODO: to implement
+  return nullptr;
+}
+
 void TraceIntelPT::TraverseInstructions(
 Thread &thread, size_t position, TraceDirection direction,
 std::function load_addr)>
Index: lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
@@ -61,7 +61,7 @@
 Args &command, CommandReturnObject &result,
 llvm::ArrayRef tids) {
   if (Error err = m_trace.Start(tids, m_options.m_thread_buffer_size))
-result.SetError(toString(std::move(err)));
+result.SetError(Status(std::move(err)));
   else
 result.SetStatus(eReturnStatusSuccessFinishResult);
 
@@ -122,7 +122,7 @@
 Args &command, CommandReturnObject &result) {
   if (Error err = m_trace.Start(m_options.m_thread_buffer_size,
 m_options.m_process_buffer_size_limit))
-result.SetError(toString(std::move(err)));
+result.SetError(Status(std::move(err)));
   else
 result.SetStatus(eReturnStatusSuccessFinishResult);
 
Index: lldb/include/lldb/lldb-forward.h
===
--- lldb/include/lldb/lldb-forward.h
+++ lldb/include/lldb/lldb-forward.h
@@ -229,6 +229,7 @@
 class ThreadSpec;
 class ThreadPostMortemTrace;
 class Trace;
+class TraceCursor;
 class TraceSessionFileParser;
 class Type;
 class TypeAndOrName;
@@ -441,6 +442,7 @@
 typedef std::weak_ptr ThreadPlanWP;
 typedef std::shared_ptr ThreadPlanTracerSP;
 typedef std::shared_ptr TraceSP;
+typedef std::unique_ptr TraceCursorUP;
 typedef std::shared_ptr TypeSP;
 typedef std::weak_ptr TypeWP;
 typedef std::shared_ptr

[Lldb-commits] [lldb] f0d0612 - [NFC][trace] remove dead function

2021-06-23 Thread Walter Erquinigo via lldb-commits 

Author: Walter Erquinigo
Date: 2021-06-23T23:18:53-07:00
New Revision: f0d06124769f477a26f8fa2589f0ace85419c120

URL: 
https://github.com/llvm/llvm-project/commit/f0d06124769f477a26f8fa2589f0ace85419c120
DIFF: 
https://github.com/llvm/llvm-project/commit/f0d06124769f477a26f8fa2589f0ace85419c120.diff

LOG: [NFC][trace] remove dead function

The Trace::GetCursorPosition function was never really implemented well and 
it's being replaced by a more correct TraceCursor object.

Added: 


Modified: 
lldb/include/lldb/Target/Trace.h
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
lldb/test/API/commands/trace/TestTraceStartStop.py

lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py

Removed: 




diff  --git a/lldb/include/lldb/Target/Trace.h 
b/lldb/include/lldb/Target/Trace.h
index bb2b93514994c..cc84270b80081 100644
--- a/lldb/include/lldb/Target/Trace.h
+++ b/lldb/include/lldb/Target/Trace.h
@@ -137,18 +137,6 @@ class Trace : public PluginInterface,
   /// The JSON schema of this Trace plug-in.
   virtual llvm::StringRef GetSchema() = 0;
 
-  /// Each decoded thread contains a cursor to the current position the user is
-  /// stopped at. When reverse debugging, each operation like reverse-next or
-  /// reverse-continue will move this cursor, which is then picked by any
-  /// subsequent dump or reverse operation.
-  ///
-  /// The initial position for this cursor is the last element of the thread,
-  /// which is the most recent chronologically.
-  ///
-  /// \return
-  /// The current position of the thread's trace or \b 0 if empty.
-  virtual size_t GetCursorPosition(Thread &thread) = 0;
-
   /// Dump \a count instructions of the given thread's trace ending at the
   /// given \a end_position position.
   ///

diff  --git a/lldb/source/Commands/CommandObjectThread.cpp 
b/lldb/source/Commands/CommandObjectThread.cpp
index 1d31f112ae63f..292f9c6a0a74e 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -2086,15 +2086,20 @@ class CommandObjectTraceDumpInstructions
 ThreadSP thread_sp =
 m_exe_ctx.GetProcessPtr()->GetThreadList().FindThreadByID(tid);
 
-size_t count = m_options.m_count;
-ssize_t position = m_options.m_position.getValueOr(
-   trace_sp->GetCursorPosition(*thread_sp)) -
-   m_consecutive_repetitions * count;
-if (position < 0)
-  result.AppendError("error: no more data");
-else
-  trace_sp->DumpTraceInstructions(*thread_sp, result.GetOutputStream(),
-  count, position, m_options.m_raw);
+if (llvm::Optional insn_count =
+trace_sp->GetInstructionCount(*thread_sp)) {
+  size_t count = m_options.m_count;
+  ssize_t position =
+  m_options.m_position.getValueOr((ssize_t)*insn_count - 1) -
+  m_consecutive_repetitions * count;
+  if (position < 0)
+result.AppendError("error: no more data");
+  else
+trace_sp->DumpTraceInstructions(*thread_sp, result.GetOutputStream(),
+count, position, m_options.m_raw);
+} else {
+  result.AppendError("error: not traced");
+}
 return true;
   }
 

diff  --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp 
b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
index 5546e2e96788a..df4e1e11b99a2 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -99,13 +99,6 @@ const DecodedThread *TraceIntelPT::Decode(Thread &thread) {
   return &it->second->Decode();
 }
 
-size_t TraceIntelPT::GetCursorPosition(Thread &thread) {
-  const DecodedThread *decoded_thread = Decode(thread);
-  if (!decoded_thread)
-return 0;
-  return decoded_thread->GetCursorPosition();
-}
-
 lldb::TraceCursorUP TraceIntelPT::GetCursor(Thread &thread) {
   // TODO: to implement
   return nullptr;

diff  --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h 
b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
index d25316e0c658d..2e414abca4ea6 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
@@ -72,8 +72,6 @@ class TraceIntelPT : public Trace {
 
   llvm::Optional GetInstructionCount(Thread &thread) override;
 
-  size_t GetCursorPosition(Thread &thread) override;
-
   lldb::TraceCursorUP GetCursor(Thread &thread) override;
 
   void DoRefreshLiveProcessState(

diff  --git a/lldb/test/API/commands/trace/TestTraceStartStop.py 
b/lldb/test/API/commands/trace/TestTraceStartStop.py
index 22aff10359820..91cf5afa26002 100644
--- a/lldb/test/API/commands/trace/TestTraceStartStop.py
+++ b/lldb/test/API/commands/trace/Test