Re: [Lldb-commits] [lldb] 8612e92 - [lldb][NFC] Remove GetASTContext call in ClangDeclVendor

2020-01-03 Thread Raphael “Teemperor” Isemann via lldb-commits 
We could use clang::Decl* etc. in these classes but that would defeat the whole 
concept that they are supposed to implement. CompilerType, CompilerDecl, etc.. 
are all classes that represent their respective equivalent in a language 
plugin. For Clang that is indeed clang::Type* and clang::Decl*, but Swift is 
storing swift::Type* and swift::Decl* in these void* pointers (and passes a 
SwiftASTContext* instead of a ClangASTContext* as a m_type_system). So if we 
change that to clang::Decl* then the all other language plugins like Swift are 
effectively broken. It would also mean that generic LLDB code using these 
classes would have a hard dependency on Clang.

And there shouldn’t be any place where we unconditionally cast this to 
clang::Decl* as this would directly crash swift-lldb. The m_type_system (which 
is either ClangASTContext or SwiftASTContext) can freely cast the void* to 
clang::Decl/swift::Decl as the checking happens through the virtual function 
call to the m_type_system interface (Swift types always have a SwiftASTContext 
as m_type_system, Clang types have a ClangASTContext as m_type_system).

Having said that, I’m not saying that the void* pointers in these classes are 
the best solution we could have here (there is a reason why I keep removing all 
these void* pointer conversions). If you see a way to get rid of them, then be 
my guest.

> On 3. Jan 2020, at 01:52, Shafik Yaghmour  wrote:
> 
> To further clarify all the member functions of CompilerDecl e.g.:
> 
> GetMangledName(…)
> GetDeclContext(…)
> 
> End up passing m_opaque_decl as an argument to a member function of 
> m_type_system and in these member functions they unconditionally cast the 
> argument to Decl* so why not make m_opaque_decl a Decl*
> 
>> On Jan 2, 2020, at 4:20 PM, Shafik Yaghmour via lldb-commits 
>>  wrote:
>> 
>> Apologies, m_opaque_decl member of CompilerDecl and operator < in 
>> CompilerDecl.h 
>> 
>>> On Jan 2, 2020, at 3:54 PM, Raphael “Teemperor” Isemann 
>>>  wrote:
>>> 
>>> Not sure if I can follow. What variable should just be a Decl* and what 
>>> operator>> 
>>> (Also yeah that reinterpret_cast could also be a static_cast)
>>> 
 On Jan 3, 2020, at 12:38 AM, Shafik Yaghmour  wrote:
 
 I am not crazy about the reinterpret_cast although if we look inside 
 CompilerDecl impl we can see that basically it is always assuming it is a 
 Decl* and just C-style casts it as such. So why not just make it a Decl*
 
 Also operator<(…) is super sketchy doing a < on void* 
 
> On Dec 28, 2019, at 6:52 AM, Raphael Isemann via lldb-commits 
>  wrote:
> 
> 
> Author: Raphael Isemann
> Date: 2019-12-28T15:20:19+01:00
> New Revision: 8612e92ed590e615f9f56e4fb86a1fdaf3a39e15
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/8612e92ed590e615f9f56e4fb86a1fdaf3a39e15
> DIFF: 
> https://github.com/llvm/llvm-project/commit/8612e92ed590e615f9f56e4fb86a1fdaf3a39e15.diff
> 
> LOG: [lldb][NFC] Remove GetASTContext call in ClangDeclVendor
> 
> Instead of returning NamedDecls and then calling GetASTContext
> to find back the ClangASTContext we used can just implement the
> FindDecl variant that returns CompilerDecls (and implement the
> other function by throwing away the ClangASTContext part of the
> compiler decl).
> 
> Added: 
> 
> 
> Modified: 
> lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp
> lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.h
> lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
> lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
> lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.h
> 
> Removed: 
> 
> 
> 
> 
> diff  --git 
> a/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp 
> b/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp
> index c59722b7b4f8..0c5796650d45 100644
> --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp
> +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp
> @@ -15,16 +15,17 @@ using namespace lldb_private;
> 
> uint32_t ClangDeclVendor::FindDecls(ConstString name, bool append,
> uint32_t max_matches,
> -std::vector &decls) {
> +std::vector 
> &decls) {
> if (!append)
> decls.clear();
> 
> -  std::vector named_decls;
> -  uint32_t ret = FindDecls(name, /*append*/ false, max_matches, 
> named_decls);
> -  for (auto *named_decl : named_decls) {
> -decls.push_back(CompilerDecl(
> -ClangASTContext::GetASTContext(&named_decl->getASTContext()),
> -named_decl));
>

[Lldb-commits] [lldb] 1711f88 - [lldb][NFC] Document TypeSystem and related Compiler* classes

2020-01-03 Thread Raphael Isemann via lldb-commits 

Author: Raphael Isemann
Date: 2020-01-03T10:38:38+01:00
New Revision: 1711f886fd801581b6b5505cc165c977294d311a

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

LOG: [lldb][NFC] Document TypeSystem and related Compiler* classes

Added: 


Modified: 
lldb/include/lldb/Symbol/CompilerDecl.h
lldb/include/lldb/Symbol/CompilerDeclContext.h
lldb/include/lldb/Symbol/CompilerType.h
lldb/include/lldb/Symbol/TypeSystem.h

Removed: 




diff  --git a/lldb/include/lldb/Symbol/CompilerDecl.h 
b/lldb/include/lldb/Symbol/CompilerDecl.h
index e4687ffb3853..4fd269d4730e 100644
--- a/lldb/include/lldb/Symbol/CompilerDecl.h
+++ b/lldb/include/lldb/Symbol/CompilerDecl.h
@@ -15,11 +15,25 @@
 
 namespace lldb_private {
 
+/// Represents a generic declaration such as a function declaration.
+///
+/// This class serves as an abstraction for a declaration inside one of the
+/// TypeSystems implemented by the language plugins. It does not have any 
actual
+/// logic in it but only stores an opaque pointer and a pointer to the
+/// TypeSystem that gives meaning to this opaque pointer. All methods of this
+/// class should call their respective method in the TypeSystem interface and
+/// pass the opaque pointer along.
+///
+/// \see lldb_private::TypeSystem
 class CompilerDecl {
 public:
   // Constructors and Destructors
   CompilerDecl() = default;
 
+  /// Creates a CompilerDecl with the given TypeSystem and opaque pointer.
+  ///
+  /// This constructor should only be called from the respective TypeSystem
+  /// implementation.
   CompilerDecl(TypeSystem *type_system, void *decl)
   : m_type_system(type_system), m_opaque_decl(decl) {}
 

diff  --git a/lldb/include/lldb/Symbol/CompilerDeclContext.h 
b/lldb/include/lldb/Symbol/CompilerDeclContext.h
index 719515242890..6db6f4d3f623 100644
--- a/lldb/include/lldb/Symbol/CompilerDeclContext.h
+++ b/lldb/include/lldb/Symbol/CompilerDeclContext.h
@@ -16,6 +16,17 @@
 
 namespace lldb_private {
 
+/// Represents a generic declaration context in a program. A declaration 
context
+/// is data structure that contains declarations (e.g. namespaces).
+///
+/// This class serves as an abstraction for a declaration context inside one of
+/// the TypeSystems implemented by the language plugins. It does not have any
+/// actual logic in it but only stores an opaque pointer and a pointer to the
+/// TypeSystem that gives meaning to this opaque pointer. All methods of this
+/// class should call their respective method in the TypeSystem interface and
+/// pass the opaque pointer along.
+///
+/// \see lldb_private::TypeSystem
 class CompilerDeclContext {
 public:
   /// Constructs an invalid CompilerDeclContext.
@@ -24,9 +35,10 @@ class CompilerDeclContext {
   /// Constructs a CompilerDeclContext with the given opaque decl context
   /// and its respective TypeSystem instance.
   ///
-  /// Do not use this constructor directly but instead call the respective
-  /// wrapper from the TypeSystem subclass.
-  /// @see 
lldb_private::ClangASTContext::CreateDeclContext(clang::DeclContext*)
+  /// This constructor should only be called from the respective TypeSystem
+  /// implementation.
+  ///
+  /// \see 
lldb_private::ClangASTContext::CreateDeclContext(clang::DeclContext*)
   CompilerDeclContext(TypeSystem *type_system, void *decl_ctx)
   : m_type_system(type_system), m_opaque_decl_ctx(decl_ctx) {}
 

diff  --git a/lldb/include/lldb/Symbol/CompilerType.h 
b/lldb/include/lldb/Symbol/CompilerType.h
index 660466be6b30..37e826291c88 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -20,16 +20,24 @@ namespace lldb_private {
 
 class DataExtractor;
 
-// A class that can carry around a clang ASTContext and a opaque clang
-// QualType. A clang::QualType can be easily reconstructed from an opaque clang
-// type and often the ASTContext is needed when doing various type related
-// tasks, so this class allows both items to travel in a single very
-// lightweight class that can be used. There are many static equivalents of the
-// member functions that allow the ASTContext and the opaque clang QualType to
-// be specified for ease of use and to avoid code duplication.
+/// Represents a generic type in a programming language.
+///
+/// This class serves as an abstraction for a type inside one of the 
TypeSystems
+/// implemented by the language plugins. It does not have any actual logic in 
it
+/// but only stores an opaque pointer and a pointer to the TypeSystem that
+/// gives meaning to this opaque pointer. All methods of this class should call
+/// their respective method in the TypeSystem interface and pass the opaque
+/// pointer along.
+///
+/// \see lldb_private::TypeSystem
 class CompilerType {
 public:
-

[Lldb-commits] [lldb] 2e03324 - [lldb][NFC] Remove forward declaration for non-existent type clang::Action and delete references to it

2020-01-03 Thread Raphael Isemann via lldb-commits 

Author: Raphael Isemann
Date: 2020-01-03T11:24:16+01:00
New Revision: 2e033244417c1b9947ee28795568bc33a1efe781

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

LOG: [lldb][NFC] Remove forward declaration for non-existent type clang::Action 
and delete references to it

There is no clang::Action anymore so our forward decl for it and the obsolete 
pointer in the
ASTStructExtractor can both go (that code anyway didn't do anything).

Added: 


Modified: 
lldb/include/lldb/Core/ClangForward.h
lldb/source/Plugins/ExpressionParser/Clang/ASTStructExtractor.cpp
lldb/source/Plugins/ExpressionParser/Clang/ASTStructExtractor.h

Removed: 




diff  --git a/lldb/include/lldb/Core/ClangForward.h 
b/lldb/include/lldb/Core/ClangForward.h
index 6b24b47c8a96..0bc331438e5c 100644
--- a/lldb/include/lldb/Core/ClangForward.h
+++ b/lldb/include/lldb/Core/ClangForward.h
@@ -17,7 +17,6 @@ namespace Builtin {
 class Context;
 }
 
-class Action;
 class ASTConsumer;
 class ASTContext;
 class ASTRecordLayout;

diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTStructExtractor.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTStructExtractor.cpp
index 190eacaa2b62..a164d48ae3e0 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTStructExtractor.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTStructExtractor.cpp
@@ -30,8 +30,8 @@ ASTStructExtractor::ASTStructExtractor(ASTConsumer 
*passthrough,
const char *struct_name,
ClangFunctionCaller &function)
 : m_ast_context(nullptr), m_passthrough(passthrough),
-  m_passthrough_sema(nullptr), m_sema(nullptr), m_action(nullptr),
-  m_function(function), m_struct_name(struct_name) {
+  m_passthrough_sema(nullptr), m_sema(nullptr), m_function(function),
+  m_struct_name(struct_name) {
   if (!m_passthrough)
 return;
 
@@ -170,7 +170,6 @@ void ASTStructExtractor::PrintStats() {
 
 void ASTStructExtractor::InitializeSema(Sema &S) {
   m_sema = &S;
-  m_action = reinterpret_cast(m_sema);
 
   if (m_passthrough_sema)
 m_passthrough_sema->InitializeSema(S);
@@ -178,7 +177,6 @@ void ASTStructExtractor::InitializeSema(Sema &S) {
 
 void ASTStructExtractor::ForgetSema() {
   m_sema = nullptr;
-  m_action = nullptr;
 
   if (m_passthrough_sema)
 m_passthrough_sema->ForgetSema();

diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTStructExtractor.h 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTStructExtractor.h
index 7aef2e254e1f..078cf095975f 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTStructExtractor.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTStructExtractor.h
@@ -121,8 +121,6 @@ class ASTStructExtractor : public clang::SemaConsumer {
///for passthrough.  NULL if it's an
///ASTConsumer.
   clang::Sema *m_sema; ///< The Sema to use.
-  clang::Action
-  *m_action; ///< The Sema to use, cast to an Action so it's usable.
 
   ClangFunctionCaller &m_function; ///< The function to populate with
///information about the argument structure.



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

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

2020-01-03 Thread Jaroslav Sevcik via Phabricator via lldb-commits 
jarin created this revision.
jarin added a project: LLDB.
Herald added a subscriber: lldb-commits.

Motivation: When formatting an array of typedefed chars, we would like to 
display the array as a string.

The string formatter currently does not trigger because the formatter lookup 
does not resolve typedefs for array elements (this behavior is inconsistent 
with pointers, for those we do look through pointee typedefs). This patch tries 
to make the array formatter lookup somewhat consistent with the pointer 
formatter lookup.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72133

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/array_typedef/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/array_typedef/TestArrayTypedef.py
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/array_typedef/main.cpp
  lldb/source/API/SBType.cpp
  lldb/source/DataFormatters/FormatManager.cpp
  lldb/source/Symbol/ClangASTContext.cpp

Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -3935,7 +3935,7 @@
 ClangASTContext::GetArrayElementType(lldb::opaque_compiler_type_t type,
  uint64_t *stride) {
   if (type) {
-clang::QualType qual_type(GetCanonicalQualType(type));
+clang::QualType qual_type(GetQualType(type));
 
 const clang::Type *array_eletype =
 qual_type.getTypePtr()->getArrayElementTypeNoTypeQual();
@@ -3943,8 +3943,7 @@
 if (!array_eletype)
   return CompilerType();
 
-CompilerType element_type =
-GetType(array_eletype->getCanonicalTypeUnqualified());
+CompilerType element_type = GetType(clang::QualType(array_eletype, 0));
 
 // TODO: the real stride will be >= this value.. find the real one!
 if (stride)
Index: lldb/source/DataFormatters/FormatManager.cpp
===
--- lldb/source/DataFormatters/FormatManager.cpp
+++ lldb/source/DataFormatters/FormatManager.cpp
@@ -238,6 +238,20 @@
 }
   }
 
+  uint64_t array_size;
+  if (compiler_type.IsArrayType(nullptr, &array_size, nullptr)) {
+CompilerType element_type = compiler_type.GetArrayElementType();
+if (element_type.IsTypedefType()) {
+  CompilerType deffed_array_type =
+  element_type.GetTypedefedType().GetArrayType(array_size);
+  GetPossibleMatches(
+  valobj, deffed_array_type,
+  reason | lldb_private::eFormatterChoiceCriterionNavigatedTypedefs,
+  use_dynamic, entries, did_strip_ptr, did_strip_ref,
+  true); // this is not exactly the usual meaning of stripping typedefs
+}
+  }
+
   for (lldb::LanguageType language_type :
GetCandidateLanguages(valobj.GetObjectRuntimeLanguage())) {
 if (Language *language = Language::FindPlugin(language_type)) {
Index: lldb/source/API/SBType.cpp
===
--- lldb/source/API/SBType.cpp
+++ lldb/source/API/SBType.cpp
@@ -212,8 +212,10 @@
 
   if (!IsValid())
 return LLDB_RECORD_RESULT(SBType());
-  return LLDB_RECORD_RESULT(SBType(TypeImplSP(
-  new TypeImpl(m_opaque_sp->GetCompilerType(true).GetArrayElementType();
+  CompilerType canonical_type =
+  m_opaque_sp->GetCompilerType(true).GetCanonicalType();
+  return LLDB_RECORD_RESULT(
+  SBType(TypeImplSP(new TypeImpl(canonical_type.GetArrayElementType();
 }
 
 SBType SBType::GetArrayType(uint64_t size) {
Index: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/array_typedef/main.cpp
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/array_typedef/main.cpp
@@ -0,0 +1,15 @@
+//===-- main.cpp *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+typedef char MCHAR;
+
+int main() {
+  MCHAR str[5] = "abcd";
+  return 0;  // break here
+}
+
Index: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/array_typedef/TestArrayTypedef.py
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/array_typedef/TestArrayTypedef.py
@@ -0,0 +1,19 @@
+"""
+Test lldb data formatter subsystem.
+"""
+
+
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class ArrayTypedefTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def test_array_typedef(self):
+self.

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

2020-01-03 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

I have some comments but otherwise this patch looks good to me. Thanks!




Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/array_typedef/TestArrayTypedef.py:2
+"""
+Test lldb data formatter subsystem.
+"""

I rather have no comment than a generic one (it's kinda obvious what is tested 
here from the name).



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/array_typedef/TestArrayTypedef.py:13
+
+mydir = TestBase.compute_mydir(__file__)
+

I think we can mark this as `NO_DEBUG_INFO_TESTCASE = True` to only run this 
once.



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/array_typedef/main.cpp:1
+//===-- main.cpp *- C++ 
-*-===//
+//

We usually don't add license headers to test files.



Comment at: lldb/source/API/SBType.cpp:218
+  return LLDB_RECORD_RESULT(
+  SBType(TypeImplSP(new TypeImpl(canonical_type.GetArrayElementType();
 }

I get that this is to preserve the previous SB API behavior but I think it's 
better if we keep this method a simple wrapper without extra functionality. 
That we return the canonical type seems like a bug for me, so it's IMHO fine to 
change this behavior.



Comment at: lldb/source/DataFormatters/FormatManager.cpp:241
 
+  uint64_t array_size;
+  if (compiler_type.IsArrayType(nullptr, &array_size, nullptr)) {

Maybe add a comment that we strip here typedefs of array element types.



Comment at: lldb/source/DataFormatters/FormatManager.cpp:245
+if (element_type.IsTypedefType()) {
+  CompilerType deffed_array_type =
+  element_type.GetTypedefedType().GetArrayType(array_size);

Maybe add a comment that this is the original array type with the element type 
typedef desugared.



Comment at: lldb/source/DataFormatters/FormatManager.cpp:251
+  use_dynamic, entries, did_strip_ptr, did_strip_ref,
+  true); // this is not exactly the usual meaning of stripping typedefs
+}

I know this is copied from a above but I think if this is more readable:
```lang=c++
// this is not exactly the usual meaning of stripping typedefs.
const bool stripped_typedef = true;
GetPossibleMatches(

  stripped_typedef);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72133



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

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

2020-01-03 Thread Jaroslav Sevcik via Phabricator via lldb-commits 
jarin updated this revision to Diff 236021.

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

https://reviews.llvm.org/D72133

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/array_typedef/Makefile
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/array_typedef/TestArrayTypedef.py
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/array_typedef/main.cpp
  lldb/source/API/SBType.cpp
  lldb/source/DataFormatters/FormatManager.cpp
  lldb/source/Symbol/ClangASTContext.cpp

Index: lldb/source/Symbol/ClangASTContext.cpp
===
--- lldb/source/Symbol/ClangASTContext.cpp
+++ lldb/source/Symbol/ClangASTContext.cpp
@@ -3935,7 +3935,7 @@
 ClangASTContext::GetArrayElementType(lldb::opaque_compiler_type_t type,
  uint64_t *stride) {
   if (type) {
-clang::QualType qual_type(GetCanonicalQualType(type));
+clang::QualType qual_type(GetQualType(type));
 
 const clang::Type *array_eletype =
 qual_type.getTypePtr()->getArrayElementTypeNoTypeQual();
@@ -3943,8 +3943,7 @@
 if (!array_eletype)
   return CompilerType();
 
-CompilerType element_type =
-GetType(array_eletype->getCanonicalTypeUnqualified());
+CompilerType element_type = GetType(clang::QualType(array_eletype, 0));
 
 // TODO: the real stride will be >= this value.. find the real one!
 if (stride)
Index: lldb/source/DataFormatters/FormatManager.cpp
===
--- lldb/source/DataFormatters/FormatManager.cpp
+++ lldb/source/DataFormatters/FormatManager.cpp
@@ -230,11 +230,33 @@
 if (non_ptr_type.IsTypedefType()) {
   CompilerType deffed_pointed_type =
   non_ptr_type.GetTypedefedType().GetPointerType();
+  const bool stripped_typedef = true;
   GetPossibleMatches(
   valobj, deffed_pointed_type,
   reason | lldb_private::eFormatterChoiceCriterionNavigatedTypedefs,
   use_dynamic, entries, did_strip_ptr, did_strip_ref,
-  true); // this is not exactly the usual meaning of stripping typedefs
+  stripped_typedef); // this is not exactly the usual meaning of
+ // stripping typedefs
+}
+  }
+
+  // For arrays with typedef-ed elements, we add a candidate with the typedef
+  // stripped.
+  uint64_t array_size;
+  if (compiler_type.IsArrayType(nullptr, &array_size, nullptr)) {
+CompilerType element_type = compiler_type.GetArrayElementType();
+if (element_type.IsTypedefType()) {
+  // Get the stripped element type and compute the stripped array type
+  // from it.
+  CompilerType deffed_array_type =
+  element_type.GetTypedefedType().GetArrayType(array_size);
+  const bool stripped_typedef = true;
+  GetPossibleMatches(
+  valobj, deffed_array_type,
+  reason | lldb_private::eFormatterChoiceCriterionNavigatedTypedefs,
+  use_dynamic, entries, did_strip_ptr, did_strip_ref,
+  stripped_typedef); // this is not exactly the usual meaning of
+ // stripping typedefs
 }
   }
 
Index: lldb/source/API/SBType.cpp
===
--- lldb/source/API/SBType.cpp
+++ lldb/source/API/SBType.cpp
@@ -212,8 +212,10 @@
 
   if (!IsValid())
 return LLDB_RECORD_RESULT(SBType());
-  return LLDB_RECORD_RESULT(SBType(TypeImplSP(
-  new TypeImpl(m_opaque_sp->GetCompilerType(true).GetArrayElementType();
+  CompilerType canonical_type =
+  m_opaque_sp->GetCompilerType(true).GetCanonicalType();
+  return LLDB_RECORD_RESULT(
+  SBType(TypeImplSP(new TypeImpl(canonical_type.GetArrayElementType();
 }
 
 SBType SBType::GetArrayType(uint64_t size) {
Index: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/array_typedef/main.cpp
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/array_typedef/main.cpp
@@ -0,0 +1,7 @@
+typedef char MCHAR;
+
+int main() {
+  MCHAR str[5] = "abcd";
+  return 0;  // break here
+}
+
Index: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/array_typedef/TestArrayTypedef.py
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/array_typedef/TestArrayTypedef.py
@@ -0,0 +1,15 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class ArrayTypedefTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_array_typedef(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here",
+lldb.SBFileSpec("main.cpp", False))
+self.expect("expr str", su

[Lldb-commits] [PATCH] D71909: [lldb] Fix crash in AccessDeclContextSanity when copying FunctionTemplateDecl inside a record.

2020-01-03 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor marked an inline comment as done.
teemperor added inline comments.



Comment at: lldb/source/Symbol/ClangASTContext.cpp:1347
+  if (decl_ctx->isRecord())
+func_tmpl_decl->setAccess(clang::AccessSpecifier::AS_public);
 

shafik wrote:
> Where is the method being added and why are we not setting the access there? 
> Are we creating it though `CreateFunctionTemplateDecl` should be be checking 
> the `DeclContext` there to see if it is a `RecordDecl`?
We actually don't add it anywhere but instead use an already created related 
FunctionDecl and set this FunctionTemplateDecl as its specialisation (see 
`lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1216`).

We could also add the FunctionTemplateDecl to the DeclContext explicitly. That 
doesn't seem to break anything and seems consistent with the other functions 
for creating Decls. On the other hand I'm not sure if this is is some untested 
workaround for something.

But in any case, whatever we returned from this function (or whether it was 
added to the DeclContext) is in an invalid state that triggers an assert as 
soon as anyone calls getAccess on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71909



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

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

2020-01-03 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor added a comment.

Just FYI, I meant that  `lldb/source/API/SBType.cpp` shouldn't be changed with 
this patch. I think it's fine that this patch will change the behavior (as the 
old behavior seems broken).

Also do you need someone to commit this for you?


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

https://reviews.llvm.org/D72133



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

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

2020-01-03 Thread Jaroslav Sevcik via Phabricator via lldb-commits 
jarin marked 7 inline comments as done.
jarin added a comment.

I have addressed the comments, thanks for the quick review.




Comment at: lldb/source/API/SBType.cpp:218
+  return LLDB_RECORD_RESULT(
+  SBType(TypeImplSP(new TypeImpl(canonical_type.GetArrayElementType();
 }

teemperor wrote:
> I get that this is to preserve the previous SB API behavior but I think it's 
> better if we keep this method a simple wrapper without extra functionality. 
> That we return the canonical type seems like a bug for me, so it's IMHO fine 
> to change this behavior.
Unfortunately, returning the non-canonicalized type breaks bunch of test (see 
below). In any case, such change should be done in a separate patch.

lldb-api :: 
functionalities/breakpoint/breakpoint_conditions/TestBreakpointConditions.py
lldb-api :: 
functionalities/data-formatter/data-formatter-stl/libcxx/unordered/TestDataFormatterUnordered.py
lldb-api :: 
functionalities/data-formatter/data-formatter-stl/libstdcpp/list/TestDataFormatterStdList.py
lldb-api :: 
functionalities/data-formatter/refpointer-recursion/TestDataFormatterRefPtrRecursion.py
lldb-api :: 
functionalities/data-formatter/varscript_formatting/TestDataFormatterVarScriptFormatting.py



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

https://reviews.llvm.org/D72133



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

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

2020-01-03 Thread Jaroslav Sevcik via Phabricator via lldb-commits 
jarin marked an inline comment as not done.
jarin added a comment.

In D72133#1802727 , @teemperor wrote:

> Also do you need someone to commit this for you?


Yes, please (once we agree what to do with SBType.cpp).


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

https://reviews.llvm.org/D72133



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

[Lldb-commits] [PATCH] D72096: [lldb/Command] Add --force option for `watchpoint delete` command

2020-01-03 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib updated this revision to Diff 236029.
mib marked an inline comment as done.
mib added a comment.

Merged `--force` flag and `auto-confirm` setting test.
Refactored implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72096

Files:
  
lldb/packages/Python/lldbsuite/test/commands/watchpoints/watchpoint_commands/TestWatchpointCommands.py
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Commands/Options.td

Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -1126,3 +1126,8 @@
 "to run as command for this watchpoint. Be sure to give a module name if "
 "appropriate.">;
 }
+
+let Command = "watchpoint delete" in {
+  def watchpoint_delete_force : Option<"force", "f">, Group<1>,
+Desc<"Delete all watchpoints without querying for confirmation.">;
+}
Index: lldb/source/Commands/CommandObjectWatchpoint.cpp
===
--- lldb/source/Commands/CommandObjectWatchpoint.cpp
+++ lldb/source/Commands/CommandObjectWatchpoint.cpp
@@ -414,6 +414,10 @@
   }
 };
 
+// CommandObjectWatchpointDelete
+#define LLDB_OPTIONS_watchpoint_delete
+#include "CommandOptions.inc"
+
 // CommandObjectWatchpointDelete
 #pragma mark Delete
 
@@ -423,7 +427,8 @@
   : CommandObjectParsed(interpreter, "watchpoint delete",
 "Delete the specified watchpoint(s).  If no "
 "watchpoints are specified, delete them all.",
-nullptr, eCommandRequiresTarget) {
+nullptr, eCommandRequiresTarget),
+m_options() {
 CommandArgumentEntry arg;
 CommandObject::AddIDsArgumentData(arg, eArgTypeWatchpointID,
   eArgTypeWatchpointIDRange);
@@ -434,6 +439,41 @@
 
   ~CommandObjectWatchpointDelete() override = default;
 
+  Options *GetOptions() override { return &m_options; }
+
+  class CommandOptions : public Options {
+  public:
+CommandOptions() : Options(), m_force(false) {}
+
+~CommandOptions() override = default;
+
+Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
+  ExecutionContext *execution_context) override {
+  const int short_option = m_getopt_table[option_idx].val;
+
+  switch (short_option) {
+  case 'f':
+m_force = true;
+break;
+  default:
+llvm_unreachable("Unimplemented option");
+  }
+
+  return {};
+}
+
+void OptionParsingStarting(ExecutionContext *execution_context) override {
+  m_force = false;
+}
+
+llvm::ArrayRef GetDefinitions() override {
+  return llvm::makeArrayRef(g_watchpoint_delete_options);
+}
+
+// Instance variables to hold the values for command options.
+bool m_force;
+  };
+
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
 Target *target = &GetSelectedTarget();
@@ -453,8 +493,9 @@
   return false;
 }
 
-if (command.GetArgumentCount() == 0) {
-  if (!m_interpreter.Confirm(
+if (command.empty()) {
+  if (!m_options.m_force &&
+  !m_interpreter.Confirm(
   "About to delete all watchpoints, do you want to do that?",
   true)) {
 result.AppendMessage("Operation cancelled...");
@@ -465,27 +506,31 @@
(uint64_t)num_watchpoints);
   }
   result.SetStatus(eReturnStatusSuccessFinishNoResult);
-} else {
-  // Particular watchpoints selected; delete them.
-  std::vector wp_ids;
-  if (!CommandObjectMultiwordWatchpoint::VerifyWatchpointIDs(
-  target, command, wp_ids)) {
-result.AppendError("Invalid watchpoints specification.");
-result.SetStatus(eReturnStatusFailed);
-return false;
-  }
+  return result.Succeeded();
+}
 
-  int count = 0;
-  const size_t size = wp_ids.size();
-  for (size_t i = 0; i < size; ++i)
-if (target->RemoveWatchpointByID(wp_ids[i]))
-  ++count;
-  result.AppendMessageWithFormat("%d watchpoints deleted.\n", count);
-  result.SetStatus(eReturnStatusSuccessFinishNoResult);
+// Particular watchpoints selected; delete them.
+std::vector wp_ids;
+if (!CommandObjectMultiwordWatchpoint::VerifyWatchpointIDs(target, command,
+   wp_ids)) {
+  result.AppendError("Invalid watchpoints specification.");
+  result.SetStatus(eReturnStatusFailed);
+  return false;
 }
 
+int count = 0;
+const size_t size = wp_ids.size();
+for (size_t i = 0; i < size; ++i)
+  if (target->RemoveWatchpointByID(wp_ids[i]))
+++count;
+result.AppendMessageWithFormat("%d watchpoints del

[Lldb-commits] [PATCH] D72096: [lldb/Command] Add --force option for `watchpoint delete` command

2020-01-03 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib marked an inline comment as done.
mib added inline comments.



Comment at: 
lldb/packages/Python/lldbsuite/test/commands/watchpoints/watchpoint_commands/TestWatchpointCommands.py:194-196
+# Delete the watchpoint immediately using the force option.
+self.expect("watchpoint delete --force",
+substrs=['All watchpoints removed.'])

friss wrote:
> Can't we just add this to the end of another test without spinning up a new 
> process?
> 
> Did you verify that the test failed before your patch? I'm asking because I 
> don't know what m_interpreter.Confirm() does when there's no PTY connected. 
> Does it default to no or yes?
I merge both tests (`auto-confirm` enabled & `--force` flag) into one.

FWIU, m_interpreter.Confirm(message, default_answer) checks first the 
auto-confirm setting, When enabled, it returns the default answer (true in this 
case) otherwise, it triggers the IOHandler for the user input.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72096



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

[Lldb-commits] [PATCH] D72152: Support Launching in External Console in lldb-vscode

2020-01-03 Thread Ben Jackson via Phabricator via lldb-commits 
puremourning created this revision.
puremourning added a reviewer: clayborg.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

When testing terminal-mode applications using lldb-vscode on macOS, it's
currently required to set LLDB_LAUNCH_FLAG_LAUNCH_IN_TTY when starting
lldb-vscode so that the application launches in a new Terminal.app
window. This is unlikely to work for VSCode as there's no obvious way to
set it.

vscode-cpptools uses `externalConsole: true` to enable this behaviour,
so we implement the same here .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72152

Files:
  
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/tools/lldb-vscode/README.md
  lldb/tools/lldb-vscode/lldb-vscode.cpp
  lldb/tools/lldb-vscode/package.json

Index: lldb/tools/lldb-vscode/package.json
===
--- lldb/tools/lldb-vscode/package.json
+++ lldb/tools/lldb-vscode/package.json
@@ -89,6 +89,11 @@
 "description": "Automatically stop after launch.",
 "default": false
 			},
+			"externalConsole": {
+"type": "boolean",
+"description": "Launch the program in a new terminal",
+"default": false
+			},
 			"disableASLR": {
 "type": "boolean",
 "description": "Enable or disable Address space layout randomization if the debugger supports it.",
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1395,6 +1395,8 @@
 flags |= lldb::eLaunchFlagDisableSTDIO;
   if (GetBoolean(arguments, "shellExpandArguments", false))
 flags |= lldb::eLaunchFlagShellExpandArguments;
+  if (GetBoolean(arguments, "externalConsole", false ))
+flags |= lldb::eLaunchFlagLaunchInTTY | lldb::eLaunchFlagCloseTTYOnExit;
   const bool detatchOnError = GetBoolean(arguments, "detachOnError", false);
   g_vsc.launch_info.SetDetachOnError(detatchOnError);
   g_vsc.launch_info.SetLaunchFlags(flags | lldb::eLaunchFlagDebug |
Index: lldb/tools/lldb-vscode/README.md
===
--- lldb/tools/lldb-vscode/README.md
+++ lldb/tools/lldb-vscode/README.md
@@ -88,6 +88,7 @@
 |**exitCommands**   |[string]| | LLDB commands executed when the program exits. Commands and command output will be sent to the debugger console when they are executed.
 |**sourceMap**  |[string[2]]| | Specify an array of path re-mappings. Each element in the array must be a two element array containing a source and destination pathname.
 |**debuggerRoot**   | string| |Specify a working directory to use when launching lldb-vscode. If the debug information in your executable contains relative paths, this option can be used so that `lldb-vscode` can find source files and object files that have relative paths.
+| **externalConsole** |boolean| | Set to TRUE to start the application in a new terminal. Useful for testing console applications.
 
 ## Attaching Settings
 
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -562,7 +562,7 @@
disableSTDIO=False, shellExpandArguments=False,
trace=False, initCommands=None, preRunCommands=None,
stopCommands=None, exitCommands=None, sourcePath=None,
-   debuggerRoot=None, launchCommands=None):
+   debuggerRoot=None, launchCommands=None, **kwargs):
 args_dict = {
 'program': program
 }
@@ -597,6 +597,8 @@
 args_dict['debuggerRoot'] = debuggerRoot
 if launchCommands:
 args_dict['launchCommands'] = launchCommands
+args_dict.update(kwargs)
+
 command_dict = {
 'command': 'launch',
 'type': 'request',
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -265,7 +265,7 @@
disableSTDIO=False, shellExpandArguments=False,
trace=False, initCommands=None, preRunCommands=None,
stopCommands=None, exitCommands=None,sourcePath= None,
-   debuggerRoot=None, launchCommands=None):
+   debuggerRoot=None, launchCommands=None, **kwargs):
 '''Sending launc

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

2020-01-03 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor added a comment.

Interesting, I actually can't reproduce those failures on any of my machines. 
Can you post the error output you get?

But I agree that we can figure this out in a separate patch, so I'll give this 
another test run and then land it. Thanks for the patch!


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

https://reviews.llvm.org/D72133



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

[Lldb-commits] [PATCH] D72161: [lldb][NFC] Use static_cast instead of reinterpret_cast where possible

2020-01-03 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor created this revision.
teemperor added reviewers: shafik, JDevlieghere, labath.
Herald added subscribers: lldb-commits, usaxena95, arphaman.
Herald added a project: LLDB.

There are a few places in LLDB where we do a `reinterpret_cast` for conversions 
that we could also do with `static_cast`. This patch moves all this code to 
`static_cast`.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D72161

Files:
  lldb/source/API/SBEvent.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Host/macosx/objcxx/Host.mm
  lldb/source/Host/posix/PipePosix.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ASTStructExtractor.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Utility/DataExtractor.cpp
  lldb/source/Utility/Environment.cpp
  lldb/source/Utility/Scalar.cpp
  lldb/source/Utility/StreamString.cpp
  lldb/tools/debugserver/source/MacOSX/DarwinLog/DarwinLogCollector.cpp
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm
  lldb/tools/debugserver/source/MacOSX/MachThread.cpp

Index: lldb/tools/debugserver/source/MacOSX/MachThread.cpp
===
--- lldb/tools/debugserver/source/MacOSX/MachThread.cpp
+++ lldb/tools/debugserver/source/MacOSX/MachThread.cpp
@@ -49,7 +49,7 @@
   DNBLogThreadedIf(LOG_THREAD | LOG_VERBOSE,
"MachThread::MachThread ( process = %p, tid = 0x%8.8" PRIx64
", seq_id = %u )",
-   reinterpret_cast(&m_process), m_unique_id, m_seq_id);
+   static_cast(&m_process), m_unique_id, m_seq_id);
 }
 
 MachThread::~MachThread() {
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -1413,29 +1413,29 @@
 bool MachProcess::Signal(int signal, const struct timespec *timeout_abstime) {
   DNBLogThreadedIf(LOG_PROCESS,
"MachProcess::Signal (signal = %d, timeout = %p)", signal,
-   reinterpret_cast(timeout_abstime));
+   static_cast(timeout_abstime));
   nub_state_t state = GetState();
   if (::kill(ProcessID(), signal) == 0) {
 // If we were running and we have a timeout, wait for the signal to stop
 if (IsRunning(state) && timeout_abstime) {
-  DNBLogThreadedIf(LOG_PROCESS, "MachProcess::Signal (signal = %d, timeout "
-"= %p) waiting for signal to stop "
-"process...",
-   signal, reinterpret_cast(timeout_abstime));
+  DNBLogThreadedIf(LOG_PROCESS,
+   "MachProcess::Signal (signal = %d, timeout "
+   "= %p) waiting for signal to stop "
+   "process...",
+   signal, static_cast(timeout_abstime));
   m_private_events.WaitForSetEvents(eEventProcessStoppedStateChanged,
 timeout_abstime);
   state = GetState();
   DNBLogThreadedIf(
   LOG_PROCESS,
   "MachProcess::Signal (signal = %d, timeout = %p) state = %s", signal,
-  reinterpret_cast(timeout_abstime),
-  DNBStateAsString(state));
+  static_cast(timeout_abstime), DNBStateAsString(state));
   return !IsRunning(state);
 }
 DNBLogThreadedIf(
 LOG_PROCESS,
 "MachProcess::Signal (signal = %d, timeout = %p) not waiting...",
-signal, reinterpret_cast(timeout_abstime));
+signal, static_cast(timeout_abstime));
 return true;
   }
   DNBError err(errno, DNBError::POSIX);
@@ -1739,10 +1739,10 @@
 bp = m_breakpoints.Add(addr, length, hardware);
 
   if (EnableBreakpoint(addr)) {
-DNBLogThreadedIf(LOG_BREAKPOINTS, "MachProcess::CreateBreakpoint ( addr = "
-  "0x%8.8llx, length = %llu) => %p",
- (uint64_t)addr, (uint64_t)length,
- reinterpret_cast(bp));
+DNBLogThreadedIf(LOG_BREAKPOINTS,
+ "MachProcess::CreateBreakpoint ( addr = "
+ "0x%8.8llx, length = %llu) => %p",
+ (uint64_t)addr, (uint64_t)length, static_cast(bp));
 return bp;
   } else if (bp->Release() == 0) {
 m_breakpoints.Remove(addr);
@@ -1771,10 +1771,10 @@
   wp->SetIsWatchpoint(watch_flags);
 
   if (EnableWatchpoint(addr)) {
-DNBLogThreadedIf(LOG_WATCHPOINTS, "MachProcess::CreateWatchpoint ( addr = "
-  "0x%8.8llx, length = %llu) => %p",
- (uint64_t)addr, (uint64_t)length,
- reinterpret_cast(wp));
+DNBLogThreadedIf(LOG_

[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

2020-01-03 Thread Jaroslav Sevcik via Phabricator via lldb-commits 
jarin added a comment.

Actually, there is something unusually flaky in my current checkout, so you 
might be right that not canonicalizing is harmless. It still makes more sense 
to land that separately.


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

https://reviews.llvm.org/D72133



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

[Lldb-commits] [PATCH] D71800: [CMake] Add $ORIGIN/../../../../lib to rpath if BUILD_SHARED_LIBS or LLVM_LINK_LLVM_DYLIB on *nix

2020-01-03 Thread Fangrui Song via Phabricator via lldb-commits 
MaskRay added a comment.

Ping:)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71800



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

[Lldb-commits] [PATCH] D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging

2020-01-03 Thread Paolo Severini via Phabricator via lldb-commits 
paolosev marked 10 inline comments as done.
paolosev added a comment.

In D71575#1793791 , @labath wrote:

> A bunch more comments from me. :)
>
> A higher level question I have is whether there's something suitable within 
> llvm for parsing wasm object files that could be reused. I know this can be 
> tricky with files loaded from memory etc., so it's fine if it isn't possible 
> to do that, but I am wondering if you have considered this option.


I have considered this option, there is indeed some code duplication because 
there is already a Wasm parser in class WasmObjectFile 
(llvm/include/llvm/Object/Wasm.h).
However class WasmObjectFile is initialized with a memory buffer that contains 
the whole module, and this is not ideal. LLDB might create an ObjectFileWasm 
either from a file or by retrieving specific module chunks from a remote, but 
it doesn't often need to load the whole module in memory.
I think this is the reason why other kind of object files (like ObjectfileELF) 
are re-implemented in LLDB rather than reusing existing code in LLVM (like 
ELFFile in llvm/include/llvm/Object/ELF.h).




Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:40
+
+  const uint32_t *version = reinterpret_cast(
+  data_sp->GetBytes() + sizeof(llvm::wasm::WasmMagic));

labath wrote:
> labath wrote:
> > This looks like it will cause problems on big endian hosts..
> One way to get around that would be to use something like 
> `llvm::support::read32le`.
Good point, modified to use read32le.



Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.cpp:252
+
+  ModuleSpec spec(file, ArchSpec("wasm32-unknown-unknown-wasm"));
+  specs.Append(spec);

labath wrote:
> I take it that wasm files don't have anything like a build id, uuid or 
> something similar?
Not yet. There is a proposal to add a uuid to wasm modules in a custom section, 
but it's not part of the standard yet.



Comment at: lldb/source/Plugins/ObjectFile/wasm/ObjectFileWasm.h:116
+
+  typedef struct section_info {
+lldb::offset_t offset;

labath wrote:
> We don't use this typedef style (except possibly in some old code which we 
> shouldn't emulate).
Yes, this was copied from old code :). Removed. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71575



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

[Lldb-commits] [PATCH] D71909: [lldb] Fix crash in AccessDeclContextSanity when copying FunctionTemplateDecl inside a record.

2020-01-03 Thread Shafik Yaghmour via Phabricator via lldb-commits 
shafik added inline comments.



Comment at: lldb/source/Symbol/ClangASTContext.cpp:1347
+  if (decl_ctx->isRecord())
+func_tmpl_decl->setAccess(clang::AccessSpecifier::AS_public);
 

teemperor wrote:
> shafik wrote:
> > Where is the method being added and why are we not setting the access 
> > there? Are we creating it though `CreateFunctionTemplateDecl` should be be 
> > checking the `DeclContext` there to see if it is a `RecordDecl`?
> We actually don't add it anywhere but instead use an already created related 
> FunctionDecl and set this FunctionTemplateDecl as its specialisation (see 
> `lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1216`).
> 
> We could also add the FunctionTemplateDecl to the DeclContext explicitly. 
> That doesn't seem to break anything and seems consistent with the other 
> functions for creating Decls. On the other hand I'm not sure if this is is 
> some untested workaround for something.
> 
> But in any case, whatever we returned from this function (or whether it was 
> added to the DeclContext) is in an invalid state that triggers an assert as 
> soon as anyone calls getAccess on it.
Right so b/c this is a templated class member function we have a large comment 
above this check in `DWARFASTParserClang.cpp`:

```
if (is_cxx_method && has_template_params)
```

explaining why it is not added as a C++ method, which means we don't add this 
via `m_ast.AddMethodToCXXRecordType(...)` which would get the access specifiers 
set at all.

I feel like we should handle this in `DWARFASTParserClang.cpp` around line 
`1216` and get the access specifier correct instead of hack where we just set 
it to public.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71909



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

[Lldb-commits] [PATCH] D72161: [lldb][NFC] Use static_cast instead of reinterpret_cast where possible

2020-01-03 Thread Shafik Yaghmour via Phabricator via lldb-commits 
shafik added a comment.

This is a great change! Moving to stricter casts clarifies intent and reduces 
the chance that later changes can introduce bugs accidentally.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D72161



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

[Lldb-commits] [PATCH] D72161: [lldb][NFC] Use static_cast instead of reinterpret_cast where possible

2020-01-03 Thread Shafik Yaghmour via Phabricator via lldb-commits 
shafik added inline comments.



Comment at: lldb/tools/debugserver/source/MacOSX/MachProcess.mm:1745
+ "0x%8.8llx, length = %llu) => %p",
+ (uint64_t)addr, (uint64_t)length, static_cast(bp));
 return bp;

`static_cast(addr)` and `static_cast(length)`



Comment at: lldb/tools/debugserver/source/MacOSX/MachProcess.mm:1777
+ "0x%8.8llx, length = %llu) => %p",
+ (uint64_t)addr, (uint64_t)length, static_cast(wp));
 return wp;

`static_cast(addr)` and `static_cast(length)`

You can use `-Wold-style-cast` to warm on C-style casts.



Comment at: lldb/tools/debugserver/source/MacOSX/MachProcess.mm:2306
   DNBLogThreadedIf(LOG_PROCESS, "MachProcess::%s (&%p[%llu]) ...", 
__FUNCTION__,
-   reinterpret_cast(buf), (uint64_t)buf_size);
+   static_cast(buf), (uint64_t)buf_size);
   PTHREAD_MUTEX_LOCKER(locker, m_stdio_mutex);

`static_cast(buf_size)` 



Comment at: lldb/tools/debugserver/source/MacOSX/MachProcess.mm:2466
   DNBLogThreadedIf(LOG_PROCESS, "MachProcess::%s (&%p[%llu]) ...", 
__FUNCTION__,
-   reinterpret_cast(buf), (uint64_t)buf_size);
+   static_cast(buf), (uint64_t)buf_size);
   PTHREAD_MUTEX_LOCKER(locker, m_profile_data_mutex);

`static_cast(buf_size)` 


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D72161



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

[Lldb-commits] [lldb] 6e6b6a5 - [lldb/Docs] Include how to generate the man page

2020-01-03 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2020-01-03T13:34:35-08:00
New Revision: 6e6b6a5754514a137729ce1a5e389db5f516c964

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

LOG: [lldb/Docs] Include how to generate the man page

Added: 


Modified: 
lldb/docs/resources/build.rst

Removed: 




diff  --git a/lldb/docs/resources/build.rst b/lldb/docs/resources/build.rst
index 499a79b88ef6..efa94de42a9d 100644
--- a/lldb/docs/resources/build.rst
+++ b/lldb/docs/resources/build.rst
@@ -365,6 +365,7 @@ To build the documentation, configure with 
``LLVM_ENABLE_SPHINX=ON`` and build t
 ::
 
   > ninja docs-lldb-html
+  > ninja docs-lldb-man
   > ninja lldb-cpp-doc
   > ninja lldb-python-doc
 



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

[Lldb-commits] [lldb] 320b43c - [lldb/Docs] Include the man page on the website

2020-01-03 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2020-01-03T13:59:19-08:00
New Revision: 320b43c39f0eb636c84815ce463893b21befdc8f

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

LOG: [lldb/Docs] Include the man page on the website

Added: 


Modified: 
lldb/docs/index.rst

Removed: 




diff  --git a/lldb/docs/index.rst b/lldb/docs/index.rst
index 0dbb160c2b31..f1e1eda7609a 100644
--- a/lldb/docs/index.rst
+++ b/lldb/docs/index.rst
@@ -152,11 +152,12 @@ interesting areas to contribute to lldb.
 .. toctree::
:hidden:
:maxdepth: 1
-   :caption: API Documentation
+   :caption: Reference
 
-   Public Python API Reference 

-   Public C++ API Reference 

-   Private C++ Reference 
+   Public Python API 
+   Public C++ API 
+   Private C++ API 
+   Man Page 
 
 .. toctree::
:hidden:



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

[Lldb-commits] [lldb] d2b19d4 - [lldb/Utility] YAML validation should be orthogonal to packet semantics.

2020-01-03 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2020-01-03T14:23:42-08:00
New Revision: d2b19d455de22fe3c8aa192320e1ff9a4eb1a365

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

LOG: [lldb/Utility] YAML validation should be orthogonal to packet semantics.

It's not up to YAML to validate the semantics of the GDB remote packet
struct. This is especially wrong here as there's nothing that says that
the amount of bytes transmitted  matches the packet payload size.

Added: 


Modified: 
lldb/source/Utility/GDBRemote.cpp

Removed: 




diff  --git a/lldb/source/Utility/GDBRemote.cpp 
b/lldb/source/Utility/GDBRemote.cpp
index 54f3a3cd8a86..f267d00fc97e 100644
--- a/lldb/source/Utility/GDBRemote.cpp
+++ b/lldb/source/Utility/GDBRemote.cpp
@@ -93,9 +93,6 @@ void yaml::MappingTraits::mapping(IO &io,
 StringRef
 yaml::MappingTraits::validate(IO &io,
GDBRemotePacket &Packet) {
-  if (Packet.bytes_transmitted != Packet.packet.data.size())
-return "BinaryData size doesn't match bytes transmitted";
-
   return {};
 }
 



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

[Lldb-commits] [PATCH] D72190: Removing C-style casts in favor of explict C++ style casts

2020-01-03 Thread Shafik Yaghmour via Phabricator via lldb-commits 
shafik created this revision.
shafik added reviewers: teemperor, JDevlieghere, labath.

C-style casts hide intent and when code changes they can also hide logic errors.

This change converts most of the C-style cast into `static_cast` and the icky 
ones are converted to `reinterpret_cast` and we can deal with fixing those icky 
cases in another change.

One case was moved to use `std::numeric_limits::max()` since it was 
simply using the fact that `-1` converted to `unsigned` results in the maximum 
unsigned value.


https://reviews.llvm.org/D72190

Files:
  lldb/include/lldb/Breakpoint/BreakpointOptions.h
  lldb/include/lldb/Breakpoint/BreakpointResolver.h
  lldb/include/lldb/Core/SearchFilter.h
  lldb/include/lldb/Core/Value.h
  lldb/include/lldb/DataFormatters/TypeSynthetic.h
  lldb/include/lldb/Expression/IRExecutionUnit.h
  lldb/include/lldb/Symbol/Symbol.h
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h

Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -42,7 +42,7 @@
 Parse();
   if (m_parse_error)
 return false;
-  return (bool)m_full;
+  return static_cast(m_full);
 }
 
 ConstString GetFullName() const { return m_full; }
Index: lldb/source/Expression/IRExecutionUnit.cpp
===
--- lldb/source/Expression/IRExecutionUnit.cpp
+++ lldb/source/Expression/IRExecutionUnit.cpp
@@ -125,7 +125,8 @@
   LLDB_LOGF(log,
 "Found function, has local address 0x%" PRIx64
 " and remote address 0x%" PRIx64,
-(uint64_t)func_local_addr, (uint64_t)func_remote_addr);
+static_cast(func_local_addr),
+static_cast(func_remote_addr));
 
   std::pair func_range;
 
@@ -302,7 +303,8 @@
   llvm::SmallVector result_path;
   std::string object_name_model =
   "jit-object-" + module->getModuleIdentifier() + "-%%%.o";
-  (void)llvm::sys::fs::createUniqueFile(object_name_model, fd, result_path);
+  static_cast(
+  llvm::sys::fs::createUniqueFile(object_name_model, fd, result_path));
   llvm::raw_fd_ostream fds(fd, true);
   fds.write(object.getBufferStart(), object.getBufferSize());
 }
@@ -466,8 +468,9 @@
   } else {
 record.dump(log);
 
-DataExtractor my_extractor((const void *)record.m_host_address,
-   record.m_size, lldb::eByteOrderBig, 8);
+DataExtractor my_extractor(
+reinterpret_cast(record.m_host_address),
+record.m_size, lldb::eByteOrderBig, 8);
 my_extractor.PutToLog(log, 0, record.m_size, record.m_host_address, 16,
   DataExtractor::TypeUInt8);
   }
@@ -593,7 +596,7 @@
   Size, Alignment, SectionID, SectionName);
 
   m_parent.m_records.push_back(AllocationRecord(
-  (uintptr_t)return_value,
+  reinterpret_cast(return_value),
   lldb::ePermissionsReadable | lldb::ePermissionsExecutable,
   GetSectionTypeFromSectionName(SectionName, AllocationKind::Code), Size,
   Alignment, SectionID, SectionName.str().c_str()));
@@ -601,7 +604,8 @@
   LLDB_LOGF(log,
 "IRExecutionUnit::allocateCodeSection(Size=0x%" PRIx64
 ", Alignment=%u, SectionID=%u) = %p",
-(uint64_t)Size, Alignment, SectionID, (void *)return_value);
+static_cast(Size), Alignment, SectionID,
+static_cast(return_value));
 
   if (m_parent.m_reported_allocations) {
 Status err;
@@ -626,13 +630,14 @@
   if (!IsReadOnly)
 permissions |= lldb::ePermissionsWritable;
   m_parent.m_records.push_back(AllocationRecord(
-  (uintptr_t)return_value, permissions,
+  reinterpret_cast(return_value), permissions,
   GetSectionTypeFromSectionName(SectionName, AllocationKind::Data), Size,
   Alignment, SectionID, SectionName.str().c_str()));
   LLDB_LOGF(log,
 "IRExecutionUnit::allocateDataSection(Size=0x%" PRIx64
 ", Alignment=%u, SectionID=%u) = %p",
-(uint64_t)Size, Alignment, SectionID, (void *)return_value);
+static_cast(Size), Alignment, SectionID,
+static_cast(return_value));
 
   if (m_parent.m_reported_allocations) {
 Status err;
@@ -1065,7 +1070,7 @@
 
 void *IRExecutionUnit::MemoryManager::getPointerToNamedFunction(
 const std::string &Name, bool AbortOnFailure) {
-  return (void *)getSymbolAddress(Name);
+  return reinterpret_cast(getSymbolAddress(Name));
 }
 
 lldb::addr_t
@@ -1085,9 +1090,10 @@
 "IRExecutionUnit::GetRemoteAddressForLocal() found 0x%" PRIx64
 " in [0x%" PRIx64 "..0x%" PRIx64 "], and returned 0x%" PRIx64
 " from [0x%" PRIx64 "..0x%" PRIx64 "].",
-

[Lldb-commits] [lldb] 6c87623 - [UserExpression] Clean up `return` after `else`.

2020-01-03 Thread Davide Italiano via lldb-commits 

Author: Davide Italiano
Date: 2020-01-03T16:52:10-08:00
New Revision: 6c87623615b3befdf62e3a5cd6c408a698f1c2d9

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

LOG: [UserExpression] Clean up `return` after `else`.

Added: 


Modified: 
lldb/source/Expression/UserExpression.cpp

Removed: 




diff  --git a/lldb/source/Expression/UserExpression.cpp 
b/lldb/source/Expression/UserExpression.cpp
index 9e3c8ad56af0..271bd9bb57aa 100644
--- a/lldb/source/Expression/UserExpression.cpp
+++ b/lldb/source/Expression/UserExpression.cpp
@@ -84,10 +84,9 @@ bool UserExpression::LockAndCheckContext(ExecutionContext 
&exe_ctx,
   if (m_address.IsValid()) {
 if (!frame_sp)
   return false;
-else
-  return (0 == Address::CompareLoadAddress(m_address,
-   frame_sp->GetFrameCodeAddress(),
-   target_sp.get()));
+return (Address::CompareLoadAddress(m_address,
+frame_sp->GetFrameCodeAddress(),
+target_sp.get()) == 0);
   }
 
   return true;



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

[Lldb-commits] [lldb] df71f92 - [lldb/Command] Add --force option for `watchpoint delete` command

2020-01-03 Thread Med Ismail Bennani via lldb-commits 

Author: Med Ismail Bennani
Date: 2020-01-04T03:11:15+01:00
New Revision: df71f92fbb7c96cfd36d247ae6fb6929cb9bce35

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

LOG: [lldb/Command] Add --force option for `watchpoint delete` command

Currently, there is no option to delete all the watchpoint without LLDB
asking for a confirmation. Besides making the watchpoint delete command
homogeneous with the breakpoint delete command, this option could also
become handy to trigger automated watchpoint deletion i.e. using
breakpoint actions.

rdar://42560586

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 

lldb/packages/Python/lldbsuite/test/commands/watchpoints/watchpoint_commands/TestWatchpointCommands.py
lldb/source/Commands/CommandObjectWatchpoint.cpp
lldb/source/Commands/Options.td

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/commands/watchpoints/watchpoint_commands/TestWatchpointCommands.py
 
b/lldb/packages/Python/lldbsuite/test/commands/watchpoints/watchpoint_commands/TestWatchpointCommands.py
index 27c332b68bb4..a428d18f12c7 100644
--- 
a/lldb/packages/Python/lldbsuite/test/commands/watchpoints/watchpoint_commands/TestWatchpointCommands.py
+++ 
b/lldb/packages/Python/lldbsuite/test/commands/watchpoints/watchpoint_commands/TestWatchpointCommands.py
@@ -105,24 +105,9 @@ def test_rw_watchpoint(self):
 @expectedFailureAll(archs=['s390x'])
 def test_rw_watchpoint_delete(self):
 """Test delete watchpoint and expect not to stop for watchpoint."""
-self.build(dictionary=self.d)
-self.setTearDownCleanup(dictionary=self.d)
-
-exe = self.getBuildArtifact(self.exe_name)
-self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
-
-# Add a breakpoint to set a watchpoint when stopped on the breakpoint.
-lldbutil.run_break_set_by_file_and_line(
-self, None, self.line, num_expected_locations=1)
-
-# Run the program.
-self.runCmd("run", RUN_SUCCEEDED)
-
-# We should be stopped again due to the breakpoint.
-# The stop reason of the thread should be breakpoint.
-self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
-substrs=['stopped',
- 'stop reason = breakpoint'])
+self.build()
+lldbutil.run_to_line_breakpoint(self, lldb.SBFileSpec(self.source),
+self.line)
 
 # Now let's set a read_write-type watchpoint for 'global'.
 # There should be two watchpoint hits (see main.c).
@@ -145,8 +130,28 @@ def test_rw_watchpoint_delete(self):
 # Restore the original setting of auto-confirm.
 self.runCmd("settings clear auto-confirm")
 
-# Use the '-v' option to do verbose listing of the watchpoint.
-self.runCmd("watchpoint list -v")
+target = self.dbg.GetSelectedTarget()
+self.assertTrue(target and not target.GetNumWatchpoints())
+
+# Now let's set a read_write-type watchpoint for 'global'.
+# There should be two watchpoint hits (see main.c).
+self.expect(
+"watchpoint set variable -w read_write global",
+WATCHPOINT_CREATED,
+substrs=[
+'Watchpoint created',
+'size = 4',
+'type = rw',
+'%s:%d' %
+(self.source,
+ self.decl)])
+
+
+# Delete the watchpoint immediately using the force option.
+self.expect("watchpoint delete --force",
+substrs=['All watchpoints removed.'])
+
+self.assertTrue(target and not target.GetNumWatchpoints())
 
 self.runCmd("process continue")
 

diff  --git a/lldb/source/Commands/CommandObjectWatchpoint.cpp 
b/lldb/source/Commands/CommandObjectWatchpoint.cpp
index 1b1d59740c86..c965d354f734 100644
--- a/lldb/source/Commands/CommandObjectWatchpoint.cpp
+++ b/lldb/source/Commands/CommandObjectWatchpoint.cpp
@@ -414,6 +414,10 @@ class CommandObjectWatchpointDisable : public 
CommandObjectParsed {
   }
 };
 
+// CommandObjectWatchpointDelete
+#define LLDB_OPTIONS_watchpoint_delete
+#include "CommandOptions.inc"
+
 // CommandObjectWatchpointDelete
 #pragma mark Delete
 
@@ -423,7 +427,8 @@ class CommandObjectWatchpointDelete : public 
CommandObjectParsed {
   : CommandObjectParsed(interpreter, "watchpoint delete",
 "Delete the specified watchpoint(s).  If no "
 "watchpoints are specified, delete them all.",
-nullptr, eCommandRequiresTarget) {
+nullptr, eCommandRequiresTarget),
+m_options() {
 CommandArg

[Lldb-commits] [PATCH] D72096: [lldb/Command] Add --force option for `watchpoint delete` command

2020-01-03 Thread Med Ismail Bennani via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
mib marked an inline comment as done.
Closed by commit rGdf71f92fbb7c: [lldb/Command] Add --force option for 
`watchpoint delete` command (authored by mib).

Changed prior to commit:
  https://reviews.llvm.org/D72096?vs=236029&id=236159#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72096

Files:
  
lldb/packages/Python/lldbsuite/test/commands/watchpoints/watchpoint_commands/TestWatchpointCommands.py
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Commands/Options.td

Index: lldb/source/Commands/Options.td
===
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -1126,3 +1126,8 @@
 "to run as command for this watchpoint. Be sure to give a module name if "
 "appropriate.">;
 }
+
+let Command = "watchpoint delete" in {
+  def watchpoint_delete_force : Option<"force", "f">, Group<1>,
+Desc<"Delete all watchpoints without querying for confirmation.">;
+}
Index: lldb/source/Commands/CommandObjectWatchpoint.cpp
===
--- lldb/source/Commands/CommandObjectWatchpoint.cpp
+++ lldb/source/Commands/CommandObjectWatchpoint.cpp
@@ -415,6 +415,10 @@
 };
 
 // CommandObjectWatchpointDelete
+#define LLDB_OPTIONS_watchpoint_delete
+#include "CommandOptions.inc"
+
+// CommandObjectWatchpointDelete
 #pragma mark Delete
 
 class CommandObjectWatchpointDelete : public CommandObjectParsed {
@@ -423,7 +427,8 @@
   : CommandObjectParsed(interpreter, "watchpoint delete",
 "Delete the specified watchpoint(s).  If no "
 "watchpoints are specified, delete them all.",
-nullptr, eCommandRequiresTarget) {
+nullptr, eCommandRequiresTarget),
+m_options() {
 CommandArgumentEntry arg;
 CommandObject::AddIDsArgumentData(arg, eArgTypeWatchpointID,
   eArgTypeWatchpointIDRange);
@@ -434,6 +439,41 @@
 
   ~CommandObjectWatchpointDelete() override = default;
 
+  Options *GetOptions() override { return &m_options; }
+
+  class CommandOptions : public Options {
+  public:
+CommandOptions() : Options(), m_force(false) {}
+
+~CommandOptions() override = default;
+
+Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
+  ExecutionContext *execution_context) override {
+  const int short_option = m_getopt_table[option_idx].val;
+
+  switch (short_option) {
+  case 'f':
+m_force = true;
+break;
+  default:
+llvm_unreachable("Unimplemented option");
+  }
+
+  return {};
+}
+
+void OptionParsingStarting(ExecutionContext *execution_context) override {
+  m_force = false;
+}
+
+llvm::ArrayRef GetDefinitions() override {
+  return llvm::makeArrayRef(g_watchpoint_delete_options);
+}
+
+// Instance variables to hold the values for command options.
+bool m_force;
+  };
+
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
 Target *target = &GetSelectedTarget();
@@ -453,8 +493,9 @@
   return false;
 }
 
-if (command.GetArgumentCount() == 0) {
-  if (!m_interpreter.Confirm(
+if (command.empty()) {
+  if (!m_options.m_force &&
+  !m_interpreter.Confirm(
   "About to delete all watchpoints, do you want to do that?",
   true)) {
 result.AppendMessage("Operation cancelled...");
@@ -465,27 +506,31 @@
(uint64_t)num_watchpoints);
   }
   result.SetStatus(eReturnStatusSuccessFinishNoResult);
-} else {
-  // Particular watchpoints selected; delete them.
-  std::vector wp_ids;
-  if (!CommandObjectMultiwordWatchpoint::VerifyWatchpointIDs(
-  target, command, wp_ids)) {
-result.AppendError("Invalid watchpoints specification.");
-result.SetStatus(eReturnStatusFailed);
-return false;
-  }
+  return result.Succeeded();
+}
 
-  int count = 0;
-  const size_t size = wp_ids.size();
-  for (size_t i = 0; i < size; ++i)
-if (target->RemoveWatchpointByID(wp_ids[i]))
-  ++count;
-  result.AppendMessageWithFormat("%d watchpoints deleted.\n", count);
-  result.SetStatus(eReturnStatusSuccessFinishNoResult);
+// Particular watchpoints selected; delete them.
+std::vector wp_ids;
+if (!CommandObjectMultiwordWatchpoint::VerifyWatchpointIDs(target, command,
+   wp_ids)) {
+  result.AppendError("Invalid watchpoints specification.");
+  result.SetStatus(eReturnStatusFailed);
+  return false;
 }
 
+int count = 0;
+const

[Lldb-commits] [PATCH] D72195: [lldb] [Process/NetBSD] Remove unused orig_*ax use

2020-01-03 Thread Michał Górny via Phabricator via lldb-commits 
mgorny created this revision.
mgorny added reviewers: krytarowski, labath.

`orig_*ax` logic is Linux-specific, and was never used on NetBSD.
In fact, its support seems to be a dead code entirely.


https://reviews.llvm.org/D72195

Files:
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp


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
@@ -725,11 +725,6 @@
   ::memcpy(dst, &m_gpr_x86_64, GetRegisterInfoInterface().GetGPRSize());
   dst += GetRegisterInfoInterface().GetGPRSize();
 
-  RegisterValue value((uint64_t)-1);
-  const RegisterInfo *reg_info =
-  GetRegisterInfoInterface().GetDynamicRegisterInfo("orig_eax");
-  if (reg_info == nullptr)
-reg_info = GetRegisterInfoInterface().GetDynamicRegisterInfo("orig_rax");
   return error;
 }
 


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
@@ -725,11 +725,6 @@
   ::memcpy(dst, &m_gpr_x86_64, GetRegisterInfoInterface().GetGPRSize());
   dst += GetRegisterInfoInterface().GetGPRSize();
 
-  RegisterValue value((uint64_t)-1);
-  const RegisterInfo *reg_info =
-  GetRegisterInfoInterface().GetDynamicRegisterInfo("orig_eax");
-  if (reg_info == nullptr)
-reg_info = GetRegisterInfoInterface().GetDynamicRegisterInfo("orig_rax");
   return error;
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits