[Lldb-commits] [lldb] [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks (PR #123918)

[Omair Javaid via lldb-commits]

https://github.com/omjavaid approved this pull request.

I agree this is as good as we can get for now with failure handling during 
expression.

https://github.com/llvm/llvm-project/pull/123918
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks (PR #123918)

[David Spickett via lldb-commits]

https://github.com/DavidSpickett updated 
https://github.com/llvm/llvm-project/pull/123918

>From c89a16ab405d4aad7bf0a59afb433dd5ecd1a402 Mon Sep 17 00:00:00 2001
From: David Spickett 
Date: Tue, 20 Aug 2024 16:06:34 +0100
Subject: [PATCH 1/4] [lldb][AArch64] Fix expression evaluation with Guarded
 Control Stacks

When the Guarded Control Stack (GCS) is enabled, returns cause the
processor to validate that the address at the location pointed to
by gcspr_el0 matches the one in the link register.

```
ret (lr=A) << pc

| GCS |
+=+
|  A  |
|  B  | << gcspr_el0

Fault: tried to return to A when you should have returned to B.
```

Therefore when an expression wraper function tries to return to
the expression return address (usually `_start` if there is a libc),
it would fault.

```
ret (lr=_start) << pc

| GCS|
++
| user_func1 |
| user_func2 | << gcspr_el0

Fault: tried to return to _start when you should have return to user_func2.
```

To fix this we must push that return address to the GCS in PrepareTrivialCall.
This value is then consumed by the final return and the expression
completes as expected.

```
ret (lr=_start) << pc

| GCS|
++
| user_func1 |
| user_func2 |
| _start | << gcspr_el0

No fault, we return to _start as normal.
```

The gcspr_el0 register will be restored after expression evaluation
so that the program can continue correctly.

However, due to restrictions in the Linux GCS ABI, we will not
restore the enable bit of gcs_features_enabled. Re-enabling GCS
via ptrace is not supported because it requires memory to be
allocated.

We could disable GCS if the expression enabled GCS, however this
would use up that state transition that the program might later
rely on. And generally it is cleaner to ignore the whole bit
rather than one state transition of it.

We will also not restore the GCS entry that was overwritten with
the expression's return address. On the grounds that:
* This entry will never be used by the program. If the program branches,
  the entry will be overwritten. If the program returns, gcspr_el0
  will point to the entry before the expression return address
  and that entry will instead be validated.
* Any expression that calls functions will overwrite even more
  entries, so the user needs to be aware of that anyway if they
  want to preserve the contents of the GCS for inspection.
* An expression could leave the program in a state where
  restoring the value makes the situation worse. Especially
  if we ever support this in bare metal debugging.

I will later document all this on https://lldb.llvm.org/use/aarch64-linux.html
as well.

Tests have been added for:
* A function call that does not interact with GCS.
* A call that does, and disables it (we do not re-enable it).
* A call that does, and enables it (we do not disable it again).
---
 .../Plugins/ABI/AArch64/ABISysV_arm64.cpp |  56 +
 .../NativeRegisterContextLinux_arm64.cpp  |  20 +-
 .../linux/aarch64/gcs/TestAArch64LinuxGCS.py  | 196 ++
 lldb/test/API/linux/aarch64/gcs/main.c|  47 -
 4 files changed, 278 insertions(+), 41 deletions(-)

diff --git a/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp 
b/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
index 93b8141e97ef86..d843e718b6875e 100644
--- a/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
+++ b/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
@@ -60,6 +60,59 @@ ABISysV_arm64::CreateInstance(lldb::ProcessSP process_sp, 
const ArchSpec &arch)
   return ABISP();
 }
 
+static bool PushToLinuxGuardedControlStack(addr_t return_addr,
+   RegisterContext *reg_ctx,
+   Thread &thread) {
+  // If the Guarded Control Stack extension is enabled we need to put the 
return
+  // address onto that stack.
+  const RegisterInfo *gcs_features_enabled_info =
+  reg_ctx->GetRegisterInfoByName("gcs_features_enabled");
+  if (!gcs_features_enabled_info)
+return false;
+
+  uint64_t gcs_features_enabled = reg_ctx->ReadRegisterAsUnsigned(
+  gcs_features_enabled_info, LLDB_INVALID_ADDRESS);
+  if (gcs_features_enabled == LLDB_INVALID_ADDRESS)
+return false;
+
+  // Only attempt this if GCS is enabled. If it's not enabled then gcspr_el0
+  // may point to unmapped memory.
+  if ((gcs_features_enabled & 1) == 0)
+return false;
+
+  const RegisterInfo *gcspr_el0_info =
+  reg_ctx->GetRegisterInfoByName("gcspr_el0");
+  if (!gcspr_el0_info)
+return false;
+
+  uint64_t gcspr_el0 =
+  reg_ctx->ReadRegisterAsUnsigned(gcspr_el0_info, LLDB_INVALID_ADDRESS);
+  if (gcspr_el0 == LLDB_INVALID_ADDRESS)
+return false;
+
+  // A link register entry on the GCS is 8 bytes.
+  gcspr_el0 -= 8;
+  if (!reg_ctx->WriteRegisterFromUnsigned(gcspr_el0_info, gcspr_el0))
+return false;
+
+  Status error;
+  size_t wrote = thread.GetProcess()->WriteMemory(gcspr_el0, &return_addr,
+

[Lldb-commits] [lldb] b31e974 - [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks (#123918)

[via lldb-commits]

Author: David Spickett
Date: 2025-01-27T13:06:33Z
New Revision: b31e9747d0866ff97a1cd4a608b7eade31c0aa0b

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

LOG: [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks 
(#123918)

When the Guarded Control Stack (GCS) is enabled, returns cause the
processor to validate that the address at the location pointed to by
gcspr_el0 matches the one in the link register.

```
ret (lr=A) << pc

| GCS |
+=+
|  A  |
|  B  | << gcspr_el0

Fault: tried to return to A when you should have returned to B.
```

Therefore when an expression wrapper function tries to return to the
expression return address (usually `_start` if there is a libc), it
would fault.

```
ret (lr=_start) << pc

| GCS|
++
| user_func1 |
| user_func2 | << gcspr_el0

Fault: tried to return to _start when you should have returned to user_func2.
```

To fix this we must push that return address to the GCS in
PrepareTrivialCall. This value is then consumed by the final return and
the expression completes as expected.

If for some reason that fails, we will manually restore the value of
gcspr_el0, because it turns out that PrepareTrivialCall
does not restore registers if it fails at all. So for now I am handling
gcspr_el0 specifically, but I have filed
https://github.com/llvm/llvm-project/issues/124269 to address the
general problem.

(the other things PrepareTrivialCall does are exceedingly likely to not
fail, so we have never noticed this)

```
ret (lr=_start) << pc

| GCS|
++
| user_func1 |
| user_func2 |
| _start | << gcspr_el0

No fault, we return to _start as normal.
```

The gcspr_el0 register will be restored after expression evaluation so
that the program can continue correctly.

However, due to restrictions in the Linux GCS ABI, we will not restore
the enable bit of gcs_features_enabled. Re-enabling GCS via ptrace is
not supported because it requires memory to be allocated by the kernel.

We could disable GCS if the expression enabled GCS, however this would
use up that state transition that the program might later rely on. And
generally it is cleaner to ignore the enable bit, rather than one state
transition of it.

We will also not restore the GCS entry that was overwritten with the
expression's return address. On the grounds that:
* This entry will never be used by the program. If the program branches,
the entry will be overwritten. If the program returns, gcspr_el0 will
point to the entry before the expression return address and that entry
will instead be validated.
* Any expression that calls functions will overwrite even more entries,
so the user needs to be aware of that anyway if they want to preserve
the contents of the GCS for inspection.
* An expression could leave the program in a state where restoring the
value makes the situation worse. Especially if we ever support this in
bare metal debugging.

I will later document all this on
https://lldb.llvm.org/use/aarch64-linux.html.

Tests have been added for:
* A function call that does not interact with GCS.
* A call that does, and disables it (we do not re-enable it).
* A call that does, and enables it (we do not disable it again).
* Failure to push an entry to the GCS stack.

Added: 


Modified: 
lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py
lldb/test/API/linux/aarch64/gcs/main.c

Removed: 




diff  --git a/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp 
b/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
index 93b8141e97ef86..74047ea65788cf 100644
--- a/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
+++ b/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
@@ -60,6 +60,69 @@ ABISysV_arm64::CreateInstance(lldb::ProcessSP process_sp, 
const ArchSpec &arch)
   return ABISP();
 }
 
+static Status PushToLinuxGuardedControlStack(addr_t return_addr,
+ RegisterContext *reg_ctx,
+ Thread &thread) {
+  Status err;
+
+  // If the Guarded Control Stack extension is present we may need to put the
+  // return address onto that stack.
+  const RegisterInfo *gcs_features_enabled_info =
+  reg_ctx->GetRegisterInfoByName("gcs_features_enabled");
+  if (!gcs_features_enabled_info)
+return err;
+
+  uint64_t gcs_features_enabled = reg_ctx->ReadRegisterAsUnsigned(
+  gcs_features_enabled_info, LLDB_INVALID_ADDRESS);
+  if (gcs_features_enabled == LLDB_INVALID_ADDRESS)
+return Status("Could not read GCS features enabled register.");
+
+  // Only attempt this if GCS is enabled. If it's not enabled then gcspr_el0
+  // may poi

[Lldb-commits] [lldb] [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks (PR #123918)

[David Spickett via lldb-commits]

https://github.com/DavidSpickett edited 
https://github.com/llvm/llvm-project/pull/123918
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (PR #124279)

[Michael Buch via lldb-commits]

https://github.com/Michael137 updated 
https://github.com/llvm/llvm-project/pull/124279

>From 23c7f4a3e067739a050964b3f1c2140787b0c31f Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 24 Jan 2025 13:33:07 +
Subject: [PATCH 1/6] [lldb][TypeSystem] Ensure that ParmVarDecls have the
 correct DeclContext

While sifting through this part of the code I noticed that when we parse C++ 
methods, `DWARFASTParserClang` creates two sets of `ParmVarDecls`, one in 
`ParseChildParameters` and once in `AddMethodToCXXRecordType`.  The former is 
unused when we're dealing with methods. Moreover, the `ParmVarDecls` we created 
in `ParseChildParameters` were created with an incorrect `clang::DeclContext` 
(namely the DeclContext of the function, and not the function itself). In 
Clang, there's `ParmVarDecl::setOwningFunction` to adjust the DeclContext of a 
parameter if the parameter was created before the FunctionDecl. But we never 
used it.

This patch removes the `ParmVarDecl` creation from `ParseChildParameters` and 
instead creates a `TypeSystemClang::CreateParameterDeclarations` that ensures 
we set the DeclContext correctly.

This wasn't causing any concrete issues (that I know of), but was quite 
surprising. And this way of setting the parameters seems easier to reason about 
(in my opinion).
---
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  |  31 ++--
 .../SymbolFile/DWARF/DWARFASTParserClang.h|   3 +-
 .../SymbolFile/NativePDB/PdbAstBuilder.cpp|   2 +-
 .../Plugins/SymbolFile/PDB/PDBASTParser.cpp   |   4 +-
 .../TypeSystem/Clang/TypeSystemClang.cpp  |  43 ++---
 .../TypeSystem/Clang/TypeSystemClang.h|  16 +-
 lldb/unittests/Symbol/TestTypeSystemClang.cpp |  42 +
 .../DWARF/DWARFASTParserClangTests.cpp| 154 ++
 8 files changed, 247 insertions(+), 48 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index e77188bfbd2e4a..30dec4bbf91c10 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1272,7 +1272,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
 return_clang_type = m_ast.GetBasicType(eBasicTypeVoid);
 
   std::vector function_param_types;
-  std::vector function_param_decls;
 
   // Parse the function children for the parameters
 
@@ -1283,8 +1282,7 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
 
   if (die.HasChildren()) {
 ParseChildParameters(containing_decl_ctx, die, is_variadic,
- has_template_params, function_param_types,
- function_param_decls);
+ has_template_params, function_param_types);
   }
 
   bool is_cxx_method = DeclKindIsCXXClass(containing_decl_ctx->getDeclKind());
@@ -1414,11 +1412,15 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE 
&die,
 
   LinkDeclContextToDIE(function_decl, die);
 
-  if (!function_param_decls.empty()) {
-m_ast.SetFunctionParameters(function_decl, function_param_decls);
+  const clang::FunctionProtoType *function_prototype(
+  llvm::cast(
+  ClangUtil::GetQualType(clang_type).getTypePtr()));
+  auto params = m_ast.CreateParameterDeclarations(function_decl,
+  *function_prototype);
+  if (!params.empty()) {
+function_decl->setParams(params);
 if (template_function_decl)
-  m_ast.SetFunctionParameters(template_function_decl,
-  function_param_decls);
+  template_function_decl->setParams(params);
   }
 
   ClangASTMetadata metadata;
@@ -2380,7 +2382,6 @@ 
DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
   bool is_variadic = false;
   bool has_template_params = false;
   std::vector param_types;
-  std::vector param_decls;
   StreamString sstr;
 
   DWARFDeclContext decl_ctx = die.GetDWARFDeclContext();
@@ -2394,7 +2395,7 @@ 
DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
   die, GetCXXObjectParameter(die, *containing_decl_ctx));
 
   ParseChildParameters(containing_decl_ctx, die, is_variadic,
-   has_template_params, param_types, param_decls);
+   has_template_params, param_types);
   sstr << "(";
   for (size_t i = 0; i < param_types.size(); i++) {
 if (i > 0)
@@ -3156,8 +3157,7 @@ bool DWARFASTParserClang::ParseChildMembers(
 void DWARFASTParserClang::ParseChildParameters(
 clang::DeclContext *containing_decl_ctx, const DWARFDIE &parent_die,
 bool &is_variadic, bool &has_template_params,
-std::vector &function_param_types,
-std::vector &function_param_decls) {
+std::vector &function_param_types) {
   if (!parent_die)
 return;
 
@@ -3168,7 +3168,6 @

[Lldb-commits] [lldb] [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks (PR #123918)

[David Spickett via lldb-commits]

https://github.com/DavidSpickett closed 
https://github.com/llvm/llvm-project/pull/123918
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [NFC][DebugInfo] Make some block-start-position methods return iterators (PR #124287)

[Orlando Cazalet-Hyams via lldb-commits]

@@ -408,9 +412,13 @@ BasicBlock::getFirstNonPHIOrDbgOrLifetime(bool 
SkipPseudoOp) const {
 if (SkipPseudoOp && isa(I))
   continue;
 
-return &I;
+BasicBlock::const_iterator It = I.getIterator();
+// Signal that this comes after any debug records.
+It.setHeadBit(false);

OCHyams wrote:

Same question again

https://github.com/llvm/llvm-project/pull/124287
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [NFC][DebugInfo] Make some block-start-position methods return iterators (PR #124287)

[Orlando Cazalet-Hyams via lldb-commits]

@@ -1656,7 +1656,8 @@ static Value 
*emitSetAndGetSwiftErrorValueAround(Instruction *Call,
 Builder.SetInsertPoint(Call->getNextNode());
   } else {
 auto Invoke = cast(Call);
-Builder.SetInsertPoint(Invoke->getNormalDest()->getFirstNonPHIOrDbg());
+BasicBlock::iterator It = Invoke->getNormalDest()->getFirstNonPHIOrDbg();
+Builder.SetInsertPoint(It);

OCHyams wrote:

Why is this change necessary?

https://github.com/llvm/llvm-project/pull/124287
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [NFC][DebugInfo] Make some block-start-position methods return iterators (PR #124287)

[Orlando Cazalet-Hyams via lldb-commits]

https://github.com/OCHyams edited 
https://github.com/llvm/llvm-project/pull/124287
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [NFC][DebugInfo] Make some block-start-position methods return iterators (PR #124287)

[Orlando Cazalet-Hyams via lldb-commits]

https://github.com/OCHyams approved this pull request.

LGTM + nits

https://github.com/llvm/llvm-project/pull/124287
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [NFC][DebugInfo] Make some block-start-position methods return iterators (PR #124287)

[Orlando Cazalet-Hyams via lldb-commits]

@@ -383,20 +383,24 @@ BasicBlock::const_iterator BasicBlock::getFirstNonPHIIt() 
const {
   return It;
 }
 
-const Instruction *BasicBlock::getFirstNonPHIOrDbg(bool SkipPseudoOp) const {
+BasicBlock::const_iterator
+BasicBlock::getFirstNonPHIOrDbg(bool SkipPseudoOp) const {
   for (const Instruction &I : *this) {
 if (isa(I) || isa(I))
   continue;
 
 if (SkipPseudoOp && isa(I))
   continue;
 
-return &I;
+BasicBlock::const_iterator It = I.getIterator();
+// Signal that this comes after any debug records.
+It.setHeadBit(false);

OCHyams wrote:

When does `Instruction::getIterator` return an iterator with head bit set? Can 
this be an assert (that it's not set) instead? 

https://github.com/llvm/llvm-project/pull/124287
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [NFC][DebugInfo] Make some block-start-position methods return iterators (PR #124287)

[Orlando Cazalet-Hyams via lldb-commits]

@@ -448,6 +455,9 @@ BasicBlock::const_iterator 
BasicBlock::getFirstNonPHIOrDbgOrAlloca() const {
   ++InsertPt;
 }
   }
+
+  // Signal that this comes after any debug records.
+  InsertPt.setHeadBit(false);

OCHyams wrote:

And again

https://github.com/llvm/llvm-project/pull/124287
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Include api-level.h header for Android (PR #124383)

[via lldb-commits]

@@ -29,6 +29,10 @@
 #include 
 #include 
 
+#ifdef __ANDROID__
+#include 

enh-google wrote:

what's the build failure you're seeing? this _should_ be included "for free" by 
all of the #includes above (via , which they should all drag in).

of course, if you're aiming for a style guide "explicitly include what you use" 
kind of thing, that's fine, but -- other than for style reasons -- you 
shouldn't ever _need_ this...

https://github.com/llvm/llvm-project/pull/124383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Include api-level.h header for Android (PR #124383)

[via lldb-commits]

https://github.com/enh-google approved this pull request.


https://github.com/llvm/llvm-project/pull/124383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Android 9 has added the spawn.h header (PR #124452)

[via lldb-commits]

@@ -26,10 +26,14 @@
 #include 
 #endif
 
+#ifdef __ANDROID__
+#include 
+#endif
+
 #if defined(__linux__) || defined(__FreeBSD__) ||  
\
 defined(__FreeBSD_kernel__) || defined(__APPLE__) ||   
\
 defined(__NetBSD__) || defined(__OpenBSD__) || defined(__EMSCRIPTEN__)
-#if !defined(__ANDROID__)
+#if !defined(__ANDROID__) || __ANDROID_API__ >= 28

enh-google wrote:

since there's no code here, you can just unconditionally _include_  -- 
it's in every supported NDK's sysroot -- even though you won't be able to _use_ 
any of the functions until api 28 (so _call sites_ will need a preprocessor 
check, but "#include sites" shouldn't, if you see what i mean).

https://github.com/llvm/llvm-project/pull/124452
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Clean up Socket headers for Android (PR #124453)

[via lldb-commits]

https://github.com/enh-google approved this pull request.


https://github.com/llvm/llvm-project/pull/124453
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 081723b - [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (#124279)

[via lldb-commits]

Author: Michael Buch
Date: 2025-01-27T14:56:47Z
New Revision: 081723b9db84e78d7dd240b46af2aeb3b51b00be

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

LOG: [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext 
(#124279)

While sifting through this part of the code I noticed that when we parse
C++ methods, `DWARFASTParserClang` creates two sets of `ParmVarDecls`,
one in `ParseChildParameters` and once in `AddMethodToCXXRecordType`.
The former is unused when we're dealing with methods. Moreover, the
`ParmVarDecls` we created in `ParseChildParameters` were created with an
incorrect `clang::DeclContext` (namely the DeclContext of the function,
and not the function itself). In Clang, there's
`ParmVarDecl::setOwningFunction` to adjust the DeclContext of a
parameter if the parameter was created before the FunctionDecl. But we
never used it.

This patch removes the `ParmVarDecl` creation from
`ParseChildParameters` and instead creates a
`TypeSystemClang::CreateParameterDeclarations` that ensures we set the
DeclContext correctly.

Note there is one differences in how `ParmVarDecl`s would be created
now: we won't set a ClangASTMetadata entry for any of the parameters. I
don't think this was ever actually useful for parameter DIEs anyway.

This wasn't causing any concrete issues (that I know of), but was quite
surprising. And this way of setting the parameters seems easier to
reason about (in my opinion).

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
lldb/unittests/Symbol/TestTypeSystemClang.cpp
lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index e77188bfbd2e4a..6602dd763ba693 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1272,7 +1272,7 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
 return_clang_type = m_ast.GetBasicType(eBasicTypeVoid);
 
   std::vector function_param_types;
-  std::vector function_param_decls;
+  llvm::SmallVector function_param_names;
 
   // Parse the function children for the parameters
 
@@ -1284,7 +1284,7 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
   if (die.HasChildren()) {
 ParseChildParameters(containing_decl_ctx, die, is_variadic,
  has_template_params, function_param_types,
- function_param_decls);
+ function_param_names);
   }
 
   bool is_cxx_method = DeclKindIsCXXClass(containing_decl_ctx->getDeclKind());
@@ -1414,12 +1414,14 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE 
&die,
 
   LinkDeclContextToDIE(function_decl, die);
 
-  if (!function_param_decls.empty()) {
-m_ast.SetFunctionParameters(function_decl, function_param_decls);
-if (template_function_decl)
-  m_ast.SetFunctionParameters(template_function_decl,
-  function_param_decls);
-  }
+  const clang::FunctionProtoType *function_prototype(
+  llvm::cast(
+  ClangUtil::GetQualType(clang_type).getTypePtr()));
+  const auto params = m_ast.CreateParameterDeclarations(
+  function_decl, *function_prototype, function_param_names);
+  function_decl->setParams(params);
+  if (template_function_decl)
+template_function_decl->setParams(params);
 
   ClangASTMetadata metadata;
   metadata.SetUserID(die.GetID());
@@ -2380,7 +2382,7 @@ 
DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
   bool is_variadic = false;
   bool has_template_params = false;
   std::vector param_types;
-  std::vector param_decls;
+  llvm::SmallVector param_names;
   StreamString sstr;
 
   DWARFDeclContext decl_ctx = die.GetDWARFDeclContext();
@@ -2394,7 +2396,7 @@ 
DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
   die, GetCXXObjectParameter(die, *containing_decl_ctx));
 
   ParseChildParameters(containing_decl_ctx, die, is_variadic,
-   has_template_params, param_types, param_decls);
+   has_template_params, param_types, param_names);
   sstr << "(";
   for (size_t i = 0; i < param_types.size(); i+

[Lldb-commits] [lldb] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (PR #124279)

[Michael Buch via lldb-commits]

https://github.com/Michael137 closed 
https://github.com/llvm/llvm-project/pull/124279
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [lldb][Windows] WoA HW Watchpoint support in LLDB (PR #108072)

[David Spickett via lldb-commits]

@@ -575,6 +575,10 @@ Changes to LLDB
  24   int main() {
   Likely cause: f->b->d accessed 0x4
   ```
+* LLDB now supports hardware watchpoints for AArch64 Windows targets. Windows
+  does not provide API to query the number of supported hardware watchpoints.
+  Therefore current implementation allows only 1 watchpoint, as tested with
+  Windows 11 on the Microsoft SQ2 and Snapdragon Elite X platforms.

DavidSpickett wrote:

What about this issue that's causing the test failures, is this something users 
are going to hit?

Or was that only about breakpoints, it's still in the PR description so I'm not 
sure.

https://github.com/llvm/llvm-project/pull/108072
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [lldb][Windows] WoA HW Watchpoint support in LLDB (PR #108072)

[David Spickett via lldb-commits]

@@ -492,23 +492,40 @@ NativeProcessWindows::OnDebugException(bool first_chance,
   }
   case DWORD(STATUS_BREAKPOINT):
   case STATUS_WX86_BREAKPOINT:

DavidSpickett wrote:

W = something related to WOW64 here right, not W for watchpoint?

And we get this even on arm64, there's no STATUS_WX86_BREAKPOINT? (and 
certainly you wouldn't get both of those in one process even if there were 2)

https://github.com/llvm/llvm-project/pull/108072
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [lldb][Windows] WoA HW Watchpoint support in LLDB (PR #108072)

[David Spickett via lldb-commits]

@@ -110,8 +110,8 @@ 
NativeRegisterContextWindows::CreateHostNativeRegisterContextWindows(
 
 NativeRegisterContextWindows_x86_64::NativeRegisterContextWindows_x86_64(
 const ArchSpec &target_arch, NativeThreadProtocol &native_thread)
-: NativeRegisterContextWindows(native_thread,
-   CreateRegisterInfoInterface(target_arch)) {}
+: NativeRegisterContextRegisterInfo(
+  native_thread, CreateRegisterInfoInterface(target_arch)) {}

DavidSpickett wrote:

So for i386 and x86_64, you've made these changes to keep them building, but 
this does not enable writing the watchpoints, correct?

For arm64, you've hooked up the debug register read and write, so there they do 
work now.

https://github.com/llvm/llvm-project/pull/108072
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] e9e06be - [lldb][AArch64][NFC] Move a comment in GCS tests

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2025-01-27T13:27:31Z
New Revision: e9e06bea8661ddd474557a0db2cdc8770a55b66f

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

LOG: [lldb][AArch64][NFC] Move a comment in GCS tests

Got put in the wrong place during a rebase.

Added: 


Modified: 
lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py

Removed: 




diff  --git a/lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py 
b/lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py
index fd46a42b3c69fe..797551b061a237 100644
--- a/lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py
+++ b/lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py
@@ -83,6 +83,8 @@ def test_gcs_fault(self):
 ],
 )
 
+# This helper reads all the GCS registers and optionally compares them
+# against a previous state, then returns the current register values.
 def check_gcs_registers(
 self,
 expected_gcs_features_enabled=None,
@@ -115,8 +117,6 @@ def check_gcs_registers(
 
 return gcs_features_enabled, gcs_features_locked, gcspr_el0
 
-# This helper reads all the GCS registers and optionally compares them
-# against a previous state, then returns the current register values.
 @skipUnlessArch("aarch64")
 @skipUnlessPlatform(["linux"])
 def test_gcs_registers(self):



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

[Lldb-commits] [lldb] [lldb][AArch64] Add Guarded Control Stack support for Linux core files (PR #124293)

[David Spickett via lldb-commits]

https://github.com/DavidSpickett updated 
https://github.com/llvm/llvm-project/pull/124293

>From d2c221b9b8cfeb8d9d902947db38c716e7a09804 Mon Sep 17 00:00:00 2001
From: David Spickett 
Date: Tue, 27 Aug 2024 11:45:52 +0100
Subject: [PATCH] [lldb][AArch64] Add Guarded Control Stack support for Linux
 core files

This allows you to read the same registers as you would for a live process.

As the content of proc/pid/smaps is not included in the core file,
we don't get the "ss" marker. The GCS region is stil in the list
though.
---
 .../RegisterContextPOSIXCore_arm64.cpp|  17 ++
 .../elf-core/RegisterContextPOSIXCore_arm64.h |   1 +
 .../Process/elf-core/RegisterUtilities.h  |   4 +++
 .../linux/aarch64/gcs/TestAArch64LinuxGCS.py  |  32 ++
 lldb/test/API/linux/aarch64/gcs/corefile  | Bin 0 -> 24576 bytes
 5 files changed, 54 insertions(+)
 create mode 100644 lldb/test/API/linux/aarch64/gcs/corefile

diff --git 
a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp 
b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
index 2ddf8440aeb035..bd02bb0e69a4d3 100644
--- a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
+++ b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
@@ -69,6 +69,15 @@ RegisterContextCorePOSIX_arm64::Create(Thread &thread, const 
ArchSpec &arch,
   if (fpmr_data.GetByteSize() >= sizeof(uint64_t))
 opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskFPMR);
 
+  DataExtractor gcs_data = getRegset(notes, arch.GetTriple(), 
AARCH64_GCS_Desc);
+  struct __attribute__((packed)) gcs_regs {
+uint64_t features_enabled;
+uint64_t features_locked;
+uint64_t gcspr_e0;
+  };
+  if (gcs_data.GetByteSize() >= sizeof(gcs_regs))
+opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskGCS);
+
   auto register_info_up =
   std::make_unique(arch, opt_regsets);
   return std::unique_ptr(
@@ -136,6 +145,9 @@ 
RegisterContextCorePOSIX_arm64::RegisterContextCorePOSIX_arm64(
   if (m_register_info_up->IsFPMRPresent())
 m_fpmr_data = getRegset(notes, target_triple, AARCH64_FPMR_Desc);
 
+  if (m_register_info_up->IsGCSPresent())
+m_gcs_data = getRegset(notes, target_triple, AARCH64_GCS_Desc);
+
   ConfigureRegisterContext();
 }
 
@@ -330,6 +342,11 @@ bool RegisterContextCorePOSIX_arm64::ReadRegister(const 
RegisterInfo *reg_info,
 assert(offset < m_mte_data.GetByteSize());
 value.SetFromMemoryData(*reg_info, m_mte_data.GetDataStart() + offset,
 reg_info->byte_size, lldb::eByteOrderLittle, 
error);
+  } else if (IsGCS(reg)) {
+offset = reg_info->byte_offset - m_register_info_up->GetGCSOffset();
+assert(offset < m_gcs_data.GetByteSize());
+value.SetFromMemoryData(*reg_info, m_gcs_data.GetDataStart() + offset,
+reg_info->byte_size, lldb::eByteOrderLittle, 
error);
   } else if (IsSME(reg)) {
 // If you had SME in the process, active or otherwise, there will at least
 // be a ZA header. No header, no SME at all.
diff --git 
a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h 
b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
index 35588c40c2eb1a..6140f805ffc78a 100644
--- a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
+++ b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
@@ -63,6 +63,7 @@ class RegisterContextCorePOSIX_arm64 : public 
RegisterContextPOSIX_arm64 {
   lldb_private::DataExtractor m_mte_data;
   lldb_private::DataExtractor m_zt_data;
   lldb_private::DataExtractor m_fpmr_data;
+  lldb_private::DataExtractor m_gcs_data;
 
   SVEState m_sve_state = SVEState::Unknown;
   uint16_t m_sve_vector_length = 0;
diff --git a/lldb/source/Plugins/Process/elf-core/RegisterUtilities.h 
b/lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
index b97279b0d735b8..59382a12cde0a2 100644
--- a/lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
+++ b/lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
@@ -148,6 +148,10 @@ constexpr RegsetDesc AARCH64_FPMR_Desc[] = {
 {llvm::Triple::Linux, llvm::Triple::aarch64, llvm::ELF::NT_ARM_FPMR},
 };
 
+constexpr RegsetDesc AARCH64_GCS_Desc[] = {
+{llvm::Triple::Linux, llvm::Triple::aarch64, llvm::ELF::NT_ARM_GCS},
+};
+
 constexpr RegsetDesc PPC_VMX_Desc[] = {
 {llvm::Triple::FreeBSD, llvm::Triple::UnknownArch, llvm::ELF::NT_PPC_VMX},
 {llvm::Triple::Linux, llvm::Triple::UnknownArch, llvm::ELF::NT_PPC_VMX},
diff --git a/lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py 
b/lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py
index 797551b061a237..adbef69c5c38b0 100644
--- a/lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py
+++ b/lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py
@@ -374,3 +374,35 @@ def test_gcs_expression_enable_gcs(self):
 # consistent with the disabled -> enabled behaviour.
 enabled |

[Lldb-commits] [lldb] [lldb][Formatters] Do not recursively dereference pointer type when creating formatter candicates list. (PR #124048)

[Pavel Labath via lldb-commits]

labath wrote:

If I **was** holding a vector as a `vector**`, then I would would to see its 
summary/size as well. And maybe even the children too. But the thing I 
questioning is how often does one hold a C++ container as a double pointer. Is 
there like some common programming pattern that produces those? The only time 
I've ever done that is when I was experimenting with how "frame variable" 
handles pointers and references. I have seen and worked with double pointers, 
but this was always in some kind of legacy/C context. A combination of double 
pointers and a std::vector (which essentially makes it a triple pointer) would 
be very.. eclectic here. Modern C++ would likely turn at least one of those 
pointers into an iterator, or a smart pointer, or a reference..

It's true that a summary provider is less intrusive (it doesn't override 
existing children -- though it can hide them!), than a synthetic child 
provider, but I think it would be very confusing to have different behavior 
here. The thing I like about doing this for one level is that (as Zequan points 
out) it nicely matches what our SBValue API does, which makes most formatters 
"just work".

https://github.com/llvm/llvm-project/pull/124048
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][AArch64] Add Guarded Control Stack support for Linux core files (PR #124293)

[David Spickett via lldb-commits]

DavidSpickett wrote:

Rebased, PR is just new code now.

https://github.com/llvm/llvm-project/pull/124293
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [lldb][Windows] WoA HW Watchpoint support in LLDB (PR #108072)

[David Spickett via lldb-commits]

DavidSpickett wrote:

Please update the PR description to reflect what the current code does, or if 
you want to leave the notes there, move them into an obvious section that you 
can cut out if/when this gets merged.

https://github.com/llvm/llvm-project/pull/108072
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [lldb][Windows] WoA HW Watchpoint support in LLDB (PR #108072)

[David Spickett via lldb-commits]

@@ -492,23 +492,40 @@ NativeProcessWindows::OnDebugException(bool first_chance,
   }
   case DWORD(STATUS_BREAKPOINT):
   case STATUS_WX86_BREAKPOINT:
-if (FindSoftwareBreakpoint(record.GetExceptionAddress())) {
-  LLDB_LOG(log, "Hit non-loader breakpoint at address {0:x}.",
-   record.GetExceptionAddress());
 
-  StopThread(record.GetThreadID(), StopReason::eStopReasonBreakpoint);
+if (NativeThreadWindows *stop_thread =
+GetThreadByID(record.GetThreadID())) {
+  auto ®_ctx = stop_thread->GetRegisterContext();
+  const auto exception_addr = record.GetExceptionAddress();
+  const auto thread_id = record.GetThreadID();
 
-  if (NativeThreadWindows *stop_thread =
-  GetThreadByID(record.GetThreadID())) {
-auto ®ister_context = stop_thread->GetRegisterContext();
-uint32_t breakpoint_size = GetSoftwareBreakpointPCOffset();
+  if (FindSoftwareBreakpoint(exception_addr)) {
+LLDB_LOG(log, "Hit non-loader breakpoint at address {0:x}.",
+ exception_addr);
 // The current PC is AFTER the BP opcode, on all architectures.
-uint64_t pc = register_context.GetPC() - breakpoint_size;
-register_context.SetPC(pc);
+reg_ctx.SetPC(reg_ctx.GetPC() - GetSoftwareBreakpointPCOffset());
+StopThread(thread_id, StopReason::eStopReasonBreakpoint);
+SetState(eStateStopped, true);
+return ExceptionResult::MaskException;
+  } else {
+const std::vector &args = record.GetExceptionArguments();
+// Check that the ExceptionInformation array of EXCEPTION_RECORD
+// contains at least two elements: the first is a read-write flag
+// indicating the type of data access operation (read or write) while
+// the second contains the virtual address of the accessed data.
+if (args.size() >= 2) {
+  uint32_t hw_id = LLDB_INVALID_INDEX32;
+  reg_ctx.GetWatchpointHitIndex(hw_id, args[1]);

DavidSpickett wrote:

Ideally every error path returns LLDB_INVALID_INDEX32 but you should check that 
the returned Status is not a failure as well, just in case.

https://github.com/llvm/llvm-project/pull/108072
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (PR #124279)

[Michael Buch via lldb-commits]

https://github.com/Michael137 updated 
https://github.com/llvm/llvm-project/pull/124279

>From 23c7f4a3e067739a050964b3f1c2140787b0c31f Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 24 Jan 2025 13:33:07 +
Subject: [PATCH 1/5] [lldb][TypeSystem] Ensure that ParmVarDecls have the
 correct DeclContext

While sifting through this part of the code I noticed that when we parse C++ 
methods, `DWARFASTParserClang` creates two sets of `ParmVarDecls`, one in 
`ParseChildParameters` and once in `AddMethodToCXXRecordType`.  The former is 
unused when we're dealing with methods. Moreover, the `ParmVarDecls` we created 
in `ParseChildParameters` were created with an incorrect `clang::DeclContext` 
(namely the DeclContext of the function, and not the function itself). In 
Clang, there's `ParmVarDecl::setOwningFunction` to adjust the DeclContext of a 
parameter if the parameter was created before the FunctionDecl. But we never 
used it.

This patch removes the `ParmVarDecl` creation from `ParseChildParameters` and 
instead creates a `TypeSystemClang::CreateParameterDeclarations` that ensures 
we set the DeclContext correctly.

This wasn't causing any concrete issues (that I know of), but was quite 
surprising. And this way of setting the parameters seems easier to reason about 
(in my opinion).
---
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  |  31 ++--
 .../SymbolFile/DWARF/DWARFASTParserClang.h|   3 +-
 .../SymbolFile/NativePDB/PdbAstBuilder.cpp|   2 +-
 .../Plugins/SymbolFile/PDB/PDBASTParser.cpp   |   4 +-
 .../TypeSystem/Clang/TypeSystemClang.cpp  |  43 ++---
 .../TypeSystem/Clang/TypeSystemClang.h|  16 +-
 lldb/unittests/Symbol/TestTypeSystemClang.cpp |  42 +
 .../DWARF/DWARFASTParserClangTests.cpp| 154 ++
 8 files changed, 247 insertions(+), 48 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index e77188bfbd2e4a..30dec4bbf91c10 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1272,7 +1272,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
 return_clang_type = m_ast.GetBasicType(eBasicTypeVoid);
 
   std::vector function_param_types;
-  std::vector function_param_decls;
 
   // Parse the function children for the parameters
 
@@ -1283,8 +1282,7 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
 
   if (die.HasChildren()) {
 ParseChildParameters(containing_decl_ctx, die, is_variadic,
- has_template_params, function_param_types,
- function_param_decls);
+ has_template_params, function_param_types);
   }
 
   bool is_cxx_method = DeclKindIsCXXClass(containing_decl_ctx->getDeclKind());
@@ -1414,11 +1412,15 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE 
&die,
 
   LinkDeclContextToDIE(function_decl, die);
 
-  if (!function_param_decls.empty()) {
-m_ast.SetFunctionParameters(function_decl, function_param_decls);
+  const clang::FunctionProtoType *function_prototype(
+  llvm::cast(
+  ClangUtil::GetQualType(clang_type).getTypePtr()));
+  auto params = m_ast.CreateParameterDeclarations(function_decl,
+  *function_prototype);
+  if (!params.empty()) {
+function_decl->setParams(params);
 if (template_function_decl)
-  m_ast.SetFunctionParameters(template_function_decl,
-  function_param_decls);
+  template_function_decl->setParams(params);
   }
 
   ClangASTMetadata metadata;
@@ -2380,7 +2382,6 @@ 
DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
   bool is_variadic = false;
   bool has_template_params = false;
   std::vector param_types;
-  std::vector param_decls;
   StreamString sstr;
 
   DWARFDeclContext decl_ctx = die.GetDWARFDeclContext();
@@ -2394,7 +2395,7 @@ 
DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
   die, GetCXXObjectParameter(die, *containing_decl_ctx));
 
   ParseChildParameters(containing_decl_ctx, die, is_variadic,
-   has_template_params, param_types, param_decls);
+   has_template_params, param_types);
   sstr << "(";
   for (size_t i = 0; i < param_types.size(); i++) {
 if (i > 0)
@@ -3156,8 +3157,7 @@ bool DWARFASTParserClang::ParseChildMembers(
 void DWARFASTParserClang::ParseChildParameters(
 clang::DeclContext *containing_decl_ctx, const DWARFDIE &parent_die,
 bool &is_variadic, bool &has_template_params,
-std::vector &function_param_types,
-std::vector &function_param_decls) {
+std::vector &function_param_types) {
   if (!parent_die)
 return;
 
@@ -3168,7 +3168,6 @

[Lldb-commits] [lldb] [llvm] [lldb][Windows] WoA HW Watchpoint support in LLDB (PR #108072)

[David Spickett via lldb-commits]

@@ -492,23 +492,40 @@ NativeProcessWindows::OnDebugException(bool first_chance,
   }
   case DWORD(STATUS_BREAKPOINT):
   case STATUS_WX86_BREAKPOINT:
-if (FindSoftwareBreakpoint(record.GetExceptionAddress())) {
-  LLDB_LOG(log, "Hit non-loader breakpoint at address {0:x}.",
-   record.GetExceptionAddress());
 
-  StopThread(record.GetThreadID(), StopReason::eStopReasonBreakpoint);
+if (NativeThreadWindows *stop_thread =
+GetThreadByID(record.GetThreadID())) {
+  auto ®_ctx = stop_thread->GetRegisterContext();
+  const auto exception_addr = record.GetExceptionAddress();
+  const auto thread_id = record.GetThreadID();
 
-  if (NativeThreadWindows *stop_thread =
-  GetThreadByID(record.GetThreadID())) {
-auto ®ister_context = stop_thread->GetRegisterContext();
-uint32_t breakpoint_size = GetSoftwareBreakpointPCOffset();
+  if (FindSoftwareBreakpoint(exception_addr)) {
+LLDB_LOG(log, "Hit non-loader breakpoint at address {0:x}.",
+ exception_addr);
 // The current PC is AFTER the BP opcode, on all architectures.
-uint64_t pc = register_context.GetPC() - breakpoint_size;
-register_context.SetPC(pc);
+reg_ctx.SetPC(reg_ctx.GetPC() - GetSoftwareBreakpointPCOffset());
+StopThread(thread_id, StopReason::eStopReasonBreakpoint);
+SetState(eStateStopped, true);
+return ExceptionResult::MaskException;
+  } else {
+const std::vector &args = record.GetExceptionArguments();

DavidSpickett wrote:

We know that once we're in this else, it's a hardware break or watch point. But 
given we don't support hardware breakpoints yet, it can only be a hardware 
watchpoint.

Assuming that's correct, I would add a comment just after the else to say that.

https://github.com/llvm/llvm-project/pull/108072
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

[Pavel Labath via lldb-commits]

@@ -0,0 +1,158 @@
+//===-- DILLexer.h --*- 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
+//
+//===--===//
+
+#ifndef LLDB_VALUEOBJECT_DILLEXER_H_
+#define LLDB_VALUEOBJECT_DILLEXER_H_
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/Support/Error.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace lldb_private {
+
+namespace dil {
+
+/// Class defining the tokens generated by the DIL lexer and used by the
+/// DIL parser.
+class Token {
+public:
+  enum Kind {
+coloncolon,
+eof,
+identifier,
+invalid,
+kw_namespace,
+l_paren,
+none,
+r_paren,
+unknown,
+  };
+
+  Token(Kind kind, std::string spelling, uint32_t start)
+  : m_kind(kind), m_spelling(spelling), m_start_pos(start) {}
+
+  Token() : m_kind(Kind::none), m_spelling(""), m_start_pos(0) {}
+
+  void SetKind(Kind kind) { m_kind = kind; }

labath wrote:

Could we get rid of these (and of the `none` token kind)? Ideally, I'd like to 
treat the token as a value type (so you can assign and copy it, but not mess 
with it), and one that is always valid (no uninitialized (none) state).

https://github.com/llvm/llvm-project/pull/123521
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

[Pavel Labath via lldb-commits]

@@ -0,0 +1,218 @@
+//===-- DILLexerTests.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/ValueObject/DILLexer.h"
+#include "llvm/ADT/StringRef.h"
+#include "gtest/gtest.h"
+#include 
+
+using llvm::StringRef;
+
+bool VerifyExpectedTokens(
+lldb_private::dil::DILLexer &lexer,
+std::vector>
+exp_tokens,
+uint32_t start_pos) {
+  if (lexer.NumLexedTokens() - start_pos < exp_tokens.size())
+return false;
+
+  if (start_pos > 0)
+lexer.ResetTokenIdx(start_pos -
+1); // GetNextToken increments the idx first.
+  for (const auto &pair : exp_tokens) {
+lldb_private::dil::Token token = lexer.GetNextToken();
+if (token.GetKind() != pair.first || token.GetSpelling() != pair.second)
+  return false;
+  }
+
+  return true;
+}
+
+TEST(DILLexerTests, SimpleTest) {
+  StringRef input_expr("simple_var");
+  uint32_t tok_len = 10;
+  lldb_private::dil::DILLexer lexer(input_expr);
+  lldb_private::dil::Token token;
+  token.SetKind(lldb_private::dil::Token::unknown);
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::unknown);
+  auto success = lexer.LexAll();
+
+  if (!success) {
+EXPECT_TRUE(false);
+  }
+  token = lexer.GetNextToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::identifier);
+  EXPECT_EQ(token.GetSpelling(), "simple_var");
+  EXPECT_EQ(token.GetLength(), tok_len);

labath wrote:

I'm wondering if the GetLength function is really necessary. How often is it 
going to be called? Because if it's not very often, we might as well use 
`GetSpelling().size()`.

https://github.com/llvm/llvm-project/pull/123521
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

[Pavel Labath via lldb-commits]

@@ -0,0 +1,218 @@
+//===-- DILLexerTests.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/ValueObject/DILLexer.h"
+#include "llvm/ADT/StringRef.h"
+#include "gtest/gtest.h"
+#include 
+
+using llvm::StringRef;
+
+bool VerifyExpectedTokens(

labath wrote:

There's a reason why I used `EXPECT_THAT(..., ElementsAre()` in my example -- 
it gives *much* better error messages in case of failure: It will print the 
*expected* vector, the *actual* vector, and also the element which differs.

With an implementation like this, all you get is `error: false != true`. Which 
one would you rather debug?

https://github.com/llvm/llvm-project/pull/123521
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

[Pavel Labath via lldb-commits]

@@ -0,0 +1,189 @@
+//===-- DILLexer.cpp 
--===//
+//
+// 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
+//
+// This implements the recursive descent parser for the Data Inspection
+// Language (DIL), and its helper functions, which will eventually underlie the
+// 'frame variable' command. The language that this parser recognizes is
+// described in lldb/docs/dil-expr-lang.ebnf
+//
+//===--===//
+
+#include "lldb/ValueObject/DILLexer.h"
+#include "lldb/Utility/Status.h"
+#include "llvm/ADT/StringSwitch.h"
+
+namespace lldb_private {
+
+namespace dil {
+
+llvm::StringRef Token::GetTokenName(Kind kind) {
+  switch (kind) {
+  case Kind::coloncolon:
+return "coloncolon";
+  case Kind::eof:
+return "eof";
+  case Kind::identifier:
+return "identifier";
+  case Kind::invalid:
+return "invalid";
+  case Kind::kw_namespace:
+return "namespace";
+  case Kind::l_paren:
+return "l_paren";
+  case Kind::none:
+return "none";
+  case Kind::r_paren:
+return "r_paren";
+  case Kind::unknown:
+return "unknown";
+  }
+}
+
+static bool IsLetter(char c) {
+  return ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z');
+}
+
+static bool IsDigit(char c) { return '0' <= c && c <= '9'; }
+
+// A word starts with a letter, underscore, or dollar sign, followed by
+// letters ('a'..'z','A'..'Z'), digits ('0'..'9'), and/or  underscores.
+llvm::iterator_range DILLexer::IsWord() {
+  llvm::StringRef::iterator start = m_cur_pos;
+  bool dollar_start = false;
+
+  // Must not start with a digit.
+  if (m_cur_pos == m_expr.end() || IsDigit(*m_cur_pos))
+return llvm::make_range(m_cur_pos, m_cur_pos);
+
+  // First character *may* be a '$', for a register name or convenience
+  // variable.
+  if (*m_cur_pos == '$') {
+dollar_start = true;
+++m_cur_pos;
+  }
+
+  // Contains only letters, digits or underscores
+  for (; m_cur_pos != m_expr.end(); ++m_cur_pos) {
+char c = *m_cur_pos;
+if (!IsLetter(c) && !IsDigit(c) && c != '_')
+  break;
+  }
+
+  // If first char is '$', make sure there's at least one mare char, or it's
+  // invalid.
+  if (dollar_start && (m_cur_pos - start <= 1)) {
+m_cur_pos = start;
+return llvm::make_range(start, start); // Empty range
+  }
+
+  return llvm::make_range(start, m_cur_pos);
+}
+
+void DILLexer::UpdateLexedTokens(Token &result, Token::Kind tok_kind,
+ std::string tok_str, uint32_t tok_pos) {
+  Token new_token(tok_kind, tok_str, tok_pos);
+  result = new_token;
+  m_lexed_tokens.push_back(std::move(new_token));
+}
+
+llvm::Expected DILLexer::LexAll() {
+  bool done = false;
+  while (!done) {
+auto tok_or_err = Lex();
+if (!tok_or_err)
+  return tok_or_err.takeError();
+Token token = *tok_or_err;
+if (token.GetKind() == Token::eof) {
+  done = true;
+}
+  }
+  return true;
+}
+
+llvm::Expected DILLexer::Lex() {
+  Token result;
+
+  // Skip over whitespace (spaces).
+  while (m_cur_pos != m_expr.end() && *m_cur_pos == ' ')
+m_cur_pos++;
+
+  // Check to see if we've reached the end of our input string.
+  if (m_cur_pos == m_expr.end()) {
+UpdateLexedTokens(result, Token::eof, "", (uint32_t)m_expr.size());
+return result;
+  }
+
+  uint32_t position = m_cur_pos - m_expr.begin();
+  llvm::StringRef::iterator start = m_cur_pos;
+  llvm::iterator_range word_range = IsWord();
+  if (!word_range.empty()) {
+uint32_t length = word_range.end() - word_range.begin();
+llvm::StringRef word(m_expr.substr(position, length));
+// We will be adding more keywords here in the future...
+Token::Kind kind = llvm::StringSwitch(word)
+   .Case("namespace", Token::kw_namespace)
+   .Default(Token::identifier);
+UpdateLexedTokens(result, kind, word.str(), position);
+return result;
+  }
+
+  m_cur_pos = start;
+  llvm::StringRef remainder(m_expr.substr(position, m_expr.end() - m_cur_pos));
+  std::vector> operators = {
+  {Token::l_paren, "("},
+  {Token::r_paren, ")"},
+  {Token::coloncolon, "::"},
+  };
+  for (auto [kind, str] : operators) {
+if (remainder.consume_front(str)) {
+  m_cur_pos += strlen(str);
+  UpdateLexedTokens(result, kind, str, position);
+  return result;
+}
+  }
+
+  // Unrecognized character(s) in string; unable to lex it.
+  Status error("Unable to lex input string");
+  return error.ToError();
+}

labath wrote:

Sorry for rewriting this for you, but I figured its easier than explaining 
everything in abstract:

The main things I wanted to achieve by this are:
- no half-initialized state (object constructed, but LexAll not called). The 
object is always construc

[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

[Pavel Labath via lldb-commits]

@@ -0,0 +1,158 @@
+//===-- DILLexer.h --*- 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
+//
+//===--===//
+
+#ifndef LLDB_VALUEOBJECT_DILLEXER_H_
+#define LLDB_VALUEOBJECT_DILLEXER_H_
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/Support/Error.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace lldb_private {
+
+namespace dil {
+
+/// Class defining the tokens generated by the DIL lexer and used by the
+/// DIL parser.
+class Token {
+public:
+  enum Kind {
+coloncolon,
+eof,
+identifier,
+invalid,
+kw_namespace,
+l_paren,
+none,
+r_paren,
+unknown,
+  };
+
+  Token(Kind kind, std::string spelling, uint32_t start)
+  : m_kind(kind), m_spelling(spelling), m_start_pos(start) {}
+
+  Token() : m_kind(Kind::none), m_spelling(""), m_start_pos(0) {}
+
+  void SetKind(Kind kind) { m_kind = kind; }
+
+  Kind GetKind() const { return m_kind; }
+
+  std::string GetSpelling() const { return m_spelling; }
+
+  uint32_t GetLength() const { return m_spelling.size(); }
+
+  bool Is(Kind kind) const { return m_kind == kind; }
+
+  bool IsNot(Kind kind) const { return m_kind != kind; }
+
+  bool IsOneOf(Kind kind1, Kind kind2) const { return Is(kind1) || Is(kind2); }
+
+  template  bool IsOneOf(Kind kind, Ts... Ks) const {
+return Is(kind) || IsOneOf(Ks...);
+  }
+
+  uint32_t GetLocation() const { return m_start_pos; }
+
+  static llvm::StringRef GetTokenName(Kind kind);
+
+private:
+  Kind m_kind;
+  std::string m_spelling;
+  uint32_t m_start_pos; // within entire expression string
+};
+
+/// Class for doing the simple lexing required by DIL.
+class DILLexer {
+public:
+  DILLexer(llvm::StringRef dil_expr) : m_expr(dil_expr) {
+m_cur_pos = m_expr.begin();
+// Use UINT_MAX to indicate invalid/uninitialized value.
+m_tokens_idx = UINT_MAX;
+m_invalid_token = Token(Token::invalid, "", 0);
+  }
+
+  llvm::Expected LexAll();
+
+  /// Return the lexed token N+1 positions ahead of the 'current' token

labath wrote:

I feel like there are too many ways to navigate the token stream here. You can 
either call GetCurrentToken+IncrementTokenIdx, or GetNextToken(which I guess 
increments the index automatically), or LookAhead+AcceptLookAhead.

I think it would be better to start with something simple (we can add more or 
revamp the existing API if it turns out to be clunky). What would you say to 
something like:
```
const Token &LookAhead(uint32_t N /* add `=1` if you want*/);
const Token &GetCurrentToken() { return LookAhead(0); } // just a fancy name 
for a look ahead of zero
void Advance(uint32_t N = 1); // advance the token stream
```

https://github.com/llvm/llvm-project/pull/123521
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

[Pavel Labath via lldb-commits]

@@ -0,0 +1,158 @@
+//===-- DILLexer.h --*- 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
+//
+//===--===//
+
+#ifndef LLDB_VALUEOBJECT_DILLEXER_H_
+#define LLDB_VALUEOBJECT_DILLEXER_H_
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/Support/Error.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace lldb_private {
+
+namespace dil {
+
+/// Class defining the tokens generated by the DIL lexer and used by the
+/// DIL parser.
+class Token {
+public:
+  enum Kind {
+coloncolon,
+eof,
+identifier,
+invalid,
+kw_namespace,
+l_paren,
+none,
+r_paren,
+unknown,
+  };
+
+  Token(Kind kind, std::string spelling, uint32_t start)
+  : m_kind(kind), m_spelling(spelling), m_start_pos(start) {}
+
+  Token() : m_kind(Kind::none), m_spelling(""), m_start_pos(0) {}
+
+  void SetKind(Kind kind) { m_kind = kind; }
+
+  Kind GetKind() const { return m_kind; }
+
+  std::string GetSpelling() const { return m_spelling; }
+
+  uint32_t GetLength() const { return m_spelling.size(); }
+
+  bool Is(Kind kind) const { return m_kind == kind; }
+
+  bool IsNot(Kind kind) const { return m_kind != kind; }
+
+  bool IsOneOf(Kind kind1, Kind kind2) const { return Is(kind1) || Is(kind2); }
+
+  template  bool IsOneOf(Kind kind, Ts... Ks) const {
+return Is(kind) || IsOneOf(Ks...);
+  }
+
+  uint32_t GetLocation() const { return m_start_pos; }
+
+  static llvm::StringRef GetTokenName(Kind kind);
+
+private:
+  Kind m_kind;
+  std::string m_spelling;
+  uint32_t m_start_pos; // within entire expression string
+};
+
+/// Class for doing the simple lexing required by DIL.
+class DILLexer {
+public:
+  DILLexer(llvm::StringRef dil_expr) : m_expr(dil_expr) {
+m_cur_pos = m_expr.begin();
+// Use UINT_MAX to indicate invalid/uninitialized value.
+m_tokens_idx = UINT_MAX;
+m_invalid_token = Token(Token::invalid, "", 0);
+  }
+
+  llvm::Expected LexAll();
+
+  /// Return the lexed token N+1 positions ahead of the 'current' token
+  /// being handled by the DIL parser.
+  const Token &LookAhead(uint32_t N);
+
+  const Token &AcceptLookAhead(uint32_t N);
+
+  const Token &GetNextToken();
+
+  /// Return the index for the 'current' token being handled by the DIL parser.
+  uint32_t GetCurrentTokenIdx() { return m_tokens_idx; }
+
+  /// Return the current token to be handled by the DIL parser.
+  const Token &GetCurrentToken() { return m_lexed_tokens[m_tokens_idx]; }
+
+  uint32_t NumLexedTokens() { return m_lexed_tokens.size(); }
+
+  /// Update the index for the 'current' token, to point to the next lexed
+  /// token.
+  bool IncrementTokenIdx() {
+if (m_tokens_idx >= m_lexed_tokens.size() - 1)
+  return false;
+
+m_tokens_idx++;
+return true;
+  }
+
+  /// Set the index for the 'current' token (to be handled by the parser)
+  /// to a particular position. Used for either committing 'look ahead' parsing
+  /// or rolling back tentative parsing.
+  bool ResetTokenIdx(uint32_t new_value) {
+if (new_value > m_lexed_tokens.size() - 1)
+  return false;
+
+m_tokens_idx = new_value;
+return true;
+  }

labath wrote:

```suggestion
  void ResetTokenIdx(uint32_t new_value) {
assert(new_value < m_lexed_tokens.size());
m_tokens_idx = new_value;
  }
```

(AIUI, the only usage of this function will be to restore a previous (and 
valid) position, so any error here is definitely a bug)

https://github.com/llvm/llvm-project/pull/123521
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

[Pavel Labath via lldb-commits]

@@ -0,0 +1,158 @@
+//===-- DILLexer.h --*- 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
+//
+//===--===//
+
+#ifndef LLDB_VALUEOBJECT_DILLEXER_H_
+#define LLDB_VALUEOBJECT_DILLEXER_H_
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/Support/Error.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace lldb_private {
+
+namespace dil {
+
+/// Class defining the tokens generated by the DIL lexer and used by the
+/// DIL parser.
+class Token {
+public:
+  enum Kind {
+coloncolon,
+eof,
+identifier,
+invalid,
+kw_namespace,
+l_paren,
+none,
+r_paren,
+unknown,
+  };
+
+  Token(Kind kind, std::string spelling, uint32_t start)
+  : m_kind(kind), m_spelling(spelling), m_start_pos(start) {}
+
+  Token() : m_kind(Kind::none), m_spelling(""), m_start_pos(0) {}
+
+  void SetKind(Kind kind) { m_kind = kind; }
+
+  Kind GetKind() const { return m_kind; }
+
+  std::string GetSpelling() const { return m_spelling; }
+
+  uint32_t GetLength() const { return m_spelling.size(); }
+
+  bool Is(Kind kind) const { return m_kind == kind; }
+
+  bool IsNot(Kind kind) const { return m_kind != kind; }
+
+  bool IsOneOf(Kind kind1, Kind kind2) const { return Is(kind1) || Is(kind2); }
+
+  template  bool IsOneOf(Kind kind, Ts... Ks) const {
+return Is(kind) || IsOneOf(Ks...);
+  }
+
+  uint32_t GetLocation() const { return m_start_pos; }
+
+  static llvm::StringRef GetTokenName(Kind kind);
+
+private:
+  Kind m_kind;
+  std::string m_spelling;
+  uint32_t m_start_pos; // within entire expression string
+};
+
+/// Class for doing the simple lexing required by DIL.
+class DILLexer {
+public:
+  DILLexer(llvm::StringRef dil_expr) : m_expr(dil_expr) {
+m_cur_pos = m_expr.begin();
+// Use UINT_MAX to indicate invalid/uninitialized value.
+m_tokens_idx = UINT_MAX;
+m_invalid_token = Token(Token::invalid, "", 0);
+  }
+
+  llvm::Expected LexAll();
+
+  /// Return the lexed token N+1 positions ahead of the 'current' token
+  /// being handled by the DIL parser.
+  const Token &LookAhead(uint32_t N);
+
+  const Token &AcceptLookAhead(uint32_t N);
+
+  const Token &GetNextToken();
+
+  /// Return the index for the 'current' token being handled by the DIL parser.
+  uint32_t GetCurrentTokenIdx() { return m_tokens_idx; }
+
+  /// Return the current token to be handled by the DIL parser.
+  const Token &GetCurrentToken() { return m_lexed_tokens[m_tokens_idx]; }
+
+  uint32_t NumLexedTokens() { return m_lexed_tokens.size(); }
+
+  /// Update the index for the 'current' token, to point to the next lexed
+  /// token.
+  bool IncrementTokenIdx() {
+if (m_tokens_idx >= m_lexed_tokens.size() - 1)
+  return false;
+
+m_tokens_idx++;
+return true;
+  }
+
+  /// Set the index for the 'current' token (to be handled by the parser)
+  /// to a particular position. Used for either committing 'look ahead' parsing
+  /// or rolling back tentative parsing.
+  bool ResetTokenIdx(uint32_t new_value) {
+if (new_value > m_lexed_tokens.size() - 1)
+  return false;
+
+m_tokens_idx = new_value;
+return true;
+  }
+
+  uint32_t GetLocation() { return m_cur_pos - m_expr.begin(); }

labath wrote:

should this be private?

https://github.com/llvm/llvm-project/pull/123521
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

[Pavel Labath via lldb-commits]

@@ -0,0 +1,189 @@
+//===-- DILLexer.cpp 
--===//
+//
+// 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
+//
+// This implements the recursive descent parser for the Data Inspection
+// Language (DIL), and its helper functions, which will eventually underlie the
+// 'frame variable' command. The language that this parser recognizes is
+// described in lldb/docs/dil-expr-lang.ebnf
+//
+//===--===//
+
+#include "lldb/ValueObject/DILLexer.h"
+#include "lldb/Utility/Status.h"
+#include "llvm/ADT/StringSwitch.h"
+
+namespace lldb_private {
+
+namespace dil {
+
+llvm::StringRef Token::GetTokenName(Kind kind) {
+  switch (kind) {
+  case Kind::coloncolon:
+return "coloncolon";
+  case Kind::eof:
+return "eof";
+  case Kind::identifier:
+return "identifier";
+  case Kind::invalid:
+return "invalid";
+  case Kind::kw_namespace:
+return "namespace";
+  case Kind::l_paren:
+return "l_paren";
+  case Kind::none:
+return "none";
+  case Kind::r_paren:
+return "r_paren";
+  case Kind::unknown:
+return "unknown";
+  }
+}
+
+static bool IsLetter(char c) {
+  return ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z');
+}
+
+static bool IsDigit(char c) { return '0' <= c && c <= '9'; }
+
+// A word starts with a letter, underscore, or dollar sign, followed by
+// letters ('a'..'z','A'..'Z'), digits ('0'..'9'), and/or  underscores.
+llvm::iterator_range DILLexer::IsWord() {
+  llvm::StringRef::iterator start = m_cur_pos;
+  bool dollar_start = false;
+
+  // Must not start with a digit.
+  if (m_cur_pos == m_expr.end() || IsDigit(*m_cur_pos))
+return llvm::make_range(m_cur_pos, m_cur_pos);
+
+  // First character *may* be a '$', for a register name or convenience
+  // variable.
+  if (*m_cur_pos == '$') {
+dollar_start = true;
+++m_cur_pos;
+  }
+
+  // Contains only letters, digits or underscores
+  for (; m_cur_pos != m_expr.end(); ++m_cur_pos) {
+char c = *m_cur_pos;
+if (!IsLetter(c) && !IsDigit(c) && c != '_')
+  break;
+  }
+
+  // If first char is '$', make sure there's at least one mare char, or it's
+  // invalid.
+  if (dollar_start && (m_cur_pos - start <= 1)) {
+m_cur_pos = start;
+return llvm::make_range(start, start); // Empty range
+  }
+
+  return llvm::make_range(start, m_cur_pos);
+}
+
+void DILLexer::UpdateLexedTokens(Token &result, Token::Kind tok_kind,
+ std::string tok_str, uint32_t tok_pos) {
+  Token new_token(tok_kind, tok_str, tok_pos);
+  result = new_token;
+  m_lexed_tokens.push_back(std::move(new_token));
+}
+
+llvm::Expected DILLexer::LexAll() {
+  bool done = false;
+  while (!done) {
+auto tok_or_err = Lex();
+if (!tok_or_err)
+  return tok_or_err.takeError();
+Token token = *tok_or_err;
+if (token.GetKind() == Token::eof) {
+  done = true;
+}
+  }
+  return true;
+}
+
+llvm::Expected DILLexer::Lex() {
+  Token result;
+
+  // Skip over whitespace (spaces).
+  while (m_cur_pos != m_expr.end() && *m_cur_pos == ' ')
+m_cur_pos++;
+
+  // Check to see if we've reached the end of our input string.
+  if (m_cur_pos == m_expr.end()) {
+UpdateLexedTokens(result, Token::eof, "", (uint32_t)m_expr.size());
+return result;
+  }
+
+  uint32_t position = m_cur_pos - m_expr.begin();
+  llvm::StringRef::iterator start = m_cur_pos;
+  llvm::iterator_range word_range = IsWord();
+  if (!word_range.empty()) {
+uint32_t length = word_range.end() - word_range.begin();
+llvm::StringRef word(m_expr.substr(position, length));
+// We will be adding more keywords here in the future...
+Token::Kind kind = llvm::StringSwitch(word)
+   .Case("namespace", Token::kw_namespace)
+   .Default(Token::identifier);
+UpdateLexedTokens(result, kind, word.str(), position);
+return result;
+  }
+
+  m_cur_pos = start;
+  llvm::StringRef remainder(m_expr.substr(position, m_expr.end() - m_cur_pos));
+  std::vector> operators = {
+  {Token::l_paren, "("},
+  {Token::r_paren, ")"},
+  {Token::coloncolon, "::"},
+  };
+  for (auto [kind, str] : operators) {
+if (remainder.consume_front(str)) {
+  m_cur_pos += strlen(str);
+  UpdateLexedTokens(result, kind, str, position);
+  return result;
+}
+  }
+
+  // Unrecognized character(s) in string; unable to lex it.
+  Status error("Unable to lex input string");
+  return error.ToError();
+}
+
+const Token &DILLexer::LookAhead(uint32_t N) {
+  if (m_tokens_idx + N + 1 < m_lexed_tokens.size())

labath wrote:

You didn't say anything about what you make of my suggestion to use an 
"infinite" stream of `eof` tokens at the end. The current implementation uses 

[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

[Pavel Labath via lldb-commits]

@@ -0,0 +1,158 @@
+//===-- DILLexer.h --*- 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
+//
+//===--===//
+
+#ifndef LLDB_VALUEOBJECT_DILLEXER_H_
+#define LLDB_VALUEOBJECT_DILLEXER_H_
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/Support/Error.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace lldb_private {
+
+namespace dil {

labath wrote:

```suggestion
namespace lldb_private::dil {
```

https://github.com/llvm/llvm-project/pull/123521
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

[Pavel Labath via lldb-commits]

https://github.com/labath commented:

There are some simplifications (and one rewrite :P) that I'd like to talk 
about, but I think we're not far.

The main thing that's bothering me is this identifier vs. keyword issue (and 
for this part, I'd like to loop in @jimingham (at least)). 

Your implementation takes a very strict view of what's a possible identifier 
(it must consist of a very specific set of characters, appearing in a specific 
order, and it must not be a keyword). In contrast, the current "frame variable" 
implementation basically treats anything it doesn't recognise (including 
whitespace) as a variable name (and it has no keywords):
```
(lldb) v a*b
error: no variable named 'a*b' found in this frame
(lldb) v "a b"
error: no variable named 'a b' found in this frame
(lldb) v 123
error: no variable named '123' found in this frame
(lldb) v namespace
error: no variable named 'namespace' found in this frame
```

Now, obviously, in order to expand the language, we need to restrict the set of 
variable names, but I don't think we need to do it so aggressively. I don't 
think anyone will complain if we make it harder for him to access a variable 
called `a*b`, but for example, `namespace`, and `💩` are perfectly valid 
variable names in many languages ([one of 
them](https://godbolt.org/z/vjfhj6dfM) is C).

For this reason, I think it'd be better to have a very liberal definition of 
what constitutes a possible variable name (identifier). Instead of a 
prescribing a character sequence, I think it be better to say that anything 
that doesn't contain one of the characters we recognize (basically: operators) 
is an identifier. IOW, to do something like `frame variable` does right now.

As for keywords, I think it'd be best to have as few of them as possible, and 
for the rest, to treat their keyword-ness as context-dependent whereever 
possible. What I mean by that is to recognise them as keywords only within 
contexts where such usage would be legal. The `namespace` keyword is a prime 
example of that. I *think* the only place where this can appear as a keyword in 
the context of the DIL is within the `(anonymous namespace)` group. If that's 
the case, then I think we should be able to detect that and disambiguate the 
usage, so that e.g. `v namespace` prints the variable called `namespace` and `v 
(anonymous namespace)::namespace` prints the variable called `namespace` in an 
anonymous namespace (in an evil language which has anonymous namespaces but 
allows you to use variables called `namespace`).

The way to do that is probably to *not* treat the string "namespace" as a 
keyword at the lexer level, but to recognize it later, when we have more 
context available.

What do you all think?

https://github.com/llvm/llvm-project/pull/123521
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

[Pavel Labath via lldb-commits]

@@ -0,0 +1,218 @@
+//===-- DILLexerTests.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/ValueObject/DILLexer.h"
+#include "llvm/ADT/StringRef.h"
+#include "gtest/gtest.h"
+#include 
+
+using llvm::StringRef;
+
+bool VerifyExpectedTokens(
+lldb_private::dil::DILLexer &lexer,
+std::vector>
+exp_tokens,
+uint32_t start_pos) {
+  if (lexer.NumLexedTokens() - start_pos < exp_tokens.size())
+return false;
+
+  if (start_pos > 0)
+lexer.ResetTokenIdx(start_pos -
+1); // GetNextToken increments the idx first.
+  for (const auto &pair : exp_tokens) {
+lldb_private::dil::Token token = lexer.GetNextToken();
+if (token.GetKind() != pair.first || token.GetSpelling() != pair.second)
+  return false;
+  }
+
+  return true;
+}
+
+TEST(DILLexerTests, SimpleTest) {
+  StringRef input_expr("simple_var");
+  uint32_t tok_len = 10;
+  lldb_private::dil::DILLexer lexer(input_expr);
+  lldb_private::dil::Token token;
+  token.SetKind(lldb_private::dil::Token::unknown);
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::unknown);
+  auto success = lexer.LexAll();
+
+  if (!success) {
+EXPECT_TRUE(false);
+  }

labath wrote:

```suggestion
  ASSERT_THAT_EXPECTED(lexer.LexAll(), llvm::HasValue(true));`
```

https://github.com/llvm/llvm-project/pull/123521
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 6087c30 - [lldb] Simplify preprocessor conditional (#124522)

[via lldb-commits]

Author: Pavel Labath
Date: 2025-01-27T13:41:18+01:00
New Revision: 6087c3049656bbaef51fffb48e2404e86f7e0d3f

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

LOG: [lldb] Simplify preprocessor conditional (#124522)

The long list of defines is just a very elaborate way to say "not
windows".

Added: 


Modified: 
lldb/source/Host/common/Host.cpp

Removed: 




diff  --git a/lldb/source/Host/common/Host.cpp 
b/lldb/source/Host/common/Host.cpp
index fdb623667bc251..d5054008268b9d 100644
--- a/lldb/source/Host/common/Host.cpp
+++ b/lldb/source/Host/common/Host.cpp
@@ -11,13 +11,19 @@
 #include 
 #include 
 #include 
+
 #ifndef _WIN32
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
+#if !defined(__ANDROID__)
+#include 
+#endif
 #endif
 
 #if defined(__APPLE__)
@@ -26,16 +32,6 @@
 #include 
 #endif
 
-#if defined(__linux__) || defined(__FreeBSD__) ||  
\
-defined(__FreeBSD_kernel__) || defined(__APPLE__) ||   
\
-defined(__NetBSD__) || defined(__OpenBSD__) || defined(__EMSCRIPTEN__)
-#if !defined(__ANDROID__)
-#include 
-#endif
-#include 
-#include 
-#endif
-
 #if defined(__FreeBSD__)
 #include 
 #endif



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

[Lldb-commits] [lldb] [lldb] Simplify preprocessor conditional (PR #124522)

[Pavel Labath via lldb-commits]

https://github.com/labath closed 
https://github.com/llvm/llvm-project/pull/124522
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (PR #124279)

[Pavel Labath via lldb-commits]

https://github.com/labath approved this pull request.


https://github.com/llvm/llvm-project/pull/124279
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (PR #124279)

[Pavel Labath via lldb-commits]

@@ -3157,7 +3158,8 @@ bool DWARFASTParserClang::ParseChildMembers(
 void DWARFASTParserClang::ParseChildParameters(
 clang::DeclContext *containing_decl_ctx, const DWARFDIE &parent_die,
 bool &is_variadic, bool &has_template_params,
-std::vector &function_param_types) {
+std::vector &function_param_types,
+llvm::SmallVector &function_param_names) {

labath wrote:

```suggestion
llvm::SmallVectorImpl &function_param_names) {
```

https://github.com/llvm/llvm-project/pull/124279
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

[Pavel Labath via lldb-commits]

https://github.com/labath edited 
https://github.com/llvm/llvm-project/pull/123521
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks (PR #123918)

[David Spickett via lldb-commits]

https://github.com/DavidSpickett updated 
https://github.com/llvm/llvm-project/pull/123918

>From c89a16ab405d4aad7bf0a59afb433dd5ecd1a402 Mon Sep 17 00:00:00 2001
From: David Spickett 
Date: Tue, 20 Aug 2024 16:06:34 +0100
Subject: [PATCH 1/3] [lldb][AArch64] Fix expression evaluation with Guarded
 Control Stacks

When the Guarded Control Stack (GCS) is enabled, returns cause the
processor to validate that the address at the location pointed to
by gcspr_el0 matches the one in the link register.

```
ret (lr=A) << pc

| GCS |
+=+
|  A  |
|  B  | << gcspr_el0

Fault: tried to return to A when you should have returned to B.
```

Therefore when an expression wraper function tries to return to
the expression return address (usually `_start` if there is a libc),
it would fault.

```
ret (lr=_start) << pc

| GCS|
++
| user_func1 |
| user_func2 | << gcspr_el0

Fault: tried to return to _start when you should have return to user_func2.
```

To fix this we must push that return address to the GCS in PrepareTrivialCall.
This value is then consumed by the final return and the expression
completes as expected.

```
ret (lr=_start) << pc

| GCS|
++
| user_func1 |
| user_func2 |
| _start | << gcspr_el0

No fault, we return to _start as normal.
```

The gcspr_el0 register will be restored after expression evaluation
so that the program can continue correctly.

However, due to restrictions in the Linux GCS ABI, we will not
restore the enable bit of gcs_features_enabled. Re-enabling GCS
via ptrace is not supported because it requires memory to be
allocated.

We could disable GCS if the expression enabled GCS, however this
would use up that state transition that the program might later
rely on. And generally it is cleaner to ignore the whole bit
rather than one state transition of it.

We will also not restore the GCS entry that was overwritten with
the expression's return address. On the grounds that:
* This entry will never be used by the program. If the program branches,
  the entry will be overwritten. If the program returns, gcspr_el0
  will point to the entry before the expression return address
  and that entry will instead be validated.
* Any expression that calls functions will overwrite even more
  entries, so the user needs to be aware of that anyway if they
  want to preserve the contents of the GCS for inspection.
* An expression could leave the program in a state where
  restoring the value makes the situation worse. Especially
  if we ever support this in bare metal debugging.

I will later document all this on https://lldb.llvm.org/use/aarch64-linux.html
as well.

Tests have been added for:
* A function call that does not interact with GCS.
* A call that does, and disables it (we do not re-enable it).
* A call that does, and enables it (we do not disable it again).
---
 .../Plugins/ABI/AArch64/ABISysV_arm64.cpp |  56 +
 .../NativeRegisterContextLinux_arm64.cpp  |  20 +-
 .../linux/aarch64/gcs/TestAArch64LinuxGCS.py  | 196 ++
 lldb/test/API/linux/aarch64/gcs/main.c|  47 -
 4 files changed, 278 insertions(+), 41 deletions(-)

diff --git a/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp 
b/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
index 93b8141e97ef86..d843e718b6875e 100644
--- a/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
+++ b/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
@@ -60,6 +60,59 @@ ABISysV_arm64::CreateInstance(lldb::ProcessSP process_sp, 
const ArchSpec &arch)
   return ABISP();
 }
 
+static bool PushToLinuxGuardedControlStack(addr_t return_addr,
+   RegisterContext *reg_ctx,
+   Thread &thread) {
+  // If the Guarded Control Stack extension is enabled we need to put the 
return
+  // address onto that stack.
+  const RegisterInfo *gcs_features_enabled_info =
+  reg_ctx->GetRegisterInfoByName("gcs_features_enabled");
+  if (!gcs_features_enabled_info)
+return false;
+
+  uint64_t gcs_features_enabled = reg_ctx->ReadRegisterAsUnsigned(
+  gcs_features_enabled_info, LLDB_INVALID_ADDRESS);
+  if (gcs_features_enabled == LLDB_INVALID_ADDRESS)
+return false;
+
+  // Only attempt this if GCS is enabled. If it's not enabled then gcspr_el0
+  // may point to unmapped memory.
+  if ((gcs_features_enabled & 1) == 0)
+return false;
+
+  const RegisterInfo *gcspr_el0_info =
+  reg_ctx->GetRegisterInfoByName("gcspr_el0");
+  if (!gcspr_el0_info)
+return false;
+
+  uint64_t gcspr_el0 =
+  reg_ctx->ReadRegisterAsUnsigned(gcspr_el0_info, LLDB_INVALID_ADDRESS);
+  if (gcspr_el0 == LLDB_INVALID_ADDRESS)
+return false;
+
+  // A link register entry on the GCS is 8 bytes.
+  gcspr_el0 -= 8;
+  if (!reg_ctx->WriteRegisterFromUnsigned(gcspr_el0_info, gcspr_el0))
+return false;
+
+  Status error;
+  size_t wrote = thread.GetProcess()->WriteMemory(gcspr_el0, &return_addr,
+

[Lldb-commits] [lldb] [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks (PR #123918)

[via lldb-commits]

github-actions[bot] wrote:




:warning: Python code formatter, darker found issues in your code. :warning:



You can test this locally with the following command:


``bash
darker --check --diff -r 
b7286dbef9dc1986860d29e390b092599e1d7db5...3436294db831d78dddacb186c93007ec05c0cfee
 lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py
``





View the diff from darker here.


``diff
--- TestAArch64LinuxGCS.py  2025-01-27 12:40:57.00 +
+++ TestAArch64LinuxGCS.py  2025-01-27 12:50:10.960144 +
@@ -299,11 +299,10 @@
 # return from this expression without causing a fault.
 before = self.check_gcs_registers()
 self.expect(expr_cmd, substrs=["(unsigned long) 1"])
 self.check_gcs_registers(*before)
 
-
 @skipUnlessPlatform(["linux"])
 def test_gcs_expression_disable_gcs(self):
 if not self.isAArch64GCS():
 self.skipTest("Target must support GCS.")
 

``




https://github.com/llvm/llvm-project/pull/123918
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)

[Pavel Labath via lldb-commits]

@@ -0,0 +1,218 @@
+//===-- DILLexerTests.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/ValueObject/DILLexer.h"
+#include "llvm/ADT/StringRef.h"
+#include "gtest/gtest.h"
+#include 
+
+using llvm::StringRef;
+
+bool VerifyExpectedTokens(
+lldb_private::dil::DILLexer &lexer,
+std::vector>
+exp_tokens,
+uint32_t start_pos) {
+  if (lexer.NumLexedTokens() - start_pos < exp_tokens.size())
+return false;
+
+  if (start_pos > 0)
+lexer.ResetTokenIdx(start_pos -
+1); // GetNextToken increments the idx first.
+  for (const auto &pair : exp_tokens) {
+lldb_private::dil::Token token = lexer.GetNextToken();
+if (token.GetKind() != pair.first || token.GetSpelling() != pair.second)
+  return false;
+  }
+
+  return true;
+}
+
+TEST(DILLexerTests, SimpleTest) {
+  StringRef input_expr("simple_var");
+  uint32_t tok_len = 10;
+  lldb_private::dil::DILLexer lexer(input_expr);
+  lldb_private::dil::Token token;
+  token.SetKind(lldb_private::dil::Token::unknown);
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::unknown);
+  auto success = lexer.LexAll();
+
+  if (!success) {
+EXPECT_TRUE(false);
+  }
+  token = lexer.GetNextToken();
+  EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::identifier);
+  EXPECT_EQ(token.GetSpelling(), "simple_var");
+  EXPECT_EQ(token.GetLength(), tok_len);
+  token = lexer.GetNextToken();
+  ;

labath wrote:

delete

https://github.com/llvm/llvm-project/pull/123521
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][AArch64] Add register fields for Guarded Control Stack registers (PR #124295)

[David Spickett via lldb-commits]

DavidSpickett wrote:

1st commit(s) is https://github.com/llvm/llvm-project/pull/124293, last commit 
is the new code.

https://github.com/llvm/llvm-project/pull/124295
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][AArch64] Add register fields for Guarded Control Stack registers (PR #124295)

[David Spickett via lldb-commits]

https://github.com/DavidSpickett updated 
https://github.com/llvm/llvm-project/pull/124295

>From d2c221b9b8cfeb8d9d902947db38c716e7a09804 Mon Sep 17 00:00:00 2001
From: David Spickett 
Date: Tue, 27 Aug 2024 11:45:52 +0100
Subject: [PATCH 1/2] [lldb][AArch64] Add Guarded Control Stack support for
 Linux core files

This allows you to read the same registers as you would for a live process.

As the content of proc/pid/smaps is not included in the core file,
we don't get the "ss" marker. The GCS region is stil in the list
though.
---
 .../RegisterContextPOSIXCore_arm64.cpp|  17 ++
 .../elf-core/RegisterContextPOSIXCore_arm64.h |   1 +
 .../Process/elf-core/RegisterUtilities.h  |   4 +++
 .../linux/aarch64/gcs/TestAArch64LinuxGCS.py  |  32 ++
 lldb/test/API/linux/aarch64/gcs/corefile  | Bin 0 -> 24576 bytes
 5 files changed, 54 insertions(+)
 create mode 100644 lldb/test/API/linux/aarch64/gcs/corefile

diff --git 
a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp 
b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
index 2ddf8440aeb035..bd02bb0e69a4d3 100644
--- a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
+++ b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp
@@ -69,6 +69,15 @@ RegisterContextCorePOSIX_arm64::Create(Thread &thread, const 
ArchSpec &arch,
   if (fpmr_data.GetByteSize() >= sizeof(uint64_t))
 opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskFPMR);
 
+  DataExtractor gcs_data = getRegset(notes, arch.GetTriple(), 
AARCH64_GCS_Desc);
+  struct __attribute__((packed)) gcs_regs {
+uint64_t features_enabled;
+uint64_t features_locked;
+uint64_t gcspr_e0;
+  };
+  if (gcs_data.GetByteSize() >= sizeof(gcs_regs))
+opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskGCS);
+
   auto register_info_up =
   std::make_unique(arch, opt_regsets);
   return std::unique_ptr(
@@ -136,6 +145,9 @@ 
RegisterContextCorePOSIX_arm64::RegisterContextCorePOSIX_arm64(
   if (m_register_info_up->IsFPMRPresent())
 m_fpmr_data = getRegset(notes, target_triple, AARCH64_FPMR_Desc);
 
+  if (m_register_info_up->IsGCSPresent())
+m_gcs_data = getRegset(notes, target_triple, AARCH64_GCS_Desc);
+
   ConfigureRegisterContext();
 }
 
@@ -330,6 +342,11 @@ bool RegisterContextCorePOSIX_arm64::ReadRegister(const 
RegisterInfo *reg_info,
 assert(offset < m_mte_data.GetByteSize());
 value.SetFromMemoryData(*reg_info, m_mte_data.GetDataStart() + offset,
 reg_info->byte_size, lldb::eByteOrderLittle, 
error);
+  } else if (IsGCS(reg)) {
+offset = reg_info->byte_offset - m_register_info_up->GetGCSOffset();
+assert(offset < m_gcs_data.GetByteSize());
+value.SetFromMemoryData(*reg_info, m_gcs_data.GetDataStart() + offset,
+reg_info->byte_size, lldb::eByteOrderLittle, 
error);
   } else if (IsSME(reg)) {
 // If you had SME in the process, active or otherwise, there will at least
 // be a ZA header. No header, no SME at all.
diff --git 
a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h 
b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
index 35588c40c2eb1a..6140f805ffc78a 100644
--- a/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
+++ b/lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h
@@ -63,6 +63,7 @@ class RegisterContextCorePOSIX_arm64 : public 
RegisterContextPOSIX_arm64 {
   lldb_private::DataExtractor m_mte_data;
   lldb_private::DataExtractor m_zt_data;
   lldb_private::DataExtractor m_fpmr_data;
+  lldb_private::DataExtractor m_gcs_data;
 
   SVEState m_sve_state = SVEState::Unknown;
   uint16_t m_sve_vector_length = 0;
diff --git a/lldb/source/Plugins/Process/elf-core/RegisterUtilities.h 
b/lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
index b97279b0d735b8..59382a12cde0a2 100644
--- a/lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
+++ b/lldb/source/Plugins/Process/elf-core/RegisterUtilities.h
@@ -148,6 +148,10 @@ constexpr RegsetDesc AARCH64_FPMR_Desc[] = {
 {llvm::Triple::Linux, llvm::Triple::aarch64, llvm::ELF::NT_ARM_FPMR},
 };
 
+constexpr RegsetDesc AARCH64_GCS_Desc[] = {
+{llvm::Triple::Linux, llvm::Triple::aarch64, llvm::ELF::NT_ARM_GCS},
+};
+
 constexpr RegsetDesc PPC_VMX_Desc[] = {
 {llvm::Triple::FreeBSD, llvm::Triple::UnknownArch, llvm::ELF::NT_PPC_VMX},
 {llvm::Triple::Linux, llvm::Triple::UnknownArch, llvm::ELF::NT_PPC_VMX},
diff --git a/lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py 
b/lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py
index 797551b061a237..adbef69c5c38b0 100644
--- a/lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py
+++ b/lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py
@@ -374,3 +374,35 @@ def test_gcs_expression_enable_gcs(self):
 # consistent with the disabled -> enabled behaviour.
 enabl

[Lldb-commits] [lldb] [lldb] Include api-level.h header for Android (PR #124383)

[Pavel Labath via lldb-commits]

@@ -29,6 +29,10 @@
 #include 
 #include 
 
+#ifdef __ANDROID__
+#include 

labath wrote:

It sounds like it's mainly for "style", but given what you've said, maybe that 
shouldn't be the style we should use (is it possible those includes were 
necessary 10 years ago?) I think this code would look much nicer if we could 
assume that __ANDROID_API__ is always defined (at least that, like, we could 
assume that any header which includes the definition of a C function we're 
about to call, also includes the definition of that macro -- which I guess must 
be true as otherwise it wouldn't be able to annotate the declaration)

https://github.com/llvm/llvm-project/pull/124383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Remove support and workarounds for Android 4 and older (PR #124047)

[Pavel Labath via lldb-commits]

labath wrote:

> > > that's especially weird because execve() doesn't use $PATH ... it's the 
> > > members of the exec family that have a 'p' in the name that do.
> > 
> > 
> > It's been a long time, but I'm pretty sure this was due to a test which ran 
> > something with an empty environment (some of our tests do that, sometimes 
> > deliberately, mostly by accident). It probably wasn't the binary itself 
> > that failed to launch, but that it then couldn't find the executable that 
> > _it_ was trying to launch. The comment could be incorrect. Maybe it's 
> > (ba)sh who inserts `PATH=/bin:/usr/bin` into the environment if the 
> > variable is missing?
> 
> it is, but mksh (Android's shell) does that too.
> 
> > Bottom line: I'm pretty sure that the only reason this needs to be there is 
> > to make the test suite happy (and maybe not even that, if things have 
> > changed since that time). I don't think anyone is running the lldb test 
> > suite on android. If you're not planning to start doing that, I think we 
> > could delete it.
> 
> even if anyone is, it might be better to delete this hack and then someone 
> should come and hassle me if/when there's a test failure so we can try to 
> work out what (if anything) is Android-specific here. if nothing else, we'll 
> have a better code comment to add next time round :-)

I made a quick test which I think should replicate the thing scenario that this 
was working around (run `execlp("env")` with an empty environment, and make 
sure it's able to find the binary in /system/bin/env -- and it seems that it 
can, at least on the API 33 device I have around. Therefore, I would be in 
favor of deleting this as well. @brad0, would you like to do the honors?

> > As I recall, we were cherry-picking the fix into all supported kernel 
> > branches (and maybe even adding conformance tests -- I'm not sure), so that 
> > at least new devices (starting from a new kernel branch) should have it. 
> > But yes, it's possible that claim is too optimistic...
> 
> ah, yes, it's coming back to me now --- it was you who wrote most of the 
> excellent tests in 
> https://cs.android.com/android/platform/superproject/main/+/main:bionic/tests/sys_ptrace_test.cpp
>  wasn't it? i _think_ that the relevant tests weren't in CTS before api 28 
> though? certainly the copyright date for that whole file was 2016, so it 
> can't have been much earlier than that, and certainly wasn't api 21.

Yes, that was me. :) In particular, the `watchpoint_stress` is what was meant 
to catch this bug. (It's not guaranteed to do that, since the bug only triggers 
when the device goes to sleep, and that won't happen if it's connected to usb 
all the time)

It's quite possible that didn't run as a part of the CTS from the start. If you 
say it didn't, then I'll take your word for it.

> 
> (fyi, these tests found a linux kernel virtualization bug just recently: 
> https://lore.kernel.org/lkml/z5fvfe9rwvnr2...@google.com/ --- so thank you 
> again for those!)

Wow. Thank *you* for following up on that. I've been aware of issues with 
running the lldb test suite (the watchpoint tests in particular) in virtualized 
environments (this is the reason why the lldb linux bot runs on a physical 
machine and not in the cloud), but I never took the time to isolate and 
reproduce the bug. It's very cool to see this fixed.

https://github.com/llvm/llvm-project/pull/124047
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (PR #124279)

[Pavel Labath via lldb-commits]

https://github.com/labath edited 
https://github.com/llvm/llvm-project/pull/124279
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (PR #124279)

[Pavel Labath via lldb-commits]

@@ -1414,11 +1412,15 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE 
&die,
 
   LinkDeclContextToDIE(function_decl, die);
 
-  if (!function_param_decls.empty()) {
-m_ast.SetFunctionParameters(function_decl, function_param_decls);
+  const clang::FunctionProtoType *function_prototype(
+  llvm::cast(
+  ClangUtil::GetQualType(clang_type).getTypePtr()));
+  auto params = m_ast.CreateParameterDeclarations(function_decl,
+  *function_prototype);
+  if (!params.empty()) {

labath wrote:

Could we drop this check (call the function with an empty vector)?

https://github.com/llvm/llvm-project/pull/124279
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (PR #124279)

[Pavel Labath via lldb-commits]

https://github.com/labath commented:

Losing it would not be the end of the world, but the extra detail in the 
diagnostic seems nice, even if it was only working for free functions.  How 
hard would it be to plumb the names in?

https://github.com/llvm/llvm-project/pull/124279
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Include api-level.h header for Android (PR #124383)

[Pavel Labath via lldb-commits]

https://github.com/labath approved this pull request.

Correct. (The alternative is to make this a "proper" cmake build-time check)

https://github.com/llvm/llvm-project/pull/124383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (PR #124279)

[Michael Buch via lldb-commits]

Michael137 wrote:

> Losing it would not be the end of the world, but the extra detail in the 
> diagnostic seems nice, even if it was only working for free functions. How 
> hard would it be to plumb the names in?

Yea shouldn't be tooo bad. We could add a `llvm::SmallVector` (or 
similar) parameter to `ParseChildParameters` and populate it via `DW_AT_name`s. 
Then just pass it to `CreateParameterDeclarations`. Happy to do that too.

https://github.com/llvm/llvm-project/pull/124279
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Simplify preprocessor conditional (PR #124522)

[David Spickett via lldb-commits]

https://github.com/DavidSpickett approved this pull request.

I suppose there might be some platform out there we don't know about that this 
breaks, but what better way to find out than changing it :)

LGTM.

https://github.com/llvm/llvm-project/pull/124522
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Extended if conditions to support alias names for registers (PR #124475)

[David Spickett via lldb-commits]

DavidSpickett wrote:

> which doesn't send the alternate name information

I don't think we do either. Unless it's in the register info structs maybe it 
is.

The way to find out is to write the tests and if they pass without any changes 
to LLDB - somehow it does work.

> (in particular, there a test there to check that we know what the aarch64 w0 
> register is even though the xml contains only x0)

This is sort of an alias but we implement it by adding another register. But 
yes, same energy for sure.

https://github.com/llvm/llvm-project/pull/124475
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Extended if conditions to support alias names for registers (PR #124475)

[Sudharsan Veeravalli via lldb-commits]

svs-quic wrote:

> A test for this would be finding the tests that check basic reads of 
> registers, and adding reads with the new names. There should be one for core 
> files and one for live processes.

Basic support was added in 
https://github.com/llvm/llvm-project/commit/847de9c332775d1841fec9fea5cb5c41592a4c8f
 and I dont see any tests added there. We may have to write a new one for live 
process at the very least.


> I haven't checked, but it's possible that this already works for core files

Yeah this should work for core files. 

```
(lldb) target create 
"lldb/test/API/functionalities/postmortem/elf-core/linux-riscv64.gpr_only.out" 
--core 
"ldb/test/API/functionalities/postmortem/elf-core/linux-riscv64.gpr_only.core"

Core file 
'lldb/test/API/functionalities/postmortem/elf-core/linux-riscv64.gpr_only.core' 
(riscv64) was loaded.

(lldb) re re x8
  fp = 0x00fff4d5fcf0  

(lldb) re re x21
  s5 = 0x00fffccd8d28
```



https://github.com/llvm/llvm-project/pull/124475
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Simplify preprocessor conditional (PR #124522)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)


Changes

The long list of defines is just a very elaborate way to say "not windows".

---
Full diff: https://github.com/llvm/llvm-project/pull/124522.diff


1 Files Affected:

- (modified) lldb/source/Host/common/Host.cpp (+6-10) 


``diff
diff --git a/lldb/source/Host/common/Host.cpp b/lldb/source/Host/common/Host.cpp
index fdb623667bc251..d5054008268b9d 100644
--- a/lldb/source/Host/common/Host.cpp
+++ b/lldb/source/Host/common/Host.cpp
@@ -11,13 +11,19 @@
 #include 
 #include 
 #include 
+
 #ifndef _WIN32
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
+#if !defined(__ANDROID__)
+#include 
+#endif
 #endif
 
 #if defined(__APPLE__)
@@ -26,16 +32,6 @@
 #include 
 #endif
 
-#if defined(__linux__) || defined(__FreeBSD__) ||  
\
-defined(__FreeBSD_kernel__) || defined(__APPLE__) ||   
\
-defined(__NetBSD__) || defined(__OpenBSD__) || defined(__EMSCRIPTEN__)
-#if !defined(__ANDROID__)
-#include 
-#endif
-#include 
-#include 
-#endif
-
 #if defined(__FreeBSD__)
 #include 
 #endif

``




https://github.com/llvm/llvm-project/pull/124522
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Android 9 has added the spawn.h header (PR #124452)

[Pavel Labath via lldb-commits]

https://github.com/labath approved this pull request.

(The messy include above this has prompted me to create #124522. You'll need to 
rebase if I merge this before you do. :) )

https://github.com/llvm/llvm-project/pull/124452
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Clean up Socket headers for Android (PR #124453)

[Pavel Labath via lldb-commits]

https://github.com/labath approved this pull request.

Yeah, I have no idea what was that about.

https://github.com/llvm/llvm-project/pull/124453
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Simplify preprocessor conditional (PR #124522)

[Pavel Labath via lldb-commits]

https://github.com/labath created 
https://github.com/llvm/llvm-project/pull/124522

The long list of defines is just a very elaborate way to say "not windows".

>From 528535210c76c5700246069321be47a8cf3dc6e1 Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Mon, 27 Jan 2025 10:07:33 +0100
Subject: [PATCH] [lldb] Simplify preprocessor conditional

The long list of defines is just a very elaborate way to say "not
windows".
---
 lldb/source/Host/common/Host.cpp | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/lldb/source/Host/common/Host.cpp b/lldb/source/Host/common/Host.cpp
index fdb623667bc251..d5054008268b9d 100644
--- a/lldb/source/Host/common/Host.cpp
+++ b/lldb/source/Host/common/Host.cpp
@@ -11,13 +11,19 @@
 #include 
 #include 
 #include 
+
 #ifndef _WIN32
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
+#if !defined(__ANDROID__)
+#include 
+#endif
 #endif
 
 #if defined(__APPLE__)
@@ -26,16 +32,6 @@
 #include 
 #endif
 
-#if defined(__linux__) || defined(__FreeBSD__) ||  
\
-defined(__FreeBSD_kernel__) || defined(__APPLE__) ||   
\
-defined(__NetBSD__) || defined(__OpenBSD__) || defined(__EMSCRIPTEN__)
-#if !defined(__ANDROID__)
-#include 
-#endif
-#include 
-#include 
-#endif
-
 #if defined(__FreeBSD__)
 #include 
 #endif

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

[Lldb-commits] [lldb] [lldb] Clean up Socket headers for Android (PR #124453)

[Pavel Labath via lldb-commits]

labath wrote:

> Some of that looks questionable and I question if most of the remaining 
> headers are actually necessary. I'd like to trim this down to only what is 
> necessary.

This is probably the consequence of some refactoring (right now, most of the 
actual work is done in the individual `***Socket.cpp` files, but maybe 
everything was in this file in the past. Feel free to remove anything that 
doesn't make sense.

https://github.com/llvm/llvm-project/pull/124453
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Extended if conditions to support alias names for registers (PR #124475)

[David Spickett via lldb-commits]

DavidSpickett wrote:

A test for this would be finding the tests that check basic reads of registers, 
and adding reads with the new names. There should be one for core files and one 
for live processes.

https://github.com/llvm/llvm-project/pull/124475
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Extended if conditions to support alias names for registers (PR #124475)

[Pavel Labath via lldb-commits]

labath wrote:

I haven't checked, but it's possible that this already works for core files. 
The original bug was specifically about talking to qemu, which doesn't send the 
alternate name information (and maybe not information about subregisters as 
well). If that's the case then a more suitable test to emulate would be 
test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py (in 
particular, there a test there to check that we know what the aarch64 `w0` 
register is even though the xml contains only `x0`)

https://github.com/llvm/llvm-project/pull/124475
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (PR #124279)

[Michael Buch via lldb-commits]

Michael137 wrote:

> > Losing it would not be the end of the world, but the extra detail in the 
> > diagnostic seems nice, even if it was only working for free functions. How 
> > hard would it be to plumb the names in?
> 
> Yea shouldn't be tooo bad. We could add a `llvm::SmallVector` 
> (or similar) parameter to `ParseChildParameters` and populate it via 
> `DW_AT_name`s. Then just pass it to `CreateParameterDeclarations`. Happy to 
> do that too.

Also tempted to just create a `struct FunctionInfo` that holds all of the 
output parameters we currently pass to `ParseChildParameters`. And return it by 
value. But that could be done separately if we wanted to

https://github.com/llvm/llvm-project/pull/124279
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)

[Ilia Kuklin via lldb-commits]

https://github.com/kuilpd updated 
https://github.com/llvm/llvm-project/pull/115005

>From 4d797371598960baf7729d05590aa1a8c7077694 Mon Sep 17 00:00:00 2001
From: Ilia Kuklin 
Date: Mon, 4 Nov 2024 14:33:45 +0500
Subject: [PATCH 1/6] [lldb] Analyze enum promotion type during parsing

---
 clang/include/clang/AST/Decl.h|  2 +-
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  | 95 ++-
 .../TypeSystem/Clang/TypeSystemClang.cpp  | 26 +
 3 files changed, 98 insertions(+), 25 deletions(-)

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 16403774e72b31..41cb47516f5803 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -3903,6 +3903,7 @@ class EnumDecl : public TagDecl {
   void setInstantiationOfMemberEnum(ASTContext &C, EnumDecl *ED,
 TemplateSpecializationKind TSK);
 
+public:
   /// Sets the width in bits required to store all the
   /// non-negative enumerators of this enum.
   void setNumPositiveBits(unsigned Num) {
@@ -3914,7 +3915,6 @@ class EnumDecl : public TagDecl {
   /// negative enumerators of this enum. (see getNumNegativeBits)
   void setNumNegativeBits(unsigned Num) { EnumDeclBits.NumNegativeBits = Num; }
 
-public:
   /// True if this tag declaration is a scoped enumeration. Only
   /// possible in C++11 mode.
   void setScoped(bool Scoped = true) { EnumDeclBits.IsScoped = Scoped; }
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index e77188bfbd2e4a..bb9c35a235c1ff 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2316,6 +2316,8 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
 return 0;
 
   size_t enumerators_added = 0;
+  unsigned NumNegativeBits = 0;
+  unsigned NumPositiveBits = 0;
 
   for (DWARFDIE die : parent_die.children()) {
 const dw_tag_t tag = die.Tag();
@@ -2367,11 +2369,102 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
 }
 
 if (name && name[0] && got_value) {
-  m_ast.AddEnumerationValueToEnumerationType(
+  auto ECD = m_ast.AddEnumerationValueToEnumerationType(
   clang_type, decl, name, enum_value, enumerator_byte_size * 8);
   ++enumerators_added;
+
+  llvm::APSInt InitVal = ECD->getInitVal();
+  // Keep track of the size of positive and negative values.
+  if (InitVal.isUnsigned() || InitVal.isNonNegative()) {
+// If the enumerator is zero that should still be counted as a positive
+// bit since we need a bit to store the value zero.
+unsigned ActiveBits = InitVal.getActiveBits();
+NumPositiveBits = std::max({NumPositiveBits, ActiveBits, 1u});
+  } else {
+NumNegativeBits =
+std::max(NumNegativeBits, (unsigned)InitVal.getSignificantBits());
+  }
 }
   }
+
+  /// The following code follows the same logic as in Sema::ActOnEnumBody
+  /// clang/lib/Sema/SemaDecl.cpp
+  // If we have an empty set of enumerators we still need one bit.
+  // From [dcl.enum]p8
+  // If the enumerator-list is empty, the values of the enumeration are as if
+  // the enumeration had a single enumerator with value 0
+  if (!NumPositiveBits && !NumNegativeBits)
+NumPositiveBits = 1;
+
+  clang::QualType qual_type(ClangUtil::GetQualType(clang_type));
+  clang::EnumDecl *enum_decl = qual_type->getAs()->getDecl();
+  enum_decl->setNumPositiveBits(NumPositiveBits);
+  enum_decl->setNumNegativeBits(NumNegativeBits);
+
+  // C++0x N3000 [conv.prom]p3:
+  //   An rvalue of an unscoped enumeration type whose underlying
+  //   type is not fixed can be converted to an rvalue of the first
+  //   of the following types that can represent all the values of
+  //   the enumeration: int, unsigned int, long int, unsigned long
+  //   int, long long int, or unsigned long long int.
+  // C99 6.4.4.3p2:
+  //   An identifier declared as an enumeration constant has type int.
+  // The C99 rule is modified by C23.
+  clang::QualType BestPromotionType;
+  unsigned BestWidth;
+
+  auto &Context = m_ast.getASTContext();
+  unsigned LongWidth = Context.getTargetInfo().getLongWidth();
+  unsigned IntWidth = Context.getTargetInfo().getIntWidth();
+  unsigned CharWidth = Context.getTargetInfo().getCharWidth();
+  unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
+
+  bool is_cpp = Language::LanguageIsCPlusPlus(
+  SymbolFileDWARF::GetLanguage(*parent_die.GetCU()));
+
+  if (NumNegativeBits) {
+// If there is a negative value, figure out the smallest integer type (of
+// int/long/longlong) that fits.
+if (NumNegativeBits <= CharWidth && NumPositiveBits < CharWidth) {
+  BestWidth = CharWidth;
+} else if (NumNegativeBits <= ShortWidth && NumPositiveBits < ShortWidth) {
+  BestWidth = ShortWidth;
+} else if (NumNegativeBits <= IntWidth && 

[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)

[Ilia Kuklin via lldb-commits]

@@ -2367,11 +2369,38 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
 }
 
 if (name && name[0] && got_value) {
-  m_ast.AddEnumerationValueToEnumerationType(
+  auto ECD = m_ast.AddEnumerationValueToEnumerationType(
   clang_type, decl, name, enum_value, enumerator_byte_size * 8);
   ++enumerators_added;
+
+  llvm::APSInt InitVal = ECD->getInitVal();
+  // Keep track of the size of positive and negative values.
+  if (InitVal.isUnsigned() || InitVal.isNonNegative()) {
+// If the enumerator is zero that should still be counted as a positive
+// bit since we need a bit to store the value zero.
+unsigned ActiveBits = InitVal.getActiveBits();
+NumPositiveBits = std::max({NumPositiveBits, ActiveBits, 1u});

kuilpd wrote:

The condition checks that the value is just non negative unsigned, so I think 
the comment means that it could be zero, in which case the max function would 
prefer `1u` over it. 

https://github.com/llvm/llvm-project/pull/115005
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)

[Ilia Kuklin via lldb-commits]

https://github.com/kuilpd edited 
https://github.com/llvm/llvm-project/pull/115005
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)

[Ilia Kuklin via lldb-commits]

https://github.com/kuilpd updated 
https://github.com/llvm/llvm-project/pull/115005

>From 4d797371598960baf7729d05590aa1a8c7077694 Mon Sep 17 00:00:00 2001
From: Ilia Kuklin 
Date: Mon, 4 Nov 2024 14:33:45 +0500
Subject: [PATCH 1/7] [lldb] Analyze enum promotion type during parsing

---
 clang/include/clang/AST/Decl.h|  2 +-
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  | 95 ++-
 .../TypeSystem/Clang/TypeSystemClang.cpp  | 26 +
 3 files changed, 98 insertions(+), 25 deletions(-)

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 16403774e72b31..41cb47516f5803 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -3903,6 +3903,7 @@ class EnumDecl : public TagDecl {
   void setInstantiationOfMemberEnum(ASTContext &C, EnumDecl *ED,
 TemplateSpecializationKind TSK);
 
+public:
   /// Sets the width in bits required to store all the
   /// non-negative enumerators of this enum.
   void setNumPositiveBits(unsigned Num) {
@@ -3914,7 +3915,6 @@ class EnumDecl : public TagDecl {
   /// negative enumerators of this enum. (see getNumNegativeBits)
   void setNumNegativeBits(unsigned Num) { EnumDeclBits.NumNegativeBits = Num; }
 
-public:
   /// True if this tag declaration is a scoped enumeration. Only
   /// possible in C++11 mode.
   void setScoped(bool Scoped = true) { EnumDeclBits.IsScoped = Scoped; }
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index e77188bfbd2e4a..bb9c35a235c1ff 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2316,6 +2316,8 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
 return 0;
 
   size_t enumerators_added = 0;
+  unsigned NumNegativeBits = 0;
+  unsigned NumPositiveBits = 0;
 
   for (DWARFDIE die : parent_die.children()) {
 const dw_tag_t tag = die.Tag();
@@ -2367,11 +2369,102 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
 }
 
 if (name && name[0] && got_value) {
-  m_ast.AddEnumerationValueToEnumerationType(
+  auto ECD = m_ast.AddEnumerationValueToEnumerationType(
   clang_type, decl, name, enum_value, enumerator_byte_size * 8);
   ++enumerators_added;
+
+  llvm::APSInt InitVal = ECD->getInitVal();
+  // Keep track of the size of positive and negative values.
+  if (InitVal.isUnsigned() || InitVal.isNonNegative()) {
+// If the enumerator is zero that should still be counted as a positive
+// bit since we need a bit to store the value zero.
+unsigned ActiveBits = InitVal.getActiveBits();
+NumPositiveBits = std::max({NumPositiveBits, ActiveBits, 1u});
+  } else {
+NumNegativeBits =
+std::max(NumNegativeBits, (unsigned)InitVal.getSignificantBits());
+  }
 }
   }
+
+  /// The following code follows the same logic as in Sema::ActOnEnumBody
+  /// clang/lib/Sema/SemaDecl.cpp
+  // If we have an empty set of enumerators we still need one bit.
+  // From [dcl.enum]p8
+  // If the enumerator-list is empty, the values of the enumeration are as if
+  // the enumeration had a single enumerator with value 0
+  if (!NumPositiveBits && !NumNegativeBits)
+NumPositiveBits = 1;
+
+  clang::QualType qual_type(ClangUtil::GetQualType(clang_type));
+  clang::EnumDecl *enum_decl = qual_type->getAs()->getDecl();
+  enum_decl->setNumPositiveBits(NumPositiveBits);
+  enum_decl->setNumNegativeBits(NumNegativeBits);
+
+  // C++0x N3000 [conv.prom]p3:
+  //   An rvalue of an unscoped enumeration type whose underlying
+  //   type is not fixed can be converted to an rvalue of the first
+  //   of the following types that can represent all the values of
+  //   the enumeration: int, unsigned int, long int, unsigned long
+  //   int, long long int, or unsigned long long int.
+  // C99 6.4.4.3p2:
+  //   An identifier declared as an enumeration constant has type int.
+  // The C99 rule is modified by C23.
+  clang::QualType BestPromotionType;
+  unsigned BestWidth;
+
+  auto &Context = m_ast.getASTContext();
+  unsigned LongWidth = Context.getTargetInfo().getLongWidth();
+  unsigned IntWidth = Context.getTargetInfo().getIntWidth();
+  unsigned CharWidth = Context.getTargetInfo().getCharWidth();
+  unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
+
+  bool is_cpp = Language::LanguageIsCPlusPlus(
+  SymbolFileDWARF::GetLanguage(*parent_die.GetCU()));
+
+  if (NumNegativeBits) {
+// If there is a negative value, figure out the smallest integer type (of
+// int/long/longlong) that fits.
+if (NumNegativeBits <= CharWidth && NumPositiveBits < CharWidth) {
+  BestWidth = CharWidth;
+} else if (NumNegativeBits <= ShortWidth && NumPositiveBits < ShortWidth) {
+  BestWidth = ShortWidth;
+} else if (NumNegativeBits <= IntWidth && 

[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)

[Ilia Kuklin via lldb-commits]

@@ -2367,11 +2369,38 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
 }
 
 if (name && name[0] && got_value) {
-  m_ast.AddEnumerationValueToEnumerationType(
+  auto ECD = m_ast.AddEnumerationValueToEnumerationType(
   clang_type, decl, name, enum_value, enumerator_byte_size * 8);
   ++enumerators_added;
+
+  llvm::APSInt InitVal = ECD->getInitVal();
+  // Keep track of the size of positive and negative values.
+  if (InitVal.isUnsigned() || InitVal.isNonNegative()) {
+// If the enumerator is zero that should still be counted as a positive
+// bit since we need a bit to store the value zero.
+unsigned ActiveBits = InitVal.getActiveBits();
+NumPositiveBits = std::max({NumPositiveBits, ActiveBits, 1u});
+  } else {
+NumNegativeBits =
+std::max(NumNegativeBits, (unsigned)InitVal.getSignificantBits());

kuilpd wrote:

This one is for when the value is just signed negative, can be anything, 
accumulates over all enum values.
This if/else is from 
[SemaDecl.cpp:L20084](https://github.com/llvm/llvm-project/blob/5d6d982df61d16b6d498e6d59dd91c059679d3d8/clang/lib/Sema/SemaDecl.cpp#L20084),
 but I don't think this few lines are worth putting into a separate function as 
well.

https://github.com/llvm/llvm-project/pull/115005
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [clang-tools-extra] [lldb] [Clang] [NFC] Rename `isAnyPointerType()` and `getPointeeOrArrayElementType()`. (PR #122938)

[Aaron Ballman via lldb-commits]

AaronBallman wrote:

Thank you for working on cleaning up these APIs! They are definitely a source 
of confusion in practice, so paying down this tech debt is great!

> The names for the first two are rather verbose; I’m open to suggestions as to 
> better names for these functions since that was the best that I managed to 
> come up with...

Given the fact that there tend to be custom predicates anyway, what about an 
API where you pass in the kinds of pointers you're after:
```
enum PointerLikeKind {
  C = 1U << 0U,
  ObjC = 1U << 1U,
  Block = 1U << 2U,
  Member = 1U << 3U,
  Reference = 1U << 4U,
  ...
};

bool isPointerLike(unsigned PLK) const;
Type *getPointeeFromPointerLike(unsigned PLK);
const Type *getPointeeFromPointerLike(unsigned PLK) const;
```
and then people can migrate to that (though we probably want to use the fancy 
enum class bitmask functionality rather than this style above, but we need to 
pick something that's not onerous to use at call sites). This would replace 
`isAnyPointerType()` and friends, so those APIs would go away.

As for the array element API, that one is really odd because it does things 
like strip qualifiers and doesn't handle multidimensional arrays gracefully. 
There are 17 uses of it, so I wonder if it makes sense to remove that API 
entirely and just handle the array case explicitly.

Would that be something you would be willing to explore to see if it leads to a 
cleaner end-state?

https://github.com/llvm/llvm-project/pull/122938
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)

[Michael Buch via lldb-commits]

@@ -2367,11 +2369,38 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
 }
 
 if (name && name[0] && got_value) {
-  m_ast.AddEnumerationValueToEnumerationType(
+  auto ECD = m_ast.AddEnumerationValueToEnumerationType(
   clang_type, decl, name, enum_value, enumerator_byte_size * 8);
   ++enumerators_added;
+
+  llvm::APSInt InitVal = ECD->getInitVal();
+  // Keep track of the size of positive and negative values.
+  if (InitVal.isUnsigned() || InitVal.isNonNegative()) {
+// If the enumerator is zero that should still be counted as a positive
+// bit since we need a bit to store the value zero.
+unsigned ActiveBits = InitVal.getActiveBits();
+NumPositiveBits = std::max({NumPositiveBits, ActiveBits, 1u});
+  } else {
+NumNegativeBits =
+std::max(NumNegativeBits, (unsigned)InitVal.getSignificantBits());
+  }
 }
   }
+
+  if (!NumPositiveBits && !NumNegativeBits)
+NumPositiveBits = 1;
+
+  clang::EnumDecl *enum_decl =
+  ClangUtil::GetQualType(clang_type)->getAs()->getDecl();
+  enum_decl->setNumPositiveBits(NumPositiveBits);
+  enum_decl->setNumNegativeBits(NumNegativeBits);
+
+  clang::QualType BestPromotionType;
+  clang::QualType BestType;
+  m_ast.getASTContext().computeBestEnumTypes(
+  false, NumNegativeBits, NumPositiveBits, BestType, BestPromotionType);

Michael137 wrote:

```suggestion
  /*IsPacked=*/false, NumNegativeBits, NumPositiveBits, BestType, 
BestPromotionType);
```

https://github.com/llvm/llvm-project/pull/115005
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)

[Michael Buch via lldb-commits]

@@ -2367,11 +2369,38 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
 }
 
 if (name && name[0] && got_value) {
-  m_ast.AddEnumerationValueToEnumerationType(
+  auto ECD = m_ast.AddEnumerationValueToEnumerationType(
   clang_type, decl, name, enum_value, enumerator_byte_size * 8);
   ++enumerators_added;
+
+  llvm::APSInt InitVal = ECD->getInitVal();
+  // Keep track of the size of positive and negative values.
+  if (InitVal.isUnsigned() || InitVal.isNonNegative()) {
+// If the enumerator is zero that should still be counted as a positive
+// bit since we need a bit to store the value zero.
+unsigned ActiveBits = InitVal.getActiveBits();
+NumPositiveBits = std::max({NumPositiveBits, ActiveBits, 1u});
+  } else {
+NumNegativeBits =
+std::max(NumNegativeBits, (unsigned)InitVal.getSignificantBits());

Michael137 wrote:

Isn't `NumNegativeBits == 0` at this point anyway? Can we just unconditionally 
assign `getSignificantBits`?

https://github.com/llvm/llvm-project/pull/115005
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)

[Michael Buch via lldb-commits]

@@ -3903,6 +3903,7 @@ class EnumDecl : public TagDecl {
   void setInstantiationOfMemberEnum(ASTContext &C, EnumDecl *ED,
 TemplateSpecializationKind TSK);
 
+public:

Michael137 wrote:

Don't need this change anymore

https://github.com/llvm/llvm-project/pull/115005
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)

[Michael Buch via lldb-commits]

@@ -2367,11 +2369,38 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
 }
 
 if (name && name[0] && got_value) {
-  m_ast.AddEnumerationValueToEnumerationType(
+  auto ECD = m_ast.AddEnumerationValueToEnumerationType(
   clang_type, decl, name, enum_value, enumerator_byte_size * 8);
   ++enumerators_added;
+
+  llvm::APSInt InitVal = ECD->getInitVal();
+  // Keep track of the size of positive and negative values.
+  if (InitVal.isUnsigned() || InitVal.isNonNegative()) {
+// If the enumerator is zero that should still be counted as a positive
+// bit since we need a bit to store the value zero.
+unsigned ActiveBits = InitVal.getActiveBits();
+NumPositiveBits = std::max({NumPositiveBits, ActiveBits, 1u});
+  } else {
+NumNegativeBits =
+std::max(NumNegativeBits, (unsigned)InitVal.getSignificantBits());

Michael137 wrote:

Also, no need to cast to `unsigned` here I think

https://github.com/llvm/llvm-project/pull/115005
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)

[Michael Buch via lldb-commits]

https://github.com/Michael137 edited 
https://github.com/llvm/llvm-project/pull/115005
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)

[Michael Buch via lldb-commits]

https://github.com/Michael137 commented:

Looks much better

Left some more clarification comments

https://github.com/llvm/llvm-project/pull/115005
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)

[Michael Buch via lldb-commits]

@@ -2367,11 +2369,38 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
 }
 
 if (name && name[0] && got_value) {
-  m_ast.AddEnumerationValueToEnumerationType(
+  auto ECD = m_ast.AddEnumerationValueToEnumerationType(
   clang_type, decl, name, enum_value, enumerator_byte_size * 8);
   ++enumerators_added;
+
+  llvm::APSInt InitVal = ECD->getInitVal();
+  // Keep track of the size of positive and negative values.
+  if (InitVal.isUnsigned() || InitVal.isNonNegative()) {
+// If the enumerator is zero that should still be counted as a positive
+// bit since we need a bit to store the value zero.
+unsigned ActiveBits = InitVal.getActiveBits();
+NumPositiveBits = std::max({NumPositiveBits, ActiveBits, 1u});
+  } else {
+NumNegativeBits =
+std::max(NumNegativeBits, (unsigned)InitVal.getSignificantBits());
+  }
 }
   }
+
+  if (!NumPositiveBits && !NumNegativeBits)
+NumPositiveBits = 1;

Michael137 wrote:

Could you leave a comment on why we do this?

https://github.com/llvm/llvm-project/pull/115005
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [NFC][DebugInfo] Make some block-start-position methods return iterators (PR #124287)

[Orlando Cazalet-Hyams via lldb-commits]

https://github.com/OCHyams approved this pull request.

Thanks, still LGTM

https://github.com/llvm/llvm-project/pull/124287
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)

[Ilia Kuklin via lldb-commits]

kuilpd wrote:

@Michael137 
Reused the code I merged in #120965 to get the best promotion type.

https://github.com/llvm/llvm-project/pull/115005
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [NFC][DebugInfo] Make some block-start-position methods return iterators (PR #124287)

[Jeremy Morse via lldb-commits]

https://github.com/jmorse updated 
https://github.com/llvm/llvm-project/pull/124287

>From 3d2aa2734d6cb49c43565e3ac8584ba8130fe302 Mon Sep 17 00:00:00 2001
From: Jeremy Morse 
Date: Thu, 23 Jan 2025 12:24:14 +
Subject: [PATCH 1/2] [NFC][DebugInfo] Make some block-start-position methods
 return iterators

As part of the "RemoveDIs" work to eliminate debug intrinsics, we're
replacing methods that use Instruction*'s as positions with iterators. A
number of these (such as getFirstNonPHIOrDbg) are sufficiently infrequently
used that we can just replace the pointer-returning version with an
iterator-returning version, hopefully without much/any disruption.

Thus this patch has getFirstNonPHIOrDbg and getFirstNonPHIOrDbgOrLifetime
return an iterator, and updates all call-sites. There are no concerns about
the iterators returned being converted to Instruction*'s and losing the
debug-info bit: because the methods skip debug intrinsics, the iterator
head bit is always false anyway.
---
 .../ExpressionParser/Clang/IRForTarget.cpp|  6 ++--
 llvm/include/llvm/IR/BasicBlock.h | 18 ++--
 llvm/lib/CodeGen/CodeGenPrepare.cpp   |  2 +-
 llvm/lib/IR/BasicBlock.cpp| 28 +--
 .../AArch64/AArch64TargetTransformInfo.cpp|  2 +-
 .../Target/AMDGPU/SIAnnotateControlFlow.cpp   |  4 +--
 llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp  |  2 +-
 llvm/lib/Transforms/Coroutines/CoroFrame.cpp  |  5 ++--
 llvm/lib/Transforms/Coroutines/CoroSplit.cpp  |  2 +-
 llvm/lib/Transforms/IPO/IROutliner.cpp|  2 +-
 llvm/lib/Transforms/IPO/SCCP.cpp  |  4 +--
 llvm/lib/Transforms/IPO/SampleProfile.cpp |  2 +-
 .../InstCombineLoadStoreAlloca.cpp|  4 +--
 .../Transforms/Scalar/CallSiteSplitting.cpp   |  2 +-
 .../Transforms/Scalar/SimpleLoopUnswitch.cpp  |  2 +-
 llvm/lib/Transforms/Utils/CodeMoverUtils.cpp  |  2 +-
 llvm/lib/Transforms/Utils/IRNormalizer.cpp|  2 +-
 llvm/lib/Transforms/Utils/SimplifyCFG.cpp |  4 +--
 .../Frontend/OpenMPIRBuilderTest.cpp  | 24 ++--
 19 files changed, 67 insertions(+), 50 deletions(-)

diff --git a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
index 6c728f34474898..a414ad652448e9 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -66,7 +66,7 @@ static llvm::Value *FindEntryInstruction(llvm::Function 
*function) {
   if (function->empty())
 return nullptr;
 
-  return function->getEntryBlock().getFirstNonPHIOrDbg();
+  return &*function->getEntryBlock().getFirstNonPHIOrDbg();
 }
 
 IRForTarget::IRForTarget(lldb_private::ClangExpressionDeclMap *decl_map,
@@ -361,7 +361,7 @@ bool IRForTarget::CreateResultVariable(llvm::Function 
&llvm_function) {
 // there's nothing to put into its equivalent persistent variable.
 
 BasicBlock &entry_block(llvm_function.getEntryBlock());
-Instruction *first_entry_instruction(entry_block.getFirstNonPHIOrDbg());
+Instruction *first_entry_instruction(&*entry_block.getFirstNonPHIOrDbg());
 
 if (!first_entry_instruction)
   return false;
@@ -1505,7 +1505,7 @@ bool IRForTarget::ReplaceVariables(Function 
&llvm_function) {
   LLDB_LOG(log, "Arg: \"{0}\"", PrintValue(argument));
 
   BasicBlock &entry_block(llvm_function.getEntryBlock());
-  Instruction *FirstEntryInstruction(entry_block.getFirstNonPHIOrDbg());
+  Instruction *FirstEntryInstruction(&*entry_block.getFirstNonPHIOrDbg());
 
   if (!FirstEntryInstruction) {
 m_error_stream.Printf("Internal error [IRForTarget]: Couldn't find the "
diff --git a/llvm/include/llvm/IR/BasicBlock.h 
b/llvm/include/llvm/IR/BasicBlock.h
index e22fe1e7e7dc8f..5df4dcecebc878 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -299,22 +299,20 @@ class BasicBlock final : public Value, // Basic blocks 
are data objects also
   /// Returns a pointer to the first instruction in this block that is not a
   /// PHINode or a debug intrinsic, or any pseudo operation if \c SkipPseudoOp
   /// is true.
-  const Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) const;
-  Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) {
-return const_cast(
-static_cast(this)->getFirstNonPHIOrDbg(
-SkipPseudoOp));
+  InstListType::const_iterator getFirstNonPHIOrDbg(bool SkipPseudoOp = true) 
const;
+  InstListType::iterator getFirstNonPHIOrDbg(bool SkipPseudoOp = true) {
+return static_cast(this)->getFirstNonPHIOrDbg(
+SkipPseudoOp).getNonConst();
   }
 
   /// Returns a pointer to the first instruction in this block that is not a
   /// PHINode, a debug intrinsic, or a lifetime intrinsic, or any pseudo
   /// operation if \c SkipPseudoOp is true.
-  const Instruction *
+  InstListType::const_iterator
   getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp = true) const;

[Lldb-commits] [lldb] [llvm] [NFC][DebugInfo] Make some block-start-position methods return iterators (PR #124287)

[Jeremy Morse via lldb-commits]

https://github.com/jmorse edited 
https://github.com/llvm/llvm-project/pull/124287
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [NFC][DebugInfo] Make some block-start-position methods return iterators (PR #124287)

[Jeremy Morse via lldb-commits]

@@ -1656,7 +1656,8 @@ static Value 
*emitSetAndGetSwiftErrorValueAround(Instruction *Call,
 Builder.SetInsertPoint(Call->getNextNode());
   } else {
 auto Invoke = cast(Call);
-Builder.SetInsertPoint(Invoke->getNormalDest()->getFirstNonPHIOrDbg());
+BasicBlock::iterator It = Invoke->getNormalDest()->getFirstNonPHIOrDbg();
+Builder.SetInsertPoint(It);

jmorse wrote:

Looks like it isn't, removing

https://github.com/llvm/llvm-project/pull/124287
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [NFC][DebugInfo] Make some block-start-position methods return iterators (PR #124287)

[Jeremy Morse via lldb-commits]

https://github.com/jmorse commented:

Adjusted two call sites, see comments on the others. I've also added an extra 
unwrap to CoroSplit.cpp due to it not compiling -- this is going to be reworked 
in #124288 anyway.

https://github.com/llvm/llvm-project/pull/124287
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [NFC][DebugInfo] Make some block-start-position methods return iterators (PR #124287)

[Jeremy Morse via lldb-commits]

@@ -448,6 +455,9 @@ BasicBlock::const_iterator 
BasicBlock::getFirstNonPHIOrDbgOrAlloca() const {
   ++InsertPt;
 }
   }
+
+  // Signal that this comes after any debug records.
+  InsertPt.setHeadBit(false);

jmorse wrote:

The other two call-sites always generate iterators with a false head bit 
anyway, but there's a path from `getFirstNonPHIIt` to this line without 
InsertPt being assigned a different iterator. `getFirstNonPHIIt` will set the 
head bit, therefore we have to explicitly clear it here to ensure the position 
is "after" any debug records.

https://github.com/llvm/llvm-project/pull/124287
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)

[Michael Buch via lldb-commits]

@@ -2367,11 +2369,38 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
 }
 
 if (name && name[0] && got_value) {
-  m_ast.AddEnumerationValueToEnumerationType(
+  auto ECD = m_ast.AddEnumerationValueToEnumerationType(
   clang_type, decl, name, enum_value, enumerator_byte_size * 8);
   ++enumerators_added;
+
+  llvm::APSInt InitVal = ECD->getInitVal();
+  // Keep track of the size of positive and negative values.
+  if (InitVal.isUnsigned() || InitVal.isNonNegative()) {
+// If the enumerator is zero that should still be counted as a positive
+// bit since we need a bit to store the value zero.
+unsigned ActiveBits = InitVal.getActiveBits();
+NumPositiveBits = std::max({NumPositiveBits, ActiveBits, 1u});

Michael137 wrote:

```suggestion
NumPositiveBits = std::max(InitVal.getActiveBits(), 1u);
```

`NumPositiveBits` is 0 at this point anyway right?

https://github.com/llvm/llvm-project/pull/115005
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][Formatters] Do not recursively dereference pointer type when creating formatter candicates list. (PR #124048)

[via lldb-commits]

jimingham wrote:


> On Jan 27, 2025, at 5:30 AM, Pavel Labath ***@***.***> wrote:
> 
> 
> If I was holding a vector as a vector**, then I would would to see its 
> summary/size as well. And maybe even the children too. But the thing I 
> questioning is how often does one hold a C++ container as a double pointer. 
> Is there like some common programming pattern that produces those? The only 
> time I've ever done that is when I was experimenting with how "frame 
> variable" handles pointers and references. I have seen and worked with double 
> pointers, but this was always in some kind of legacy/C context. A combination 
> of double pointers and a std::vector (which essentially makes it a triple 
> pointer) would be very.. eclectic here. Modern C++ would likely turn at least 
> one of those pointers into an iterator, or a smart pointer, or a reference..
> 
If this were super difficult, I'd be uncomfortable about telling other people 
that their usages were too esoteric to support but willing to do so as a 
tradeoff.  But I don't think it's a good idea to do that unless we have to.  
After all, you're basing the argument on common uses of vectors, but really it 
should be about "the usage of any data type someone might have pointers to, and 
want to see the summary of."  We're providing the tools, not the uses.
> It's true that a summary provider is less intrusive (it doesn't override 
> existing children -- though it can hide them!), than a synthetic child 
> provider, but I think it would be very confusing to have different behavior 
> here. The thing I like about doing this for one level is that (as Zequan 
> points out) it nicely matches what our SBValue API does, which makes most 
> formatters "just work".
> 
I still don't see the force of this argument, it's just arguing by analogy.  
But given one can write a regex matcher to implement more general formatters, 
so long as the limitations of the normal formatter matching is clearly spelled 
out, I'll go along with whatever the consensus is here.

Jim

> —
> Reply to this email directly, view it on GitHub 
> , 
> or unsubscribe 
> .
> You are receiving this because you were mentioned.
> 



https://github.com/llvm/llvm-project/pull/124048
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (PR #124279)

[LLVM Continuous Integration via lldb-commits]

llvm-ci wrote:

LLVM Buildbot has detected a new failure on builder `llvm-x86_64-debian-dylib` 
running on `gribozavr4` while building `lldb` at step 5 "build-unified-tree".

Full details are available at: 
https://lab.llvm.org/buildbot/#/builders/60/builds/18062


Here is the relevant piece of the build log for the reference

```
Step 5 (build-unified-tree) failure: build (failure)
...
97.780 [117/22/7019] Building CXX object 
tools/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/CMakeFiles/lldbPluginDynamicLoaderMacOSXDYLD.dir/DynamicLoaderDarwin.cpp.o
98.338 [117/21/7020] Building CXX object 
tools/lldb/source/Plugins/SymbolFile/CTF/CMakeFiles/lldbPluginSymbolFileCTF.dir/SymbolFileCTF.cpp.o
98.741 [117/20/7021] Building CXX object 
tools/lldb/source/Plugins/ExpressionParser/Clang/CMakeFiles/lldbPluginExpressionParserClang.dir/ClangUtilityFunction.cpp.o
99.196 [117/19/7022] Building CXX object 
tools/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/CMakeFiles/lldbPluginAppleObjCRuntime.dir/AppleObjCRuntimeV2.cpp.o
99.444 [117/18/7023] Building CXX object 
tools/lldb/source/Plugins/ExpressionParser/Clang/CMakeFiles/lldbPluginExpressionParserClang.dir/ASTResultSynthesizer.cpp.o
99.973 [117/17/7024] Building CXX object 
tools/lldb/source/Plugins/ExpressionParser/Clang/CMakeFiles/lldbPluginExpressionParserClang.dir/CxxModuleHandler.cpp.o
100.421 [117/16/7025] Building CXX object 
tools/lldb/source/Plugins/ExpressionParser/Clang/CMakeFiles/lldbPluginExpressionParserClang.dir/ClangUserExpression.cpp.o
100.930 [117/15/7026] Building CXX object 
tools/lldb/source/Plugins/ExpressionParser/Clang/CMakeFiles/lldbPluginExpressionParserClang.dir/ClangASTSource.cpp.o
101.305 [117/14/7027] Building CXX object 
tools/lldb/source/Plugins/ExpressionParser/Clang/CMakeFiles/lldbPluginExpressionParserClang.dir/ClangFunctionCaller.cpp.o
103.634 [117/13/7028] Building CXX object 
tools/lldb/source/Plugins/TypeSystem/Clang/CMakeFiles/lldbPluginTypeSystemClang.dir/TypeSystemClang.cpp.o
FAILED: 
tools/lldb/source/Plugins/TypeSystem/Clang/CMakeFiles/lldbPluginTypeSystemClang.dir/TypeSystemClang.cpp.o
 
CCACHE_CPP2=yes CCACHE_HASHDIR=yes /usr/bin/ccache /usr/bin/clang++ 
-DGTEST_HAS_RTTI=0 -DHAVE_ROUND -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-I/b/1/llvm-x86_64-debian-dylib/build/tools/lldb/source/Plugins/TypeSystem/Clang
 
-I/b/1/llvm-x86_64-debian-dylib/llvm-project/lldb/source/Plugins/TypeSystem/Clang
 -I/b/1/llvm-x86_64-debian-dylib/llvm-project/lldb/include 
-I/b/1/llvm-x86_64-debian-dylib/build/tools/lldb/include 
-I/b/1/llvm-x86_64-debian-dylib/build/include 
-I/b/1/llvm-x86_64-debian-dylib/llvm-project/llvm/include 
-I/b/1/llvm-x86_64-debian-dylib/llvm-project/llvm/../clang/include 
-I/b/1/llvm-x86_64-debian-dylib/build/tools/lldb/../clang/include 
-I/b/1/llvm-x86_64-debian-dylib/llvm-project/lldb/source 
-I/b/1/llvm-x86_64-debian-dylib/build/tools/lldb/source -isystem 
/usr/include/libxml2 -fPIC -fvisibility-inlines-hidden -Werror=date-time 
-Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
-Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
-Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
-Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
-Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
-Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color 
-ffunction-sections -fdata-sections -Wno-unknown-pragmas -Wno-strict-aliasing 
-Wno-vla-extension -O3 -DNDEBUG  -fno-exceptions -funwind-tables -fno-rtti 
-UNDEBUG -std=c++17 -MD -MT 
tools/lldb/source/Plugins/TypeSystem/Clang/CMakeFiles/lldbPluginTypeSystemClang.dir/TypeSystemClang.cpp.o
 -MF 
tools/lldb/source/Plugins/TypeSystem/Clang/CMakeFiles/lldbPluginTypeSystemClang.dir/TypeSystemClang.cpp.o.d
 -o 
tools/lldb/source/Plugins/TypeSystem/Clang/CMakeFiles/lldbPluginTypeSystemClang.dir/TypeSystemClang.cpp.o
 -c 
/b/1/llvm-x86_64-debian-dylib/llvm-project/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
/b/1/llvm-x86_64-debian-dylib/llvm-project/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:7728:10:
 warning: local variable 'params' will be copied despite being returned by name 
[-Wreturn-std-move]
  return params;
 ^~
/b/1/llvm-x86_64-debian-dylib/llvm-project/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:7728:10:
 note: call 'std::move' explicitly to avoid copying
  return params;
 ^~
 std::move(params)
/b/1/llvm-x86_64-debian-dylib/llvm-project/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:7728:10:
 error: no viable conversion from returned value of type 'SmallVector<[...], 
12>' to function return type 'SmallVector<[...], (default) 
CalculateSmallVectorDefaultInlinedElements::value aka 6>'
  return params;
 ^~
/b/1/llvm-x86_64-debian-dylib/llvm-project/llvm/include/llvm/ADT/SmallVector.h:1226:3:
 note: candidate const

[Lldb-commits] [lldb] [llvm] [NFC][DebugInfo] Make some block-start-position methods return iterators (PR #124287)

[Jeremy Morse via lldb-commits]

https://github.com/jmorse updated 
https://github.com/llvm/llvm-project/pull/124287

>From 3d2aa2734d6cb49c43565e3ac8584ba8130fe302 Mon Sep 17 00:00:00 2001
From: Jeremy Morse 
Date: Thu, 23 Jan 2025 12:24:14 +
Subject: [PATCH 1/3] [NFC][DebugInfo] Make some block-start-position methods
 return iterators

As part of the "RemoveDIs" work to eliminate debug intrinsics, we're
replacing methods that use Instruction*'s as positions with iterators. A
number of these (such as getFirstNonPHIOrDbg) are sufficiently infrequently
used that we can just replace the pointer-returning version with an
iterator-returning version, hopefully without much/any disruption.

Thus this patch has getFirstNonPHIOrDbg and getFirstNonPHIOrDbgOrLifetime
return an iterator, and updates all call-sites. There are no concerns about
the iterators returned being converted to Instruction*'s and losing the
debug-info bit: because the methods skip debug intrinsics, the iterator
head bit is always false anyway.
---
 .../ExpressionParser/Clang/IRForTarget.cpp|  6 ++--
 llvm/include/llvm/IR/BasicBlock.h | 18 ++--
 llvm/lib/CodeGen/CodeGenPrepare.cpp   |  2 +-
 llvm/lib/IR/BasicBlock.cpp| 28 +--
 .../AArch64/AArch64TargetTransformInfo.cpp|  2 +-
 .../Target/AMDGPU/SIAnnotateControlFlow.cpp   |  4 +--
 llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp  |  2 +-
 llvm/lib/Transforms/Coroutines/CoroFrame.cpp  |  5 ++--
 llvm/lib/Transforms/Coroutines/CoroSplit.cpp  |  2 +-
 llvm/lib/Transforms/IPO/IROutliner.cpp|  2 +-
 llvm/lib/Transforms/IPO/SCCP.cpp  |  4 +--
 llvm/lib/Transforms/IPO/SampleProfile.cpp |  2 +-
 .../InstCombineLoadStoreAlloca.cpp|  4 +--
 .../Transforms/Scalar/CallSiteSplitting.cpp   |  2 +-
 .../Transforms/Scalar/SimpleLoopUnswitch.cpp  |  2 +-
 llvm/lib/Transforms/Utils/CodeMoverUtils.cpp  |  2 +-
 llvm/lib/Transforms/Utils/IRNormalizer.cpp|  2 +-
 llvm/lib/Transforms/Utils/SimplifyCFG.cpp |  4 +--
 .../Frontend/OpenMPIRBuilderTest.cpp  | 24 ++--
 19 files changed, 67 insertions(+), 50 deletions(-)

diff --git a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
index 6c728f34474898..a414ad652448e9 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -66,7 +66,7 @@ static llvm::Value *FindEntryInstruction(llvm::Function 
*function) {
   if (function->empty())
 return nullptr;
 
-  return function->getEntryBlock().getFirstNonPHIOrDbg();
+  return &*function->getEntryBlock().getFirstNonPHIOrDbg();
 }
 
 IRForTarget::IRForTarget(lldb_private::ClangExpressionDeclMap *decl_map,
@@ -361,7 +361,7 @@ bool IRForTarget::CreateResultVariable(llvm::Function 
&llvm_function) {
 // there's nothing to put into its equivalent persistent variable.
 
 BasicBlock &entry_block(llvm_function.getEntryBlock());
-Instruction *first_entry_instruction(entry_block.getFirstNonPHIOrDbg());
+Instruction *first_entry_instruction(&*entry_block.getFirstNonPHIOrDbg());
 
 if (!first_entry_instruction)
   return false;
@@ -1505,7 +1505,7 @@ bool IRForTarget::ReplaceVariables(Function 
&llvm_function) {
   LLDB_LOG(log, "Arg: \"{0}\"", PrintValue(argument));
 
   BasicBlock &entry_block(llvm_function.getEntryBlock());
-  Instruction *FirstEntryInstruction(entry_block.getFirstNonPHIOrDbg());
+  Instruction *FirstEntryInstruction(&*entry_block.getFirstNonPHIOrDbg());
 
   if (!FirstEntryInstruction) {
 m_error_stream.Printf("Internal error [IRForTarget]: Couldn't find the "
diff --git a/llvm/include/llvm/IR/BasicBlock.h 
b/llvm/include/llvm/IR/BasicBlock.h
index e22fe1e7e7dc8f..5df4dcecebc878 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -299,22 +299,20 @@ class BasicBlock final : public Value, // Basic blocks 
are data objects also
   /// Returns a pointer to the first instruction in this block that is not a
   /// PHINode or a debug intrinsic, or any pseudo operation if \c SkipPseudoOp
   /// is true.
-  const Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) const;
-  Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) {
-return const_cast(
-static_cast(this)->getFirstNonPHIOrDbg(
-SkipPseudoOp));
+  InstListType::const_iterator getFirstNonPHIOrDbg(bool SkipPseudoOp = true) 
const;
+  InstListType::iterator getFirstNonPHIOrDbg(bool SkipPseudoOp = true) {
+return static_cast(this)->getFirstNonPHIOrDbg(
+SkipPseudoOp).getNonConst();
   }
 
   /// Returns a pointer to the first instruction in this block that is not a
   /// PHINode, a debug intrinsic, or a lifetime intrinsic, or any pseudo
   /// operation if \c SkipPseudoOp is true.
-  const Instruction *
+  InstListType::const_iterator
   getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp = true) const;

[Lldb-commits] [lldb] 5f5cdf4 - [lldb][TypeSystemClang] CreateParameterDeclarations: don't specify SmallVector size

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2025-01-27T16:15:30Z
New Revision: 5f5cdf40382f06a8c417c42ec591f97aa76eeb67

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

LOG: [lldb][TypeSystemClang] CreateParameterDeclarations: don't specify 
SmallVector size

This was causing Ubuntu buildbot failures:
```
/home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/llvm-project/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:
 In member function ‘llvm::SmallVector 
lldb_private::TypeSystemClang::CreateParameterDeclarations(clang::FunctionDecl*,
 const clang::FunctionProtoType&, const llvm::SmallVector&)’:
/home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/llvm-project/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:7728:10:
 error: could not convert ‘params’ from ‘SmallVector<[...],12>’ to 
‘SmallVector<[...],6>’
 7728 |   return params;
  |  ^~
  |  |
  |  SmallVector<[...],12>
```

It's unclear why 12 was chosen here. Given we don't set the
size in other places where we parse parameters, this patch
just removes the constant.

Added: 


Modified: 
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Removed: 




diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index fc3dbfa311c9b8..cb246fde976c2f 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -7710,7 +7710,7 @@ TypeSystemClang::CreateParameterDeclarations(
   assert(parameter_names.empty() ||
  parameter_names.size() == prototype.getNumParams());
 
-  llvm::SmallVector params;
+  llvm::SmallVector params;
   for (unsigned param_index = 0; param_index < prototype.getNumParams();
++param_index) {
 llvm::StringRef name =



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

[Lldb-commits] [lldb] 81d18ad - [NFC][DebugInfo] Make some block-start-position methods return iterators (#124287)

[via lldb-commits]

Author: Jeremy Morse
Date: 2025-01-27T16:27:54Z
New Revision: 81d18ad86419fc612c7071e888d11aa923eaeb8a

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

LOG: [NFC][DebugInfo] Make some block-start-position methods return iterators 
(#124287)

As part of the "RemoveDIs" work to eliminate debug intrinsics, we're
replacing methods that use Instruction*'s as positions with iterators. A
number of these (such as getFirstNonPHIOrDbg) are sufficiently
infrequently used that we can just replace the pointer-returning version
with an iterator-returning version, hopefully without much/any
disruption.

Thus this patch has getFirstNonPHIOrDbg and
getFirstNonPHIOrDbgOrLifetime return an iterator, and updates all
call-sites. There are no concerns about the iterators returned being
converted to Instruction*'s and losing the debug-info bit: because the
methods skip debug intrinsics, the iterator head bit is always false
anyway.

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
llvm/include/llvm/IR/BasicBlock.h
llvm/lib/CodeGen/CodeGenPrepare.cpp
llvm/lib/IR/BasicBlock.cpp
llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp
llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp
llvm/lib/Transforms/Coroutines/CoroFrame.cpp
llvm/lib/Transforms/Coroutines/CoroSplit.cpp
llvm/lib/Transforms/IPO/IROutliner.cpp
llvm/lib/Transforms/IPO/SCCP.cpp
llvm/lib/Transforms/IPO/SampleProfile.cpp
llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
llvm/lib/Transforms/Utils/IRNormalizer.cpp
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
index 6c728f34474898..a414ad652448e9 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -66,7 +66,7 @@ static llvm::Value *FindEntryInstruction(llvm::Function 
*function) {
   if (function->empty())
 return nullptr;
 
-  return function->getEntryBlock().getFirstNonPHIOrDbg();
+  return &*function->getEntryBlock().getFirstNonPHIOrDbg();
 }
 
 IRForTarget::IRForTarget(lldb_private::ClangExpressionDeclMap *decl_map,
@@ -361,7 +361,7 @@ bool IRForTarget::CreateResultVariable(llvm::Function 
&llvm_function) {
 // there's nothing to put into its equivalent persistent variable.
 
 BasicBlock &entry_block(llvm_function.getEntryBlock());
-Instruction *first_entry_instruction(entry_block.getFirstNonPHIOrDbg());
+Instruction *first_entry_instruction(&*entry_block.getFirstNonPHIOrDbg());
 
 if (!first_entry_instruction)
   return false;
@@ -1505,7 +1505,7 @@ bool IRForTarget::ReplaceVariables(Function 
&llvm_function) {
   LLDB_LOG(log, "Arg: \"{0}\"", PrintValue(argument));
 
   BasicBlock &entry_block(llvm_function.getEntryBlock());
-  Instruction *FirstEntryInstruction(entry_block.getFirstNonPHIOrDbg());
+  Instruction *FirstEntryInstruction(&*entry_block.getFirstNonPHIOrDbg());
 
   if (!FirstEntryInstruction) {
 m_error_stream.Printf("Internal error [IRForTarget]: Couldn't find the "

diff  --git a/llvm/include/llvm/IR/BasicBlock.h 
b/llvm/include/llvm/IR/BasicBlock.h
index e22fe1e7e7dc8f..f2e34a7d24d06b 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -299,22 +299,24 @@ class BasicBlock final : public Value, // Basic blocks 
are data objects also
   /// Returns a pointer to the first instruction in this block that is not a
   /// PHINode or a debug intrinsic, or any pseudo operation if \c SkipPseudoOp
   /// is true.
-  const Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) const;
-  Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) {
-return const_cast(
-static_cast(this)->getFirstNonPHIOrDbg(
-SkipPseudoOp));
+  InstListType::const_iterator
+  getFirstNonPHIOrDbg(bool SkipPseudoOp = true) const;
+  InstListType::iterator getFirstNonPHIOrDbg(bool SkipPseudoOp = true) {
+return static_cast(this)
+->getFirstNonPHIOrDbg(SkipPseudoOp)
+.getNonConst();
   }
 
   /// Returns a pointer to the first instruction in this block that is not a
   /// PHINode, a debug intrinsic, or a lifetime intrinsic, or any pseudo
   /// operation if \c SkipPseudoOp is true.
-  const Instruction *
+  InstListType::const_iterator
   getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp = true) const;
-  Instruction *ge

[Lldb-commits] [lldb] [llvm] [NFC][DebugInfo] Make some block-start-position methods return iterators (PR #124287)

[Jeremy Morse via lldb-commits]

https://github.com/jmorse closed 
https://github.com/llvm/llvm-project/pull/124287
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)

[Ilia Kuklin via lldb-commits]

https://github.com/kuilpd updated 
https://github.com/llvm/llvm-project/pull/115005

>From 4d797371598960baf7729d05590aa1a8c7077694 Mon Sep 17 00:00:00 2001
From: Ilia Kuklin 
Date: Mon, 4 Nov 2024 14:33:45 +0500
Subject: [PATCH 1/8] [lldb] Analyze enum promotion type during parsing

---
 clang/include/clang/AST/Decl.h|  2 +-
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  | 95 ++-
 .../TypeSystem/Clang/TypeSystemClang.cpp  | 26 +
 3 files changed, 98 insertions(+), 25 deletions(-)

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 16403774e72b31..41cb47516f5803 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -3903,6 +3903,7 @@ class EnumDecl : public TagDecl {
   void setInstantiationOfMemberEnum(ASTContext &C, EnumDecl *ED,
 TemplateSpecializationKind TSK);
 
+public:
   /// Sets the width in bits required to store all the
   /// non-negative enumerators of this enum.
   void setNumPositiveBits(unsigned Num) {
@@ -3914,7 +3915,6 @@ class EnumDecl : public TagDecl {
   /// negative enumerators of this enum. (see getNumNegativeBits)
   void setNumNegativeBits(unsigned Num) { EnumDeclBits.NumNegativeBits = Num; }
 
-public:
   /// True if this tag declaration is a scoped enumeration. Only
   /// possible in C++11 mode.
   void setScoped(bool Scoped = true) { EnumDeclBits.IsScoped = Scoped; }
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index e77188bfbd2e4a..bb9c35a235c1ff 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2316,6 +2316,8 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
 return 0;
 
   size_t enumerators_added = 0;
+  unsigned NumNegativeBits = 0;
+  unsigned NumPositiveBits = 0;
 
   for (DWARFDIE die : parent_die.children()) {
 const dw_tag_t tag = die.Tag();
@@ -2367,11 +2369,102 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
 }
 
 if (name && name[0] && got_value) {
-  m_ast.AddEnumerationValueToEnumerationType(
+  auto ECD = m_ast.AddEnumerationValueToEnumerationType(
   clang_type, decl, name, enum_value, enumerator_byte_size * 8);
   ++enumerators_added;
+
+  llvm::APSInt InitVal = ECD->getInitVal();
+  // Keep track of the size of positive and negative values.
+  if (InitVal.isUnsigned() || InitVal.isNonNegative()) {
+// If the enumerator is zero that should still be counted as a positive
+// bit since we need a bit to store the value zero.
+unsigned ActiveBits = InitVal.getActiveBits();
+NumPositiveBits = std::max({NumPositiveBits, ActiveBits, 1u});
+  } else {
+NumNegativeBits =
+std::max(NumNegativeBits, (unsigned)InitVal.getSignificantBits());
+  }
 }
   }
+
+  /// The following code follows the same logic as in Sema::ActOnEnumBody
+  /// clang/lib/Sema/SemaDecl.cpp
+  // If we have an empty set of enumerators we still need one bit.
+  // From [dcl.enum]p8
+  // If the enumerator-list is empty, the values of the enumeration are as if
+  // the enumeration had a single enumerator with value 0
+  if (!NumPositiveBits && !NumNegativeBits)
+NumPositiveBits = 1;
+
+  clang::QualType qual_type(ClangUtil::GetQualType(clang_type));
+  clang::EnumDecl *enum_decl = qual_type->getAs()->getDecl();
+  enum_decl->setNumPositiveBits(NumPositiveBits);
+  enum_decl->setNumNegativeBits(NumNegativeBits);
+
+  // C++0x N3000 [conv.prom]p3:
+  //   An rvalue of an unscoped enumeration type whose underlying
+  //   type is not fixed can be converted to an rvalue of the first
+  //   of the following types that can represent all the values of
+  //   the enumeration: int, unsigned int, long int, unsigned long
+  //   int, long long int, or unsigned long long int.
+  // C99 6.4.4.3p2:
+  //   An identifier declared as an enumeration constant has type int.
+  // The C99 rule is modified by C23.
+  clang::QualType BestPromotionType;
+  unsigned BestWidth;
+
+  auto &Context = m_ast.getASTContext();
+  unsigned LongWidth = Context.getTargetInfo().getLongWidth();
+  unsigned IntWidth = Context.getTargetInfo().getIntWidth();
+  unsigned CharWidth = Context.getTargetInfo().getCharWidth();
+  unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
+
+  bool is_cpp = Language::LanguageIsCPlusPlus(
+  SymbolFileDWARF::GetLanguage(*parent_die.GetCU()));
+
+  if (NumNegativeBits) {
+// If there is a negative value, figure out the smallest integer type (of
+// int/long/longlong) that fits.
+if (NumNegativeBits <= CharWidth && NumPositiveBits < CharWidth) {
+  BestWidth = CharWidth;
+} else if (NumNegativeBits <= ShortWidth && NumPositiveBits < ShortWidth) {
+  BestWidth = ShortWidth;
+} else if (NumNegativeBits <= IntWidth && 

[Lldb-commits] [clang] [clang-tools-extra] [lldb] [Clang] [NFC] Rename `isAnyPointerType()` and `getPointeeOrArrayElementType()`. (PR #122938)

[via lldb-commits]

Sirraide wrote:

Aaron and I just talked about this a bit, and I agree it would be cleaner to 
just make a single function that can be used everywhere and whose behaviour is 
customised with an enum. I’ll experiment with that approach and see how it 
goes. Also, my plan was to try and remove `isPointerOrObjCObjectPointerType()` 
entirely in a follow-up pr, and it does make more sense to just do that right 
away.

https://github.com/llvm/llvm-project/pull/122938
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [clang-tools-extra] [lldb] [Clang] [NFC] Rename `isAnyPointerType()` and `getPointeeOrArrayElementType()`. (PR #122938)

[via lldb-commits]

Sirraide wrote:

ping

https://github.com/llvm/llvm-project/pull/122938
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Remove support and workarounds for Android 4 and older (PR #124047)

[via lldb-commits]

enh-google wrote:

> It's quite possible that didn't run as a part of the CTS from the start. If 
> you say it didn't, then I'll take your word for it.

sorry, i didn't mean to imply that these tests didn't immediately go into CTS 
when they went into bionic: they will have automatically been included in the 
next release. it's just that with a copyright date of 2016, they clearly 
weren't in bionic early enough for api 21. (2016 would be api 24 at the 
earliest.)

> Wow. Thank _you_ for following up on that. I've been aware of issues with 
> running the lldb test suite (the watchpoint tests in particular) in 
> virtualized environments (this is the reason why the lldb linux bot runs on a 
> physical machine and not in the cloud), but I never took the time to isolate 
> and reproduce the bug. It's very cool to see this fixed.

i'll pass that on to the folks who did all the work --- i just handed the CI's 
auto-filed bug to the systems team and jstultz and others did the real work :-)

https://github.com/llvm/llvm-project/pull/124047
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Include api-level.h header for Android (PR #124383)

[via lldb-commits]

@@ -29,6 +29,10 @@
 #include 
 #include 
 
+#ifdef __ANDROID__
+#include 

enh-google wrote:

> is it possible those includes were necessary 10 years ago?

yeah, one reason i'm so confidently asserting that you shouldn't need this now 
is because i know we had a big cleanup to ensure that we hadn't missed any :-)

> we could assume that any header which includes the definition of a C function 
> we're about to call, also includes the definition of that macro -- which I 
> guess must be true as otherwise it wouldn't be able to annotate the 
> declaration

correct.

https://github.com/llvm/llvm-project/pull/124383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits