[Lldb-commits] [PATCH] D69931: Add cmake variables to specify a python framework to ship with lldb

[Pavel Labath via Phabricator via lldb-commits]

labath added a reviewer: beanz.
labath added a subscriber: beanz.
labath added a comment.

In D69931#1738097 , @smeenai wrote:

> I agree on not getting into the business of installing Python ourselves, but 
> there's also the rpath issue. The Windows case is different because you just 
> put all your DLLs in the same directory (or some other directory in your 
> PATH) and you're done. With macOS, you have to specify an rpath for liblldb 
> to be able to find the Python framework. It'd be really nice to support 
> adding additional rpaths in the build system itself (it could be a more 
> generic mechanism; it doesn't have to be Python-specific). CMake does offer 
> `CMAKE_INSTALL_RPATH`, but that applies to every single target. You could 
> also use install_name_tool after building to modify the rpath, but that feels 
> like a kludge.
>
> How would people feel about a patch that just adds the ability to set 
> additional rpaths on liblldb?


+ @beanz for any cmake/osx/framework insights.

Having the ability to add additional rpaths to the (lib)lldb framework seems 
fine, if there's no other way to arrange that simply dropping dependent 
libraries into some directory "just works". It's not just windows where this is 
possible. On linux (and I assume this applies to non-framework osx builds too), 
you can do that by just dropping things into the "$prefix/lib" directory -- in 
our default setup both executables and shared libraries get the rpath of 
"$ORIGIN/../lib" meaning that both executables and shared libraries can find 
any additional deps there. Is it possible to do something like that with the 
framework build too? I.e., set the rpath of the LLDB.framework so that it 
searches for dependencies next to the framework itself? Then you'd just need to 
drop the python framework next to lldb...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69931



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

Re: [Lldb-commits] [lldb] 0877dd1 - [Driver] Force llvm to install its handlers before lldb's

[Pavel Labath via lldb-commits]

On 08/11/2019 01:33, via lldb-commits wrote:

Hey JF, Pavel,

We're still seeing crashes due to SIGPIPE on some lldb bots. This workaround in 
the lldb driver is insufficient, because liblldb.dylib may install its own a 
fresh set of llvm signal handlers (the `NumRegisteredSignals` global is not 
shared by all images loaded in a process). Here is a trace that illustrates the 
issue: https://gist.github.com/vedantk/2d0cc1df9bea9f0fa74ee101d240b82c.

I think we should fix this by changing llvm's default behavior: let's have it 
ignore SIGPIPE. Driver programs (like clang, dwarfdump, etc.) can then opt-in 
to exiting when SIGPIPE is received. Wdyt?


Ah, yes.. I should've realized that wasn't enough.

I agree (and I've alluded to this in the past) that changing the llvm 
behavior is the best option, though ideally, I'd take this a step 
further, and have llvm avoid installing *any* signal handlers by default.


This kind of race is not just limited to SIGPIPE. Other signals suffer 
from the same issues too, it's just that the manifestation of that is 
more subtle.


lldb and liblldb are still racing in who sets the SIGSEGV (etc.) 
handlers last. Since the main effect of those handlers is to produce a 
backtrace and exit, you won't notice this most of the time. But another 
effect of these handlers is to run the RemoveFileOnSignal actions, and 
so the set of files removed might differ since the two lists of files to 
delete are independent too.


(BTW, the fact that this is two copies of llvm racing with each other 
here is unfortunate but irrelevant for this discussion -- the same kind 
of problem would occur if we had llvm and some other entity both trying 
to handle the same signals.)


I think the right behavior for the llvm *library* should be to not 
install *any* signal handlers by default. Instead it should expose some 
API like `runAbnormalExitActions()`, which it's *users* can invoke from 
a signal handler, if they choose so.


Then, the client programs can opt into installing the signal handlers 
and calling this function. This can be done as a part of InitLLVM, and 
it can be the default behavior of InitLLVM. The lldb driver can then 
pass some flag to InitLLVM to disable this handling, or it can just 
continue to override it after the fact, like it does now.





Some alternatives include:

- Add a static initializer to liblldb.dylib that copies the workaround in the 
driver. This should work fine, but it's a duct tape on top of a bandaid.
- Have the dynamic linker coalesce all copies of `NumRegisteredSignals` in a 
process. This would incur an app launch time hit on iOS (where libllvm is hot 
code), and it doesn't seem very portable.


A third option might be to link liblldb and the lldb driver to libllvm 
dynamically. That would solve this problem by not having two copies of 
llvm around, but it would also come with some runtime and binary size 
costs...


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

[Lldb-commits] [lldb] 79b3cce - [lldb][NFC] Refactor some IsClangType checks in ClangASTContext

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-11-08T12:03:28+01:00
New Revision: 79b3cce7f143ebcbc57a3c4599cbd7a1541a742b

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

LOG: [lldb][NFC] Refactor some IsClangType checks in ClangASTContext

Summary:
All type in these functions need be valid and Clang types, so
we might as well replace these checks with IsClangType.

Also lets IsClangType explicitly check for validity instead of
assuming that the TypeSystem is a nullptr.

Subscribers: abidh, JDevlieghere, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Symbol/ClangASTContext.cpp
lldb/source/Symbol/ClangUtil.cpp

Removed: 




diff  --git a/lldb/source/Symbol/ClangASTContext.cpp 
b/lldb/source/Symbol/ClangASTContext.cpp
index 61b08ab9f516..31b62ebb615e 100644
--- a/lldb/source/Symbol/ClangASTContext.cpp
+++ b/lldb/source/Symbol/ClangASTContext.cpp
@@ -3601,7 +3601,7 @@ bool 
ClangASTContext::IsDefined(lldb::opaque_compiler_type_t type) {
 }
 
 bool ClangASTContext::IsObjCClassType(const CompilerType &type) {
-  if (type) {
+  if (ClangUtil::IsClangType(type)) {
 clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));
 
 const clang::ObjCObjectPointerType *obj_pointer_type =
@@ -3886,7 +3886,7 @@ bool 
ClangASTContext::IsBeingDefined(lldb::opaque_compiler_type_t type) {
 
 bool ClangASTContext::IsObjCObjectPointerType(const CompilerType &type,
   CompilerType *class_type_ptr) {
-  if (!type)
+  if (!ClangUtil::IsClangType(type))
 return false;
 
   clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));

diff  --git a/lldb/source/Symbol/ClangUtil.cpp 
b/lldb/source/Symbol/ClangUtil.cpp
index 86be895fadcb..71ff36a5ebab 100644
--- a/lldb/source/Symbol/ClangUtil.cpp
+++ b/lldb/source/Symbol/ClangUtil.cpp
@@ -15,6 +15,10 @@ using namespace clang;
 using namespace lldb_private;
 
 bool ClangUtil::IsClangType(const CompilerType &ct) {
+  // Invalid types are never Clang types.
+  if (!ct)
+return false;
+
   if (llvm::dyn_cast_or_null(ct.GetTypeSystem()) == nullptr)
 return false;
 



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

[Lldb-commits] [PATCH] D70001: [lldb][NFC] Refactor some IsClangType checks in ClangASTContext

[Raphael Isemann via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG79b3cce7f143: [lldb][NFC] Refactor some IsClangType checks 
in ClangASTContext (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70001

Files:
  lldb/source/Symbol/ClangASTContext.cpp
  lldb/source/Symbol/ClangUtil.cpp


Index: lldb/source/Symbol/ClangUtil.cpp
===
--- lldb/source/Symbol/ClangUtil.cpp
+++ lldb/source/Symbol/ClangUtil.cpp
@@ -15,6 +15,10 @@
 using namespace lldb_private;
 
 bool ClangUtil::IsClangType(const CompilerType &ct) {
+  // Invalid types are never Clang types.
+  if (!ct)
+return false;
+
   if (llvm::dyn_cast_or_null(ct.GetTypeSystem()) == nullptr)
 return false;
 
Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -3601,7 +3601,7 @@
 }
 
 bool ClangASTContext::IsObjCClassType(const CompilerType &type) {
-  if (type) {
+  if (ClangUtil::IsClangType(type)) {
 clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));
 
 const clang::ObjCObjectPointerType *obj_pointer_type =
@@ -3886,7 +3886,7 @@
 
 bool ClangASTContext::IsObjCObjectPointerType(const CompilerType &type,
   CompilerType *class_type_ptr) {
-  if (!type)
+  if (!ClangUtil::IsClangType(type))
 return false;
 
   clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));


Index: lldb/source/Symbol/ClangUtil.cpp
===
--- lldb/source/Symbol/ClangUtil.cpp
+++ lldb/source/Symbol/ClangUtil.cpp
@@ -15,6 +15,10 @@
 using namespace lldb_private;
 
 bool ClangUtil::IsClangType(const CompilerType &ct) {
+  // Invalid types are never Clang types.
+  if (!ct)
+return false;
+
   if (llvm::dyn_cast_or_null(ct.GetTypeSystem()) == nullptr)
 return false;
 
Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -3601,7 +3601,7 @@
 }
 
 bool ClangASTContext::IsObjCClassType(const CompilerType &type) {
-  if (type) {
+  if (ClangUtil::IsClangType(type)) {
 clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));
 
 const clang::ObjCObjectPointerType *obj_pointer_type =
@@ -3886,7 +3886,7 @@
 
 bool ClangASTContext::IsObjCObjectPointerType(const CompilerType &type,
   CompilerType *class_type_ptr) {
-  if (!type)
+  if (!ClangUtil::IsClangType(type))
 return false;
 
   clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70001: [lldb][NFC] Refactor some IsClangType checks in ClangASTContext

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
Herald added subscribers: lldb-commits, JDevlieghere, abidh.
Herald added a project: LLDB.

All type in these functions need be valid and Clang types, so
we might as well replace these checks with IsClangType.

  

Also lets IsClangType explicitly check for validity instead of
assuming that the TypeSystem is a nullptr.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D70001

Files:
  lldb/source/Symbol/ClangASTContext.cpp
  lldb/source/Symbol/ClangUtil.cpp


Index: lldb/source/Symbol/ClangUtil.cpp
===
--- lldb/source/Symbol/ClangUtil.cpp
+++ lldb/source/Symbol/ClangUtil.cpp
@@ -15,6 +15,10 @@
 using namespace lldb_private;
 
 bool ClangUtil::IsClangType(const CompilerType &ct) {
+  // Invalid types are never Clang types.
+  if (!ct)
+return false;
+
   if (llvm::dyn_cast_or_null(ct.GetTypeSystem()) == nullptr)
 return false;
 
Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -3601,7 +3601,7 @@
 }
 
 bool ClangASTContext::IsObjCClassType(const CompilerType &type) {
-  if (type) {
+  if (ClangUtil::IsClangType(type)) {
 clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));
 
 const clang::ObjCObjectPointerType *obj_pointer_type =
@@ -3886,7 +3886,7 @@
 
 bool ClangASTContext::IsObjCObjectPointerType(const CompilerType &type,
   CompilerType *class_type_ptr) {
-  if (!type)
+  if (!ClangUtil::IsClangType(type))
 return false;
 
   clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));


Index: lldb/source/Symbol/ClangUtil.cpp
===
--- lldb/source/Symbol/ClangUtil.cpp
+++ lldb/source/Symbol/ClangUtil.cpp
@@ -15,6 +15,10 @@
 using namespace lldb_private;
 
 bool ClangUtil::IsClangType(const CompilerType &ct) {
+  // Invalid types are never Clang types.
+  if (!ct)
+return false;
+
   if (llvm::dyn_cast_or_null(ct.GetTypeSystem()) == nullptr)
 return false;
 
Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -3601,7 +3601,7 @@
 }
 
 bool ClangASTContext::IsObjCClassType(const CompilerType &type) {
-  if (type) {
+  if (ClangUtil::IsClangType(type)) {
 clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));
 
 const clang::ObjCObjectPointerType *obj_pointer_type =
@@ -3886,7 +3886,7 @@
 
 bool ClangASTContext::IsObjCObjectPointerType(const CompilerType &type,
   CompilerType *class_type_ptr) {
-  if (!type)
+  if (!ClangUtil::IsClangType(type))
 return false;
 
   clang::QualType qual_type(ClangUtil::GetCanonicalQualType(type));
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 5a1bac4 - [lldb] Make Target* a Target& in CommandObjectExpression::DoExecute REPL logic

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-11-08T12:28:49+01:00
New Revision: 5a1bac4d1daee2bcbf13365a8254a26d242d8c46

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

LOG: [lldb] Make Target* a Target& in CommandObjectExpression::DoExecute REPL 
logic

Subscribers: JDevlieghere, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectExpression.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectExpression.cpp 
b/lldb/source/Commands/CommandObjectExpression.cpp
index 1f7bfd127dbf..ed63423b11e7 100644
--- a/lldb/source/Commands/CommandObjectExpression.cpp
+++ b/lldb/source/Commands/CommandObjectExpression.cpp
@@ -589,60 +589,58 @@ bool CommandObjectExpression::DoExecute(llvm::StringRef 
command,
   return false;
 
 if (m_repl_option.GetOptionValue().GetCurrentValue()) {
-  Target *target = m_interpreter.GetExecutionContext().GetTargetPtr();
-  if (target) {
-// Drop into REPL
-m_expr_lines.clear();
-m_expr_line_count = 0;
-
-Debugger &debugger = target->GetDebugger();
-
-// Check if the LLDB command interpreter is sitting on top of a REPL
-// that launched it...
-if 
(debugger.CheckTopIOHandlerTypes(IOHandler::Type::CommandInterpreter,
-IOHandler::Type::REPL)) {
-  // the LLDB command interpreter is sitting on top of a REPL that
-  // launched it, so just say the command interpreter is done and
-  // fall back to the existing REPL
-  m_interpreter.GetIOHandler(false)->SetIsDone(true);
-} else {
-  // We are launching the REPL on top of the current LLDB command
-  // interpreter, so just push one
-  bool initialize = false;
-  Status repl_error;
-  REPLSP repl_sp(target->GetREPL(repl_error, 
m_command_options.language,
- nullptr, false));
-
-  if (!repl_sp) {
-initialize = true;
-repl_sp = target->GetREPL(repl_error, m_command_options.language,
-  nullptr, true);
-if (!repl_error.Success()) {
-  result.SetError(repl_error);
-  return result.Succeeded();
-}
+  Target &target = GetSelectedOrDummyTarget();
+  // Drop into REPL
+  m_expr_lines.clear();
+  m_expr_line_count = 0;
+
+  Debugger &debugger = target.GetDebugger();
+
+  // Check if the LLDB command interpreter is sitting on top of a REPL
+  // that launched it...
+  if (debugger.CheckTopIOHandlerTypes(IOHandler::Type::CommandInterpreter,
+  IOHandler::Type::REPL)) {
+// the LLDB command interpreter is sitting on top of a REPL that
+// launched it, so just say the command interpreter is done and
+// fall back to the existing REPL
+m_interpreter.GetIOHandler(false)->SetIsDone(true);
+  } else {
+// We are launching the REPL on top of the current LLDB command
+// interpreter, so just push one
+bool initialize = false;
+Status repl_error;
+REPLSP repl_sp(target.GetREPL(repl_error, m_command_options.language,
+   nullptr, false));
+
+if (!repl_sp) {
+  initialize = true;
+  repl_sp = target.GetREPL(repl_error, m_command_options.language,
+nullptr, true);
+  if (!repl_error.Success()) {
+result.SetError(repl_error);
+return result.Succeeded();
   }
+}
 
-  if (repl_sp) {
-if (initialize) {
-  repl_sp->SetEvaluateOptions(
-  GetExprOptions(exe_ctx, m_command_options));
-  repl_sp->SetFormatOptions(m_format_options);
-  repl_sp->SetValueObjectDisplayOptions(m_varobj_options);
-}
+if (repl_sp) {
+  if (initialize) {
+repl_sp->SetEvaluateOptions(
+GetExprOptions(exe_ctx, m_command_options));
+repl_sp->SetFormatOptions(m_format_options);
+repl_sp->SetValueObjectDisplayOptions(m_varobj_options);
+  }
 
-IOHandlerSP io_handler_sp(repl_sp->GetIOHandler());
+  IOHandlerSP io_handler_sp(repl_sp->GetIOHandler());
 
-io_handler_sp->SetIsDone(false);
+  io_handler_sp->SetIsDone(false);
 
-debugger.PushIOHandler(io_handler_sp);
-  } else {
-repl_error.SetErrorStringWithFormat(
-"Couldn't create a REPL for %s",
-Language::GetNameForLanguageType(m_command_options.la

[Lldb-commits] [PATCH] D70002: [lldb] Make Target* a Target& in CommandObjectExpression::DoExecute REPL logic

[Raphael Isemann via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5a1bac4d1dae: [lldb] Make Target* a Target& in 
CommandObjectExpression::DoExecute REPL logic (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70002

Files:
  lldb/source/Commands/CommandObjectExpression.cpp

Index: lldb/source/Commands/CommandObjectExpression.cpp
===
--- lldb/source/Commands/CommandObjectExpression.cpp
+++ lldb/source/Commands/CommandObjectExpression.cpp
@@ -589,60 +589,58 @@
   return false;
 
 if (m_repl_option.GetOptionValue().GetCurrentValue()) {
-  Target *target = m_interpreter.GetExecutionContext().GetTargetPtr();
-  if (target) {
-// Drop into REPL
-m_expr_lines.clear();
-m_expr_line_count = 0;
-
-Debugger &debugger = target->GetDebugger();
-
-// Check if the LLDB command interpreter is sitting on top of a REPL
-// that launched it...
-if (debugger.CheckTopIOHandlerTypes(IOHandler::Type::CommandInterpreter,
-IOHandler::Type::REPL)) {
-  // the LLDB command interpreter is sitting on top of a REPL that
-  // launched it, so just say the command interpreter is done and
-  // fall back to the existing REPL
-  m_interpreter.GetIOHandler(false)->SetIsDone(true);
-} else {
-  // We are launching the REPL on top of the current LLDB command
-  // interpreter, so just push one
-  bool initialize = false;
-  Status repl_error;
-  REPLSP repl_sp(target->GetREPL(repl_error, m_command_options.language,
- nullptr, false));
-
-  if (!repl_sp) {
-initialize = true;
-repl_sp = target->GetREPL(repl_error, m_command_options.language,
-  nullptr, true);
-if (!repl_error.Success()) {
-  result.SetError(repl_error);
-  return result.Succeeded();
-}
+  Target &target = GetSelectedOrDummyTarget();
+  // Drop into REPL
+  m_expr_lines.clear();
+  m_expr_line_count = 0;
+
+  Debugger &debugger = target.GetDebugger();
+
+  // Check if the LLDB command interpreter is sitting on top of a REPL
+  // that launched it...
+  if (debugger.CheckTopIOHandlerTypes(IOHandler::Type::CommandInterpreter,
+  IOHandler::Type::REPL)) {
+// the LLDB command interpreter is sitting on top of a REPL that
+// launched it, so just say the command interpreter is done and
+// fall back to the existing REPL
+m_interpreter.GetIOHandler(false)->SetIsDone(true);
+  } else {
+// We are launching the REPL on top of the current LLDB command
+// interpreter, so just push one
+bool initialize = false;
+Status repl_error;
+REPLSP repl_sp(target.GetREPL(repl_error, m_command_options.language,
+   nullptr, false));
+
+if (!repl_sp) {
+  initialize = true;
+  repl_sp = target.GetREPL(repl_error, m_command_options.language,
+nullptr, true);
+  if (!repl_error.Success()) {
+result.SetError(repl_error);
+return result.Succeeded();
   }
+}
 
-  if (repl_sp) {
-if (initialize) {
-  repl_sp->SetEvaluateOptions(
-  GetExprOptions(exe_ctx, m_command_options));
-  repl_sp->SetFormatOptions(m_format_options);
-  repl_sp->SetValueObjectDisplayOptions(m_varobj_options);
-}
+if (repl_sp) {
+  if (initialize) {
+repl_sp->SetEvaluateOptions(
+GetExprOptions(exe_ctx, m_command_options));
+repl_sp->SetFormatOptions(m_format_options);
+repl_sp->SetValueObjectDisplayOptions(m_varobj_options);
+  }
 
-IOHandlerSP io_handler_sp(repl_sp->GetIOHandler());
+  IOHandlerSP io_handler_sp(repl_sp->GetIOHandler());
 
-io_handler_sp->SetIsDone(false);
+  io_handler_sp->SetIsDone(false);
 
-debugger.PushIOHandler(io_handler_sp);
-  } else {
-repl_error.SetErrorStringWithFormat(
-"Couldn't create a REPL for %s",
-Language::GetNameForLanguageType(m_command_options.language));
-result.SetError(repl_error);
-return result.Succeeded();
-  }
+  debugger.PushIOHandler(io_handler_sp);
+} else {
+  repl_error.SetErrorStringWithFormat(
+  "Couldn't create a REPL for %s",
+  L

[Lldb-commits] [PATCH] D70002: [lldb] Make Target* a Target& in CommandObjectExpression::DoExecute REPL logic

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
Herald added subscribers: lldb-commits, JDevlieghere.
Herald added a project: LLDB.

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D70002

Files:
  lldb/source/Commands/CommandObjectExpression.cpp

Index: lldb/source/Commands/CommandObjectExpression.cpp
===
--- lldb/source/Commands/CommandObjectExpression.cpp
+++ lldb/source/Commands/CommandObjectExpression.cpp
@@ -589,60 +589,58 @@
   return false;
 
 if (m_repl_option.GetOptionValue().GetCurrentValue()) {
-  Target *target = m_interpreter.GetExecutionContext().GetTargetPtr();
-  if (target) {
-// Drop into REPL
-m_expr_lines.clear();
-m_expr_line_count = 0;
-
-Debugger &debugger = target->GetDebugger();
-
-// Check if the LLDB command interpreter is sitting on top of a REPL
-// that launched it...
-if (debugger.CheckTopIOHandlerTypes(IOHandler::Type::CommandInterpreter,
-IOHandler::Type::REPL)) {
-  // the LLDB command interpreter is sitting on top of a REPL that
-  // launched it, so just say the command interpreter is done and
-  // fall back to the existing REPL
-  m_interpreter.GetIOHandler(false)->SetIsDone(true);
-} else {
-  // We are launching the REPL on top of the current LLDB command
-  // interpreter, so just push one
-  bool initialize = false;
-  Status repl_error;
-  REPLSP repl_sp(target->GetREPL(repl_error, m_command_options.language,
- nullptr, false));
-
-  if (!repl_sp) {
-initialize = true;
-repl_sp = target->GetREPL(repl_error, m_command_options.language,
-  nullptr, true);
-if (!repl_error.Success()) {
-  result.SetError(repl_error);
-  return result.Succeeded();
-}
+  Target &target = GetSelectedOrDummyTarget();
+  // Drop into REPL
+  m_expr_lines.clear();
+  m_expr_line_count = 0;
+
+  Debugger &debugger = target.GetDebugger();
+
+  // Check if the LLDB command interpreter is sitting on top of a REPL
+  // that launched it...
+  if (debugger.CheckTopIOHandlerTypes(IOHandler::Type::CommandInterpreter,
+  IOHandler::Type::REPL)) {
+// the LLDB command interpreter is sitting on top of a REPL that
+// launched it, so just say the command interpreter is done and
+// fall back to the existing REPL
+m_interpreter.GetIOHandler(false)->SetIsDone(true);
+  } else {
+// We are launching the REPL on top of the current LLDB command
+// interpreter, so just push one
+bool initialize = false;
+Status repl_error;
+REPLSP repl_sp(target.GetREPL(repl_error, m_command_options.language,
+   nullptr, false));
+
+if (!repl_sp) {
+  initialize = true;
+  repl_sp = target.GetREPL(repl_error, m_command_options.language,
+nullptr, true);
+  if (!repl_error.Success()) {
+result.SetError(repl_error);
+return result.Succeeded();
   }
+}
 
-  if (repl_sp) {
-if (initialize) {
-  repl_sp->SetEvaluateOptions(
-  GetExprOptions(exe_ctx, m_command_options));
-  repl_sp->SetFormatOptions(m_format_options);
-  repl_sp->SetValueObjectDisplayOptions(m_varobj_options);
-}
+if (repl_sp) {
+  if (initialize) {
+repl_sp->SetEvaluateOptions(
+GetExprOptions(exe_ctx, m_command_options));
+repl_sp->SetFormatOptions(m_format_options);
+repl_sp->SetValueObjectDisplayOptions(m_varobj_options);
+  }
 
-IOHandlerSP io_handler_sp(repl_sp->GetIOHandler());
+  IOHandlerSP io_handler_sp(repl_sp->GetIOHandler());
 
-io_handler_sp->SetIsDone(false);
+  io_handler_sp->SetIsDone(false);
 
-debugger.PushIOHandler(io_handler_sp);
-  } else {
-repl_error.SetErrorStringWithFormat(
-"Couldn't create a REPL for %s",
-Language::GetNameForLanguageType(m_command_options.language));
-result.SetError(repl_error);
-return result.Succeeded();
-  }
+  debugger.PushIOHandler(io_handler_sp);
+} else {
+  repl_error.SetErrorStringWithFormat(
+  "Couldn't create a REPL for %s",
+  Language::GetNameForLanguageType(m_command_options.language));
+  result.SetError(repl_error);
+  return result.Succeeded();
 }
   }
 }
___
lldb-commits mailing list
lldb-commits@lis

[Lldb-commits] [PATCH] D63540: Fix lookup of symbols at the same address with no size vs. size

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid reopened this revision.
omjavaid added a comment.
This revision is now accepted and ready to land.

Hi Jan,

This change has introduced a regression in 32bit arm linux testsuite with 
around 60 new test failures.

Here are list of tests which are failing with this change. Mostly tests are 
failing when an expression is being evaluated. I think we should immediately 
revert this change from 9.x branch as well as master.

It.py

  lldb-api :: 
commands/expression/argument_passing_restrictions/TestArgumentPassingRestrictions.py
  lldb-api :: 
commands/expression/call-overridden-method/TestCallOverriddenMethod.py
  lldb-api :: commands/expression/call-restarts/TestCallThatRestarts.py
  lldb-api :: commands/expression/char/TestExprsChar.py
  lldb-api :: 
commands/expression/class_template_specialization_empty_pack/TestClassTemplateSpecializationParametersHandling.py
  lldb-api :: commands/expression/context-object/TestContextObject.py
  lldb-api :: commands/expression/dont_allow_jit/TestAllowJIT.py
  lldb-api :: commands/expression/expr-in-syscall/TestExpressionInSyscall.py
  lldb-api :: commands/expression/inline-namespace/TestInlineNamespace.py
  lldb-api :: commands/expression/no-deadlock/TestExprDoesntBlock.py
  lldb-api :: commands/expression/persistent_types/TestNestedPersistentTypes.py
  lldb-api :: commands/expression/persistent_types/TestPersistentTypes.py
  lldb-api :: commands/expression/pr35310/TestExprsBug35310.py
  lldb-api :: commands/expression/radar_9531204/TestPrintfAfterUp.py
  lldb-api :: commands/expression/radar_9673664/TestExprHelpExamples.py
  lldb-api :: commands/expression/rdar44436068/Test128BitsInteger.py
  lldb-api :: commands/expression/save_jit_objects/TestSaveJITObjects.py
  lldb-api :: commands/expression/static-initializers/TestStaticInitializers.py
  lldb-api :: commands/expression/test/TestExprs.py
  lldb-api :: commands/expression/timeout/TestCallWithTimeout.py
  lldb-api :: commands/expression/unwind_expression/TestUnwindExpression.py
  lldb-api :: commands/expression/xvalue/TestXValuePrinting.py
  lldb-api :: commands/register/register/register_command/TestRegisters.py
  lldb-api :: commands/source/info/TestSourceInfo.py
  lldb-api :: 
functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py
  lldb-api :: 
functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py
  lldb-api :: 
functionalities/data-formatter/data-formatter-smart-array/TestDataFormatterSmartArray.py
  lldb-api :: 
functionalities/data-formatter/data-formatter-synthval/TestDataFormatterSynthVal.py
  lldb-api :: functionalities/load_unload/TestLoadUnload.py
  lldb-api :: functionalities/load_using_paths/TestLoadUsingPaths.py
  lldb-api :: functionalities/memory/find/TestMemoryFind.py
  lldb-api :: functionalities/process_group/TestChangeProcessGroup.py
  lldb-api :: functionalities/show_location/TestShowLocationDwarf5.py
  lldb-api :: functionalities/signal/handle-abrt/TestHandleAbort.py
  lldb-api :: functionalities/thread/num_threads/TestNumThreads.py
  lldb-api :: lang/c/anonymous/TestAnonymous.py
  lldb-api :: lang/c/bitfields/TestBitfields.py
  lldb-api :: lang/c/enum_types/TestEnumTypes.py
  lldb-api :: lang/c/function_types/TestFunctionTypes.py
  lldb-api :: lang/c/shared_lib/TestSharedLib.py
  lldb-api :: lang/c/strings/TestCStrings.py
  lldb-api :: lang/c/struct_types/TestStructTypes.py
  lldb-api :: lang/cpp/auto/TestCPPAuto.py
  lldb-api :: lang/cpp/call-function/TestCallCPPFunction.py
  lldb-api :: lang/cpp/chained-calls/TestCppChainedCalls.py
  lldb-api :: 
lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py
  lldb-api :: lang/cpp/extern_c/TestExternCSymbols.py
  lldb-api :: lang/cpp/global_operators/TestCppGlobalOperators.py
  lldb-api :: lang/cpp/lambdas/TestLambdas.py
  lldb-api :: lang/cpp/llvm-style/TestLLVMStyle.py
  lldb-api :: lang/cpp/namespace/TestNamespace.py
  lldb-api :: lang/cpp/namespace/TestNamespaceLookup.py
  lldb-api :: lang/cpp/namespace_conflicts/TestNamespaceConflicts.py
  lldb-api :: lang/cpp/namespace_definitions/TestNamespaceDefinitions.py
  lldb-api :: lang/cpp/operators/TestCppOperators.py
  lldb-api :: lang/cpp/overloaded-functions/TestOverloadedFunctions.py
  lldb-api :: lang/cpp/rvalue-references/TestRvalueReferences.py
  lldb-api :: lang/cpp/scope/TestCppScope.py
  lldb-api :: lang/cpp/static_methods/TestCPPStaticMethods.py
  lldb-api :: lang/cpp/symbols/TestSymbols.py
  lldb-api :: lang/cpp/template-function/TestTemplateFunctions.py
  lldb-api :: lang/cpp/template/TestTemplateArgs.py
  lldb-api :: lang/cpp/this/TestCPPThis.py
  lldb-api :: lang/cpp/trivial_abi/TestTrivialABI.py
  lldb-api :: lang/cpp/unicode-literals/TestUnicodeLiterals.py
  lldb-api :: lang/cpp/virtual/TestVirtual.py
  lldb-api :: python_api/sbvalue_persist/TestSBValuePersist.py
  lldb-api :: python_api/thread/TestThreadAPI.py
  lldb-api :: python_api/value/TestValueAPI.py


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https:

[Lldb-commits] [lldb] cdc38c9 - [lldb] Skip parts of TestCallOverriddenMethod.py on Linux

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2019-11-08T15:55:02+01:00
New Revision: cdc38c93fa22f0bee1bd7b84a27c32abb4a3aba8

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

LOG: [lldb] Skip parts of TestCallOverriddenMethod.py on Linux

The function call and the constructor call fail now several Linux
bots (Swift CI, my own bot and Stella's Debian system), so let's disable
the relevant test parts until we can figure out why it is failing.

Added: 


Modified: 

lldb/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
 
b/lldb/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
index 09369f43819c..9c25597391de 100644
--- 
a/lldb/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
+++ 
b/lldb/packages/Python/lldbsuite/test/commands/expression/call-overridden-method/TestCallOverriddenMethod.py
@@ -26,7 +26,7 @@ def setUp(self):
 # Find the line number to break for main.c.
 self.line = line_number('main.cpp', '// Set breakpoint here')
 
-def test(self):
+def test_call_on_base(self):
 """Test calls to overridden methods in derived classes."""
 self.build()
 
@@ -42,14 +42,28 @@ def test(self):
 # class method is never an override).
 self.expect("expr b->foo()", substrs=["2"])
 
+# Test calling the base class.
+self.expect("expr realbase.foo()", substrs=["1"])
+
+@skipIfLinux # Returns wrong result code on some platforms.
+def test_call_on_derived(self):
+"""Test calls to overridden methods in derived classes."""
+self.build()
+
+# Set breakpoint in main and run exe
+self.runCmd("file " + self.getBuildArtifact("a.out"),
+CURRENT_EXECUTABLE_SET)
+lldbutil.run_break_set_by_file_and_line(
+self, "main.cpp", self.line, num_expected_locations=-1, 
loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
 # Test call to overridden method in derived class (this will fail if 
the
 # overrides table is not correctly set up, as Derived::foo will be 
assigned
 # a vtable entry that does not exist in the compiled program).
 self.expect("expr d.foo()", substrs=["2"])
 
-# Test calling the base class.
-self.expect("expr realbase.foo()", substrs=["1"])
-
+@skipIfLinux # Calling constructor causes SIGABRT
 @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr43707")
 def test_call_on_temporary(self):
 """Test calls to overridden methods in derived classes."""



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

[Lldb-commits] [lldb] 454acae - Adapt LLDB to clang API change in ObjCMethodDecl::create().

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2019-11-08T08:59:22-08:00
New Revision: 454acae97ca4ad25cac582afe66c616ad46e4dd1

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

LOG: Adapt LLDB to clang API change in ObjCMethodDecl::create().

Added: 


Modified: 

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
lldb/source/Symbol/ClangASTContext.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
index 1f27a4f0b3ed..e13b0d1b0cd2 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
@@ -330,7 +330,8 @@ class ObjCRuntimeMethodType {
 
 const bool isInstance = instance;
 const bool isVariadic = false;
-const bool isSynthesized = false;
+const bool isPropertyAccessor = false;
+const bool isSynthesizedAccessorStub = false;
 const bool isImplicitlyDeclared = true;
 const bool isDefined = false;
 const clang::ObjCMethodDecl::ImplementationControl impControl =
@@ -377,8 +378,8 @@ class ObjCRuntimeMethodType {
 clang::ObjCMethodDecl *ret = clang::ObjCMethodDecl::Create(
 ast_ctx, clang::SourceLocation(), clang::SourceLocation(), sel,
 ret_type, nullptr, interface_decl, isInstance, isVariadic,
-isSynthesized, isImplicitlyDeclared, isDefined, impControl,
-HasRelatedResultType);
+isPropertyAccessor, isSynthesizedAccessorStub, isImplicitlyDeclared,
+isDefined, impControl, HasRelatedResultType);
 
 std::vector parm_vars;
 

diff  --git a/lldb/source/Symbol/ClangASTContext.cpp 
b/lldb/source/Symbol/ClangASTContext.cpp
index 31b62ebb615e..5c7000ac2f36 100644
--- a/lldb/source/Symbol/ClangASTContext.cpp
+++ b/lldb/source/Symbol/ClangASTContext.cpp
@@ -8489,7 +8489,8 @@ bool ClangASTContext::AddObjCClassProperty(
   ? class_interface_decl->lookupInstanceMethod(getter_sel)
   : class_interface_decl->lookupClassMethod(getter_sel))) {
   const bool isVariadic = false;
-  const bool isSynthesized = false;
+  const bool isPropertyAccessor = false;
+  const bool isSynthesizedAccessorStub = false;
   const bool isImplicitlyDeclared = true;
   const bool isDefined = false;
   const clang::ObjCMethodDecl::ImplementationControl impControl =
@@ -8500,7 +8501,8 @@ bool ClangASTContext::AddObjCClassProperty(
   *clang_ast, clang::SourceLocation(), clang::SourceLocation(),
   getter_sel, 
ClangUtil::GetQualType(property_clang_type_to_access),
   nullptr, class_interface_decl, isInstance, isVariadic,
-  isSynthesized, isImplicitlyDeclared, isDefined, impControl,
+  isPropertyAccessor, isSynthesizedAccessorStub,
+  isImplicitlyDeclared, isDefined, impControl,
   HasRelatedResultType);
 
   if (getter && metadata)
@@ -8521,7 +8523,8 @@ bool ClangASTContext::AddObjCClassProperty(
   : class_interface_decl->lookupClassMethod(setter_sel))) {
   clang::QualType result_type = clang_ast->VoidTy;
   const bool isVariadic = false;
-  const bool isSynthesized = false;
+  const bool isPropertyAccessor = true;
+  const bool isSynthesizedAccessorStub = false;
   const bool isImplicitlyDeclared = true;
   const bool isDefined = false;
   const clang::ObjCMethodDecl::ImplementationControl impControl =
@@ -8531,8 +8534,9 @@ bool ClangASTContext::AddObjCClassProperty(
   clang::ObjCMethodDecl *setter = clang::ObjCMethodDecl::Create(
   *clang_ast, clang::SourceLocation(), clang::SourceLocation(),
   setter_sel, result_type, nullptr, class_interface_decl,
-  isInstance, isVariadic, isSynthesized, isImplicitlyDeclared,
-  isDefined, impControl, HasRelatedResultType);
+  isInstance, isVariadic, isPropertyAccessor,
+  isSynthesizedAccessorStub, isImplicitlyDeclared, isDefined,
+  impControl, HasRelatedResultType);
 
   if (setter && metadata)
 ClangASTContext::SetMetadata(clang_ast, setter, *metadata);
@@ -8634,10 +8638,16 @@ clang::ObjCMethodDecl 
*ClangASTContext::AddMethodToObjCObjectType(
   if (!method_function_prototype)
 return nullptr;
 
-  bool is_synthesized = false;
-  bool is_defined = false;
-  clang::ObjCMethodDecl::ImplementationControl imp_control =
+  const bool isInstance = (name[0] == '-');
+  const bool isVariadic = false;
+  const bool isPropertyAccessor = fals

[Lldb-commits] [lldb] 6b44a41 - [lldb] Prevent Asan/SIP workaround from affecting Python in /usr/local/bin

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2019-11-08T09:08:27-08:00
New Revision: 6b44a41fefc7a62a5024c64fb7453f5ee93f3bb6

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

LOG: [lldb] Prevent Asan/SIP workaround from affecting Python in /usr/local/bin

The code that works around SIP was unintentionally being triggered for
/usr/local/bin/python as well. That caused trouble on GreenDragon where
we were swapping out a Python 3 executable with the system's Python 2
executable.

Added: 


Modified: 
lldb/test/API/lldbtest.py

Removed: 




diff  --git a/lldb/test/API/lldbtest.py b/lldb/test/API/lldbtest.py
index 99eb1457259e..a12544c7cbc6 100644
--- a/lldb/test/API/lldbtest.py
+++ b/lldb/test/API/lldbtest.py
@@ -72,7 +72,7 @@ def execute(self, test, litConfig):
 # copying the binary into a 
diff erent location.
 if 'DYLD_INSERT_LIBRARIES' in test.config.environment and \
 (sys.executable.startswith('/System/') or \
-sys.executable.startswith('/usr/')):
+sys.executable.startswith('/usr/bin/')):
 builddir = getBuildDir(cmd)
 mkdir_p(builddir)
 copied_python = os.path.join(builddir, 'copied-system-python')



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

[Lldb-commits] [lldb] 4d0e07f - [lldb] Make Asan/SIP workaround work for Python 3

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2019-11-08T09:19:56-08:00
New Revision: 4d0e07f7862b832fb49a466feb8046770ea5b792

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

LOG: [lldb] Make Asan/SIP workaround work for Python 3

Make the check generic instead of hard-coding the path to Python 2. This
also fixes the print-syntax to be compatible with both versions.

Added: 


Modified: 
lldb/test/API/lldbtest.py

Removed: 




diff  --git a/lldb/test/API/lldbtest.py b/lldb/test/API/lldbtest.py
index a12544c7cbc6..e7ba5eb3b04d 100644
--- a/lldb/test/API/lldbtest.py
+++ b/lldb/test/API/lldbtest.py
@@ -79,8 +79,10 @@ def execute(self, test, litConfig):
 if not os.path.isfile(copied_python):
 import shutil, subprocess
 python = subprocess.check_output([
-'/usr/bin/python2.7', '-c',
-'import sys; print sys.executable']).strip()
+sys.executable,
+'-c',
+'import sys; print(sys.executable)'
+]).decode('utf-8').strip()
 shutil.copy(python, copied_python)
 cmd[0] = copied_python
 



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

[Lldb-commits] [PATCH] D69931: Add cmake variables to specify a python framework to ship with lldb

[António Afonso via Phabricator via lldb-commits]

aadsm added a comment.

That should work fine yeah. It should be a matter of adding 
`"@loader_path/../../../"` to the rpath.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69931



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

[Lldb-commits] [lldb] 8204d9f - Properly propagate is_variadic.

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2019-11-08T09:53:51-08:00
New Revision: 8204d9ff7ef59bbec9137a5b6679718d03b9cb1b

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

LOG: Properly propagate is_variadic.

This fixes a copy&paste error made when adapting to new clang API
which was promptly caught by the bots.

Added: 


Modified: 
lldb/source/Symbol/ClangASTContext.cpp

Removed: 




diff  --git a/lldb/source/Symbol/ClangASTContext.cpp 
b/lldb/source/Symbol/ClangASTContext.cpp
index 5c7000ac2f36..290ad93b9532 100644
--- a/lldb/source/Symbol/ClangASTContext.cpp
+++ b/lldb/source/Symbol/ClangASTContext.cpp
@@ -8639,7 +8639,7 @@ clang::ObjCMethodDecl 
*ClangASTContext::AddMethodToObjCObjectType(
 return nullptr;
 
   const bool isInstance = (name[0] == '-');
-  const bool isVariadic = false;
+  const bool isVariadic = is_variadic;
   const bool isPropertyAccessor = false;
   const bool isSynthesizedAccessorStub = false;
   /// Force this to true because we don't have source locations.



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

[Lldb-commits] [PATCH] D69931: Add cmake variables to specify a python framework to ship with lldb

[Chris Bieneman via Phabricator via lldb-commits]

beanz added a comment.

It is a bit gross that Python does an @rpath install name, that is generally 
against convention. I can see adding an option to add rpaths to liblldb making 
sense. I certainly agree LLDB shouldn't be in the business of packaging 
transitive dependencies.

In a broader sense, Frameworks are dylibs. They can have all the same rpath 
magic that a dylib has. When you link a macho, the static linker pulls the 
install_name from each dylib you link against and embeds those as the dylibs to 
load. Since Python 3.7's install_name is @rpath based, it means that the 
dynamic linker will look for it in all the @rpath-relative locations specified 
by liblldb when linking. The proposed path in this patch, `-rpath 
"@loader_path/../../../"`, uses the @loader_path expansion which is the 
directory containing the binary that the load command is in (in this case 
liblldb's directory). Popping 3 directories up from that is likely not sane in 
most build configurations, but if it works for you meh...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69931



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

[Lldb-commits] [PATCH] D70002: [lldb] Make Target* a Target& in CommandObjectExpression::DoExecute REPL logic

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

As long as target.GetREPL (..., true) returns a sensible error for the Dummy 
target, this is fine.  Actually having the dummy target run the REPL seems like 
a bad idea, since that's a pretty heavy-weight activity and the dummy target is 
supposed to be just for priming settings for future targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70002



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

[Lldb-commits] [PATCH] D69931: Add cmake variables to specify a python framework to ship with lldb

[Shoaib Meenai via Phabricator via lldb-commits]

smeenai added a comment.

In D69931#1739063 , @beanz wrote:

> It is a bit gross that Python does an @rpath install name, that is generally 
> against convention. I can see adding an option to add rpaths to liblldb 
> making sense. I certainly agree LLDB shouldn't be in the business of 
> packaging transitive dependencies.
>
> In a broader sense, Frameworks are dylibs. They can have all the same rpath 
> magic that a dylib has. When you link a macho, the static linker pulls the 
> install_name from each dylib you link against and embeds those as the dylibs 
> to load. Since Python 3.7's install_name is @rpath based, it means that the 
> dynamic linker will look for it in all the @rpath-relative locations 
> specified by liblldb when linking. The proposed path in this patch, `-rpath 
> "@loader_path/../../../"`, uses the @loader_path expansion which is the 
> directory containing the binary that the load command is in (in this case 
> liblldb's directory). Popping 3 directories up from that is likely not sane 
> in most build configurations, but if it works for you meh...


The assumption here is that you'd have a directory layout like the following:

  |- LLDB.framework
|- Versions
  |- A
|- LLDB
  |- Python3.framework
|- Versions
  |- 3.7
|- Python3

`Python3`'s install name is `@rpath/Python3.framework/Versions/3.7/Python3`, so 
by adding `-rpath "@loader_path/../../.."` to `LLDB`, you make @labath's 
request work:

In D69931#1738285 , @labath wrote:

> Is it possible to do something like that with the framework build too? I.e., 
> set the rpath of the LLDB.framework so that it searches for dependencies next 
> to the framework itself? Then you'd just need to drop the python framework 
> next to lldb...


The assumption, of course, is that LLDB is being built as a framework as well, 
but I believe that's the standard setup on Darwin.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69931



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

[Lldb-commits] [PATCH] D70023: [lldb] [Process/NetBSD] Copy watchpoints to newly-created threads

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, krytarowski.
mgorny added a parent revision: D70022: [lldb] [Process/NetBSD] Improve 
threading support.

NetBSD ptrace interface does not populate watchpoints to newly-created
threads.  Solve this via copying the watchpoints from the current thread
when new thread is reported via TRAP_LWP.


https://reviews.llvm.org/D70023

Files:
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h
  lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h

Index: lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h
===
--- lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h
+++ lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h
@@ -62,6 +62,8 @@
   void SetRunning();
   void SetStepping();
 
+  Status CopyWatchpointsFrom(NativeThreadNetBSD& source);
+
   // Member Variables
   lldb::StateType m_state;
   ThreadStopInfo m_stop_info;
Index: lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
===
--- lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
@@ -247,3 +247,15 @@
 
   return Status("Clearing hardware breakpoint failed.");
 }
+
+Status NativeThreadNetBSD::CopyWatchpointsFrom(NativeThreadNetBSD &source) {
+  Status s = static_cast(GetRegisterContext())
+ .CopyHardwareWatchpointsFrom(
+ static_cast(
+ source.GetRegisterContext()));
+  if (!s.Fail()) {
+m_watchpoint_index_map = source.m_watchpoint_index_map;
+m_hw_break_index_map = source.m_hw_break_index_map;
+  }
+  return s;
+}
Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h
===
--- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h
+++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h
@@ -74,6 +74,10 @@
 
   uint32_t NumSupportedHardwareWatchpoints() override;
 
+  Status
+  CopyHardwareWatchpointsFrom(NativeRegisterContextNetBSD &source)
+  override;
+
 private:
   // Private member types.
   enum { GPRegSet, FPRegSet, XStateRegSet, DBRegSet };
Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
@@ -995,4 +995,15 @@
   return 4;
 }
 
+Status NativeRegisterContextNetBSD_x86_64::CopyHardwareWatchpointsFrom(
+NativeRegisterContextNetBSD &source) {
+  auto &r_source = static_cast(source);
+  Status res = r_source.ReadRegisterSet(DBRegSet);
+  if (!res.Fail()) {
+m_dbr_x86_64 = r_source.m_dbr_x86_64;
+res = WriteRegisterSet(DBRegSet);
+  }
+  return res;
+}
+
 #endif // defined(__x86_64__)
Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h
===
--- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h
+++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h
@@ -30,6 +30,8 @@
   static NativeRegisterContextNetBSD *
   CreateHostNativeRegisterContextNetBSD(const ArchSpec &target_arch,
 NativeThreadProtocol &native_thread);
+  virtual Status
+  CopyHardwareWatchpointsFrom(NativeRegisterContextNetBSD &source) = 0;
 
 protected:
   Status DoRegisterSet(int req, void *buf);
Index: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
===
--- lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
@@ -273,12 +273,14 @@
 }
 
 switch (pst.pe_report_event) {
-  case PTRACE_LWP_CREATE:
+  case PTRACE_LWP_CREATE: {
 LLDB_LOG(log,
  "monitoring new thread, pid = {0}, LWP = {1}",
  GetID(), pst.pe_lwp);
-AddThread(pst.pe_lwp);
-break;
+NativeThreadNetBSD& t = AddThread(pst.pe_lwp);
+t.CopyWatchpointsFrom(
+static_cast(*GetCurrentThread()));
+  } break;
   case PTRACE_LWP_EXIT:
 LLDB_LOG(log,
  "removing exited thread, pid = {0}, LWP = {1}",
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70022: [lldb] [Process/NetBSD] Improve threading support

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, krytarowski.
mgorny added a child revision: D70023: [lldb] [Process/NetBSD] Copy watchpoints 
to newly-created threads.

Implement major improvements to multithreaded program support.  Notably,
support tracking new and exited threads, associate signals and events
with correct threads and support controlling individual threads when
resuming.

Firstly, use PT_SET_EVENT_MASK to enable reporting of created and exited
threads via SIGTRAP.  Handle TRAP_LWP events to keep track
of the currently running threads.

Secondly, update the signal (both generic and SIGTRAP) handling code
to account for per-thread signals correctly.  Signals delivered
to the whole process are reported on all threads, while per-thread
signals and events are reported only to the specific thread.
The remaining threads are marked as 'stopped with no reason'.  Note that
NetBSD always stops all threads on debugger events.

Thirdly, implement the ability to set every thread as running, stopped
or single-stepping separately while continuing the process.  This also
provides the ability to send a signal to the whole process or to one
of its thread while resuming.

Note 1: this is not perfect but the whole series is good enough to commit and 
work on fixing remaining bugs afterwards.

Note 2: the series in general fixes a lot of tests but I'm going to wait with 
test status updates until buildbot confirms.


https://reviews.llvm.org/D70022

Files:
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h
  lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h

Index: lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h
===
--- lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h
+++ lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h
@@ -48,11 +48,16 @@
 private:
   // Interface for friend classes
 
+  Status Resume();
+  Status SingleStep();
+  Status Suspend();
+
   void SetStoppedBySignal(uint32_t signo, const siginfo_t *info = nullptr);
   void SetStoppedByBreakpoint();
   void SetStoppedByTrace();
   void SetStoppedByExec();
   void SetStoppedByWatchpoint(uint32_t wp_index);
+  void SetStoppedWithNoReason();
   void SetStopped();
   void SetRunning();
   void SetStepping();
Index: lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
===
--- lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
@@ -17,6 +17,11 @@
 #include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/State.h"
 
+// clang-format off
+#include 
+#include 
+// clang-format on
+
 #include 
 
 using namespace lldb;
@@ -30,6 +35,38 @@
 NativeRegisterContextNetBSD::CreateHostNativeRegisterContextNetBSD(process.GetArchitecture(), *this)
 ), m_stop_description() {}
 
+Status NativeThreadNetBSD::Resume() {
+  Status ret = NativeProcessNetBSD::PtraceWrapper(PT_RESUME, m_process.GetID(),
+  nullptr, GetID());
+  if (!ret.Success())
+return ret;
+  ret = NativeProcessNetBSD::PtraceWrapper(PT_CLEARSTEP, m_process.GetID(),
+   nullptr, GetID());
+  if (ret.Success())
+SetRunning();
+  return ret;
+}
+
+Status NativeThreadNetBSD::SingleStep() {
+  Status ret = NativeProcessNetBSD::PtraceWrapper(PT_RESUME, m_process.GetID(),
+  nullptr, GetID());
+  if (!ret.Success())
+return ret;
+  ret = NativeProcessNetBSD::PtraceWrapper(PT_SETSTEP, m_process.GetID(),
+   nullptr, GetID());
+  if (ret.Success())
+SetStepping();
+  return ret;
+}
+
+Status NativeThreadNetBSD::Suspend() {
+  Status ret = NativeProcessNetBSD::PtraceWrapper(PT_SUSPEND, m_process.GetID(),
+  nullptr, GetID());
+  if (ret.Success())
+SetStopped();
+  return ret;
+}
+
 void NativeThreadNetBSD::SetStoppedBySignal(uint32_t signo,
 const siginfo_t *info) {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_THREAD));
@@ -89,6 +126,13 @@
   m_stop_info.details.signal.signo = SIGTRAP;
 }
 
+void NativeThreadNetBSD::SetStoppedWithNoReason() {
+  SetStopped();
+
+  m_stop_info.reason = StopReason::eStopReasonNone;
+  m_stop_info.details.signal.signo = 0;
+}
+
 void NativeThreadNetBSD::SetStopped() {
   const StateType new_state = StateType::eStateStopped;
   m_state = new_state;
Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h

[Lldb-commits] [PATCH] D70025: [lldb] [Process/NetBSD] Fix handling concurrent watchpoint events

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, krytarowski.

Fix handling concurrent watchpoint events so that they are reported
correctly in LLDB.

If multiple watchpoints are hit concurrently, the NetBSD kernel reports
them as series of SIGTRAPs with a thread specified, and the debugger
investigates DR6 in order to establish which watchpoint was hit.  This
is normally fine.

However, LLDB disables and reenables the watchpoint on all threads after
each hit, which results in the hit status from DR6 being wiped.
As a result, it can't establish which watchpoint was hit in successive
SIGTRAP processing.

In order to workaround this problem, clear DR6 only if the breakpoint
is overwritten with a new one.  More specifically, move cleaning DR6
from ClearHardwareWatchpoint() to SetHardwareWatchpointWithIndex(),
and do that only if the newly requested watchpoint is different
from the one being set previously.  This ensures that the disable-enable
logic of LLDB does not clear watchpoint hit status for the remaining
threads.

This also involves refactoring of watchpoint logic.  With the old logic,
clearing watchpoint involved wiping dr6 & dr7, and setting it setting
dr{0..3} & dr7.  With the new logic, only enable bit is cleared
from dr7, and the remaining bits are cleared/overwritten while setting
new watchpoint.


https://reviews.llvm.org/D70025

Files:
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h

Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h
===
--- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h
+++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h
@@ -61,6 +61,8 @@
 
   bool ClearHardwareWatchpoint(uint32_t wp_index) override;
 
+  Status ClearWatchpointHit(uint32_t wp_index) override;
+
   Status ClearAllHardwareWatchpoints() override;
 
   Status SetHardwareWatchpointWithIndex(lldb::addr_t addr, size_t size,
Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
@@ -860,10 +860,10 @@
   if (!is_vacant)
 return Status("Watchpoint index not vacant");
 
-  RegisterValue reg_value;
   const RegisterInfo *const reg_info_dr7 =
   GetRegisterInfoAtIndex(lldb_dr7_x86_64);
-  error = ReadRegister(reg_info_dr7, reg_value);
+  RegisterValue dr7_value;
+  error = ReadRegister(reg_info_dr7, dr7_value);
   if (error.Fail())
 return error;
 
@@ -881,16 +881,41 @@
 
   uint64_t bit_mask = (0x3 << (2 * wp_index)) | (0xF << (16 + 4 * wp_index));
 
-  uint64_t control_bits = reg_value.GetAsUInt64() & ~bit_mask;
+  uint64_t control_bits = dr7_value.GetAsUInt64() & ~bit_mask;
 
   control_bits |= enable_bit | rw_bits | size_bits;
 
   const RegisterInfo *const reg_info_drN =
   GetRegisterInfoAtIndex(lldb_dr0_x86_64 + wp_index);
-  error = WriteRegister(reg_info_drN, RegisterValue(addr));
+  RegisterValue drN_value;
+  error = ReadRegister(reg_info_drN, drN_value);
   if (error.Fail())
 return error;
 
+  // clear dr6 if address or bits changed (i.e. we're not reenabling the same
+  // watchpoint)
+  if (drN_value.GetAsUInt64() != addr ||
+  (dr7_value.GetAsUInt64() & bit_mask) != (rw_bits | size_bits)) {
+// for watchpoints 0, 1, 2, or 3, respectively, clear bits 0, 1, 2, or 3 of
+// the debug status register (DR6)
+const RegisterInfo *const reg_info_dr6 =
+GetRegisterInfoAtIndex(lldb_dr6_x86_64);
+RegisterValue dr6_value;
+error = ReadRegister(reg_info_dr6, dr6_value);
+if (error.Fail())
+  return error;
+
+uint64_t dr6_bit_mask = 1 << wp_index;
+uint64_t dr6_bits = dr6_value.GetAsUInt64() & ~dr6_bit_mask;
+error = WriteRegister(reg_info_dr6, RegisterValue(dr6_bits));
+if (error.Fail())
+  return error;
+
+error = WriteRegister(reg_info_drN, RegisterValue(addr));
+if (error.Fail())
+  return error;
+  }
+
   error = WriteRegister(reg_info_dr7, RegisterValue(control_bits));
   if (error.Fail())
 return error;
@@ -904,32 +929,36 @@
   if (wp_index >= NumSupportedHardwareWatchpoints())
 return false;
 
+  // for watchpoints 0, 1, 2, or 3, respectively, clear bits 0-1, 2-3, 4-5
+  // or 6-7 of the debug control register (DR7)
+  const RegisterInfo *const reg_info_dr7 =
+  GetRegisterInfoAtIndex(lldb_dr7_x86_64);
   RegisterValue reg_value;
+  Status error = ReadRegister(reg_info_dr7, reg_value);
+  if (error.Fail())
+return false;
+  uint64_t bit_mask = 0x3 << (2 * wp_index);
+  uint64_t control_

[Lldb-commits] [PATCH] D64647: [lldb] [Process/NetBSD] Multithread support

[Michał Górny via Phabricator via lldb-commits]

mgorny abandoned this revision.
mgorny added a comment.

I'm going to upload a new series shortly.


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

https://reviews.llvm.org/D64647



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

[Lldb-commits] [PATCH] D70022: [lldb] [Process/NetBSD] Improve threading support

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:241
   case TRAP_BRKPT:
-for (const auto &thread : m_threads) {
-  static_cast(*thread).SetStoppedByBreakpoint();
-  FixupBreakpointPCAsNeeded(static_cast(*thread));
+if (thread) {
+  thread->SetStoppedByBreakpoint();

This shall be always true unless there is a kernel issue.
But this check is fine, I would just add a comment.



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:248
   case TRAP_TRACE:
-for (const auto &thread : m_threads)
-  static_cast(*thread).SetStoppedByTrace();
+if (thread)
+  thread->SetStoppedByTrace();

Same here. Isn't there a fixup for PC?

It's not needed in x86 and can be delayed into future.



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:268
+ptrace_state_t pst;
+Status error = PtraceWrapper(PT_GET_PROCESS_STATE, GetID(), &pst,
+ sizeof(pst));

Maybe `GetID()` -> `pid`? Same later.



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:790
+void NativeProcessNetBSD::RemoveThread(lldb::tid_t thread_id) {
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_THREAD));
+  LLDB_LOG(log, "pid {0} removing thread with tid {1}", GetID(), thread_id);

I would add an assert `thread_id > 0`.


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

https://reviews.llvm.org/D70022



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

[Lldb-commits] [PATCH] D70023: [lldb] [Process/NetBSD] Copy watchpoints to newly-created threads

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

How does it deal with `security.models.extensions.user_set_dbregs`? If there is 
a handled error than it's fine.


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

https://reviews.llvm.org/D70023



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

[Lldb-commits] [PATCH] D70025: [lldb] [Process/NetBSD] Fix handling concurrent watchpoint events

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.



Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:334
 
 thread->SetStoppedByTrace();
 SetState(StateType::eStateStopped, true);

I presume that in this code path we land into a scenario that:

1. Trap on a different LWP
2. User sets new watchpoints
3. We land here with a SIGTRAP on old watchpoint that was wiped out.

If so, we shall ignore this report, bail out and resume execution with 
`PT_CONTINUE`.

I think that this path could be some remnant from Linux shared trap reasons.


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

https://reviews.llvm.org/D70025



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

[Lldb-commits] [lldb] 2bbc4fd - Add a testcase for .dSYM path remapping dictionaries.

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2019-11-08T14:07:35-08:00
New Revision: 2bbc4fdd8fa0ed58d610ab6260cb664c7cfef204

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

LOG: Add a testcase for .dSYM path remapping dictionaries.

rdar://problem/56924558

Added: 

lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c
lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile

lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py

Modified: 


Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c
 
b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c
new file mode 100644
index ..556bda3c17d1
--- /dev/null
+++ 
b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c
@@ -0,0 +1,8 @@
+void stop() {}
+
+int main()
+{
+  stop();
+  // Hello World!
+  return 0;
+}

diff  --git 
a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile 
b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile
new file mode 100644
index ..f36a8dc1e671
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile
@@ -0,0 +1,5 @@
+BOTDIR = $(BUILDDIR)/buildbot
+USERDIR = $(BUILDDIR)/user
+C_SOURCES = $(BOTDIR)/main.c
+
+include Makefile.rules

diff  --git 
a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
 
b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
new file mode 100644
index ..d13a04748672
--- /dev/null
+++ 
b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
@@ -0,0 +1,56 @@
+import lldb
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbtest as lldbtest
+import lldbsuite.test.lldbutil as lldbutil
+import os
+import unittest2
+
+
+class TestDSYMSourcePathRemapping(lldbtest.TestBase):
+
+mydir = lldbtest.TestBase.compute_mydir(__file__)
+
+def build(self):
+botdir = self.getBuildArtifact('buildbot')
+userdir = self.getBuildArtifact('user')
+inputs = self.getSourcePath('Inputs')
+lldbutil.mkdir_p(botdir)
+lldbutil.mkdir_p(userdir)
+import shutil
+for f in ['main.c']:
+shutil.copyfile(os.path.join(inputs, f), os.path.join(botdir, f))
+shutil.copyfile(os.path.join(inputs, f), os.path.join(userdir, f))
+
+super(TestDSYMSourcePathRemapping, self).build()
+
+# Remove the build sources.
+self.assertTrue(os.path.isdir(botdir))
+shutil.rmtree(botdir)
+
+# Create a plist.
+import subprocess
+dsym = self.getBuildArtifact('a.out.dSYM')
+uuid = subprocess.check_output(["/usr/bin/dwarfdump", "--uuid", dsym]
+  ).decode("utf-8").split(" ")[1]
+import re
+self.assertTrue(re.match(r'[0-9a-fA-F-]+', uuid))
+plist = os.path.join(dsym, 'Contents', 'Resources', uuid + '.plist')
+   with open(plist, 'w') as f:
+f.write('\n')
+f.write('http://www.apple.com/DTDs/PropertyList-1.0.dtd";>\n')
+   f.write('\n')
+   f.write('\n')
+   f.write('  DBGSourcePathRemapping\n')
+   f.write('  \n')
+   f.write('' + botdir + '\n')
+   f.write('' + userdir + '\n')
+   f.write('  \n')
+   f.write('\n')
+   f.write('\n')
+
+
+@skipIf(debug_info=no_match("dsym"))
+def test(self):
+self.build()
+lldbutil.run_to_name_breakpoint(self, 'main')
+self.expect("source list", substrs=["Hello World"])



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

[Lldb-commits] [PATCH] D69913: Re-enable std::function formatter with fixes to improve non-cached lookup performance

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik updated this revision to Diff 228533.
shafik marked 6 inline comments as done.
shafik added a comment.

After updating branch the stepping test started failing. It was failing because 
the __invoke case was subtly broken in this patch. Fixing that case opened up a 
lot of refactoring opportunities. So this patch ends up being a large refactor 
and in many ways a simplification now.


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

https://reviews.llvm.org/D69913

Files:
  lldb/include/lldb/Symbol/CompileUnit.h
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/function/TestLibCxxFunction.py
  
lldb/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py
  
lldb/packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Symbol/CompileUnit.cpp

Index: lldb/source/Symbol/CompileUnit.cpp
===
--- lldb/source/Symbol/CompileUnit.cpp
+++ lldb/source/Symbol/CompileUnit.cpp
@@ -12,6 +12,7 @@
 #include "lldb/Symbol/SymbolFile.h"
 #include "lldb/Symbol/VariableList.h"
 #include "lldb/Target/Language.h"
+#include "lldb/Utility/Timer.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -81,6 +82,31 @@
   return;
 }
 
+lldb::FunctionSP CompileUnit::FindFunction(
+llvm::function_ref matching_lambda) {
+  static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
+  Timer scoped_timer(func_cat, "CompileUnit::FindFunction");
+
+  lldb::ModuleSP module = CalculateSymbolContextModule();
+
+  if (!module)
+return {};
+
+  SymbolFile *symbol_file = module->GetSymbolFile();
+
+  if (!symbol_file)
+return {};
+
+  // m_functions_by_uid is filled in lazily but we need all the entries.
+  symbol_file->ParseFunctions(*this);
+
+  for (auto &p : m_functions_by_uid) {
+if (matching_lambda(p.second->GetName().GetStringRef()))
+  return p.second;
+  }
+  return {};
+}
+
 // Dump the current contents of this object. No functions that cause on demand
 // parsing of functions, globals, statics are called, so this is a good
 // function to call to get an idea of the current contents of the CompileUnit
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -828,6 +828,8 @@
 }
 
 size_t SymbolFileDWARF::ParseFunctions(CompileUnit &comp_unit) {
+  static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
+  Timer scoped_timer(func_cat, "SymbolFileDWARF::ParseFunctions");
   std::lock_guard guard(GetModuleMutex());
   DWARFUnit *dwarf_cu = GetDWARFCompileUnit(&comp_unit);
   if (!dwarf_cu)
Index: lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -21,6 +21,7 @@
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Core/UniqueCStringMap.h"
 #include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Target/ABI.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/RegisterContext.h"
@@ -28,6 +29,7 @@
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Target/ThreadPlanRunToAddress.h"
 #include "lldb/Target/ThreadPlanStepInRange.h"
+#include "lldb/Utility/Timer.h"
 
 using namespace lldb;
 using namespace lldb_private;
@@ -58,9 +60,53 @@
   return false;
 }
 
+bool contains_lambda_identifier(llvm::StringRef &str_ref) {
+  return str_ref.contains("$_") || str_ref.contains("'lambda'");
+};
+
+CPPLanguageRuntime::LibCppStdFunctionCallableInfo
+line_entry_helper(Target &target, const SymbolContext &sc, Symbol *symbol,
+  llvm::StringRef first_template_param_sref,
+  bool has___invoke) {
+
+  CPPLanguageRuntime::LibCppStdFunctionCallableInfo optional_info;
+
+  AddressRange range;
+  sc.GetAddressRange(eSymbolContextEverything, 0, false, range);
+
+  Address address = range.GetBaseAddress();
+
+  Address addr;
+  if (target.ResolveLoadAddress(address.GetCallableLoadAddress(&target),
+addr)) {
+LineEntry line_entry;
+addr.CalculateSymbolContextLineEntry(line_entry);
+
+if (contains_lambda_identifier(first_template_param_sref) || has___invoke) {
+  // Case 1 and 2
+  optional_info.callable_case = lldb_private::CPPLanguageRuntime::
+  LibCppStdFunctionCallableCase::Lambda;
+} else {
+  // Case 3
+  optional_info.callable_case = lldb_private::C

[Lldb-commits] [PATCH] D69913: Re-enable std::function formatter with fixes to improve non-cached lookup performance

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

@aprantl you will want to take a second look at this, I did some major 
refactoring.


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

https://reviews.llvm.org/D69913



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

[Lldb-commits] [PATCH] D69913: Re-enable std::function formatter with fixes to improve non-cached lookup performance

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Why not have the FindFunctions lambda take a FunctionSP?  It would be easy to 
get the Function name out of the function in the lambda, and that would make 
the function more general (and also match what the Foreach does...


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

https://reviews.llvm.org/D69913



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

[Lldb-commits] [lldb] 1478f36 - Test case to verify that lldb falls back to p/P if g is unsupported

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2019-11-08T15:57:54-08:00
New Revision: 1478f36f27cfe06c5da75ef11fab2d409f2beafe

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

LOG: Test case to verify that lldb falls back to p/P if g is unsupported
and that lldb uses the expedited register values in the ? packet
aka stop packet (T11 etc) and does not re-fetch them with the p packet.

This test is currently failing from the "[lldb-server] Add setting to
force 'g' packet use" commit; I'm marking it as @expectedFailureAll
until we can get this fixed.

Added: 

lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoGPacketSupported.py

Modified: 


Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoGPacketSupported.py
 
b/lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoGPacketSupported.py
new file mode 100644
index ..b22367682030
--- /dev/null
+++ 
b/lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoGPacketSupported.py
@@ -0,0 +1,98 @@
+from __future__ import print_function
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+# This test case is testing three things:
+#
+#  1. three register values will be provided in the ? stop packet (T11) -
+# registers 0 ("rax"), 1 ("rbx"), and 3 ("rip")
+#  2. ReadRegister packet will provide the value of register 2 ("rsi")
+#  3. The "g" read-all-registers packet is not supported; p must be used
+# to get the value of register 2 ("rsi")
+#
+# Forcing lldb to use the expedited registers in the stop packet and
+# marking it an error to request that register value is to prevent
+# performance regressions.
+# 
+# Some gdb RSP stubs only implement p/P, they do not support g/G.
+# lldb must be able to work with either.
+
+class TestNoGPacketSupported(GDBRemoteTestBase):
+
+@skipIfXmlSupportMissing
+@expectedFailureAll
+def test(self):
+class MyResponder(MockGDBServerResponder):
+def haltReason(self):
+return 
"T11thread:1ff0d;threads:1ff0d;thread-pcs:00010001bc00;00:7882773ce0ff;01:1122334455667788;03:00bc01000100;"
+
+def threadStopInfo(self, threadnum):
+return 
"T11thread:1ff0d;threads:1ff0d;thread-pcs:00010001bc00;00:7882773ce0ff;01:1122334455667788;03:00bc01000100;"
+
+def writeRegisters(self):
+return "E02"
+
+def readRegisters(self):
+return "E01"
+
+def readRegister(self, regnum):
+# lldb will try sending "p0" to see if the p packet is 
supported,
+# give a bogus value; in theory lldb could use this value in 
the
+# register context and that would be valid behavior.
+
+# notably, don't give values for registers 1 & 3 -- lldb should
+# get those from the ? stop packet ("T11") and it is a pref 
regression
+# if lldb is asking for these register values.
+if regnum == 0:
+return ""
+if regnum == 2:
+return "c04825ebfe7f" # 0x7ffeeb2548c0
+
+return "E03"
+
+def writeRegister(self, regnum):
+return "OK"
+
+def qXferRead(self, obj, annex, offset, length):
+if annex == "target.xml":
+return """
+
+  i386:x86-64
+  
+
+
+
+
+  
+""", False
+else:
+return None, False
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+self.runCmd("log enable gdb-remote packets")
+if self.TraceOn():
+  self.runCmd("log enable gdb-remote packets")
+  self.addTearDownHook(
+lambda: self.runCmd("log disable gdb-remote packets"))
+process = self.connect(target)
+
+thread = process.GetThreadAtIndex(0)
+frame = thread.GetFrameAtIndex(0)
+rax = frame.FindRegister("rax").GetValueAsUnsigned()
+rbx = frame.FindRegister("rbx").GetValueAsUnsigned()
+rsi = frame.FindRegister("rsi").GetValueAsUnsigned()
+pc = frame.GetPC()
+rip = frame.FindRegister("rip").GetValueAsUnsigned()
+
+if self.TraceOn():
+print("Register values: rax == 0x%x, rbx == 0x%x, rsi == 0x%x, pc 
== 0x%x, rip == 0x%x" % (rax, rbx, rsi

Re: [Lldb-commits] [PATCH] D62931: [lldb-server] Add setting to force 'g' packet use

[Jason Molenda via lldb-commits]

A heads-up - lldb is failing to detect the case where the remote gdb RSP stub 
does not support the 'g' packet.  I found this while doing some bare board 
debugging; g fails and doesn't fall back to fetching register values 
individually.  

I wrote a test TestNoGPacketSupported.py to show this behavior - it's currently 
marked as @expectedFailureAll.  If I add the 
plugin.process.gdb-remote.use-g-packet-for-reading = false setting, the test 
case passes, but of course we can't require people to use that.  lldb has to be 
adaptive to the packets that the remote stub supports.


I'll try to look at the updating the changes to work correctly in this 
environment, but I wanted to raise the issue more widely in case anyone has a 
chance before me. 


J
 

> On Oct 30, 2019, at 9:30 AM, Guilherme Andrade via Phabricator 
>  wrote:
> 
> guiandrade added a comment.
> 
> In D62931#1726865 , @labath wrote:
> 
>> This looks fine to me. Thanks for your patience. Do you still need someone 
>> to commit this for you?
> 
> 
> Np. Yes, I do. Could you please do it for me?
> 
> Thanks!
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D62931/new/
> 
> https://reviews.llvm.org/D62931
> 
> 
> 

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

Re: [Lldb-commits] [PATCH] D62931: [lldb-server] Add setting to force 'g' packet use

[Guilherme Andrade via lldb-commits]

Hey Jason,

Sorry, I think I ended up accidentally dropping the verification in the
final version (
https://github.com/llvm/llvm-project/blob/b1b70f6761266c3eecaf8bd71529eaf51994207b/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp#L307).
I had
implemented GDBRemoteCommunicationClient::GetgPacketSupported previously,
though (https://reviews.llvm.org/D62931?id=226779). So maybe we could just
hook that up again? I.e., read_all_registers_at_once = !pSupported ||
(gdb_process->m_use_g_packet_for_reading && gSupported).

Thanks!

On Fri, Nov 8, 2019 at 7:15 PM Jason Molenda  wrote:

> A heads-up - lldb is failing to detect the case where the remote gdb RSP
> stub does not support the 'g' packet.  I found this while doing some bare
> board debugging; g fails and doesn't fall back to fetching register values
> individually.
>
> I wrote a test TestNoGPacketSupported.py to show this behavior - it's
> currently marked as @expectedFailureAll.  If I add the
> plugin.process.gdb-remote.use-g-packet-for-reading = false setting, the
> test case passes, but of course we can't require people to use that.  lldb
> has to be adaptive to the packets that the remote stub supports.
>
>
> I'll try to look at the updating the changes to work correctly in this
> environment, but I wanted to raise the issue more widely in case anyone has
> a chance before me.
>
>
> J
>
>
> > On Oct 30, 2019, at 9:30 AM, Guilherme Andrade via Phabricator <
> revi...@reviews.llvm.org> wrote:
> >
> > guiandrade added a comment.
> >
> > In D62931#1726865 , @labath
> wrote:
> >
> >> This looks fine to me. Thanks for your patience. Do you still need
> someone to commit this for you?
> >
> >
> > Np. Yes, I do. Could you please do it for me?
> >
> > Thanks!
> >
> >
> > Repository:
> >  rG LLVM Github Monorepo
> >
> > CHANGES SINCE LAST ACTION
> >  https://reviews.llvm.org/D62931/new/
> >
> > https://reviews.llvm.org/D62931
> >
> >
> >
>
>

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

Re: [Lldb-commits] [PATCH] D62931: [lldb-server] Add setting to force 'g' packet use

[Jason Molenda via lldb-commits]

Hm, a follow-on problem is that there's some bug between debugserver and lldb 
with the g/G packets which is causing bot failures on macos systems. lldb has 
never used g/G before (if p/P was available) because debugserver seeds all of 
the GPR values with the stop packet (? aka Tnn) or with the jThreadsInfo packet 
when we have a public stop and want the GPRs for all the threads, so there was 
no driving perf need.  

g/G would be a perf benefit if you were fetching the floating point registers 
on every step, which could definitely happen in an IDE, but it's not common, so 
we stuck with the simpler p/P.

Given that TestRegisters.py and some filecheck tests are failing on macos the 
bots, I think it might be best to change the default value for 
plugin.process.gdb-remote.use-g-packet-for-reading to false until next week 
when we can get to the bottom of the debugserver g/G issue.  

I'm not sure of the configuration of the bots; I think some of them use the 
installed debugserver binaries (which are code signed by apple etc) instead of 
the just-built one.  I'm not sure how we'll structure TestRegisters.py and the 
filecheck tests to handle the difference correctly.  We can figure that out 
next week.


J

> On Nov 8, 2019, at 4:15 PM, Jason Molenda  wrote:
> 
> A heads-up - lldb is failing to detect the case where the remote gdb RSP stub 
> does not support the 'g' packet.  I found this while doing some bare board 
> debugging; g fails and doesn't fall back to fetching register values 
> individually.  
> 
> I wrote a test TestNoGPacketSupported.py to show this behavior - it's 
> currently marked as @expectedFailureAll.  If I add the 
> plugin.process.gdb-remote.use-g-packet-for-reading = false setting, the test 
> case passes, but of course we can't require people to use that.  lldb has to 
> be adaptive to the packets that the remote stub supports.
> 
> 
> I'll try to look at the updating the changes to work correctly in this 
> environment, but I wanted to raise the issue more widely in case anyone has a 
> chance before me. 
> 
> 
> J
> 
> 
>> On Oct 30, 2019, at 9:30 AM, Guilherme Andrade via Phabricator 
>>  wrote:
>> 
>> guiandrade added a comment.
>> 
>> In D62931#1726865 , @labath wrote:
>> 
>>> This looks fine to me. Thanks for your patience. Do you still need someone 
>>> to commit this for you?
>> 
>> 
>> Np. Yes, I do. Could you please do it for me?
>> 
>> Thanks!
>> 
>> 
>> Repository:
>> rG LLVM Github Monorepo
>> 
>> CHANGES SINCE LAST ACTION
>> https://reviews.llvm.org/D62931/new/
>> 
>> https://reviews.llvm.org/D62931
>> 
>> 
>> 
> 

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

[Lldb-commits] [PATCH] D70037: Fix a regression in macOS-style path remapping.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: JDevlieghere, labath, jasonmolenda.

When we switched to the LLVM .debug_line parser, the .dSYM-style path remapping 
logic stopped working for relative paths because of how RemapSourceFile 
silently fails for relative paths. This patch both makes the code more readable 
and fixes this particular bug.

One interesting thing I learned is that `Module::RemapSourceFile()` is a 
macOS-only code path that operates on on the `lldb::Module` level and is 
completely separate from `target.source-map`, which operates on a per-Target 
level.


https://reviews.llvm.org/D70037

Files:
  
lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c
  
lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/relative.c
  lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile
  
lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -179,6 +179,23 @@
   return *line_table;
 }
 
+static llvm::Optional
+GetFileByIndex(const llvm::DWARFDebugLine::Prologue &prologue, size_t idx,
+   llvm::StringRef compile_dir, FileSpec::Style style) {
+  // Try to get an absolute path first.
+  std::string abs_path;
+  auto absolute = llvm::DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath;
+  if (prologue.getFileNameByIndex(idx, compile_dir, absolute, abs_path, style))
+return std::move(abs_path);
+
+  // Otherwise ask for a relative path.
+  std::string rel_path;
+  auto relative = llvm::DILineInfoSpecifier::FileLineInfoKind::Default;
+  if (!prologue.getFileNameByIndex(idx, compile_dir, relative, rel_path, style))
+return {};
+  return std::move(rel_path);
+}
+
 static FileSpecList ParseSupportFilesFromPrologue(
 const lldb::ModuleSP &module,
 const llvm::DWARFDebugLine::Prologue &prologue, FileSpec::Style style,
@@ -188,27 +205,12 @@
 
   const size_t number_of_files = prologue.FileNames.size();
   for (size_t idx = 1; idx <= number_of_files; ++idx) {
-std::string original_file;
-if (!prologue.getFileNameByIndex(
-idx, compile_dir,
-llvm::DILineInfoSpecifier::FileLineInfoKind::Default, original_file,
-style)) {
-  // Always add an entry so the indexes remain correct.
-  support_files.EmplaceBack();
-  continue;
-}
-
 std::string remapped_file;
-if (!prologue.getFileNameByIndex(
-idx, compile_dir,
-llvm::DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath,
-remapped_file, style)) {
-  // Always add an entry so the indexes remain correct.
-  support_files.EmplaceBack(original_file, style);
-  continue;
-}
+if (auto file_path = GetFileByIndex(prologue, idx, compile_dir, style))
+  if (!module->RemapSourceFile(llvm::StringRef(*file_path), remapped_file))
+remapped_file = std::move(*file_path);
 
-module->RemapSourceFile(llvm::StringRef(original_file), remapped_file);
+// Unconditionally add an entry, so the indices match up.
 support_files.EmplaceBack(remapped_file, style);
   }
 
Index: lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
===
--- lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
+++ lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
@@ -17,7 +17,7 @@
 lldbutil.mkdir_p(botdir)
 lldbutil.mkdir_p(userdir)
 import shutil
-for f in ['main.c']:
+for f in ['main.c', 'relative.c']:
 shutil.copyfile(os.path.join(inputs, f), os.path.join(botdir, f))
 shutil.copyfile(os.path.join(inputs, f), os.path.join(userdir, f))
 
@@ -52,5 +52,10 @@
 @skipIf(debug_info=no_match("dsym"))
 def test(self):
 self.build()
-lldbutil.run_to_name_breakpoint(self, 'main')
-self.expect("source list", substrs=["Hello World"])
+
+target, process, _, _ = lldbutil.run_to_name_breakpoint(
+self, 'main')
+self.expect("source list -n main", substrs=["Hello Absolute"])
+bkpt = target.BreakpointCreateByName('relative')
+lldbutil.continue_to_breakpoint(process, bkpt)
+self.expect("source list -n relative", substrs=["Hello Relative"])
Index: lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile
===
--- lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile
+++ lldb/packa

[Lldb-commits] [PATCH] D70037: Fix a regression in macOS-style path remapping.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

Sorry for breaking this functionality & thank you for fixing it!


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

https://reviews.llvm.org/D70037



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

Re: [Lldb-commits] [lldb] 0877dd1 - [Driver] Force llvm to install its handlers before lldb's

[via lldb-commits]

> On Nov 8, 2019, at 1:17 AM, Pavel Labath  wrote:
> 
> On 08/11/2019 01:33, via lldb-commits wrote:
>> Hey JF, Pavel,
>> We're still seeing crashes due to SIGPIPE on some lldb bots. This workaround 
>> in the lldb driver is insufficient, because liblldb.dylib may install its 
>> own a fresh set of llvm signal handlers (the `NumRegisteredSignals` global 
>> is not shared by all images loaded in a process). Here is a trace that 
>> illustrates the issue: 
>> https://gist.github.com/vedantk/2d0cc1df9bea9f0fa74ee101d240b82c.
>> I think we should fix this by changing llvm's default behavior: let's have 
>> it ignore SIGPIPE. Driver programs (like clang, dwarfdump, etc.) can then 
>> opt-in to exiting when SIGPIPE is received. Wdyt?
> 
> Ah, yes.. I should've realized that wasn't enough.
> 
> I agree (and I've alluded to this in the past) that changing the llvm 
> behavior is the best option, though ideally, I'd take this a step further, 
> and have llvm avoid installing *any* signal handlers by default.
> 
> This kind of race is not just limited to SIGPIPE. Other signals suffer from 
> the same issues too, it's just that the manifestation of that is more subtle.
> 
> lldb and liblldb are still racing in who sets the SIGSEGV (etc.) handlers 
> last. Since the main effect of those handlers is to produce a backtrace and 
> exit, you won't notice this most of the time. But another effect of these 
> handlers is to run the RemoveFileOnSignal actions, and so the set of files 
> removed might differ since the two lists of files to delete are independent 
> too.
> 
> (BTW, the fact that this is two copies of llvm racing with each other here is 
> unfortunate but irrelevant for this discussion -- the same kind of problem 
> would occur if we had llvm and some other entity both trying to handle the 
> same signals.)

One wrinkle is that the two copies of llvm will track different sets of files 
to remove on a signal. In practice, this doesn't seem to be an issue. But the 
current situation is that if the lldb process gets a SIGINT, any 
files-to-be-removed registered by liblldb get removed, but any 
files-to-be-removed registered by the lldb driver stick around.


> I think the right behavior for the llvm *library* should be to not install 
> *any* signal handlers by default. Instead it should expose some API like 
> `runAbnormalExitActions()`, which it's *users* can invoke from a signal 
> handler, if they choose so.
> 
> Then, the client programs can opt into installing the signal handlers and 
> calling this function. This can be done as a part of InitLLVM, and it can be 
> the default behavior of InitLLVM. The lldb driver can then pass some flag to 
> InitLLVM to disable this handling, or it can just continue to override it 
> after the fact, like it does now.

One advantage of this approach is that existing llvm tools without custom 
signal handling wouldn't have to change.

To make this work, the dylib would need to install an interrupt handler that 1) 
runs llvm::runAbnormalExitActions() to clean up files, then 2) locates & runs 
the lldb driver's handler (to do `GetDebugger().DispatchInputInterrupt()`).

And we couldn't use a static initializer to do it, because *then* 
liblldb.dylib's handler would get installed before the lldb driver's. I think 
this approach necessitates a special entry point into the dylib that just 
installs handlers.


>> Some alternatives include:
>> - Add a static initializer to liblldb.dylib that copies the workaround in 
>> the driver. This should work fine, but it's a duct tape on top of a bandaid.
>> - Have the dynamic linker coalesce all copies of `NumRegisteredSignals` in a 
>> process. This would incur an app launch time hit on iOS (where libllvm is 
>> hot code), and it doesn't seem very portable.
> 
> A third option might be to link liblldb and the lldb driver to libllvm 
> dynamically. That would solve this problem by not having two copies of llvm 
> around, but it would also come with some runtime and binary size costs...

Yeah, that might be too big a hammer.

I just talked to Jonas about it offline and both of us think we should just 
make llvm's current SIGPIPE handling opt-in, as (imo) it's a lot simpler than 
the alternatives we've mentioned so far. @Pavel it sounded like you were ok 
with this in principle -- anyone have any objections?

vedant

> 
> pl

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

[Lldb-commits] [lldb] 441a785 - Revert "Add a testcase for .dSYM path remapping dictionaries."

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2019-11-08T18:16:44-08:00
New Revision: 441a78533e61cfffb3fd59e2c169ca7ff7b286dc

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

LOG: Revert "Add a testcase for .dSYM path remapping dictionaries."

This reverts commit 2bbc4fdd8fa0ed58d610ab6260cb664c7cfef204.

Added: 


Modified: 


Removed: 

lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c
lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile

lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py



diff  --git 
a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c
 
b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c
deleted file mode 100644
index 556bda3c17d1..
--- 
a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c
+++ /dev/null
@@ -1,8 +0,0 @@
-void stop() {}
-
-int main()
-{
-  stop();
-  // Hello World!
-  return 0;
-}

diff  --git 
a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile 
b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile
deleted file mode 100644
index f36a8dc1e671..
--- a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile
+++ /dev/null
@@ -1,5 +0,0 @@
-BOTDIR = $(BUILDDIR)/buildbot
-USERDIR = $(BUILDDIR)/user
-C_SOURCES = $(BOTDIR)/main.c
-
-include Makefile.rules

diff  --git 
a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
 
b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
deleted file mode 100644
index d13a04748672..
--- 
a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
+++ /dev/null
@@ -1,56 +0,0 @@
-import lldb
-from lldbsuite.test.decorators import *
-import lldbsuite.test.lldbtest as lldbtest
-import lldbsuite.test.lldbutil as lldbutil
-import os
-import unittest2
-
-
-class TestDSYMSourcePathRemapping(lldbtest.TestBase):
-
-mydir = lldbtest.TestBase.compute_mydir(__file__)
-
-def build(self):
-botdir = self.getBuildArtifact('buildbot')
-userdir = self.getBuildArtifact('user')
-inputs = self.getSourcePath('Inputs')
-lldbutil.mkdir_p(botdir)
-lldbutil.mkdir_p(userdir)
-import shutil
-for f in ['main.c']:
-shutil.copyfile(os.path.join(inputs, f), os.path.join(botdir, f))
-shutil.copyfile(os.path.join(inputs, f), os.path.join(userdir, f))
-
-super(TestDSYMSourcePathRemapping, self).build()
-
-# Remove the build sources.
-self.assertTrue(os.path.isdir(botdir))
-shutil.rmtree(botdir)
-
-# Create a plist.
-import subprocess
-dsym = self.getBuildArtifact('a.out.dSYM')
-uuid = subprocess.check_output(["/usr/bin/dwarfdump", "--uuid", dsym]
-  ).decode("utf-8").split(" ")[1]
-import re
-self.assertTrue(re.match(r'[0-9a-fA-F-]+', uuid))
-plist = os.path.join(dsym, 'Contents', 'Resources', uuid + '.plist')
-   with open(plist, 'w') as f:
-f.write('\n')
-f.write('http://www.apple.com/DTDs/PropertyList-1.0.dtd";>\n')
-   f.write('\n')
-   f.write('\n')
-   f.write('  DBGSourcePathRemapping\n')
-   f.write('  \n')
-   f.write('' + botdir + '\n')
-   f.write('' + userdir + '\n')
-   f.write('  \n')
-   f.write('\n')
-   f.write('\n')
-
-
-@skipIf(debug_info=no_match("dsym"))
-def test(self):
-self.build()
-lldbutil.run_to_name_breakpoint(self, 'main')
-self.expect("source list", substrs=["Hello World"])



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

Re: [Lldb-commits] [lldb] 2bbc4fd - Add a testcase for .dSYM path remapping dictionaries.

[Jonas Devlieghere via lldb-commits]

This is failing on GreenDragon, so I've reverted it. Can you please
have a look on Monday?

On Fri, Nov 8, 2019 at 2:07 PM Adrian Prantl via lldb-commits
 wrote:
>
>
> Author: Adrian Prantl
> Date: 2019-11-08T14:07:35-08:00
> New Revision: 2bbc4fdd8fa0ed58d610ab6260cb664c7cfef204
>
> URL: 
> https://github.com/llvm/llvm-project/commit/2bbc4fdd8fa0ed58d610ab6260cb664c7cfef204
> DIFF: 
> https://github.com/llvm/llvm-project/commit/2bbc4fdd8fa0ed58d610ab6260cb664c7cfef204.diff
>
> LOG: Add a testcase for .dSYM path remapping dictionaries.
>
> rdar://problem/56924558
>
> Added:
> 
> lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c
> lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile
> 
> lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
>
> Modified:
>
>
> Removed:
>
>
>
> 
> diff  --git 
> a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c
>  
> b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c
> new file mode 100644
> index ..556bda3c17d1
> --- /dev/null
> +++ 
> b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Inputs/main.c
> @@ -0,0 +1,8 @@
> +void stop() {}
> +
> +int main()
> +{
> +  stop();
> +  // Hello World!
> +  return 0;
> +}
>
> diff  --git 
> a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile 
> b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile
> new file mode 100644
> index ..f36a8dc1e671
> --- /dev/null
> +++ 
> b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/Makefile
> @@ -0,0 +1,5 @@
> +BOTDIR = $(BUILDDIR)/buildbot
> +USERDIR = $(BUILDDIR)/user
> +C_SOURCES = $(BOTDIR)/main.c
> +
> +include Makefile.rules
>
> diff  --git 
> a/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
>  
> b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
> new file mode 100644
> index ..d13a04748672
> --- /dev/null
> +++ 
> b/lldb/packages/Python/lldbsuite/test/macosx/DBGSourcePathRemapping/TestDSYMSourcePathRemapping.py
> @@ -0,0 +1,56 @@
> +import lldb
> +from lldbsuite.test.decorators import *
> +import lldbsuite.test.lldbtest as lldbtest
> +import lldbsuite.test.lldbutil as lldbutil
> +import os
> +import unittest2
> +
> +
> +class TestDSYMSourcePathRemapping(lldbtest.TestBase):
> +
> +mydir = lldbtest.TestBase.compute_mydir(__file__)
> +
> +def build(self):
> +botdir = self.getBuildArtifact('buildbot')
> +userdir = self.getBuildArtifact('user')
> +inputs = self.getSourcePath('Inputs')
> +lldbutil.mkdir_p(botdir)
> +lldbutil.mkdir_p(userdir)
> +import shutil
> +for f in ['main.c']:
> +shutil.copyfile(os.path.join(inputs, f), os.path.join(botdir, f))
> +shutil.copyfile(os.path.join(inputs, f), os.path.join(userdir, 
> f))
> +
> +super(TestDSYMSourcePathRemapping, self).build()
> +
> +# Remove the build sources.
> +self.assertTrue(os.path.isdir(botdir))
> +shutil.rmtree(botdir)
> +
> +# Create a plist.
> +import subprocess
> +dsym = self.getBuildArtifact('a.out.dSYM')
> +uuid = subprocess.check_output(["/usr/bin/dwarfdump", "--uuid", dsym]
> +  ).decode("utf-8").split(" ")[1]
> +import re
> +self.assertTrue(re.match(r'[0-9a-fA-F-]+', uuid))
> +plist = os.path.join(dsym, 'Contents', 'Resources', uuid + '.plist')
> +   with open(plist, 'w') as f:
> +f.write('\n')
> +f.write(' "http://www.apple.com/DTDs/PropertyList-1.0.dtd";>\n')
> +   f.write('\n')
> +   f.write('\n')
> +   f.write('  DBGSourcePathRemapping\n')
> +   f.write('  \n')
> +   f.write('' + botdir + '\n')
> +   f.write('' + userdir + '\n')
> +   f.write('  \n')
> +   f.write('\n')
> +   f.write('\n')
> +
> +
> +@skipIf(debug_info=no_match("dsym"))
> +def test(self):
> +self.build()
> +lldbutil.run_to_name_breakpoint(self, 'main')
> +self.expect("source list", substrs=["Hello World"])
>
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 60ab30e - Temporarily change the default for use-g-packet-for-reading to false,

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2019-11-08T18:21:57-08:00
New Revision: 60ab30ebce833c87bd4776f67cd9a82fe162ef9c

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

LOG: Temporarily change the default for use-g-packet-for-reading to false,
until we can automatically fall back to p/P if g/G are not supported;
it looks like there is a bug in debugserver's g/G packets taht needs
to be fixed, or debugserver should stop supporting g/G until that bug
is fixed.  But we need lldb to be able to fall back to p/P correctly
for that to be a viable workaround.

Added: 


Modified: 

lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py

lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoGPacketSupported.py
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
 
b/lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
index fd13b1f2cd64..8f0ed9a4933d 100644
--- 
a/lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ 
b/lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -52,6 +52,10 @@ def vAttach(self, pid):
 
 def test_read_registers_using_g_packets(self):
 """Test reading registers using 'g' packets (default behavior)"""
+self.dbg.HandleCommand(
+"settings set 
plugin.process.gdb-remote.use-g-packet-for-reading true")
+self.addTearDownHook(lambda: 
+self.runCmd("settings set 
plugin.process.gdb-remote.use-g-packet-for-reading false"))
 self.server.responder = self.gPacketResponder()
 target = self.createTarget("a.yaml")
 process = self.connect(target)

diff  --git 
a/lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoGPacketSupported.py
 
b/lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoGPacketSupported.py
index b22367682030..989684c62229 100644
--- 
a/lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoGPacketSupported.py
+++ 
b/lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestNoGPacketSupported.py
@@ -23,7 +23,6 @@
 class TestNoGPacketSupported(GDBRemoteTestBase):
 
 @skipIfXmlSupportMissing
-@expectedFailureAll
 def test(self):
 class MyResponder(MockGDBServerResponder):
 def haltReason(self):

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
index 4c8202945501..9cbe3d40ca2c 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
@@ -15,6 +15,6 @@ let Definition = "processgdbremote" in {
 Desc<"If true, the libraries-svr4 feature will be used to get a hold of 
the process's loaded modules.">;
   def UseGPacketForReading: Property<"use-g-packet-for-reading", "Boolean">,
 Global,
-DefaultTrue,
+DefaultFalse,
 Desc<"Specify if the server should use 'g' packets to read registers.">;
 }



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

Re: [Lldb-commits] [PATCH] D62931: [lldb-server] Add setting to force 'g' packet use

[Jason Molenda via lldb-commits]

I'm switching the default for at least the weekend via 
60ab30ebce833c87bd4776f67cd9a82fe162ef9c / 
https://reviews.llvm.org/rG60ab30ebce83 so the bots aren't failing because of 
this, we can all look into this next week.  I think the best solution is to get 
lldb to fall back to p/P if g/G are not supported (which we need to talk to 
some targets), and disable debugserver's g/G packet support until I can debug 
where the bug is over there.

I'm still concerned about some of the macos CI bots which use the installed 
debugservers, which will continue to have this g/G bug for a while, we'll 
figure all that out next week.



> On Nov 8, 2019, at 5:41 PM, Jason Molenda  wrote:
> 
> Hm, a follow-on problem is that there's some bug between debugserver and lldb 
> with the g/G packets which is causing bot failures on macos systems. lldb has 
> never used g/G before (if p/P was available) because debugserver seeds all of 
> the GPR values with the stop packet (? aka Tnn) or with the jThreadsInfo 
> packet when we have a public stop and want the GPRs for all the threads, so 
> there was no driving perf need.  
> 
> g/G would be a perf benefit if you were fetching the floating point registers 
> on every step, which could definitely happen in an IDE, but it's not common, 
> so we stuck with the simpler p/P.
> 
> Given that TestRegisters.py and some filecheck tests are failing on macos the 
> bots, I think it might be best to change the default value for 
> plugin.process.gdb-remote.use-g-packet-for-reading to false until next week 
> when we can get to the bottom of the debugserver g/G issue.  
> 
> I'm not sure of the configuration of the bots; I think some of them use the 
> installed debugserver binaries (which are code signed by apple etc) instead 
> of the just-built one.  I'm not sure how we'll structure TestRegisters.py and 
> the filecheck tests to handle the difference correctly.  We can figure that 
> out next week.
> 
> 
> J
> 
>> On Nov 8, 2019, at 4:15 PM, Jason Molenda  wrote:
>> 
>> A heads-up - lldb is failing to detect the case where the remote gdb RSP 
>> stub does not support the 'g' packet.  I found this while doing some bare 
>> board debugging; g fails and doesn't fall back to fetching register values 
>> individually.  
>> 
>> I wrote a test TestNoGPacketSupported.py to show this behavior - it's 
>> currently marked as @expectedFailureAll.  If I add the 
>> plugin.process.gdb-remote.use-g-packet-for-reading = false setting, the test 
>> case passes, but of course we can't require people to use that.  lldb has to 
>> be adaptive to the packets that the remote stub supports.
>> 
>> 
>> I'll try to look at the updating the changes to work correctly in this 
>> environment, but I wanted to raise the issue more widely in case anyone has 
>> a chance before me. 
>> 
>> 
>> J
>> 
>> 
>>> On Oct 30, 2019, at 9:30 AM, Guilherme Andrade via Phabricator 
>>>  wrote:
>>> 
>>> guiandrade added a comment.
>>> 
>>> In D62931#1726865 , @labath wrote:
>>> 
 This looks fine to me. Thanks for your patience. Do you still need someone 
 to commit this for you?
>>> 
>>> 
>>> Np. Yes, I do. Could you please do it for me?
>>> 
>>> Thanks!
>>> 
>>> 
>>> Repository:
>>> rG LLVM Github Monorepo
>>> 
>>> CHANGES SINCE LAST ACTION
>>> https://reviews.llvm.org/D62931/new/
>>> 
>>> https://reviews.llvm.org/D62931
>>> 
>>> 
>>> 
>> 
> 

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