[Lldb-commits] [PATCH] D150043: [InferAddressSpaces] Handle vector of pointers type

2023-05-08 Thread CaprYang via Phabricator via lldb-commits 
CaprYang updated this revision to Diff 520296.
CaprYang removed reviewers: bollu, ldionne, nicolasvasilache, rafauler, Amir, 
maksfb, NoQ, njames93, libc++, libc++abi, libunwind, rymiel, 
HazardyKnusperkeks, owenpan, MyDeveloperDay.
CaprYang removed projects: clang-format, Flang, clang-tools-extra, MLIR, 
libunwind, libc++abi, libc-project, OpenMP, libc++, LLDB, Sanitizers, clang.

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

https://reviews.llvm.org/D150043

Files:
  llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
  llvm/test/Transforms/InferAddressSpaces/AMDGPU/icmp.ll
  llvm/test/Transforms/InferAddressSpaces/masked-gather-scatter.ll

Index: llvm/test/Transforms/InferAddressSpaces/masked-gather-scatter.ll
===
--- /dev/null
+++ llvm/test/Transforms/InferAddressSpaces/masked-gather-scatter.ll
@@ -0,0 +1,25 @@
+; RUN: opt -S -passes=infer-address-spaces -assume-default-is-flat-addrspace %s | FileCheck %s
+
+; CHECK-LABEL: @masked_gather_inferas(
+; CHECK: tail call <4 x i32> @llvm.masked.gather.v4i32.v4p1
+define <4 x i32> @masked_gather_inferas(ptr addrspace(1) %out, <4 x i64> %index) {
+entry:
+  %out.1 = addrspacecast ptr addrspace(1) %out to ptr
+  %ptrs = getelementptr inbounds i32, ptr %out.1, <4 x i64> %index
+  %value = tail call <4 x i32> @llvm.masked.gather.v4i32.v4p0(<4 x ptr> %ptrs, i32 4, <4 x i1> , <4 x i32> poison)
+  ret <4 x i32> %value
+}
+
+; CHECK-LABEL: @masked_scatter_inferas(
+; CHECK: tail call void @llvm.masked.scatter.v4i32.v4p1
+define void @masked_scatter_inferas(ptr addrspace(1) %out, <4 x i64> %index, <4 x i32> %value) {
+entry:
+  %out.1 = addrspacecast ptr addrspace(1) %out to ptr
+  %ptrs = getelementptr inbounds i32, ptr %out.1, <4 x i64> %index
+  tail call void @llvm.masked.scatter.v4i32.v4p0(<4 x i32> %value, <4 x ptr> %ptrs, i32 4, <4 x i1> )
+  ret void
+}
+
+declare <4 x i32> @llvm.masked.gather.v4i32.v4p0(<4 x ptr>, i32 immarg, <4 x i1>, <4 x i32>)
+
+declare void @llvm.masked.scatter.v4i32.v4p0(<4 x i32>, <4 x ptr>, i32 immarg, <4 x i1>)
\ No newline at end of file
Index: llvm/test/Transforms/InferAddressSpaces/AMDGPU/icmp.ll
===
--- llvm/test/Transforms/InferAddressSpaces/AMDGPU/icmp.ll
+++ llvm/test/Transforms/InferAddressSpaces/AMDGPU/icmp.ll
@@ -147,9 +147,8 @@
   ret i1 %cmp
 }
 
-; TODO: Should be handled
 ; CHECK-LABEL: @icmp_flat_flat_from_group_vector(
-; CHECK: %cmp = icmp eq <2 x ptr> %cast0, %cast1
+; CHECK: %cmp = icmp eq <2 x ptr addrspace(3)> %group.ptr.0, %group.ptr.1
 define <2 x i1> @icmp_flat_flat_from_group_vector(<2 x ptr addrspace(3)> %group.ptr.0, <2 x ptr addrspace(3)> %group.ptr.1) #0 {
   %cast0 = addrspacecast <2 x ptr addrspace(3)> %group.ptr.0 to <2 x ptr>
   %cast1 = addrspacecast <2 x ptr addrspace(3)> %group.ptr.1 to <2 x ptr>
Index: llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
===
--- llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
+++ llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
@@ -256,6 +256,48 @@
 INITIALIZE_PASS_END(InferAddressSpaces, DEBUG_TYPE, "Infer address spaces",
 false, false)
 
+static unsigned getPtrOrVecOfPtrsAddressSpace(Type *Ty) {
+  if (Ty->isVectorTy()) {
+Ty = cast(Ty)->getElementType();
+  }
+  assert(Ty->isPointerTy());
+  return Ty->getPointerAddressSpace();
+}
+
+static bool isPtrOrVecOfPtrsType(Type *Ty) {
+  if (Ty->isVectorTy()) {
+Ty = cast(Ty)->getElementType();
+  }
+  return Ty->isPointerTy();
+}
+
+static Type *getPtrOrVecOfPtrsWithNewAS(Type *Ty, unsigned NewAddrSpace) {
+  if (!Ty->isVectorTy()) {
+assert(Ty->isPointerTy());
+return PointerType::getWithSamePointeeType(cast(Ty),
+   NewAddrSpace);
+  }
+
+  Type *PT = cast(Ty)->getElementType();
+  assert(PT->isPointerTy());
+
+  Type *NPT =
+  PointerType::getWithSamePointeeType(cast(PT), NewAddrSpace);
+  return VectorType::get(NPT, cast(Ty)->getElementCount());
+}
+
+static bool hasSameElementOfPtrOrVecPtrs(Type *Ty1, Type *Ty2) {
+  assert(isPtrOrVecOfPtrsType(Ty1) && isPtrOrVecOfPtrsType(Ty2));
+  assert(Ty1->isVectorTy() == Ty2->isVectorTy());
+  if (Ty1->isVectorTy()) {
+Ty1 = cast(Ty1)->getElementType();
+Ty2 = cast(Ty2)->getElementType();
+  }
+
+  assert(Ty1->isPointerTy() && Ty2->isPointerTy());
+  return cast(Ty1)->hasSameElementTypeAs(cast(Ty2));
+}
+
 // Check whether that's no-op pointer bicast using a pair of
 // `ptrtoint`/`inttoptr` due to the missing no-op pointer bitcast over
 // different address spaces.
@@ -279,8 +321,9 @@
   // arithmetic may also be undefined after invalid pointer reinterpret cast.
   // However, as we confirm through the target hooks that it's a no-op
   // addrspacecast, it doesn't matter since the bits should be the same.
-  unsigned P2IOp0AS = P2I->getOperand(0)->getType()->getPointerAddre

[Lldb-commits] [PATCH] D150043: [InferAddressSpaces] Handle vector of pointers type & Support intrinsic masked gather/scatter

2023-05-08 Thread CaprYang via Phabricator via lldb-commits 
CaprYang updated this revision to Diff 520302.
CaprYang retitled this revision from "[InferAddressSpaces] Handle vector of 
pointers type" to "[InferAddressSpaces] Handle vector of pointers type & 
Support intrinsic masked gather/scatter".

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

https://reviews.llvm.org/D150043

Files:
  llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
  llvm/test/Transforms/InferAddressSpaces/AMDGPU/icmp.ll
  llvm/test/Transforms/InferAddressSpaces/masked-gather-scatter.ll

Index: llvm/test/Transforms/InferAddressSpaces/masked-gather-scatter.ll
===
--- /dev/null
+++ llvm/test/Transforms/InferAddressSpaces/masked-gather-scatter.ll
@@ -0,0 +1,25 @@
+; RUN: opt -S -passes=infer-address-spaces -assume-default-is-flat-addrspace %s | FileCheck %s
+
+; CHECK-LABEL: @masked_gather_inferas(
+; CHECK: tail call <4 x i32> @llvm.masked.gather.v4i32.v4p1
+define <4 x i32> @masked_gather_inferas(ptr addrspace(1) %out, <4 x i64> %index) {
+entry:
+  %out.1 = addrspacecast ptr addrspace(1) %out to ptr
+  %ptrs = getelementptr inbounds i32, ptr %out.1, <4 x i64> %index
+  %value = tail call <4 x i32> @llvm.masked.gather.v4i32.v4p0(<4 x ptr> %ptrs, i32 4, <4 x i1> , <4 x i32> poison)
+  ret <4 x i32> %value
+}
+
+; CHECK-LABEL: @masked_scatter_inferas(
+; CHECK: tail call void @llvm.masked.scatter.v4i32.v4p1
+define void @masked_scatter_inferas(ptr addrspace(1) %out, <4 x i64> %index, <4 x i32> %value) {
+entry:
+  %out.1 = addrspacecast ptr addrspace(1) %out to ptr
+  %ptrs = getelementptr inbounds i32, ptr %out.1, <4 x i64> %index
+  tail call void @llvm.masked.scatter.v4i32.v4p0(<4 x i32> %value, <4 x ptr> %ptrs, i32 4, <4 x i1> )
+  ret void
+}
+
+declare <4 x i32> @llvm.masked.gather.v4i32.v4p0(<4 x ptr>, i32 immarg, <4 x i1>, <4 x i32>)
+
+declare void @llvm.masked.scatter.v4i32.v4p0(<4 x i32>, <4 x ptr>, i32 immarg, <4 x i1>)
\ No newline at end of file
Index: llvm/test/Transforms/InferAddressSpaces/AMDGPU/icmp.ll
===
--- llvm/test/Transforms/InferAddressSpaces/AMDGPU/icmp.ll
+++ llvm/test/Transforms/InferAddressSpaces/AMDGPU/icmp.ll
@@ -147,9 +147,8 @@
   ret i1 %cmp
 }
 
-; TODO: Should be handled
 ; CHECK-LABEL: @icmp_flat_flat_from_group_vector(
-; CHECK: %cmp = icmp eq <2 x ptr> %cast0, %cast1
+; CHECK: %cmp = icmp eq <2 x ptr addrspace(3)> %group.ptr.0, %group.ptr.1
 define <2 x i1> @icmp_flat_flat_from_group_vector(<2 x ptr addrspace(3)> %group.ptr.0, <2 x ptr addrspace(3)> %group.ptr.1) #0 {
   %cast0 = addrspacecast <2 x ptr addrspace(3)> %group.ptr.0 to <2 x ptr>
   %cast1 = addrspacecast <2 x ptr addrspace(3)> %group.ptr.1 to <2 x ptr>
Index: llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
===
--- llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
+++ llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
@@ -256,6 +256,33 @@
 INITIALIZE_PASS_END(InferAddressSpaces, DEBUG_TYPE, "Infer address spaces",
 false, false)
 
+static Type *getPtrOrVecOfPtrsWithNewAS(Type *Ty, unsigned NewAddrSpace) {
+  if (!Ty->isVectorTy()) {
+assert(Ty->isPointerTy());
+return PointerType::getWithSamePointeeType(cast(Ty),
+   NewAddrSpace);
+  }
+
+  Type *PT = cast(Ty)->getElementType();
+  assert(PT->isPointerTy());
+
+  Type *NPT =
+  PointerType::getWithSamePointeeType(cast(PT), NewAddrSpace);
+  return VectorType::get(NPT, cast(Ty)->getElementCount());
+}
+
+static bool hasSameElementOfPtrOrVecPtrs(Type *Ty1, Type *Ty2) {
+  assert(Ty1->isPtrOrPtrVectorTy() && Ty2->isPtrOrPtrVectorTy());
+  assert(Ty1->isVectorTy() == Ty2->isVectorTy());
+  if (Ty1->isVectorTy()) {
+Ty1 = cast(Ty1)->getElementType();
+Ty2 = cast(Ty2)->getElementType();
+  }
+
+  assert(Ty1->isPointerTy() && Ty2->isPointerTy());
+  return cast(Ty1)->hasSameElementTypeAs(cast(Ty2));
+}
+
 // Check whether that's no-op pointer bicast using a pair of
 // `ptrtoint`/`inttoptr` due to the missing no-op pointer bitcast over
 // different address spaces.
@@ -301,14 +328,14 @@
 
   switch (Op->getOpcode()) {
   case Instruction::PHI:
-assert(Op->getType()->isPointerTy());
+assert(Op->getType()->isPtrOrPtrVectorTy());
 return true;
   case Instruction::BitCast:
   case Instruction::AddrSpaceCast:
   case Instruction::GetElementPtr:
 return true;
   case Instruction::Select:
-return Op->getType()->isPointerTy();
+return Op->getType()->isPtrOrPtrVectorTy();
   case Instruction::Call: {
 const IntrinsicInst *II = dyn_cast(&V);
 return II && II->getIntrinsicID() == Intrinsic::ptrmask;
@@ -373,6 +400,24 @@
   case Intrinsic::ptrmask:
 // This is handled as an address expression, not as a use memory operation.
 return false;
+  case Intrinsic::masked_gather: {
+Type *RetTy = II->getType();
+Type *NewPtrTy = NewV

[Lldb-commits] [PATCH] D150043: [InferAddressSpaces] Handle vector of pointers type & Support intrinsic masked gather/scatter

2023-05-08 Thread CaprYang via Phabricator via lldb-commits 
CaprYang updated this revision to Diff 520308.

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

https://reviews.llvm.org/D150043

Files:
  llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
  llvm/test/Transforms/InferAddressSpaces/AMDGPU/icmp.ll
  llvm/test/Transforms/InferAddressSpaces/masked-gather-scatter.ll

Index: llvm/test/Transforms/InferAddressSpaces/masked-gather-scatter.ll
===
--- /dev/null
+++ llvm/test/Transforms/InferAddressSpaces/masked-gather-scatter.ll
@@ -0,0 +1,25 @@
+; RUN: opt -S -passes=infer-address-spaces -assume-default-is-flat-addrspace %s | FileCheck %s
+
+; CHECK-LABEL: @masked_gather_inferas(
+; CHECK: tail call <4 x i32> @llvm.masked.gather.v4i32.v4p1
+define <4 x i32> @masked_gather_inferas(ptr addrspace(1) %out, <4 x i64> %index) {
+entry:
+  %out.1 = addrspacecast ptr addrspace(1) %out to ptr
+  %ptrs = getelementptr inbounds i32, ptr %out.1, <4 x i64> %index
+  %value = tail call <4 x i32> @llvm.masked.gather.v4i32.v4p0(<4 x ptr> %ptrs, i32 4, <4 x i1> , <4 x i32> poison)
+  ret <4 x i32> %value
+}
+
+; CHECK-LABEL: @masked_scatter_inferas(
+; CHECK: tail call void @llvm.masked.scatter.v4i32.v4p1
+define void @masked_scatter_inferas(ptr addrspace(1) %out, <4 x i64> %index, <4 x i32> %value) {
+entry:
+  %out.1 = addrspacecast ptr addrspace(1) %out to ptr
+  %ptrs = getelementptr inbounds i32, ptr %out.1, <4 x i64> %index
+  tail call void @llvm.masked.scatter.v4i32.v4p0(<4 x i32> %value, <4 x ptr> %ptrs, i32 4, <4 x i1> )
+  ret void
+}
+
+declare <4 x i32> @llvm.masked.gather.v4i32.v4p0(<4 x ptr>, i32 immarg, <4 x i1>, <4 x i32>)
+
+declare void @llvm.masked.scatter.v4i32.v4p0(<4 x i32>, <4 x ptr>, i32 immarg, <4 x i1>)
\ No newline at end of file
Index: llvm/test/Transforms/InferAddressSpaces/AMDGPU/icmp.ll
===
--- llvm/test/Transforms/InferAddressSpaces/AMDGPU/icmp.ll
+++ llvm/test/Transforms/InferAddressSpaces/AMDGPU/icmp.ll
@@ -147,9 +147,8 @@
   ret i1 %cmp
 }
 
-; TODO: Should be handled
 ; CHECK-LABEL: @icmp_flat_flat_from_group_vector(
-; CHECK: %cmp = icmp eq <2 x ptr> %cast0, %cast1
+; CHECK: %cmp = icmp eq <2 x ptr addrspace(3)> %group.ptr.0, %group.ptr.1
 define <2 x i1> @icmp_flat_flat_from_group_vector(<2 x ptr addrspace(3)> %group.ptr.0, <2 x ptr addrspace(3)> %group.ptr.1) #0 {
   %cast0 = addrspacecast <2 x ptr addrspace(3)> %group.ptr.0 to <2 x ptr>
   %cast1 = addrspacecast <2 x ptr addrspace(3)> %group.ptr.1 to <2 x ptr>
Index: llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
===
--- llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
+++ llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp
@@ -256,6 +256,21 @@
 INITIALIZE_PASS_END(InferAddressSpaces, DEBUG_TYPE, "Infer address spaces",
 false, false)
 
+static Type *getPtrOrVecOfPtrsWithNewAS(Type *Ty, unsigned NewAddrSpace) {
+  if (!Ty->isVectorTy()) {
+assert(Ty->isPointerTy());
+return PointerType::getWithSamePointeeType(cast(Ty),
+   NewAddrSpace);
+  }
+
+  Type *PT = cast(Ty)->getElementType();
+  assert(PT->isPointerTy());
+
+  Type *NPT =
+  PointerType::getWithSamePointeeType(cast(PT), NewAddrSpace);
+  return VectorType::get(NPT, cast(Ty)->getElementCount());
+}
+
 // Check whether that's no-op pointer bicast using a pair of
 // `ptrtoint`/`inttoptr` due to the missing no-op pointer bitcast over
 // different address spaces.
@@ -301,14 +316,14 @@
 
   switch (Op->getOpcode()) {
   case Instruction::PHI:
-assert(Op->getType()->isPointerTy());
+assert(Op->getType()->isPtrOrPtrVectorTy());
 return true;
   case Instruction::BitCast:
   case Instruction::AddrSpaceCast:
   case Instruction::GetElementPtr:
 return true;
   case Instruction::Select:
-return Op->getType()->isPointerTy();
+return Op->getType()->isPtrOrPtrVectorTy();
   case Instruction::Call: {
 const IntrinsicInst *II = dyn_cast(&V);
 return II && II->getIntrinsicID() == Intrinsic::ptrmask;
@@ -373,6 +388,24 @@
   case Intrinsic::ptrmask:
 // This is handled as an address expression, not as a use memory operation.
 return false;
+  case Intrinsic::masked_gather: {
+Type *RetTy = II->getType();
+Type *NewPtrTy = NewV->getType();
+Function *NewDecl =
+Intrinsic::getDeclaration(M, II->getIntrinsicID(), {RetTy, NewPtrTy});
+II->setArgOperand(0, NewV);
+II->setCalledFunction(NewDecl);
+return true;
+  }
+  case Intrinsic::masked_scatter: {
+Type *ValueTy = II->getOperand(0)->getType();
+Type *NewPtrTy = NewV->getType();
+Function *NewDecl =
+Intrinsic::getDeclaration(M, II->getIntrinsicID(), {ValueTy, NewPtrTy});
+II->setArgOperand(1, NewV);
+II->setCalledFunction(NewDecl);
+return true;
+  }
   default: {
 Value *Rewrite = TTI->rewriteIntrinsicWithAddre

[Lldb-commits] [lldb] ba902ef - Skip test when compiling with older versions of clang

2023-05-08 Thread Adrian Prantl via lldb-commits 

Author: Adrian Prantl
Date: 2023-05-08T09:24:40-07:00
New Revision: ba902efa499a092405c207d71a98af1e38853d7b

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

LOG: Skip test when compiling with older versions of clang

Added: 


Modified: 
lldb/test/API/commands/expression/macros/TestMacros.py

Removed: 




diff  --git a/lldb/test/API/commands/expression/macros/TestMacros.py 
b/lldb/test/API/commands/expression/macros/TestMacros.py
index b39572b6e4408..278683e62ea05 100644
--- a/lldb/test/API/commands/expression/macros/TestMacros.py
+++ b/lldb/test/API/commands/expression/macros/TestMacros.py
@@ -8,6 +8,7 @@
 
 class TestMacros(TestBase):
 
+@skipIf(compiler="clang", compiler_version=['<', '9.0'])
 @expectedFailureAll(
 compiler="clang",
 bugnumber="clang does not emit .debug_macro[.dwo] sections.")



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

[Lldb-commits] [PATCH] D150032: [lldb, NetBSD] getValue => operator* for Optional migration

2023-05-08 Thread Ed Maste via Phabricator via lldb-commits 
emaste added a comment.
Herald added a subscriber: JDevlieghere.

> because Phabricator is a PoS that converts patches to useless diffs

FWIW the Phorge folks have taken an interest in these sorts of complaints 
brought by the FreeBSD community, e.g. https://we.phorge.it/Q46, 
https://we.phorge.it/T15249, https://we.phorge.it/T15096, and 
https://we.phorge.it/T15364


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150032

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

[Lldb-commits] [lldb] 7652377 - [lldb] Prevent mutation of CommandAlias::GetOptionArguments

2023-05-08 Thread Dave Lee via lldb-commits 

Author: Dave Lee
Date: 2023-05-08T09:45:26-07:00
New Revision: 765237779ce4dbb60f4faf9ebc91386ce5218e15

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

LOG: [lldb] Prevent mutation of CommandAlias::GetOptionArguments

Fix a mutation of `CommandAlias::m_option_args_sp`, which resulted in cases 
where
aliases would fail to run on second, and subsequent times.

For example, an alias such as:

```
command alias p1 p 1
```

When run the second time, the following error would be reported to the user:

```
error: expression failed to parse:
error: :1:1: expression is not assignable
--  1
^   ~
```

To fix this, `CommandAlias::Desugar` now constructs options to a freshly 
constructed
vector, rather than by appending to the results of `GetOptionArguments`.

rdar://107770836

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

Added: 


Modified: 
lldb/source/Interpreter/CommandAlias.cpp
lldb/test/API/commands/command/nested_alias/TestNestedAlias.py

Removed: 




diff  --git a/lldb/source/Interpreter/CommandAlias.cpp 
b/lldb/source/Interpreter/CommandAlias.cpp
index e36edcac15a59..f6eaeacff81ef 100644
--- a/lldb/source/Interpreter/CommandAlias.cpp
+++ b/lldb/source/Interpreter/CommandAlias.cpp
@@ -8,6 +8,7 @@
 
 #include "lldb/Interpreter/CommandAlias.h"
 
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/ErrorHandling.h"
 
 #include "lldb/Interpreter/CommandInterpreter.h"
@@ -211,9 +212,9 @@ std::pair 
CommandAlias::Desugar() {
 // FIXME: This doesn't work if the original alias fills a slot in the
 // underlying alias, since this just appends the two lists.
 auto desugared = ((CommandAlias *)underlying.get())->Desugar();
-auto options = GetOptionArguments();
-options->insert(options->begin(), desugared.second->begin(),
-desugared.second->end());
+OptionArgVectorSP options = std::make_shared();
+llvm::append_range(*options, *desugared.second);
+llvm::append_range(*options, *GetOptionArguments());
 return {desugared.first, options};
   }
 

diff  --git a/lldb/test/API/commands/command/nested_alias/TestNestedAlias.py 
b/lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
index 29b4af335028d..7cce6138aa3e0 100644
--- a/lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
+++ b/lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
@@ -101,3 +101,7 @@ def cleanup():
 self.expect('command alias two expr -- 2')
 self.expect('command alias add_two two +')
 self.expect('add_two 3', patterns=[' = 5$'])
+# Check that aliases to aliases to raw input commands work the second
+# and subsequent times.
+self.expect('add_two 3', patterns=[' = 5$'])
+self.expect('add_two 3', patterns=[' = 5$'])



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

[Lldb-commits] [PATCH] D150078: [lldb] Prevent mutation of CommandAlias::GetOptionArguments

2023-05-08 Thread Dave Lee via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG765237779ce4: [lldb] Prevent mutation of 
CommandAlias::GetOptionArguments (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150078

Files:
  lldb/source/Interpreter/CommandAlias.cpp
  lldb/test/API/commands/command/nested_alias/TestNestedAlias.py


Index: lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
===
--- lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
+++ lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
@@ -101,3 +101,7 @@
 self.expect('command alias two expr -- 2')
 self.expect('command alias add_two two +')
 self.expect('add_two 3', patterns=[' = 5$'])
+# Check that aliases to aliases to raw input commands work the second
+# and subsequent times.
+self.expect('add_two 3', patterns=[' = 5$'])
+self.expect('add_two 3', patterns=[' = 5$'])
Index: lldb/source/Interpreter/CommandAlias.cpp
===
--- lldb/source/Interpreter/CommandAlias.cpp
+++ lldb/source/Interpreter/CommandAlias.cpp
@@ -8,6 +8,7 @@
 
 #include "lldb/Interpreter/CommandAlias.h"
 
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/ErrorHandling.h"
 
 #include "lldb/Interpreter/CommandInterpreter.h"
@@ -211,9 +212,9 @@
 // FIXME: This doesn't work if the original alias fills a slot in the
 // underlying alias, since this just appends the two lists.
 auto desugared = ((CommandAlias *)underlying.get())->Desugar();
-auto options = GetOptionArguments();
-options->insert(options->begin(), desugared.second->begin(),
-desugared.second->end());
+OptionArgVectorSP options = std::make_shared();
+llvm::append_range(*options, *desugared.second);
+llvm::append_range(*options, *GetOptionArguments());
 return {desugared.first, options};
   }
 


Index: lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
===
--- lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
+++ lldb/test/API/commands/command/nested_alias/TestNestedAlias.py
@@ -101,3 +101,7 @@
 self.expect('command alias two expr -- 2')
 self.expect('command alias add_two two +')
 self.expect('add_two 3', patterns=[' = 5$'])
+# Check that aliases to aliases to raw input commands work the second
+# and subsequent times.
+self.expect('add_two 3', patterns=[' = 5$'])
+self.expect('add_two 3', patterns=[' = 5$'])
Index: lldb/source/Interpreter/CommandAlias.cpp
===
--- lldb/source/Interpreter/CommandAlias.cpp
+++ lldb/source/Interpreter/CommandAlias.cpp
@@ -8,6 +8,7 @@
 
 #include "lldb/Interpreter/CommandAlias.h"
 
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/ErrorHandling.h"
 
 #include "lldb/Interpreter/CommandInterpreter.h"
@@ -211,9 +212,9 @@
 // FIXME: This doesn't work if the original alias fills a slot in the
 // underlying alias, since this just appends the two lists.
 auto desugared = ((CommandAlias *)underlying.get())->Desugar();
-auto options = GetOptionArguments();
-options->insert(options->begin(), desugared.second->begin(),
-desugared.second->end());
+OptionArgVectorSP options = std::make_shared();
+llvm::append_range(*options, *desugared.second);
+llvm::append_range(*options, *GetOptionArguments());
 return {desugared.first, options};
   }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150129: [lldb] Refine call to decl printing helper (NFC)

2023-05-08 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione created this revision.
kastiglione added reviewers: aprantl, JDevlieghere, jingham.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

When `ValueObjectPrinter` calls its `m_decl_printing_helper`, not all state is 
passed to
the helper. In particular, the helper doesn't have access to `m_curr_depth`, 
and thus
can't act on the logic within `ShouldShowName`.

To address this, this change passes in a modified copy of `m_options`. The 
modified copy
has has `m_hide_name` set according to the results of `ShouldShowName`. This 
allows
helper functions to know whether the name should be shown or hidden, without 
having
access to `ValueObjectPrinter`'s full state.

This is NFC in mainline lldb, as the only decl printing helper doesn't make use 
of this.
However in swift-lldb at least, there are decl printing helpers that do need 
this
information passed to them. See https://github.com/apple/llvm-project/pull/6795 
where a
test is also included.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150129

Files:
  lldb/source/DataFormatters/ValueObjectPrinter.cpp


Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -300,9 +300,14 @@
 ConstString type_name_cstr(typeName.GetString());
 ConstString var_name_cstr(varName.GetString());
 
+DumpValueObjectOptions decl_print_options = m_options;
+// Pass printing helpers an option object that indicates whether the name
+// should be shown or hidden.
+decl_print_options.SetHideName(!ShouldShowName());
+
 StreamString dest_stream;
 if (m_options.m_decl_printing_helper(type_name_cstr, var_name_cstr,
- m_options, dest_stream)) {
+ decl_print_options, dest_stream)) {
   decl_printed = true;
   m_stream->PutCString(dest_stream.GetString());
 }


Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -300,9 +300,14 @@
 ConstString type_name_cstr(typeName.GetString());
 ConstString var_name_cstr(varName.GetString());
 
+DumpValueObjectOptions decl_print_options = m_options;
+// Pass printing helpers an option object that indicates whether the name
+// should be shown or hidden.
+decl_print_options.SetHideName(!ShouldShowName());
+
 StreamString dest_stream;
 if (m_options.m_decl_printing_helper(type_name_cstr, var_name_cstr,
- m_options, dest_stream)) {
+ decl_print_options, dest_stream)) {
   decl_printed = true;
   m_stream->PutCString(dest_stream.GetString());
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150032: [lldb, NetBSD] getValue => operator* for Optional migration

2023-05-08 Thread Nikita Ronja Gillmann via Phabricator via lldb-commits 
nikicoon added a comment.

In D150032#4327156 , @emaste wrote:

>> because Phabricator is a PoS that converts patches to useless diffs
>
> FWIW the Phorge folks have taken an interest in these sorts of complaints 
> brought by the FreeBSD community, e.g. https://we.phorge.it/Q46, 
> https://we.phorge.it/T15249, https://we.phorge.it/T15096, and 
> https://we.phorge.it/T15364

we have some (iirc not that many) build unbreaking patches in pkgsrc for 
various llvm projects which I plan on upstreaming if possible, so that we just 
have to rely on build-system adjustments for pkgsrc specific settings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150032

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

[Lldb-commits] [PATCH] D149774: [lldb] Use templates to simplify {Get, Set}PropertyAtIndex (NFC)

2023-05-08 Thread Jorge Gorbe Moya via Phabricator via lldb-commits 
jgorbe added inline comments.



Comment at: lldb/source/Target/Target.cpp:4520
   const uint32_t idx = ePropertyMaxSummaryLength;
-  return m_collection_sp->GetPropertyAtIndexAsSInt64(idx).value_or(
-  g_target_properties[idx].default_uint_value);
+  return GetPropertyAtIndexAs(
+  idx, g_target_properties[idx].default_uint_value);

I think this is causing `settings set target.max-string-summary-length` not to 
work properly.

As far as I can tell, the problem is that `GetPropertyAtIndexAs` will 
end up calling `OptionValue::GetAsUInt64()` which checks the declared type of 
the property and returns null if there's a mismatch. 
`target.max-string-summary-length` is, for some reason, defined as signed, so 
this will always return the default value regardless of what it was set to.

You should be able to reproduce it by writing a simple program with a 
`std::string`, setting `target.max-string-summary-length` to some small value 
and checking that the string is not truncated.

`GetMaximumMemReadSize`, just below this one, seems to be mismatched too 
because the diff shows a change from `GetPropertyAtIndexAsSInt64` to 
`GetPropertyAtIndexAs` but I haven't actually tested that one. It 
would probably be a good idea to double check the rest of the patch looking for 
other mismatches.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149774

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

[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

2023-05-08 Thread Andrey Ali Khan Bolshakov via Phabricator via lldb-commits 
bolshakov-a updated this revision to Diff 520478.
bolshakov-a added a subscriber: hubert.reinterpretcast.
bolshakov-a added a comment.

Avoid binding references in template arguments to bit-fields. @erichkeane, 
@hubert.reinterpretcast, please verify.


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

https://reviews.llvm.org/D140996

Files:
  clang-tools-extra/clangd/DumpAST.cpp
  clang-tools-extra/clangd/FindTarget.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ODRHash.h
  clang/include/clang/AST/PropertiesBase.td
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/TemplateArgumentVisitor.h
  clang/include/clang/AST/TemplateBase.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/ODRHash.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TemplateBase.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Index/USRGeneration.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CXX/drs/dr12xx.cpp
  clang/test/CXX/temp/temp.arg/temp.arg.nontype/p1.cpp
  clang/test/CodeGenCXX/mangle-ms-templates.cpp
  clang/test/CodeGenCXX/mangle-template.cpp
  clang/test/CodeGenCXX/template-arguments.cpp
  clang/test/Index/USR/structural-value-tpl-arg.cpp
  clang/test/Modules/odr_hash.cpp
  clang/test/SemaCXX/warn-bool-conversion.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/CXCursor.cpp
  clang/www/cxx_status.html
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -7305,6 +7305,9 @@
 
   case clang::TemplateArgument::Pack:
 return eTemplateArgumentKindPack;
+
+  case clang::TemplateArgument::StructuralValue:
+return eTemplateArgumentKindStructuralValue;
   }
   llvm_unreachable("Unhandled clang::TemplateArgument::ArgKind");
 }
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -849,6 +849,7 @@
   eTemplateArgumentKindExpression,
   eTemplateArgumentKindPack,
   eTemplateArgumentKindNullPtr,
+  eTemplateArgumentKindStructuralValue,
 };
 
 /// Type of match to be performed when looking for a formatter for a data type.
Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1053,13 +1053,21 @@
 
 
 
-  Class types as non-type template parameters
+  Class types as non-type template parameters
   https://wg21.link/p0732r2";>P0732R2
-  Partial
+  Clang 12
+
+ 
+  Generalized non-type template parameters of scalar type
+  https://wg21.link/p1907r1";>P1907R1
+  
+
+  Clang 17 (Partial)
+  Reference type template arguments referring to instantiation-dependent objects and subobjects
+  (i.e. declared inside a template but neither type- nor value-dependent) aren't fully supported.
+
+  
 
-   
-https://wg21.link/p1907r1";>P1907R1
-  
 
   Destroying operator delete
   https://wg21.link/p0722r3";>P0722R3
Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -1463,6 +1463,9 @@
 return CXTemplateArgumentKind_NullPtr;
   case TemplateArgument::Integral:
 return CXTemplateArgumentKind_Integral;
+  case TemplateArgument::StructuralValue:
+// FIXME: Expose these values.
+return CXTemplateArgumentKind_Invalid;
   case TemplateArgument::Template:
 return CXTemplateArgumentKind_Template;
   case TemplateArgument::TemplateExpansion:
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1570,6 +1570,11 @@
   return Visit(MakeCXCursor(E, StmtParent, TU, RegionOfInterest));
 return false;
 
+  case TemplateArgument::StructuralVal

[Lldb-commits] [PATCH] D150158: Add optional to debugserver's "tell me about all binaries in the process" packet, to limit it to just load addresses and names

2023-05-08 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda created this revision.
jasonmolenda added a reviewer: bulbazord.
jasonmolenda added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

debugserver has a memory use problem when a process has many (1000+) binaries 
loaded in it.  lldb sends the `jGetLoadedDynamicLibrariesInfos` packet to get a 
JSON description of those binaries -- load address, filepath, mach-o header, 
mach-o load commands.  This can result in such a large packet that it will 
temporarily cause debugserver's memory use to balloon up for the duration of 
the packet processes.  I made a first crack at this problem a year ago ( 
https://reviews.llvm.org/D122848 and https://reviews.llvm.org/D122882 ) to 
reduce unnecessary copies while creating this response, but inevitably there 
will be more binaries and we'll be back here again.

This patch adds a new option to the `jGetLoadedDynamicLibrariesInfos 
fetch-all-binaries` packet to only report the mach-o load addresses and the 
filepaths -- to skip the mach-o header & load command scan and JSONification.  
The default behavior is unchanged.

In a future patch, I will change DynamicLoaderMacOS to request binaries in 
batches of maybe 300 per request.  When it attaches to a new process, it will 
ask for the load addresses of the binaries, and get the full information in 
these batches.  On notification of new binaries being loaded, it will already 
have the load addresses and can skip this first request.

As long as I was updating this code, I also removed support for our pre-iOS 
10/macOS 10.12 way of querying binaries loaded in a process.  We can't build 
debugserver with a deployment target that old any more; it is not testable.

This patch does have a regression on 
API/macosx/unregistered-macho/TestUnregisteredMacho.py that I wrote last 
summer; I have the patch generate JSON for a binary that it had an address for, 
but could not read the mach-o headers.  This test case is specifically testing 
the address of something that is not a mach-o in memory and testing that nothin 
is returned.  I need to revisit why I wrote this before I fix it by changing my 
patch or removing the test, but the reason for the test failure is obvious, 
it's a minor revision either way.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150158

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNB.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm
  lldb/tools/debugserver/source/RNBRemote.cpp

Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -5927,17 +5927,10 @@
   return SendPacket("OK");
 }
 
-//  This packet may be called in one of three ways:
-//
-//  jGetLoadedDynamicLibrariesInfos:{"image_count":40,"image_list_address":4295244704}
-//  Look for an array of the old dyld_all_image_infos style of binary infos
-//  at the image_list_address.
-//  This an array of {void* load_addr, void* mod_date, void* pathname}
+//  This packet may be called in one of two ways:
 //
 //  jGetLoadedDynamicLibrariesInfos:{"fetch_all_solibs":true}
-//  Use the new style (macOS 10.12, tvOS 10, iOS 10, watchOS 3) dyld SPI to
-//  get a list of all the
-//  libraries loaded
+//  Use the new dyld SPI to get a list of all the libraries loaded
 //
 //  jGetLoadedDynamicLibrariesInfos:{"solib_addresses":[8382824135,3258302053,830202858503]}
 //  Use the new style (macOS 10.12, tvOS 10, iOS 10, watchOS 3) dyld SPI to
@@ -5964,24 +5957,17 @@
 
 std::vector macho_addresses;
 bool fetch_all_solibs = false;
+bool report_load_commands = true;
+get_boolean_value_for_key_name_from_json("report_load_commands", p,
+ report_load_commands);
+
 if (get_boolean_value_for_key_name_from_json("fetch_all_solibs", p,
  fetch_all_solibs) &&
 fetch_all_solibs) {
-  json_sp = DNBGetAllLoadedLibrariesInfos(pid);
+  json_sp = DNBGetAllLoadedLibrariesInfos(pid, report_load_commands);
 } else if (get_array_of_ints_value_for_key_name_from_json(
"solib_addresses", p, macho_addresses)) {
   json_sp = DNBGetLibrariesInfoForAddresses(pid, macho_addresses);
-} else {
-  nub_addr_t image_list_address =
-  get_integer_value_for_key_name_from_json("image_list_address", p);
-  nub_addr_t image_count =
-  get_integer_value_for_key_name_from_json("image_count", p);
-
-  if (image_list_address != INVALID_NUB_ADDRESS &&
-  image_count != INVALID_NUB_ADDRESS) {
-json_sp = DNBGetLoadedDynamicLibrariesInfos(pid, im

[Lldb-commits] [PATCH] D150160: [lldb] Simplify Log::PutString (NFC)

2023-05-08 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione created this revision.
kastiglione added reviewers: mib, bulbazord.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

- As no format string is involved, avoid unecessary call into `Printf`
- Eliminate creation of a `std::string` to print a `StringRef`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150160

Files:
  lldb/source/Utility/Log.cpp


Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -131,8 +131,15 @@
   return m_mask.load(std::memory_order_relaxed);
 }
 
-void Log::PutCString(const char *cstr) { Printf("%s", cstr); }
-void Log::PutString(llvm::StringRef str) { PutCString(str.str().c_str()); }
+void Log::PutCString(const char *cstr) { PutString(cstr); }
+
+void Log::PutString(llvm::StringRef str) {
+  std::string FinalMessage;
+  llvm::raw_string_ostream Stream(FinalMessage);
+  WriteHeader(Stream, "", "");
+  Stream << str << "\n";
+  WriteMessage(FinalMessage);
+}
 
 // Simple variable argument logging with flags.
 void Log::Printf(const char *format, ...) {
@@ -146,16 +153,9 @@
 // callback registered, then we call the logging callback. If we have a valid
 // file handle, we also log to the file.
 void Log::VAPrintf(const char *format, va_list args) {
-  std::string FinalMessage;
-  llvm::raw_string_ostream Stream(FinalMessage);
-  WriteHeader(Stream, "", "");
-
   llvm::SmallString<64> Content;
   lldb_private::VASprintf(Content, format, args);
-
-  Stream << Content << "\n";
-
-  WriteMessage(FinalMessage);
+  PutString(Content);
 }
 
 // Printing of errors that are not fatal.


Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -131,8 +131,15 @@
   return m_mask.load(std::memory_order_relaxed);
 }
 
-void Log::PutCString(const char *cstr) { Printf("%s", cstr); }
-void Log::PutString(llvm::StringRef str) { PutCString(str.str().c_str()); }
+void Log::PutCString(const char *cstr) { PutString(cstr); }
+
+void Log::PutString(llvm::StringRef str) {
+  std::string FinalMessage;
+  llvm::raw_string_ostream Stream(FinalMessage);
+  WriteHeader(Stream, "", "");
+  Stream << str << "\n";
+  WriteMessage(FinalMessage);
+}
 
 // Simple variable argument logging with flags.
 void Log::Printf(const char *format, ...) {
@@ -146,16 +153,9 @@
 // callback registered, then we call the logging callback. If we have a valid
 // file handle, we also log to the file.
 void Log::VAPrintf(const char *format, va_list args) {
-  std::string FinalMessage;
-  llvm::raw_string_ostream Stream(FinalMessage);
-  WriteHeader(Stream, "", "");
-
   llvm::SmallString<64> Content;
   lldb_private::VASprintf(Content, format, args);
-
-  Stream << Content << "\n";
-
-  WriteMessage(FinalMessage);
+  PutString(Content);
 }
 
 // Printing of errors that are not fatal.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150160: [lldb] Simplify Log::PutString (NFC)

2023-05-08 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord accepted this revision.
bulbazord added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: lldb/source/Utility/Log.cpp:135
-void Log::PutCString(const char *cstr) { Printf("%s", cstr); }
-void Log::PutString(llvm::StringRef str) { PutCString(str.str().c_str()); }
 

Amazing



Comment at: lldb/source/Utility/Log.cpp:151-154
 
 // All logging eventually boils down to this function call. If we have a
 // callback registered, then we call the logging callback. If we have a valid
 // file handle, we also log to the file.

We can probably update this comment as it's no longer true: `Log::PutCString` 
will skip `Printf` -> `VAPrintf` entirely now...



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150160

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

[Lldb-commits] [PATCH] D150160: [lldb] Simplify Log::PutString (NFC)

2023-05-08 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione updated this revision to Diff 520541.
kastiglione added a comment.

Update comments too


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150160

Files:
  lldb/source/Utility/Log.cpp


Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -131,8 +131,15 @@
   return m_mask.load(std::memory_order_relaxed);
 }
 
-void Log::PutCString(const char *cstr) { Printf("%s", cstr); }
-void Log::PutString(llvm::StringRef str) { PutCString(str.str().c_str()); }
+void Log::PutCString(const char *cstr) { PutString(cstr); }
+
+void Log::PutString(llvm::StringRef str) {
+  std::string FinalMessage;
+  llvm::raw_string_ostream Stream(FinalMessage);
+  WriteHeader(Stream, "", "");
+  Stream << str << "\n";
+  WriteMessage(FinalMessage);
+}
 
 // Simple variable argument logging with flags.
 void Log::Printf(const char *format, ...) {
@@ -142,20 +149,10 @@
   va_end(args);
 }
 
-// All logging eventually boils down to this function call. If we have a
-// callback registered, then we call the logging callback. If we have a valid
-// file handle, we also log to the file.
 void Log::VAPrintf(const char *format, va_list args) {
-  std::string FinalMessage;
-  llvm::raw_string_ostream Stream(FinalMessage);
-  WriteHeader(Stream, "", "");
-
   llvm::SmallString<64> Content;
   lldb_private::VASprintf(Content, format, args);
-
-  Stream << Content << "\n";
-
-  WriteMessage(FinalMessage);
+  PutString(Content);
 }
 
 // Printing of errors that are not fatal.
@@ -344,6 +341,8 @@
   }
 }
 
+// If we have a callback registered, then we call the logging callback. If we
+// have a valid file handle, we also log to the file.
 void Log::WriteMessage(llvm::StringRef message) {
   // Make a copy of our stream shared pointer in case someone disables our log
   // while we are logging and releases the stream


Index: lldb/source/Utility/Log.cpp
===
--- lldb/source/Utility/Log.cpp
+++ lldb/source/Utility/Log.cpp
@@ -131,8 +131,15 @@
   return m_mask.load(std::memory_order_relaxed);
 }
 
-void Log::PutCString(const char *cstr) { Printf("%s", cstr); }
-void Log::PutString(llvm::StringRef str) { PutCString(str.str().c_str()); }
+void Log::PutCString(const char *cstr) { PutString(cstr); }
+
+void Log::PutString(llvm::StringRef str) {
+  std::string FinalMessage;
+  llvm::raw_string_ostream Stream(FinalMessage);
+  WriteHeader(Stream, "", "");
+  Stream << str << "\n";
+  WriteMessage(FinalMessage);
+}
 
 // Simple variable argument logging with flags.
 void Log::Printf(const char *format, ...) {
@@ -142,20 +149,10 @@
   va_end(args);
 }
 
-// All logging eventually boils down to this function call. If we have a
-// callback registered, then we call the logging callback. If we have a valid
-// file handle, we also log to the file.
 void Log::VAPrintf(const char *format, va_list args) {
-  std::string FinalMessage;
-  llvm::raw_string_ostream Stream(FinalMessage);
-  WriteHeader(Stream, "", "");
-
   llvm::SmallString<64> Content;
   lldb_private::VASprintf(Content, format, args);
-
-  Stream << Content << "\n";
-
-  WriteMessage(FinalMessage);
+  PutString(Content);
 }
 
 // Printing of errors that are not fatal.
@@ -344,6 +341,8 @@
   }
 }
 
+// If we have a callback registered, then we call the logging callback. If we
+// have a valid file handle, we also log to the file.
 void Log::WriteMessage(llvm::StringRef message) {
   // Make a copy of our stream shared pointer in case someone disables our log
   // while we are logging and releases the stream
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150158: Add optional to debugserver's "tell me about all binaries in the process" packet, to limit it to just load addresses and names

2023-05-08 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord added a comment.

This looks good to me.

> This patch does have a regression on 
> API/macosx/unregistered-macho/TestUnregisteredMacho.py that I wrote last 
> summer; I have the patch generate JSON for a binary that it had an address 
> for, but could not read the mach-o headers. This test case is specifically 
> testing the address of something that is not a mach-o in memory and testing 
> that nothin is returned. I need to revisit why I wrote this before I fix it 
> by changing my patch or removing the test, but the reason for the test 
> failure is obvious, it's a minor revision either way.

I assume you plan on addressing this failure before landing this change or 
otherwise disabling the test?




Comment at: lldb/tools/debugserver/source/MacOSX/MachProcess.mm:930-931
+if (!image_infos[i].is_valid_mach_header) {
+  image_infos_array_sp->AddItem(image_info_dict_sp);
+  continue;
+}

Why do we still want to add information about this dylib if the mach header 
isn't valid? 



Comment at: lldb/tools/debugserver/source/RNBRemote.cpp:5932-5933
 //
 //  jGetLoadedDynamicLibrariesInfos:{"fetch_all_solibs":true}
-//  Use the new style (macOS 10.12, tvOS 10, iOS 10, watchOS 3) dyld SPI to
-//  get a list of all the
-//  libraries loaded
+//  Use the new dyld SPI to get a list of all the libraries loaded
 //

Should probably update this comment with `"report_load_commands":false` as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150158

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

[Lldb-commits] [PATCH] D150158: Add optional to debugserver's "tell me about all binaries in the process" packet, to limit it to just load addresses and names

2023-05-08 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda added a comment.

Thanks for the feedback.  Testing now & will update the patch in a bit.




Comment at: lldb/tools/debugserver/source/MacOSX/MachProcess.mm:930-931
+if (!image_infos[i].is_valid_mach_header) {
+  image_infos_array_sp->AddItem(image_info_dict_sp);
+  continue;
+}

bulbazord wrote:
> Why do we still want to add information about this dylib if the mach header 
> isn't valid? 
Yeah, I was being too clever here, it's not clear what I'm doing (and it's the 
cause of the test failure I was looking at).  I tried to overload "could not 
read the header/load commands" for both "it failed" and "lldb told us to not 
read them".  I'll pass down the bool for whether we're including the 
header/loadcmds to make it a lot clearer.



Comment at: lldb/tools/debugserver/source/RNBRemote.cpp:5932-5933
 //
 //  jGetLoadedDynamicLibrariesInfos:{"fetch_all_solibs":true}
-//  Use the new style (macOS 10.12, tvOS 10, iOS 10, watchOS 3) dyld SPI to
-//  get a list of all the
-//  libraries loaded
+//  Use the new dyld SPI to get a list of all the libraries loaded
 //

bulbazord wrote:
> Should probably update this comment with `"report_load_commands":false` as 
> well.
Good catch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150158

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

[Lldb-commits] [PATCH] D150158: Add optional to debugserver's "tell me about all binaries in the process" packet, to limit it to just load addresses and names

2023-05-08 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda updated this revision to Diff 520545.
jasonmolenda added a comment.

Update to address Alex's comments, fix the testsuite failure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150158

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNB.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm
  lldb/tools/debugserver/source/RNBRemote.cpp

Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -5927,21 +5927,17 @@
   return SendPacket("OK");
 }
 
-//  This packet may be called in one of three ways:
-//
-//  jGetLoadedDynamicLibrariesInfos:{"image_count":40,"image_list_address":4295244704}
-//  Look for an array of the old dyld_all_image_infos style of binary infos
-//  at the image_list_address.
-//  This an array of {void* load_addr, void* mod_date, void* pathname}
+//  This packet may be called in one of two ways:
 //
 //  jGetLoadedDynamicLibrariesInfos:{"fetch_all_solibs":true}
-//  Use the new style (macOS 10.12, tvOS 10, iOS 10, watchOS 3) dyld SPI to
-//  get a list of all the
-//  libraries loaded
+//  Use the new dyld SPI to get a list of all the libraries loaded.
+//  If "report_load_commands":false" is present, only the dyld SPI
+//  provided information (load address, filepath) is returned.
+//  lldb can ask for the mach-o header/load command details in a
+//  separate packet.
 //
 //  jGetLoadedDynamicLibrariesInfos:{"solib_addresses":[8382824135,3258302053,830202858503]}
-//  Use the new style (macOS 10.12, tvOS 10, iOS 10, watchOS 3) dyld SPI to
-//  get the information
+//  Use the dyld SPI and Mach-O parsing in memory to get the information
 //  about the libraries loaded at these addresses.
 //
 rnb_err_t
@@ -5964,24 +5960,17 @@
 
 std::vector macho_addresses;
 bool fetch_all_solibs = false;
+bool report_load_commands = true;
+get_boolean_value_for_key_name_from_json("report_load_commands", p,
+ report_load_commands);
+
 if (get_boolean_value_for_key_name_from_json("fetch_all_solibs", p,
  fetch_all_solibs) &&
 fetch_all_solibs) {
-  json_sp = DNBGetAllLoadedLibrariesInfos(pid);
+  json_sp = DNBGetAllLoadedLibrariesInfos(pid, report_load_commands);
 } else if (get_array_of_ints_value_for_key_name_from_json(
"solib_addresses", p, macho_addresses)) {
   json_sp = DNBGetLibrariesInfoForAddresses(pid, macho_addresses);
-} else {
-  nub_addr_t image_list_address =
-  get_integer_value_for_key_name_from_json("image_list_address", p);
-  nub_addr_t image_count =
-  get_integer_value_for_key_name_from_json("image_count", p);
-
-  if (image_list_address != INVALID_NUB_ADDRESS &&
-  image_count != INVALID_NUB_ADDRESS) {
-json_sp = DNBGetLoadedDynamicLibrariesInfos(pid, image_list_address,
-image_count);
-  }
 }
 
 if (json_sp.get()) {
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -912,14 +912,15 @@
 // create a JSONGenerator object
 // with all the details we want to send to lldb.
 JSONGenerator::ObjectSP MachProcess::FormatDynamicLibrariesIntoJSON(
-const std::vector &image_infos) {
+const std::vector &image_infos,
+bool report_load_commands) {
 
   JSONGenerator::ArraySP image_infos_array_sp(new JSONGenerator::Array());
 
   const size_t image_count = image_infos.size();
 
   for (size_t i = 0; i < image_count; i++) {
-if (!image_infos[i].is_valid_mach_header)
+if (report_load_commands && !image_infos[i].is_valid_mach_header)
   continue;
 JSONGenerator::DictionarySP image_info_dict_sp(
 new JSONGenerator::Dictionary());
@@ -928,6 +929,11 @@
 image_info_dict_sp->AddIntegerItem("mod_date", image_infos[i].mod_date);
 image_info_dict_sp->AddStringItem("pathname", image_infos[i].filename);
 
+if (!report_load_commands) {
+  image_infos_array_sp->AddItem(image_info_dict_sp);
+  continue;
+}
+
 uuid_string_t uuidstr;
 uuid_unparse_upper(image_infos[i].macho_info.uuid, uuidstr);
 image_info_dict_sp->AddStringItem("uuid", uuidstr);
@@ -1000,109 +1006,6 @@
   return reply_sp;
 }
 
-// Get the shared library information using the old (pre-macOS 10.12, pre-iOS
-// 10, pre-tvOS 10, pre-watchOS 3)
-// code path.  We'll be given the address of an array of

[Lldb-commits] [PATCH] D150158: Add optional to debugserver's "tell me about all binaries in the process" packet, to limit it to just load addresses and names

2023-05-08 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda updated this revision to Diff 520546.
jasonmolenda added a comment.

Update the patch to stop checking for the `mod_date` key in the JSON reply for 
each binary.  This value is always zero with dyld for the past five+ years.  It 
increases the size of the packet unnecessarily, but lldb is currently checking 
that the field is included for this to be a valid response.  I was getting 
annoyed at this required field and how I couldn't remove this unnecessary 
field, and then I decided to start the process by removing lldb's requiring of 
it.

Change debugserver to hardcode returning mod_date 0 and a comment that this 
should be removed in the near future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150158

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNB.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm
  lldb/tools/debugserver/source/RNBRemote.cpp

Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -5927,21 +5927,17 @@
   return SendPacket("OK");
 }
 
-//  This packet may be called in one of three ways:
-//
-//  jGetLoadedDynamicLibrariesInfos:{"image_count":40,"image_list_address":4295244704}
-//  Look for an array of the old dyld_all_image_infos style of binary infos
-//  at the image_list_address.
-//  This an array of {void* load_addr, void* mod_date, void* pathname}
+//  This packet may be called in one of two ways:
 //
 //  jGetLoadedDynamicLibrariesInfos:{"fetch_all_solibs":true}
-//  Use the new style (macOS 10.12, tvOS 10, iOS 10, watchOS 3) dyld SPI to
-//  get a list of all the
-//  libraries loaded
+//  Use the new dyld SPI to get a list of all the libraries loaded.
+//  If "report_load_commands":false" is present, only the dyld SPI
+//  provided information (load address, filepath) is returned.
+//  lldb can ask for the mach-o header/load command details in a
+//  separate packet.
 //
 //  jGetLoadedDynamicLibrariesInfos:{"solib_addresses":[8382824135,3258302053,830202858503]}
-//  Use the new style (macOS 10.12, tvOS 10, iOS 10, watchOS 3) dyld SPI to
-//  get the information
+//  Use the dyld SPI and Mach-O parsing in memory to get the information
 //  about the libraries loaded at these addresses.
 //
 rnb_err_t
@@ -5964,24 +5960,17 @@
 
 std::vector macho_addresses;
 bool fetch_all_solibs = false;
+bool report_load_commands = true;
+get_boolean_value_for_key_name_from_json("report_load_commands", p,
+ report_load_commands);
+
 if (get_boolean_value_for_key_name_from_json("fetch_all_solibs", p,
  fetch_all_solibs) &&
 fetch_all_solibs) {
-  json_sp = DNBGetAllLoadedLibrariesInfos(pid);
+  json_sp = DNBGetAllLoadedLibrariesInfos(pid, report_load_commands);
 } else if (get_array_of_ints_value_for_key_name_from_json(
"solib_addresses", p, macho_addresses)) {
   json_sp = DNBGetLibrariesInfoForAddresses(pid, macho_addresses);
-} else {
-  nub_addr_t image_list_address =
-  get_integer_value_for_key_name_from_json("image_list_address", p);
-  nub_addr_t image_count =
-  get_integer_value_for_key_name_from_json("image_count", p);
-
-  if (image_list_address != INVALID_NUB_ADDRESS &&
-  image_count != INVALID_NUB_ADDRESS) {
-json_sp = DNBGetLoadedDynamicLibrariesInfos(pid, image_list_address,
-image_count);
-  }
 }
 
 if (json_sp.get()) {
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -912,22 +912,34 @@
 // create a JSONGenerator object
 // with all the details we want to send to lldb.
 JSONGenerator::ObjectSP MachProcess::FormatDynamicLibrariesIntoJSON(
-const std::vector &image_infos) {
+const std::vector &image_infos,
+bool report_load_commands) {
 
   JSONGenerator::ArraySP image_infos_array_sp(new JSONGenerator::Array());
 
   const size_t image_count = image_infos.size();
 
   for (size_t i = 0; i < image_count; i++) {
-if (!image_infos[i].is_valid_mach_header)
+// If we should report the Mach-O header and load commands,
+// and those were unreadable, don'

[Lldb-commits] [PATCH] D150168: [lldb] Simplify predicates of find_if in BroadcastManager

2023-05-08 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, jingham, mib.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

We had some custom classes that were used as the predicate for
`std::find_if`. It would be a lot simpler if we used lambdas instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150168

Files:
  lldb/include/lldb/Utility/Broadcaster.h
  lldb/source/Utility/Broadcaster.cpp

Index: lldb/source/Utility/Broadcaster.cpp
===
--- lldb/source/Utility/Broadcaster.cpp
+++ lldb/source/Utility/Broadcaster.cpp
@@ -336,10 +336,13 @@
   collection::iterator iter = m_event_map.begin(), end_iter = m_event_map.end();
   uint32_t available_bits = event_spec.GetEventBits();
 
+  auto class_matches = [&event_spec](const event_listener_key &input) -> bool {
+return input.first.GetBroadcasterClass() ==
+   event_spec.GetBroadcasterClass();
+  };
+
   while (iter != end_iter &&
- (iter = find_if(iter, end_iter,
- BroadcasterClassMatches(
- event_spec.GetBroadcasterClass( != end_iter) {
+ (iter = find_if(iter, end_iter, class_matches)) != end_iter) {
 available_bits &= ~((*iter).first.GetEventBits());
 iter++;
   }
@@ -362,7 +365,13 @@
   if (m_listeners.erase(listener_sp) == 0)
 return false;
 
-  ListenerMatchesAndSharedBits predicate(event_spec, listener_sp);
+  auto listener_matches_and_shared_bits =
+  [&listener_sp, &event_spec](const event_listener_key &input) -> bool {
+return input.first.GetBroadcasterClass() ==
+   event_spec.GetBroadcasterClass() &&
+   (input.first.GetEventBits() & event_spec.GetEventBits()) != 0 &&
+   input.second == listener_sp;
+  };
   std::vector to_be_readded;
   uint32_t event_bits_to_remove = event_spec.GetEventBits();
 
@@ -370,7 +379,8 @@
   // matches that weren't exact to re-add:
   while (true) {
 collection::iterator iter, end_iter = m_event_map.end();
-iter = find_if(m_event_map.begin(), end_iter, predicate);
+iter = find_if(m_event_map.begin(), end_iter,
+   listener_matches_and_shared_bits);
 if (iter == end_iter) {
   break;
 }
@@ -397,9 +407,13 @@
 const BroadcastEventSpec &event_spec) const {
   std::lock_guard guard(m_manager_mutex);
 
+  auto event_spec_matches =
+  [&event_spec](const event_listener_key &input) -> bool {
+return input.first.IsContainedIn(event_spec);
+  };
+
   collection::const_iterator iter, end_iter = m_event_map.end();
-  iter = find_if(m_event_map.begin(), end_iter,
- BroadcastEventSpecMatches(event_spec));
+  iter = find_if(m_event_map.begin(), end_iter, event_spec_matches);
   if (iter != end_iter)
 return (*iter).second;
 
@@ -408,17 +422,25 @@
 
 void BroadcasterManager::RemoveListener(Listener *listener) {
   std::lock_guard guard(m_manager_mutex);
-  ListenerMatchesPointer predicate(listener);
+  auto listeners_predicate =
+  [&listener](const lldb::ListenerSP &input) -> bool {
+return input.get() == listener;
+  };
+
   listener_collection::iterator iter = m_listeners.begin(),
 end_iter = m_listeners.end();
 
-  iter = std::find_if(iter, end_iter, predicate);
+  iter = std::find_if(iter, end_iter, listeners_predicate);
   if (iter != end_iter)
 m_listeners.erase(iter);
 
   while (true) {
+auto events_predicate =
+[&listener](const event_listener_key &input) -> bool {
+  return input.second.get() == listener;
+};
 collection::iterator iter, end_iter = m_event_map.end();
-iter = find_if(m_event_map.begin(), end_iter, predicate);
+iter = find_if(m_event_map.begin(), end_iter, events_predicate);
 if (iter == end_iter)
   break;
 
@@ -428,14 +450,18 @@
 
 void BroadcasterManager::RemoveListener(const lldb::ListenerSP &listener_sp) {
   std::lock_guard guard(m_manager_mutex);
-  ListenerMatches predicate(listener_sp);
+
+  auto listener_matches =
+  [&listener_sp](const event_listener_key &input) -> bool {
+return input.second == listener_sp;
+  };
 
   if (m_listeners.erase(listener_sp) == 0)
 return;
 
   while (true) {
 collection::iterator iter, end_iter = m_event_map.end();
-iter = find_if(m_event_map.begin(), end_iter, predicate);
+iter = find_if(m_event_map.begin(), end_iter, listener_matches);
 if (iter == end_iter)
   break;
 
@@ -449,10 +475,13 @@
 
   collection::iterator iter = m_event_map.begin(), end_iter = m_event_map.end();
 
+  auto class_matches = [&broadcaster](const event_listener_key &input) -> bool {
+return input.first.GetBroadcasterClass() ==
+   broadcaster.GetBroadcasterClass();
+  };
+
   while (iter != end_iter &&
- (iter = find_if(iter, end_iter,
- Broadca

[Lldb-commits] [PATCH] D148776: [Modules] Move modulemaps to header search directories. NFC intended.

2023-05-08 Thread David Blaikie via Phabricator via lldb-commits 
dblaikie added a comment.

Got a link to a design discussion motivating this change?

I'd have thought it made sense to put modulemaps in subdirectories - since they 
cover the whole directory, putting them in the root of an include path would be 
problematic if there are multiple distinct projects in that same path. And I 
didn't think this involved searching through subdirectories, but walking up 
parent directories from the included file...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148776

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

[Lldb-commits] [PATCH] D150158: Add optional to debugserver's "tell me about all binaries in the process" packet, to limit it to just load addresses and names

2023-05-08 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord accepted this revision.
bulbazord added a comment.
This revision is now accepted and ready to land.

I have no problem with removing mod_date since you're only removing the 
requirement in the Darwin-specific code. Also, be sure to remove the portion of 
the commit message that says this introduces a regression since you've fixed 
the test now. It would be confusing for future code archeologists.

LGTM!




Comment at: lldb/tools/debugserver/source/MacOSX/MachProcess.mm:1141
 }
-return FormatDynamicLibrariesIntoJSON(image_infos);
+return FormatDynamicLibrariesIntoJSON(image_infos, true);
 }

nit: Could you add a comment specifying what the bool argument is for?
```
return FormatDynamicLibrariesIntoJSON(image_infos, /* report_load_commands = */ 
true);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150158

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

[Lldb-commits] [PATCH] D148776: [Modules] Move modulemaps to header search directories. NFC intended.

2023-05-08 Thread Volodymyr Sapsai via Phabricator via lldb-commits 
vsapsai added a comment.

In D148776#4328425 , @dblaikie wrote:

> Got a link to a design discussion motivating this change?

No design discussion. I though that doing less work is not contentious.

> I'd have thought it made sense to put modulemaps in subdirectories - since 
> they cover the whole directory, putting them in the root of an include path 
> would be problematic if there are multiple distinct projects in that same 
> path. And I didn't think this involved searching through subdirectories, but 
> walking up parent directories from the included file...

When we look for a module map covering a header - you are right, we are walking 
up parent directories from the included file. But when we are looking up a 
module by name (for example, when we load a module), there is no included file 
and we need to look for a module top-down instead of bottom-up.

Your suggested approach to put module maps in corresponding subdirectories 
works when the names are the same, for example, module "blue" in directory 
"blue", that remains unchanged. But, for example, module "LLVM_DebugInfo" in 
directory "llvm" is not obvious. And there is no indication it //must// be in 
directory "llvm" and not in "llvm-c" or in some other directory, so clang 
checks all subdirectories.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148776

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

[Lldb-commits] [PATCH] D150043: [InferAddressSpaces] Handle vector of pointers type & Support intrinsic masked gather/scatter

2023-05-08 Thread CaprYang via Phabricator via lldb-commits 
CaprYang added inline comments.



Comment at: llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp:259-265
+static unsigned getPtrOrVecOfPtrsAddressSpace(Type *Ty) {
+  if (Ty->isVectorTy()) {
+Ty = cast(Ty)->getElementType();
+  }
+  assert(Ty->isPointerTy());
+  return Ty->getPointerAddressSpace();
+}

arsenm wrote:
> This is reinventing Type::getPointerAddressSpace
Yes.. it seems like I am doing some useless work.



Comment at: llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp:268-271
+  if (Ty->isVectorTy()) {
+Ty = cast(Ty)->getElementType();
+  }
+  return Ty->isPointerTy();

arsenm wrote:
> isPtrOrPtrVectorTy
Oh thanks! I just learned that there are these APIs, let me replace them.



Comment at: llvm/lib/Transforms/Scalar/InferAddressSpaces.cpp:285
+  Type *NPT =
+  PointerType::getWithSamePointeeType(cast(PT), NewAddrSpace);
+  return VectorType::get(NPT, cast(Ty)->getElementCount());

arsenm wrote:
> Don't need to bother trying to maintain pointer element types anymore 
Sorry.. I don't know what to do, can you tell me?



Comment at: llvm/test/Transforms/InferAddressSpaces/AMDGPU/icmp.ll:151
 ; CHECK-LABEL: @icmp_flat_flat_from_group_vector(
-; CHECK: %cmp = icmp eq <2 x ptr> %cast0, %cast1
+; CHECK: %cmp = icmp eq <2 x ptr addrspace(3)> %group.ptr.0, %group.ptr.1
 define <2 x i1> @icmp_flat_flat_from_group_vector(<2 x ptr addrspace(3)> 
%group.ptr.0, <2 x ptr addrspace(3)> %group.ptr.1) #0 {

arsenm wrote:
> You touched a lot more than just icmp, so this needs more tests to cover all 
> the newly handled cases 
I will add more tests later


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

https://reviews.llvm.org/D150043

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

[Lldb-commits] [lldb] 4e93f91 - Add a new report_load_commands option to jGetLoadedDynamicLibrariesInfos

2023-05-08 Thread Jason Molenda via lldb-commits 

Author: Jason Molenda
Date: 2023-05-08T20:34:58-07:00
New Revision: 4e93f91148ae4698b31ee397fb60410441df8b6b

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

LOG: Add a new report_load_commands option to jGetLoadedDynamicLibrariesInfos

jGetLoadedDynamicLibrariesInfos has a mode where it will list
every binary in the process - the load address and filepath from dyld
SPI, and the mach-o header and load commands from a scan by debugserver
for perf reasons.  With a large enough number of libraries, creating
that StructuredData representation of all of this, and formatting it
into an ascii string to send up to lldb, can grow debugserver's heap
size too large for some environments.

This patch adds a new report_load_commands:false boolean to the
jGetLoadedDynamicLibrariesInfos packet, where debugserver will now
only report the dyld SPI load address and filepath for all of the
binaries.  lldb can then ask for the detailed information on
the process binaries in smaller chunks, and avoid debugserver
having ever growing heap use as the number of binaries inevitably
increases.

This patch also removes a version of jGetLoadedDynamicLibrariesInfos
for pre-iOS 10 and pre-macOS 10.12 systems where we did not use
dyld SPI.  We can't back compile to those OS builds any longer
with modern Xcode.

Finally, it removes a requirement in DynamicLoaderMacOS that the
JSON reply from jGetLoadedDynamicLibrariesInfos include the
mod_date field for each binary.  This has always been reported as
0 in modern dyld, and is another reason for packet growth in
the reply.  debugserver still puts the mod_date field in its replies
for interop with existing lldb's, but we will be able to remove it
the field from debugserver's output after the next release cycle
when this patch has had time to circulate.

I'll add lldb support for requesting the load addresses only
and splitting the request up into chunks in a separate patch.

Differential Revision: https://reviews.llvm.org/D150158
rdar://107848326

Added: 


Modified: 
lldb/docs/lldb-gdb-remote.txt
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
lldb/tools/debugserver/source/DNB.cpp
lldb/tools/debugserver/source/DNB.h
lldb/tools/debugserver/source/MacOSX/MachProcess.h
lldb/tools/debugserver/source/MacOSX/MachProcess.mm
lldb/tools/debugserver/source/RNBRemote.cpp

Removed: 




diff  --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt
index b426978a94907..28180df714e00 100644
--- a/lldb/docs/lldb-gdb-remote.txt
+++ b/lldb/docs/lldb-gdb-remote.txt
@@ -2001,19 +2001,16 @@ for this region.
 //  This packet asks the remote debug stub to send the details about libraries
 //  being added/removed from the process as a performance optimization.
 //
-//  There are three ways this packet can be used.  All three return a 
dictionary of
+//  There are two ways this packet can be used.  Both return a dictionary of
 //  binary images formatted the same way.
 //
-//  On OS X 10.11, iOS 9, tvOS 9, watchOS 2 and earlier, the packet is used 
like
-//   
jGetLoadedDynamicLibrariesInfos:{"image_count":1,"image_list_address":140734800075128}
-//  where the image_list_address is an array of {void* load_addr, void* 
mod_date, void* pathname}
-//  in the inferior process memory (and image_count is the number of elements 
in this array).
-//  lldb is using information from the dyld_all_image_infos structure to make 
these requests to
-//  debugserver.  This use is not supported on macOS 10.12, iOS 10, tvOS 10, 
watchOS 3 or newer.
-//
-//  On macOS 10.12, iOS 10, tvOS 10, watchOS 3 and newer, there are two calls. 
 One requests information
-//  on all shared libraries:
+//  One requests information on all shared libraries:
 //   jGetLoadedDynamicLibrariesInfos:{"fetch_all_solibs":true}
+//  with an optional `"report_load_commands":false` which can be added, asking
+//  that only the dyld SPI information (load addresses, filenames) be returned.
+//  The default behavior is that debugserver scans the mach-o header and load 
+//  commands of each binary, and returns it in the JSON reply.
+//
 //  And the second requests information about a list of shared libraries, 
given their load addresses:
 //   
jGetLoadedDynamicLibrariesInfos:{"solib_addresses":[8382824135,3258302053,830202858503]}
 //

diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index 6506d000668e9..8bcc93589ae8f 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/l

[Lldb-commits] [PATCH] D150158: Add optional to debugserver's "tell me about all binaries in the process" packet, to limit it to just load addresses and names

2023-05-08 Thread Jason Molenda via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4e93f91148ae: Add a new report_load_commands option to 
jGetLoadedDynamicLibrariesInfos (authored by jasonmolenda).

Changed prior to commit:
  https://reviews.llvm.org/D150158?vs=520546&id=520578#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150158

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNB.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm
  lldb/tools/debugserver/source/RNBRemote.cpp

Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -5927,21 +5927,17 @@
   return SendPacket("OK");
 }
 
-//  This packet may be called in one of three ways:
-//
-//  jGetLoadedDynamicLibrariesInfos:{"image_count":40,"image_list_address":4295244704}
-//  Look for an array of the old dyld_all_image_infos style of binary infos
-//  at the image_list_address.
-//  This an array of {void* load_addr, void* mod_date, void* pathname}
+//  This packet may be called in one of two ways:
 //
 //  jGetLoadedDynamicLibrariesInfos:{"fetch_all_solibs":true}
-//  Use the new style (macOS 10.12, tvOS 10, iOS 10, watchOS 3) dyld SPI to
-//  get a list of all the
-//  libraries loaded
+//  Use the new dyld SPI to get a list of all the libraries loaded.
+//  If "report_load_commands":false" is present, only the dyld SPI
+//  provided information (load address, filepath) is returned.
+//  lldb can ask for the mach-o header/load command details in a
+//  separate packet.
 //
 //  jGetLoadedDynamicLibrariesInfos:{"solib_addresses":[8382824135,3258302053,830202858503]}
-//  Use the new style (macOS 10.12, tvOS 10, iOS 10, watchOS 3) dyld SPI to
-//  get the information
+//  Use the dyld SPI and Mach-O parsing in memory to get the information
 //  about the libraries loaded at these addresses.
 //
 rnb_err_t
@@ -5964,24 +5960,17 @@
 
 std::vector macho_addresses;
 bool fetch_all_solibs = false;
+bool report_load_commands = true;
+get_boolean_value_for_key_name_from_json("report_load_commands", p,
+ report_load_commands);
+
 if (get_boolean_value_for_key_name_from_json("fetch_all_solibs", p,
  fetch_all_solibs) &&
 fetch_all_solibs) {
-  json_sp = DNBGetAllLoadedLibrariesInfos(pid);
+  json_sp = DNBGetAllLoadedLibrariesInfos(pid, report_load_commands);
 } else if (get_array_of_ints_value_for_key_name_from_json(
"solib_addresses", p, macho_addresses)) {
   json_sp = DNBGetLibrariesInfoForAddresses(pid, macho_addresses);
-} else {
-  nub_addr_t image_list_address =
-  get_integer_value_for_key_name_from_json("image_list_address", p);
-  nub_addr_t image_count =
-  get_integer_value_for_key_name_from_json("image_count", p);
-
-  if (image_list_address != INVALID_NUB_ADDRESS &&
-  image_count != INVALID_NUB_ADDRESS) {
-json_sp = DNBGetLoadedDynamicLibrariesInfos(pid, image_list_address,
-image_count);
-  }
 }
 
 if (json_sp.get()) {
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -912,22 +912,34 @@
 // create a JSONGenerator object
 // with all the details we want to send to lldb.
 JSONGenerator::ObjectSP MachProcess::FormatDynamicLibrariesIntoJSON(
-const std::vector &image_infos) {
+const std::vector &image_infos,
+bool report_load_commands) {
 
   JSONGenerator::ArraySP image_infos_array_sp(new JSONGenerator::Array());
 
   const size_t image_count = image_infos.size();
 
   for (size_t i = 0; i < image_count; i++) {
-if (!image_infos[i].is_valid_mach_header)
+// If we should report the Mach-O header and load commands,
+// and those were unreadable, don't report anything about this
+// binary.
+if (report_load_commands && !image_infos[i].is_valid_mach_header)
   continue;
 JSONGenerator::DictionarySP image_info_dict_sp(
 new JSONGenerator::Dictionary());
 image_info_dict_sp->AddIntegerItem("load_address",
image_infos[i].load_address);
-image_info_dict_sp->A

[Lldb-commits] [PATCH] D150158: Add optional to debugserver's "tell me about all binaries in the process" packet, to limit it to just load addresses and names

2023-05-08 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda updated this revision to Diff 520580.
jasonmolenda added a comment.

Updating to reflect what I committed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150158

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNB.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm
  lldb/tools/debugserver/source/RNBRemote.cpp

Index: lldb/tools/debugserver/source/RNBRemote.cpp
===
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -5927,21 +5927,17 @@
   return SendPacket("OK");
 }
 
-//  This packet may be called in one of three ways:
-//
-//  jGetLoadedDynamicLibrariesInfos:{"image_count":40,"image_list_address":4295244704}
-//  Look for an array of the old dyld_all_image_infos style of binary infos
-//  at the image_list_address.
-//  This an array of {void* load_addr, void* mod_date, void* pathname}
+//  This packet may be called in one of two ways:
 //
 //  jGetLoadedDynamicLibrariesInfos:{"fetch_all_solibs":true}
-//  Use the new style (macOS 10.12, tvOS 10, iOS 10, watchOS 3) dyld SPI to
-//  get a list of all the
-//  libraries loaded
+//  Use the new dyld SPI to get a list of all the libraries loaded.
+//  If "report_load_commands":false" is present, only the dyld SPI
+//  provided information (load address, filepath) is returned.
+//  lldb can ask for the mach-o header/load command details in a
+//  separate packet.
 //
 //  jGetLoadedDynamicLibrariesInfos:{"solib_addresses":[8382824135,3258302053,830202858503]}
-//  Use the new style (macOS 10.12, tvOS 10, iOS 10, watchOS 3) dyld SPI to
-//  get the information
+//  Use the dyld SPI and Mach-O parsing in memory to get the information
 //  about the libraries loaded at these addresses.
 //
 rnb_err_t
@@ -5964,24 +5960,17 @@
 
 std::vector macho_addresses;
 bool fetch_all_solibs = false;
+bool report_load_commands = true;
+get_boolean_value_for_key_name_from_json("report_load_commands", p,
+ report_load_commands);
+
 if (get_boolean_value_for_key_name_from_json("fetch_all_solibs", p,
  fetch_all_solibs) &&
 fetch_all_solibs) {
-  json_sp = DNBGetAllLoadedLibrariesInfos(pid);
+  json_sp = DNBGetAllLoadedLibrariesInfos(pid, report_load_commands);
 } else if (get_array_of_ints_value_for_key_name_from_json(
"solib_addresses", p, macho_addresses)) {
   json_sp = DNBGetLibrariesInfoForAddresses(pid, macho_addresses);
-} else {
-  nub_addr_t image_list_address =
-  get_integer_value_for_key_name_from_json("image_list_address", p);
-  nub_addr_t image_count =
-  get_integer_value_for_key_name_from_json("image_count", p);
-
-  if (image_list_address != INVALID_NUB_ADDRESS &&
-  image_count != INVALID_NUB_ADDRESS) {
-json_sp = DNBGetLoadedDynamicLibrariesInfos(pid, image_list_address,
-image_count);
-  }
 }
 
 if (json_sp.get()) {
Index: lldb/tools/debugserver/source/MacOSX/MachProcess.mm
===
--- lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -912,22 +912,34 @@
 // create a JSONGenerator object
 // with all the details we want to send to lldb.
 JSONGenerator::ObjectSP MachProcess::FormatDynamicLibrariesIntoJSON(
-const std::vector &image_infos) {
+const std::vector &image_infos,
+bool report_load_commands) {
 
   JSONGenerator::ArraySP image_infos_array_sp(new JSONGenerator::Array());
 
   const size_t image_count = image_infos.size();
 
   for (size_t i = 0; i < image_count; i++) {
-if (!image_infos[i].is_valid_mach_header)
+// If we should report the Mach-O header and load commands,
+// and those were unreadable, don't report anything about this
+// binary.
+if (report_load_commands && !image_infos[i].is_valid_mach_header)
   continue;
 JSONGenerator::DictionarySP image_info_dict_sp(
 new JSONGenerator::Dictionary());
 image_info_dict_sp->AddIntegerItem("load_address",
image_infos[i].load_address);
-image_info_dict_sp->AddIntegerItem("mod_date", image_infos[i].mod_date);
+// TODO: lldb currently rejects a response without this, but it
+// is always zero from dyld.  It can be removed