date:20240315

[Lldb-commits] [lldb] Add register lookup as another fallback computation for address-expressions (PR #85492)

2024-03-15 Thread Adrian Prantl via lldb-commits


adrian-prantl wrote:

This is going to be incredibly useful, thanks!

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

[Lldb-commits] [lldb] Add register lookup as another fallback computation for address-expressions (PR #85492)

2024-03-15 Thread Adrian Prantl via lldb-commits



@@ -233,24 +235,57 @@ OptionArgParser::DoToAddress(const ExecutionContext 
*exe_ctx, llvm::StringRef s,
   // Since the compiler can't handle things like "main + 12" we should try to
   // do this for now. The compiler doesn't like adding offsets to function
   // pointer types.
+  // Some languages also don't have a natural representation for register
+  // values (e.g. swift) so handle simple uses of them here as well.
   static RegularExpression g_symbol_plus_offset_regex(
-  "^(.*)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*$");
+  "^(\\$[^ +-]+)|(([^ 
+-]+)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*)$");
 
   llvm::SmallVector matches;
   if (g_symbol_plus_offset_regex.Execute(sref, &matches)) {
 uint64_t offset = 0;
-llvm::StringRef name = matches[1];
-llvm::StringRef sign = matches[2];
-llvm::StringRef str_offset = matches[3];
-if (!str_offset.getAsInteger(0, offset)) {
+llvm::StringRef name;
+if (!matches[1].empty())
+  name = matches[1];
+else
+  name = matches[3];

adrian-prantl wrote:

It might help to comment with an example what match[1,3] are: 
```
// $pc + 0  ... match[1] == "$pc"
```

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

[Lldb-commits] [lldb] Add register lookup as another fallback computation for address-expressions (PR #85492)

2024-03-15 Thread Adrian Prantl via lldb-commits



@@ -233,24 +235,57 @@ OptionArgParser::DoToAddress(const ExecutionContext 
*exe_ctx, llvm::StringRef s,
   // Since the compiler can't handle things like "main + 12" we should try to
   // do this for now. The compiler doesn't like adding offsets to function
   // pointer types.
+  // Some languages also don't have a natural representation for register
+  // values (e.g. swift) so handle simple uses of them here as well.
   static RegularExpression g_symbol_plus_offset_regex(
-  "^(.*)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*$");
+  "^(\\$[^ +-]+)|(([^ 
+-]+)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*)$");
 
   llvm::SmallVector matches;
   if (g_symbol_plus_offset_regex.Execute(sref, &matches)) {
 uint64_t offset = 0;
-llvm::StringRef name = matches[1];
-llvm::StringRef sign = matches[2];
-llvm::StringRef str_offset = matches[3];
-if (!str_offset.getAsInteger(0, offset)) {
+llvm::StringRef name;
+if (!matches[1].empty())
+  name = matches[1];
+else
+  name = matches[3];
+
+llvm::StringRef sign = matches[4];
+llvm::StringRef str_offset = matches[5];
+std::optional register_value;
+StackFrame *frame = exe_ctx->GetFramePtr();
+if (frame && !name.empty() && name[0] == '$') {

adrian-prantl wrote:

or even better:
StringRef regname = name.consume_front("$");
if (!regname.empty) ...

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

[Lldb-commits] [lldb] Add register lookup as another fallback computation for address-expressions (PR #85492)

2024-03-15 Thread Adrian Prantl via lldb-commits



@@ -233,24 +235,57 @@ OptionArgParser::DoToAddress(const ExecutionContext 
*exe_ctx, llvm::StringRef s,
   // Since the compiler can't handle things like "main + 12" we should try to
   // do this for now. The compiler doesn't like adding offsets to function
   // pointer types.
+  // Some languages also don't have a natural representation for register
+  // values (e.g. swift) so handle simple uses of them here as well.
   static RegularExpression g_symbol_plus_offset_regex(
-  "^(.*)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*$");
+  "^(\\$[^ +-]+)|(([^ 
+-]+)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*)$");
 
   llvm::SmallVector matches;
   if (g_symbol_plus_offset_regex.Execute(sref, &matches)) {
 uint64_t offset = 0;
-llvm::StringRef name = matches[1];
-llvm::StringRef sign = matches[2];
-llvm::StringRef str_offset = matches[3];
-if (!str_offset.getAsInteger(0, offset)) {
+llvm::StringRef name;
+if (!matches[1].empty())
+  name = matches[1];
+else
+  name = matches[3];
+
+llvm::StringRef sign = matches[4];
+llvm::StringRef str_offset = matches[5];
+std::optional register_value;
+StackFrame *frame = exe_ctx->GetFramePtr();
+if (frame && !name.empty() && name[0] == '$') {

adrian-prantl wrote:

```suggestion
if (frame && name.starts_with("$") {
```

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

[Lldb-commits] [lldb] Add register lookup as another fallback computation for address-expressions (PR #85492)

2024-03-15 Thread via lldb-commits


github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 9405d5af65853ac548cce2656497195010db1d86 
59320299f5aa3f9e03695e762c9fec237362c460 -- 
lldb/test/API/commands/target/modules/lookup/main.c 
lldb/source/Interpreter/OptionArgParser.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/source/Interpreter/OptionArgParser.cpp 
b/lldb/source/Interpreter/OptionArgParser.cpp
index dd914138fe..98e0971a8d 100644
--- a/lldb/source/Interpreter/OptionArgParser.cpp
+++ b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -238,7 +238,8 @@ OptionArgParser::DoToAddress(const ExecutionContext 
*exe_ctx, llvm::StringRef s,
   // Some languages also don't have a natural representation for register
   // values (e.g. swift) so handle simple uses of them here as well.
   static RegularExpression g_symbol_plus_offset_regex(
-  "^(\\$[^ +-]+)|(([^ 
+-]+)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*)$");
+  "^(\\$[^ +-]+)|(([^ "
+  "+-]+)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*)$");
 
   llvm::SmallVector matches;
   if (g_symbol_plus_offset_regex.Execute(sref, &matches)) {
@@ -259,7 +260,8 @@ OptionArgParser::DoToAddress(const ExecutionContext 
*exe_ctx, llvm::StringRef s,
   RegisterContextSP reg_ctx_sp = frame->GetRegisterContext();
   if (reg_ctx_sp) {
 llvm::StringRef base_name = name.substr(1);
-const RegisterInfo *reg_info = 
reg_ctx_sp->GetRegisterInfoByName(base_name);
+const RegisterInfo *reg_info =
+reg_ctx_sp->GetRegisterInfoByName(base_name);
 if (reg_info) {
   RegisterValue reg_val;
   bool success = reg_ctx_sp->ReadRegister(reg_info, reg_val);
@@ -269,7 +271,7 @@ OptionArgParser::DoToAddress(const ExecutionContext 
*exe_ctx, llvm::StringRef s,
   register_value.reset();
   }
 }
-  } 
+  }
 }
 if (!str_offset.empty() && !str_offset.getAsInteger(0, offset)) {
   Status error;
@@ -283,7 +285,7 @@ OptionArgParser::DoToAddress(const ExecutionContext 
*exe_ctx, llvm::StringRef s,
 return addr - offset;
   }
 } else if (register_value)
-  // In the case of register values, someone might just want to get the 
+  // In the case of register values, someone might just want to get the
   // value in a language whose expression parser doesn't support registers.
   return register_value.value();
   }
diff --git a/lldb/test/API/commands/target/modules/lookup/main.c 
b/lldb/test/API/commands/target/modules/lookup/main.c
index afe962f309..f2be761a72 100644
--- a/lldb/test/API/commands/target/modules/lookup/main.c
+++ b/lldb/test/API/commands/target/modules/lookup/main.c
@@ -1,15 +1,11 @@
 #include 
 
-void
-doSomething()
-{
-  printf ("Set a breakpoint here.\n");
-  printf ("Need a bit more code.\n");
+void doSomething() {
+  printf("Set a breakpoint here.\n");
+  printf("Need a bit more code.\n");
 }
 
-int
-main()
-{
+int main() {
   doSomething();
   return 0;
 }

``




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

[Lldb-commits] [lldb] Add register lookup as another fallback computation for address-expressions (PR #85492)

2024-03-15 Thread 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 
9405d5af65853ac548cce2656497195010db1d86...59320299f5aa3f9e03695e762c9fec237362c460
 lldb/test/API/commands/target/modules/lookup/TestImageLookupPCExpression.py
``





View the diff from darker here.


``diff
--- TestImageLookupPCExpression.py  2024-03-16 00:56:20.00 +
+++ TestImageLookupPCExpression.py  2024-03-16 01:08:56.473285 +
@@ -22,6 +22,5 @@
 self, "Set a breakpoint here", self.main_source_file
 )
 
 self.expect("target modules lookup -va $pc", substrs=["doSomething"])
 self.expect("target modules lookup -va $pc+4", substrs=["doSomething"])
-

``




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

[Lldb-commits] [lldb] Add register lookup as another fallback computation for address-expressions (PR #85492)

2024-03-15 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: None (jimingham)


Changes

The idea behind the address-expression is that it handles all the common 
expressions that produce addresses.  It handles actual valid expressions that 
return a scalar, and it handles useful cases that the various source languages 
don't support.  At present, the fallback handles:

{+-}

which isn't valid C but is very handy.

This patch adds handling of:

$

and

${+-}

That's kind of pointless in C because the C expression parser handles that 
expression already.  But some languages don't have a straightforward way to 
represent register values like this (swift) so having this fallback is quite a 
quality of life improvement.

I added a test which tests that I didn't mess up either of these fallbacks, 
though it doesn't test the actually handling of registers that I added, since 
the expression parser for C succeeds in that case and returns before this code 
gets run.

I will add a test on the swift fork for that checks that this works the same 
way for a swift frame after this check.

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


4 Files Affected:

- (modified) lldb/source/Interpreter/OptionArgParser.cpp (+42-7) 
- (added) lldb/test/API/commands/target/modules/lookup/Makefile (+4) 
- (added) 
lldb/test/API/commands/target/modules/lookup/TestImageLookupPCExpression.py 
(+27) 
- (added) lldb/test/API/commands/target/modules/lookup/main.c (+15) 


``diff
diff --git a/lldb/source/Interpreter/OptionArgParser.cpp 
b/lldb/source/Interpreter/OptionArgParser.cpp
index 75ccad87467e95..dd914138fe0e6b 100644
--- a/lldb/source/Interpreter/OptionArgParser.cpp
+++ b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -9,7 +9,9 @@
 #include "lldb/Interpreter/OptionArgParser.h"
 #include "lldb/DataFormatters/FormatManager.h"
 #include "lldb/Target/ABI.h"
+#include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/StreamString.h"
 
@@ -233,24 +235,57 @@ OptionArgParser::DoToAddress(const ExecutionContext 
*exe_ctx, llvm::StringRef s,
   // Since the compiler can't handle things like "main + 12" we should try to
   // do this for now. The compiler doesn't like adding offsets to function
   // pointer types.
+  // Some languages also don't have a natural representation for register
+  // values (e.g. swift) so handle simple uses of them here as well.
   static RegularExpression g_symbol_plus_offset_regex(
-  "^(.*)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*$");
+  "^(\\$[^ +-]+)|(([^ 
+-]+)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*)$");
 
   llvm::SmallVector matches;
   if (g_symbol_plus_offset_regex.Execute(sref, &matches)) {
 uint64_t offset = 0;
-llvm::StringRef name = matches[1];
-llvm::StringRef sign = matches[2];
-llvm::StringRef str_offset = matches[3];
-if (!str_offset.getAsInteger(0, offset)) {
+llvm::StringRef name;
+if (!matches[1].empty())
+  name = matches[1];
+else
+  name = matches[3];
+
+llvm::StringRef sign = matches[4];
+llvm::StringRef str_offset = matches[5];
+std::optional register_value;
+StackFrame *frame = exe_ctx->GetFramePtr();
+if (frame && !name.empty() && name[0] == '$') {
+  // Some languages don't have a natural type for register values, but it
+  // is still useful to look them up here:
+  RegisterContextSP reg_ctx_sp = frame->GetRegisterContext();
+  if (reg_ctx_sp) {
+llvm::StringRef base_name = name.substr(1);
+const RegisterInfo *reg_info = 
reg_ctx_sp->GetRegisterInfoByName(base_name);
+if (reg_info) {
+  RegisterValue reg_val;
+  bool success = reg_ctx_sp->ReadRegister(reg_info, reg_val);
+  if (success && reg_val.GetType() != RegisterValue::eTypeInvalid) {
+register_value = reg_val.GetAsUInt64(0, &success);
+if (!success)
+  register_value.reset();
+  }
+}
+  } 
+}
+if (!str_offset.empty() && !str_offset.getAsInteger(0, offset)) {
   Status error;
-  addr = ToAddress(exe_ctx, name, LLDB_INVALID_ADDRESS, &error);
+  if (register_value)
+addr = register_value.value();
+  else
+addr = ToAddress(exe_ctx, name, LLDB_INVALID_ADDRESS, &error);
   if (addr != LLDB_INVALID_ADDRESS) {
 if (sign[0] == '+')
   return addr + offset;
 return addr - offset;
   }
-}
+} else if (register_value)
+  // In the case of register values, someone might just want to get the 
+  // value in a language whose expression parser doesn't support registers.
+  return register_value.value();
   }
 
   if (error_ptr)
diff --git a/lldb/test/API/commands/target/modules/lookup/Makefile 
b/lldb/test/API/commands/target/modules/lookup/Makefile
new file m

[Lldb-commits] [lldb] Add register lookup as another fallback computation for address-expressions (PR #85492)

2024-03-15 Thread via lldb-commits


https://github.com/jimingham created 
https://github.com/llvm/llvm-project/pull/85492

The idea behind the address-expression is that it handles all the common 
expressions that produce addresses.  It handles actual valid expressions that 
return a scalar, and it handles useful cases that the various source languages 
don't support.  At present, the fallback handles:

{+-}

which isn't valid C but is very handy.

This patch adds handling of:

$

and

${+-}

That's kind of pointless in C because the C expression parser handles that 
expression already.  But some languages don't have a straightforward way to 
represent register values like this (swift) so having this fallback is quite a 
quality of life improvement.

I added a test which tests that I didn't mess up either of these fallbacks, 
though it doesn't test the actually handling of registers that I added, since 
the expression parser for C succeeds in that case and returns before this code 
gets run.

I will add a test on the swift fork for that checks that this works the same 
way for a swift frame after this check.

>From 59320299f5aa3f9e03695e762c9fec237362c460 Mon Sep 17 00:00:00 2001
From: Jim Ingham 
Date: Fri, 15 Mar 2024 17:56:20 -0700
Subject: [PATCH] Add register lookup as another fallback computation for
 address-expressions

The idea behind the address-expression is that it handles all the common
expressions that produce addresses.  It handles actual valid expressions
that return a scalar, and it handles useful cases that the various source 
languages
don't support.  At present, the fallback handles:

{+-}

which isn't valid C but is very handy.

This patch adds handling of:

$

and

${+-}

That's kind of pointless in C because the C expression parser handles
that expression already.  But some languages don't have a straightforward
way to represent register values like this (swift) so having this fallback
is quite a quality of life improvement.

I added a test which tests that I didn't mess up either of these fallbacks,
though it doesn't test the actually handling of registers that I added, since
the expression parser for C succeeds in that case and returns before this code
gets run.

I will add a test on the swift fork for that checks that this works the same
way for a swift frame after this check.
---
 lldb/source/Interpreter/OptionArgParser.cpp   | 49 ---
 .../commands/target/modules/lookup/Makefile   |  4 ++
 .../lookup/TestImageLookupPCExpression.py | 27 ++
 .../API/commands/target/modules/lookup/main.c | 15 ++
 4 files changed, 88 insertions(+), 7 deletions(-)
 create mode 100644 lldb/test/API/commands/target/modules/lookup/Makefile
 create mode 100644 
lldb/test/API/commands/target/modules/lookup/TestImageLookupPCExpression.py
 create mode 100644 lldb/test/API/commands/target/modules/lookup/main.c

diff --git a/lldb/source/Interpreter/OptionArgParser.cpp 
b/lldb/source/Interpreter/OptionArgParser.cpp
index 75ccad87467e95..dd914138fe0e6b 100644
--- a/lldb/source/Interpreter/OptionArgParser.cpp
+++ b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -9,7 +9,9 @@
 #include "lldb/Interpreter/OptionArgParser.h"
 #include "lldb/DataFormatters/FormatManager.h"
 #include "lldb/Target/ABI.h"
+#include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/StreamString.h"
 
@@ -233,24 +235,57 @@ OptionArgParser::DoToAddress(const ExecutionContext 
*exe_ctx, llvm::StringRef s,
   // Since the compiler can't handle things like "main + 12" we should try to
   // do this for now. The compiler doesn't like adding offsets to function
   // pointer types.
+  // Some languages also don't have a natural representation for register
+  // values (e.g. swift) so handle simple uses of them here as well.
   static RegularExpression g_symbol_plus_offset_regex(
-  "^(.*)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*$");
+  "^(\\$[^ +-]+)|(([^ 
+-]+)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*)$");
 
   llvm::SmallVector matches;
   if (g_symbol_plus_offset_regex.Execute(sref, &matches)) {
 uint64_t offset = 0;
-llvm::StringRef name = matches[1];
-llvm::StringRef sign = matches[2];
-llvm::StringRef str_offset = matches[3];
-if (!str_offset.getAsInteger(0, offset)) {
+llvm::StringRef name;
+if (!matches[1].empty())
+  name = matches[1];
+else
+  name = matches[3];
+
+llvm::StringRef sign = matches[4];
+llvm::StringRef str_offset = matches[5];
+std::optional register_value;
+StackFrame *frame = exe_ctx->GetFramePtr();
+if (frame && !name.empty() && name[0] == '$') {
+  // Some languages don't have a natural type for register values, but it
+  // is still useful to look them up here:
+  RegisterContextSP reg_ctx_sp = frame->GetRegisterContext();
+  if (reg_ctx_sp) {
+llvm::StringRef base_name = name.substr(1);
+

[Lldb-commits] [lldb] 113214e - [lldb] Update SymbolFilePDBTests for LineEntry change (d5a277d309e9)

2024-03-15 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2024-03-15T17:47:04-07:00
New Revision: 113214e15b5ce3f3ec313eb1fa91a7038ecd072f

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

LOG: [lldb] Update SymbolFilePDBTests for LineEntry change (d5a277d309e9)

Added: 


Modified: 
lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

Removed: 




diff  --git a/lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp 
b/lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
index f237dd63ab1cce..4379ffac9d744e 100644
--- a/lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
+++ b/lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
@@ -102,7 +102,7 @@ class SymbolFilePDBTests : public testing::Test {
 EXPECT_EQ(line, entry.line);
 EXPECT_EQ(address, entry.range.GetBaseAddress());
 
-EXPECT_TRUE(FileSpecMatchesAsBaseOrFull(spec, entry.file));
+EXPECT_TRUE(FileSpecMatchesAsBaseOrFull(spec, entry.GetFile()));
   }
 
   bool ContainsCompileUnit(const SymbolContextList &sc_list,



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

[Lldb-commits] [lldb] [lldb] Fix dwim-print to not delete non-result persistent variables (PR #85152)

2024-03-15 Thread via lldb-commits

jimingham wrote:

> > I'm not sure what you mean by "mixed language expressions".
> 
> I was thinking about the case where a user attempts `p $some_c_var + 1` in 
> Swift frame – which is valid swift as far as the debugger is concerned. I 
> said mixed because it's got a C var inside a Swift expression.

The important thing is not whether this is abstractly a valid swift expression, 
but rather that is an expression that would execute correctly in the Swift 
Expression parser.  That won't happen with this expression because the swift 
compiler can't find `$some_c_var` and and even if it could it wouldn't know how 
to use it.

My take is that if I do `p some_var` and you return `some_var` successfully, 
then when I do `p some_var->some_method()` you will use the language in which 
you found that symbol to evaluate that expression that uses it.  I don't see 
how you will know that directly, I think the best you can do is try them all in 
some reasonable order.

`dwim-print` has a language option because it's really trying to be the 
expression parser but done as efficiently as possible.  Having it just 
sometimes ignore whatever that language option is supposed to instruct it to do 
seems wrong, and having it sometimes ignore the language and other times not is 
even more wrong, IMO.

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

[Lldb-commits] [lldb] [lldb] Fix dwim-print to not delete non-result persistent variables (PR #85152)

2024-03-15 Thread Dave Lee via lldb-commits


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

[Lldb-commits] [lldb] 4da2b54 - [lldb] Fix dwim-print to not delete non-result persistent variables (#85152)

2024-03-15 Thread via lldb-commits


Author: Dave Lee
Date: 2024-03-15T16:09:24-07:00
New Revision: 4da2b542b142dac441722e044ee75da2475d9a20

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

LOG: [lldb] Fix dwim-print to not delete non-result persistent variables 
(#85152)

`EvaluateExpression` does not always create a new persistent result. If the 
expression 
is a bare persistent variable, then a new persistent result is not created. 
This means 
the caller can't assume a new persistent result is created for each evaluation. 
However, `dwim-print` was doing exactly that: assuming a new persistent result 
for each 
evaluation. This resulted in a bug:

```
(lldb) p int $j = 23
(lldb) p $j
(lldb) p $j
```

The first `p $j` would not create a persistent result, and so `dwim-print` 
would 
inadvertently delete `$j`. The second `p $j` would fail.

The fix is to try `expr` as a persistent variable, after trying `expr` as a 
frame 
variable. For persistent variables, this avoids calling `EvaluateExpression`.

Resolves https://github.com/llvm/llvm-project/issues/84806

rdar://124688427

Added: 


Modified: 
lldb/source/Commands/CommandObjectDWIMPrint.cpp
lldb/test/API/commands/dwim-print/TestDWIMPrint.py

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp 
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index b183cb423111fb..a88da986bb384f 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -23,7 +23,6 @@
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-forward.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Support/FormatVariadic.h"
 
 #include 
 
@@ -161,7 +160,17 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 }
   }
 
-  // Second, also lastly, try `expr` as a source expression to evaluate.
+  // Second, try `expr` as a persistent variable.
+  if (expr.starts_with("$"))
+if (auto *state = target.GetPersistentExpressionStateForLanguage(language))
+  if (auto var_sp = state->GetVariable(expr))
+if (auto valobj_sp = var_sp->GetValueObject()) {
+  valobj_sp->Dump(result.GetOutputStream(), dump_options);
+  result.SetStatus(eReturnStatusSuccessFinishResult);
+  return;
+}
+
+  // Third, and lastly, try `expr` as a source expression to evaluate.
   {
 auto *exe_scope = m_exe_ctx.GetBestExecutionContextScope();
 ValueObjectSP valobj_sp;

diff  --git a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py 
b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
index 040632096c70e7..c650b1e3533e08 100644
--- a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -146,3 +146,15 @@ def test_void_result(self):
 self, "// break here", lldb.SBFileSpec("main.c")
 )
 self.expect("dwim-print (void)15", matching=False, 
patterns=["(?i)error"])
+
+def test_preserves_persistent_variables(self):
+"""Test dwim-print does not delete persistent variables."""
+self.build()
+lldbutil.run_to_source_breakpoint(
+self, "// break here", lldb.SBFileSpec("main.c")
+)
+self.expect("dwim-print int $i = 15")
+# Run the same expression twice and verify success. This ensures the
+# first command does not delete the persistent variable.
+for _ in range(2):
+self.expect("dwim-print $i", startstr="(int) 15")



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

[Lldb-commits] [lldb] [lldb] Fix dwim-print to not delete non-result persistent variables (PR #85152)

2024-03-15 Thread Dave Lee via lldb-commits


kastiglione wrote:

> I'm not sure what you mean by "mixed language expressions". 

I was thinking about the case where a user attempts `p $some_c_var + 1` in 
Swift frame – which is valid swift as far as the debugger is concerned.

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

[Lldb-commits] [lldb] 8f2632c - [lldb][test] Fix -Wctad-maybe-unsupported in AlarmTest.cpp (NFC)

2024-03-15 Thread Jie Fu via lldb-commits


Author: Jie Fu
Date: 2024-03-16T06:59:07+08:00
New Revision: 8f2632c45f54d1e91248be81db5d4908d1036213

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

LOG: [lldb][test] Fix -Wctad-maybe-unsupported in AlarmTest.cpp (NFC)

llvm-project/lldb/unittests/Host/AlarmTest.cpp:49:7:
error: 'lock_guard' may not intend to support class template argument deduction 
[-Werror,-Wctad-maybe-unsupported]
  std::lock_guard guard(m);
  ^

Added: 


Modified: 
lldb/unittests/Host/AlarmTest.cpp

Removed: 




diff  --git a/lldb/unittests/Host/AlarmTest.cpp 
b/lldb/unittests/Host/AlarmTest.cpp
index e5895574376e38..9f6ad189dee970 100644
--- a/lldb/unittests/Host/AlarmTest.cpp
+++ b/lldb/unittests/Host/AlarmTest.cpp
@@ -46,7 +46,7 @@ TEST(AlarmTest, Create) {
 ALARM_TIMEOUT);
 
 alarm.Create([&callbacks_actual, &m, i]() {
-  std::lock_guard guard(m);
+  std::lock_guard guard(m);
   callbacks_actual[i] = std::chrono::system_clock::now();
 });
 
@@ -75,7 +75,7 @@ TEST(AlarmTest, Exit) {
   callbacks.emplace_back(false);
 
   handles.push_back(alarm.Create([&callbacks, &m, i]() {
-std::lock_guard guard(m);
+std::lock_guard guard(m);
 callbacks[i] = true;
   }));
 }
@@ -101,7 +101,7 @@ TEST(AlarmTest, Cancel) {
 callbacks.emplace_back(false);
 
 handles.push_back(alarm.Create([&callbacks, &m, i]() {
-  std::lock_guard guard(m);
+  std::lock_guard guard(m);
   callbacks[i] = true;
 }));
   }
@@ -137,7 +137,7 @@ TEST(AlarmTest, Restart) {
 ALARM_TIMEOUT);
 
 handles.push_back(alarm.Create([&callbacks_actual, &m, i]() {
-  std::lock_guard guard(m);
+  std::lock_guard guard(m);
   callbacks_actual[i] = std::chrono::system_clock::now();
 }));
 



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

[Lldb-commits] [lldb] ba97dc8 - [lldb] Fix -Wctad-maybe-unsupported in Alarm.cpp (NFC)

2024-03-15 Thread Jie Fu via lldb-commits


Author: Jie Fu
Date: 2024-03-16T06:52:12+08:00
New Revision: ba97dc8c7a8fc26516fbdfe822343bc4d38fe3db

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

LOG: [lldb] Fix -Wctad-maybe-unsupported in Alarm.cpp (NFC)

llvm-project/lldb/source/Host/common/Alarm.cpp:37:5:
error: 'lock_guard' may not intend to support class template argument deduction 
[-Werror,-Wctad-maybe-unsupported]
std::lock_guard alarm_guard(m_alarm_mutex);
^

Added: 


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

Removed: 




diff  --git a/lldb/source/Host/common/Alarm.cpp 
b/lldb/source/Host/common/Alarm.cpp
index 80c544773d7650..245cdc7ae5c2da 100644
--- a/lldb/source/Host/common/Alarm.cpp
+++ b/lldb/source/Host/common/Alarm.cpp
@@ -34,7 +34,7 @@ Alarm::Handle Alarm::Create(std::function callback) {
   Handle handle = INVALID_HANDLE;
 
   {
-std::lock_guard alarm_guard(m_alarm_mutex);
+std::lock_guard alarm_guard(m_alarm_mutex);
 
 // Create a new unique entry and remember its handle.
 m_entries.emplace_back(callback, expiration);
@@ -59,7 +59,7 @@ bool Alarm::Restart(Handle handle) {
   const TimePoint expiration = GetNextExpiration();
 
   {
-std::lock_guard alarm_guard(m_alarm_mutex);
+std::lock_guard alarm_guard(m_alarm_mutex);
 
 // Find the entry corresponding to the given handle.
 const auto it =
@@ -86,7 +86,7 @@ bool Alarm::Cancel(Handle handle) {
 return false;
 
   {
-std::lock_guard alarm_guard(m_alarm_mutex);
+std::lock_guard alarm_guard(m_alarm_mutex);
 
 const auto it =
 std::find_if(m_entries.begin(), m_entries.end(),
@@ -126,7 +126,7 @@ void Alarm::StartAlarmThread() {
 void Alarm::StopAlarmThread() {
   if (m_alarm_thread.IsJoinable()) {
 {
-  std::lock_guard alarm_guard(m_alarm_mutex);
+  std::lock_guard alarm_guard(m_alarm_mutex);
   m_exit = true;
 }
 m_alarm_cv.notify_one();
@@ -154,7 +154,7 @@ lldb::thread_result_t Alarm::AlarmThread() {
 //
 // Below we only deal with the timeout expiring and fall through for 
dealing
 // with the rest.
-std::unique_lock alarm_lock(m_alarm_mutex);
+std::unique_lock alarm_lock(m_alarm_mutex);
 if (next_alarm) {
   if (!m_alarm_cv.wait_until(alarm_lock, *next_alarm, predicate)) {
 // The timeout for the next alarm expired.



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

[Lldb-commits] [lldb] [lldb] Fix dwim-print to not delete non-result persistent variables (PR #85152)

2024-03-15 Thread via lldb-commits


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

LGTM

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

[Lldb-commits] [lldb] [lldb] Fix dwim-print to not delete non-result persistent variables (PR #85152)

2024-03-15 Thread via lldb-commits

jimingham wrote:

> I don't consider it a failure to not be able to perform mixed language 
> expressions. But it would have to be presented to the user in a way that is 
> understandable, which could be a challenge.
> 
> Anyway, I pushed the change that scopes the lookup to the expected language.
> 
> thank you for the reviews

I'm not sure what you mean by "mixed language expressions".  What I'm trying to 
avoid is, if stopped in a swift frame, you get:

(lldb) p cpp_object_ptr

(lldb) p cpp_object_ptr->cpp_method()

That's not a cross language expression, that's a perfectly good C++ expression. 
 I don't think it makes sense to present simple variables of all language 
types, but not evaluate expressions using all the available expression parsers, 
starting with the obvious one first.

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

[Lldb-commits] [lldb] d5a277d - [lldb] Store SupportFile in FileEntry (NFC) (#85468)

2024-03-15 Thread via lldb-commits


Author: Jonas Devlieghere
Date: 2024-03-15T15:03:54-07:00
New Revision: d5a277d309e92b1d3e493da6036cffdf815105b1

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

LOG: [lldb] Store SupportFile in FileEntry (NFC) (#85468)

This is another step towards supporting DWARF5 checksums and inline
source code in LLDB.

Added: 


Modified: 
lldb/include/lldb/Core/Disassembler.h
lldb/include/lldb/Symbol/LineEntry.h
lldb/include/lldb/Utility/SupportFile.h
lldb/source/API/SBLineEntry.cpp
lldb/source/API/SBThread.cpp
lldb/source/Breakpoint/BreakpointResolver.cpp
lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
lldb/source/Commands/CommandObjectBreakpoint.cpp
lldb/source/Commands/CommandObjectSource.cpp
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Core/Address.cpp
lldb/source/Core/Disassembler.cpp
lldb/source/Core/FormatEntity.cpp
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/source/Core/SourceManager.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
lldb/source/Symbol/CompileUnit.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Symbol/LineEntry.cpp
lldb/source/Symbol/LineTable.cpp
lldb/source/Symbol/SymbolContext.cpp
lldb/source/Target/StackFrame.cpp
lldb/source/Target/StackFrameList.cpp
lldb/source/Target/Thread.cpp
lldb/source/Target/TraceDumper.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Disassembler.h 
b/lldb/include/lldb/Core/Disassembler.h
index 885ac1bb4a7ef8..e037a49f152c74 100644
--- a/lldb/include/lldb/Core/Disassembler.h
+++ b/lldb/include/lldb/Core/Disassembler.h
@@ -538,7 +538,7 @@ class Disassembler : public 
std::enable_shared_from_this,
   ElideMixedSourceAndDisassemblyLine(const ExecutionContext &exe_ctx,
  const SymbolContext &sc, LineEntry &line) 
{
 SourceLine sl;
-sl.file = line.file;
+sl.file = line.GetFile();
 sl.line = line.line;
 sl.column = line.column;
 return ElideMixedSourceAndDisassemblyLine(exe_ctx, sc, sl);

diff  --git a/lldb/include/lldb/Symbol/LineEntry.h 
b/lldb/include/lldb/Symbol/LineEntry.h
index c2daba916e3f98..eb8ad6ee9368d2 100644
--- a/lldb/include/lldb/Symbol/LineEntry.h
+++ b/lldb/include/lldb/Symbol/LineEntry.h
@@ -130,18 +130,28 @@ struct LineEntry {
   /// Shared pointer to the target this LineEntry belongs to.
   void ApplyFileMappings(lldb::TargetSP target_sp);
 
-  // Member variables.
-  AddressRange range; ///< The section offset address range for this line 
entry.
-  FileSpec file; ///< The source file, possibly mapped by the target.source-map
- ///setting
-  lldb::SupportFileSP
-  original_file_sp; ///< The original source file, from debug info.
-  uint32_t line = LLDB_INVALID_LINE_NUMBER; ///< The source line number, or 
zero
-///< if there is no line number
-/// information.
-  uint16_t column =
-  0; ///< The column number of the source line, or zero if there
- /// is no column information.
+  const FileSpec &GetFile() const {
+assert(file_sp);
+return file_sp->GetSpecOnly();
+  }
+
+  /// The section offset address range for this line entry.
+  AddressRange range;
+
+  /// The source file, possibly mapped by the target.source-map setting.
+  lldb::SupportFileSP file_sp;
+
+  /// The original source file, from debug info.
+  lldb::SupportFileSP original_file_sp;
+
+  /// The source line number, or LLDB_INVALID_LINE_NUMBER if there is no line
+  /// number information.
+  uint32_t line = LLDB_INVALID_LINE_NUMBER;
+
+  /// The column number of the source line, or zero if there is no column
+  /// information.
+  uint16_t column = 0;
+
   uint16_t is_start_of_statement : 1, ///< Indicates this entry is the 
beginning
   ///of a statement.
   is_start_of_basic_block : 1, ///< Indicates this entry is the beginning 
of

diff  --git a/lldb/include/lldb/Utility/SupportFile.h 
b/lldb/include/lldb/Utility/SupportFile.h
index 0ea0ca4e7c97a1..7505d7f345c5dd 100644
--- a/lldb/include/lldb/Utility/SupportFile.h
+++ b/lldb/include/lldb/Utility/SupportFile.h
@@ -45,6 +45,9 @@ class SupportFile {
   /// Materialize the file to disk and return the path to that temporary file.
   virtual const FileSpec &Materialize() { return m_file_spec; }
 
+  /// Change the file name.
+  void Update(const FileSpec &file_spec) { m_file_spec = file_spec; }
+
 protected:
   FileSpec m_file_spec;
   Checksum m_checksum;

diff  --git a/lldb/source/API/SBLineEntry.cpp b/lldb/source/API/SBLineEntry.cpp
index 28d12e65fdaf8a..f9fd750c2ee64e 100644

[Lldb-commits] [lldb] [lldb] Store SupportFile in FileEntry (NFC) (PR #85468)

2024-03-15 Thread Jonas Devlieghere via lldb-commits


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

[Lldb-commits] [lldb] [lldb] Fix dwim-print to not delete non-result persistent variables (PR #85152)

2024-03-15 Thread Dave Lee via lldb-commits


kastiglione wrote:

I don't consider it a failure to not be able to perform mixed language 
expressions. But it would have to be presented to the user in a way that is 
understandable, which could be a challenge.

Anyway, I pushed the change that scopes the lookup to the expected language.

thank you for the reviews

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

[Lldb-commits] [lldb] [lldb] Fix dwim-print to not delete non-result persistent variables (PR #85152)

2024-03-15 Thread Dave Lee via lldb-commits


https://github.com/kastiglione updated 
https://github.com/llvm/llvm-project/pull/85152

>From 970cf82fa3d64c5a4e1b3929c110b42974ef13cd Mon Sep 17 00:00:00 2001
From: Dave Lee 
Date: Wed, 13 Mar 2024 14:49:23 -0700
Subject: [PATCH 1/3] [lldb] Fix dwim-print to not delete non-result persistent
 variables

---
 lldb/source/Commands/CommandObjectDWIMPrint.cpp| 12 ++--
 lldb/test/API/commands/dwim-print/TestDWIMPrint.py | 12 
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp 
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index b183cb423111fb..5c043dfd101be6 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -170,6 +170,14 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 ExpressionResults expr_result = target.EvaluateExpression(
 expr, exe_scope, valobj_sp, eval_options, &fixed_expression);
 
+auto persistent_name = valobj_sp->GetName();
+// EvaluateExpression doesn't generate a new persistent result (`$0`) when
+// the expression is already just a persistent variable (`$var`). Instead,
+// the same persistent variable is reused. Take note of when a persistent
+// result is created, to prevent unintentional deletion of a user's
+// persistent variable.
+bool did_persist_result = persistent_name != expr;
+
 // Only mention Fix-Its if the expression evaluator applied them.
 // Compiler errors refer to the final expression after applying Fix-It(s).
 if (!fixed_expression.empty() && target.GetEnableNotifyAboutFixIts()) {
@@ -199,9 +207,9 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 }
   }
 
-  if (suppress_result)
+  if (did_persist_result && suppress_result)
 if (auto result_var_sp =
-target.GetPersistentVariable(valobj_sp->GetName())) {
+target.GetPersistentVariable(persistent_name)) {
   auto language = valobj_sp->GetPreferredDisplayLanguage();
   if (auto *persistent_state =
   target.GetPersistentExpressionStateForLanguage(language))
diff --git a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py 
b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
index 040632096c70e7..c650b1e3533e08 100644
--- a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -146,3 +146,15 @@ def test_void_result(self):
 self, "// break here", lldb.SBFileSpec("main.c")
 )
 self.expect("dwim-print (void)15", matching=False, 
patterns=["(?i)error"])
+
+def test_preserves_persistent_variables(self):
+"""Test dwim-print does not delete persistent variables."""
+self.build()
+lldbutil.run_to_source_breakpoint(
+self, "// break here", lldb.SBFileSpec("main.c")
+)
+self.expect("dwim-print int $i = 15")
+# Run the same expression twice and verify success. This ensures the
+# first command does not delete the persistent variable.
+for _ in range(2):
+self.expect("dwim-print $i", startstr="(int) 15")

>From 6503e22c894d9ed3f0d94d30a2165cf7f1914e47 Mon Sep 17 00:00:00 2001
From: Dave Lee 
Date: Fri, 15 Mar 2024 11:02:24 -0700
Subject: [PATCH 2/3] Try expr as a persistent variable

---
 .../Commands/CommandObjectDWIMPrint.cpp   | 24 +--
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp 
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 5c043dfd101be6..9db0c5a1e73ea1 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -23,7 +23,6 @@
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-forward.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Support/FormatVariadic.h"
 
 #include 
 
@@ -161,7 +160,16 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 }
   }
 
-  // Second, also lastly, try `expr` as a source expression to evaluate.
+  // Second, try `expr` as a persistent variable.
+  if (expr.starts_with("$"))
+if (auto var_sp = target.GetPersistentVariable(ConstString(expr)))
+  if (auto valobj_sp = var_sp->GetValueObject()) {
+valobj_sp->Dump(result.GetOutputStream(), dump_options);
+result.SetStatus(eReturnStatusSuccessFinishResult);
+return;
+  }
+
+  // Third, and lastly, try `expr` as a source expression to evaluate.
   {
 auto *exe_scope = m_exe_ctx.GetBestExecutionContextScope();
 ValueObjectSP valobj_sp;
@@ -170,14 +178,6 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 ExpressionResults expr_result = target.EvaluateExpression(
 expr, exe_scope, valobj_sp, eval_options, &fixed_expression);
 
-auto persistent_name = valobj_sp->GetName();
-// EvaluateExpression doesn't generate a new persis

[Lldb-commits] [lldb] [lldb] Store SupportFile in FileEntry (NFC) (PR #85468)

2024-03-15 Thread Jonas Devlieghere via lldb-commits


https://github.com/JDevlieghere updated 
https://github.com/llvm/llvm-project/pull/85468

>From 0caa7e713711c8211a10d3e6a249213746d41e9e Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Fri, 15 Mar 2024 11:42:44 -0700
Subject: [PATCH 1/3] [lldb] Outline Doxygen comments in LineEntry.h (NFC)

---
 lldb/include/lldb/Symbol/LineEntry.h | 28 
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/lldb/include/lldb/Symbol/LineEntry.h 
b/lldb/include/lldb/Symbol/LineEntry.h
index c2daba916e3f98..31d90eb6c256e5 100644
--- a/lldb/include/lldb/Symbol/LineEntry.h
+++ b/lldb/include/lldb/Symbol/LineEntry.h
@@ -130,18 +130,22 @@ struct LineEntry {
   /// Shared pointer to the target this LineEntry belongs to.
   void ApplyFileMappings(lldb::TargetSP target_sp);
 
-  // Member variables.
-  AddressRange range; ///< The section offset address range for this line 
entry.
-  FileSpec file; ///< The source file, possibly mapped by the target.source-map
- ///setting
-  lldb::SupportFileSP
-  original_file_sp; ///< The original source file, from debug info.
-  uint32_t line = LLDB_INVALID_LINE_NUMBER; ///< The source line number, or 
zero
-///< if there is no line number
-/// information.
-  uint16_t column =
-  0; ///< The column number of the source line, or zero if there
- /// is no column information.
+  /// The section offset address range for this line entry.
+  AddressRange range;
+
+  /// The source file, possibly mapped by the target.source-map setting.
+  FileSpec file;
+
+  /// The original source file, from debug info.
+  lldb::SupportFileSP original_file_sp;
+
+  /// The source line number, or zero if there is no line number information.
+  uint32_t line = LLDB_INVALID_LINE_NUMBER;
+
+  /// The column number of the source line, or zero if there is no column
+  /// information.
+  uint16_t column = 0;
+
   uint16_t is_start_of_statement : 1, ///< Indicates this entry is the 
beginning
   ///of a statement.
   is_start_of_basic_block : 1, ///< Indicates this entry is the beginning 
of

>From bc239e02430811818220a165c33bf00f6597b05d Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Fri, 15 Mar 2024 11:46:10 -0700
Subject: [PATCH 2/3] [lldb] Store SupportFile in FileEntry (NFC)

This is another step towards supporting DWARF5 checksums and inline
source code in LLDB.
---
 lldb/include/lldb/Core/Disassembler.h   |  2 +-
 lldb/include/lldb/Symbol/LineEntry.h|  7 ++-
 lldb/include/lldb/Utility/SupportFile.h |  3 +++
 lldb/source/API/SBLineEntry.cpp | 10 +-
 lldb/source/API/SBThread.cpp|  2 +-
 lldb/source/Breakpoint/BreakpointResolver.cpp   |  2 +-
 .../Breakpoint/BreakpointResolverFileLine.cpp   |  7 ---
 .../source/Commands/CommandObjectBreakpoint.cpp |  4 ++--
 lldb/source/Commands/CommandObjectSource.cpp| 14 +++---
 lldb/source/Commands/CommandObjectThread.cpp|  2 +-
 lldb/source/Core/Address.cpp|  2 +-
 lldb/source/Core/Disassembler.cpp   |  8 
 lldb/source/Core/FormatEntity.cpp   |  2 +-
 lldb/source/Core/IOHandlerCursesGUI.cpp | 11 ++-
 lldb/source/Core/SourceManager.cpp  |  2 +-
 .../Clang/ClangExpressionSourceCode.cpp |  2 +-
 .../Plugins/Language/CPlusPlus/LibCxx.cpp   |  4 ++--
 lldb/source/Symbol/CompileUnit.cpp  |  2 +-
 lldb/source/Symbol/Function.cpp |  4 ++--
 lldb/source/Symbol/LineEntry.cpp| 17 ++---
 lldb/source/Symbol/LineTable.cpp|  4 ++--
 lldb/source/Symbol/SymbolContext.cpp|  4 ++--
 lldb/source/Target/StackFrame.cpp   |  2 +-
 lldb/source/Target/StackFrameList.cpp   |  4 ++--
 lldb/source/Target/Thread.cpp   |  8 
 lldb/source/Target/TraceDumper.cpp  |  4 ++--
 26 files changed, 73 insertions(+), 60 deletions(-)

diff --git a/lldb/include/lldb/Core/Disassembler.h 
b/lldb/include/lldb/Core/Disassembler.h
index 885ac1bb4a7ef8..e037a49f152c74 100644
--- a/lldb/include/lldb/Core/Disassembler.h
+++ b/lldb/include/lldb/Core/Disassembler.h
@@ -538,7 +538,7 @@ class Disassembler : public 
std::enable_shared_from_this,
   ElideMixedSourceAndDisassemblyLine(const ExecutionContext &exe_ctx,
  const SymbolContext &sc, LineEntry &line) 
{
 SourceLine sl;
-sl.file = line.file;
+sl.file = line.GetFile();
 sl.line = line.line;
 sl.column = line.column;
 return ElideMixedSourceAndDisassemblyLine(exe_ctx, sc, sl);
diff --git a/lldb/include/lldb/Symbol/LineEntry.h 
b/lldb/include/lldb/Symbol/LineEntry.h
index 31d90eb6c256e5..aff476f19bcb93 100644
--- a/lldb/include/lldb/Symbol/LineEntry.h
+++ b/lldb/include/lldb/Symbol/Li

[Lldb-commits] [lldb] [lldb] Fix dwim-print to not delete non-result persistent variables (PR #85152)

2024-03-15 Thread via lldb-commits

jimingham wrote:

The counter to this is that `dwim-print` differs from `frame var` in that it 
supports complex expressions.  So having it work for simple statements of a 
variable name (what frame var and target var already do just fine) but then 
fail when the SAME name is used in a slightly more complicated expression is a 
serious failure to do its particular job.

Jim

> On Mar 15, 2024, at 2:28 PM, Dave Lee ***@***.***> wrote:
> 
> 
> I'll change this to scope lookup of persistent variables to the expected 
> language.
> 
> However, there's an argument to be made that dwim-print $my_var should work 
> anywhere. I think that follows the spirit of "do what I mean". Maybe in the 
> future I'll propose that in a separate change.
> 
> —
> Reply to this email directly, view it on GitHub 
> , or 
> unsubscribe 
> .
> You are receiving this because you are on a team that was mentioned.
> 

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

[Lldb-commits] [lldb] [lldb] Store SupportFile in FileEntry (NFC) (PR #85468)

2024-03-15 Thread Jonas Devlieghere via lldb-commits



@@ -130,18 +130,27 @@ struct LineEntry {
   /// Shared pointer to the target this LineEntry belongs to.
   void ApplyFileMappings(lldb::TargetSP target_sp);
 
-  // Member variables.
-  AddressRange range; ///< The section offset address range for this line 
entry.
-  FileSpec file; ///< The source file, possibly mapped by the target.source-map
- ///setting
-  lldb::SupportFileSP
-  original_file_sp; ///< The original source file, from debug info.
-  uint32_t line = LLDB_INVALID_LINE_NUMBER; ///< The source line number, or 
zero
-///< if there is no line number
-/// information.
-  uint16_t column =
-  0; ///< The column number of the source line, or zero if there
- /// is no column information.
+  const FileSpec &GetFile() const {
+assert(file_sp);
+return file_sp->GetSpecOnly();
+  }
+
+  /// The section offset address range for this line entry.
+  AddressRange range;
+
+  /// The source file, possibly mapped by the target.source-map setting.
+  lldb::SupportFileSP file_sp;
+
+  /// The original source file, from debug info.
+  lldb::SupportFileSP original_file_sp;
+
+  /// The source line number, or zero if there is no line number information.
+  uint32_t line = LLDB_INVALID_LINE_NUMBER;

JDevlieghere wrote:

Actually... it's not: `LLDB_INVALID_LINE_NUMBER` is `UINT32_MAX`. I'll update 
the comment since I'm already touching the line. 

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

[Lldb-commits] [lldb] [lldb] Store SupportFile in FileEntry (NFC) (PR #85468)

2024-03-15 Thread Adrian Prantl via lldb-commits


https://github.com/adrian-prantl approved this pull request.


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

[Lldb-commits] [lldb] [lldb] Store SupportFile in FileEntry (NFC) (PR #85468)

2024-03-15 Thread Adrian Prantl via lldb-commits



@@ -130,18 +130,27 @@ struct LineEntry {
   /// Shared pointer to the target this LineEntry belongs to.
   void ApplyFileMappings(lldb::TargetSP target_sp);
 
-  // Member variables.
-  AddressRange range; ///< The section offset address range for this line 
entry.
-  FileSpec file; ///< The source file, possibly mapped by the target.source-map
- ///setting
-  lldb::SupportFileSP
-  original_file_sp; ///< The original source file, from debug info.
-  uint32_t line = LLDB_INVALID_LINE_NUMBER; ///< The source line number, or 
zero
-///< if there is no line number
-/// information.
-  uint16_t column =
-  0; ///< The column number of the source line, or zero if there
- /// is no column information.
+  const FileSpec &GetFile() const {
+assert(file_sp);
+return file_sp->GetSpecOnly();
+  }
+
+  /// The section offset address range for this line entry.
+  AddressRange range;
+
+  /// The source file, possibly mapped by the target.source-map setting.
+  lldb::SupportFileSP file_sp;
+
+  /// The original source file, from debug info.
+  lldb::SupportFileSP original_file_sp;
+
+  /// The source line number, or zero if there is no line number information.
+  uint32_t line = LLDB_INVALID_LINE_NUMBER;

adrian-prantl wrote:

Is LLDB_INVALID_LINE_NUMBER 0?

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

[Lldb-commits] [lldb] [lldb] Fix dwim-print to not delete non-result persistent variables (PR #85152)

2024-03-15 Thread Dave Lee via lldb-commits


kastiglione wrote:

I'll change this to scope lookup of persistent variables to the expected 
language.

However, there's an argument to be made that `dwim-print $my_var` should work 
anywhere. I think that follows the spirit of "do what I mean". Maybe in the 
future I'll propose that in a separate change.

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

[Lldb-commits] [lldb] [lldb] Store SupportFile in FileEntry (NFC) (PR #85468)

2024-03-15 Thread via lldb-commits


llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)


Changes

This is another step towards supporting DWARF5 checksums and inline
source code in LLDB.

---

Patch is 28.14 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/85468.diff


26 Files Affected:

- (modified) lldb/include/lldb/Core/Disassembler.h (+1-1) 
- (modified) lldb/include/lldb/Symbol/LineEntry.h (+21-12) 
- (modified) lldb/include/lldb/Utility/SupportFile.h (+3) 
- (modified) lldb/source/API/SBLineEntry.cpp (+5-5) 
- (modified) lldb/source/API/SBThread.cpp (+1-1) 
- (modified) lldb/source/Breakpoint/BreakpointResolver.cpp (+1-1) 
- (modified) lldb/source/Breakpoint/BreakpointResolverFileLine.cpp (+4-3) 
- (modified) lldb/source/Commands/CommandObjectBreakpoint.cpp (+2-2) 
- (modified) lldb/source/Commands/CommandObjectSource.cpp (+7-7) 
- (modified) lldb/source/Commands/CommandObjectThread.cpp (+1-1) 
- (modified) lldb/source/Core/Address.cpp (+1-1) 
- (modified) lldb/source/Core/Disassembler.cpp (+4-4) 
- (modified) lldb/source/Core/FormatEntity.cpp (+1-1) 
- (modified) lldb/source/Core/IOHandlerCursesGUI.cpp (+6-5) 
- (modified) lldb/source/Core/SourceManager.cpp (+1-1) 
- (modified) 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp (+1-1) 
- (modified) lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp (+2-2) 
- (modified) lldb/source/Symbol/CompileUnit.cpp (+1-1) 
- (modified) lldb/source/Symbol/Function.cpp (+2-2) 
- (modified) lldb/source/Symbol/LineEntry.cpp (+10-7) 
- (modified) lldb/source/Symbol/LineTable.cpp (+2-2) 
- (modified) lldb/source/Symbol/SymbolContext.cpp (+2-2) 
- (modified) lldb/source/Target/StackFrame.cpp (+1-1) 
- (modified) lldb/source/Target/StackFrameList.cpp (+2-2) 
- (modified) lldb/source/Target/Thread.cpp (+4-4) 
- (modified) lldb/source/Target/TraceDumper.cpp (+2-2) 


``diff
diff --git a/lldb/include/lldb/Core/Disassembler.h 
b/lldb/include/lldb/Core/Disassembler.h
index 885ac1bb4a7ef8..e037a49f152c74 100644
--- a/lldb/include/lldb/Core/Disassembler.h
+++ b/lldb/include/lldb/Core/Disassembler.h
@@ -538,7 +538,7 @@ class Disassembler : public 
std::enable_shared_from_this,
   ElideMixedSourceAndDisassemblyLine(const ExecutionContext &exe_ctx,
  const SymbolContext &sc, LineEntry &line) 
{
 SourceLine sl;
-sl.file = line.file;
+sl.file = line.GetFile();
 sl.line = line.line;
 sl.column = line.column;
 return ElideMixedSourceAndDisassemblyLine(exe_ctx, sc, sl);
diff --git a/lldb/include/lldb/Symbol/LineEntry.h 
b/lldb/include/lldb/Symbol/LineEntry.h
index c2daba916e3f98..aff476f19bcb93 100644
--- a/lldb/include/lldb/Symbol/LineEntry.h
+++ b/lldb/include/lldb/Symbol/LineEntry.h
@@ -130,18 +130,27 @@ struct LineEntry {
   /// Shared pointer to the target this LineEntry belongs to.
   void ApplyFileMappings(lldb::TargetSP target_sp);
 
-  // Member variables.
-  AddressRange range; ///< The section offset address range for this line 
entry.
-  FileSpec file; ///< The source file, possibly mapped by the target.source-map
- ///setting
-  lldb::SupportFileSP
-  original_file_sp; ///< The original source file, from debug info.
-  uint32_t line = LLDB_INVALID_LINE_NUMBER; ///< The source line number, or 
zero
-///< if there is no line number
-/// information.
-  uint16_t column =
-  0; ///< The column number of the source line, or zero if there
- /// is no column information.
+  const FileSpec &GetFile() const {
+assert(file_sp);
+return file_sp->GetSpecOnly();
+  }
+
+  /// The section offset address range for this line entry.
+  AddressRange range;
+
+  /// The source file, possibly mapped by the target.source-map setting.
+  lldb::SupportFileSP file_sp;
+
+  /// The original source file, from debug info.
+  lldb::SupportFileSP original_file_sp;
+
+  /// The source line number, or zero if there is no line number information.
+  uint32_t line = LLDB_INVALID_LINE_NUMBER;
+
+  /// The column number of the source line, or zero if there is no column
+  /// information.
+  uint16_t column = 0;
+
   uint16_t is_start_of_statement : 1, ///< Indicates this entry is the 
beginning
   ///of a statement.
   is_start_of_basic_block : 1, ///< Indicates this entry is the beginning 
of
diff --git a/lldb/include/lldb/Utility/SupportFile.h 
b/lldb/include/lldb/Utility/SupportFile.h
index 0ea0ca4e7c97a1..7505d7f345c5dd 100644
--- a/lldb/include/lldb/Utility/SupportFile.h
+++ b/lldb/include/lldb/Utility/SupportFile.h
@@ -45,6 +45,9 @@ class SupportFile {
   /// Materialize the file to disk and return the path to that temporary file.
   virtual const FileSpec &Materialize() { return m_file_spec; }
 
+  /// Change the file name.
+  void Update(const FileSpec &file_spec) { m_file_spec = file_spec; }
+

[Lldb-commits] [lldb] [lldb] Store SupportFile in FileEntry (NFC) (PR #85468)

2024-03-15 Thread Jonas Devlieghere via lldb-commits


https://github.com/JDevlieghere created 
https://github.com/llvm/llvm-project/pull/85468

This is another step towards supporting DWARF5 checksums and inline
source code in LLDB.

>From 0caa7e713711c8211a10d3e6a249213746d41e9e Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Fri, 15 Mar 2024 11:42:44 -0700
Subject: [PATCH 1/2] [lldb] Outline Doxygen comments in LineEntry.h (NFC)

---
 lldb/include/lldb/Symbol/LineEntry.h | 28 
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/lldb/include/lldb/Symbol/LineEntry.h 
b/lldb/include/lldb/Symbol/LineEntry.h
index c2daba916e3f98..31d90eb6c256e5 100644
--- a/lldb/include/lldb/Symbol/LineEntry.h
+++ b/lldb/include/lldb/Symbol/LineEntry.h
@@ -130,18 +130,22 @@ struct LineEntry {
   /// Shared pointer to the target this LineEntry belongs to.
   void ApplyFileMappings(lldb::TargetSP target_sp);
 
-  // Member variables.
-  AddressRange range; ///< The section offset address range for this line 
entry.
-  FileSpec file; ///< The source file, possibly mapped by the target.source-map
- ///setting
-  lldb::SupportFileSP
-  original_file_sp; ///< The original source file, from debug info.
-  uint32_t line = LLDB_INVALID_LINE_NUMBER; ///< The source line number, or 
zero
-///< if there is no line number
-/// information.
-  uint16_t column =
-  0; ///< The column number of the source line, or zero if there
- /// is no column information.
+  /// The section offset address range for this line entry.
+  AddressRange range;
+
+  /// The source file, possibly mapped by the target.source-map setting.
+  FileSpec file;
+
+  /// The original source file, from debug info.
+  lldb::SupportFileSP original_file_sp;
+
+  /// The source line number, or zero if there is no line number information.
+  uint32_t line = LLDB_INVALID_LINE_NUMBER;
+
+  /// The column number of the source line, or zero if there is no column
+  /// information.
+  uint16_t column = 0;
+
   uint16_t is_start_of_statement : 1, ///< Indicates this entry is the 
beginning
   ///of a statement.
   is_start_of_basic_block : 1, ///< Indicates this entry is the beginning 
of

>From bc239e02430811818220a165c33bf00f6597b05d Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Fri, 15 Mar 2024 11:46:10 -0700
Subject: [PATCH 2/2] [lldb] Store SupportFile in FileEntry (NFC)

This is another step towards supporting DWARF5 checksums and inline
source code in LLDB.
---
 lldb/include/lldb/Core/Disassembler.h   |  2 +-
 lldb/include/lldb/Symbol/LineEntry.h|  7 ++-
 lldb/include/lldb/Utility/SupportFile.h |  3 +++
 lldb/source/API/SBLineEntry.cpp | 10 +-
 lldb/source/API/SBThread.cpp|  2 +-
 lldb/source/Breakpoint/BreakpointResolver.cpp   |  2 +-
 .../Breakpoint/BreakpointResolverFileLine.cpp   |  7 ---
 .../source/Commands/CommandObjectBreakpoint.cpp |  4 ++--
 lldb/source/Commands/CommandObjectSource.cpp| 14 +++---
 lldb/source/Commands/CommandObjectThread.cpp|  2 +-
 lldb/source/Core/Address.cpp|  2 +-
 lldb/source/Core/Disassembler.cpp   |  8 
 lldb/source/Core/FormatEntity.cpp   |  2 +-
 lldb/source/Core/IOHandlerCursesGUI.cpp | 11 ++-
 lldb/source/Core/SourceManager.cpp  |  2 +-
 .../Clang/ClangExpressionSourceCode.cpp |  2 +-
 .../Plugins/Language/CPlusPlus/LibCxx.cpp   |  4 ++--
 lldb/source/Symbol/CompileUnit.cpp  |  2 +-
 lldb/source/Symbol/Function.cpp |  4 ++--
 lldb/source/Symbol/LineEntry.cpp| 17 ++---
 lldb/source/Symbol/LineTable.cpp|  4 ++--
 lldb/source/Symbol/SymbolContext.cpp|  4 ++--
 lldb/source/Target/StackFrame.cpp   |  2 +-
 lldb/source/Target/StackFrameList.cpp   |  4 ++--
 lldb/source/Target/Thread.cpp   |  8 
 lldb/source/Target/TraceDumper.cpp  |  4 ++--
 26 files changed, 73 insertions(+), 60 deletions(-)

diff --git a/lldb/include/lldb/Core/Disassembler.h 
b/lldb/include/lldb/Core/Disassembler.h
index 885ac1bb4a7ef8..e037a49f152c74 100644
--- a/lldb/include/lldb/Core/Disassembler.h
+++ b/lldb/include/lldb/Core/Disassembler.h
@@ -538,7 +538,7 @@ class Disassembler : public 
std::enable_shared_from_this,
   ElideMixedSourceAndDisassemblyLine(const ExecutionContext &exe_ctx,
  const SymbolContext &sc, LineEntry &line) 
{
 SourceLine sl;
-sl.file = line.file;
+sl.file = line.GetFile();
 sl.line = line.line;
 sl.column = line.column;
 return ElideMixedSourceAndDisassemblyLine(exe_ctx, sc, sl);
diff --git a/lldb/include/lldb/Symbol/LineEntry.h 
b/lldb/include/lldb/Symbol/LineEntry.h
index 31d90eb6c256e5..aff476f1

[Lldb-commits] [lldb] [lldb] Show module name in progress update for downloading symbols (PR #85342)

2024-03-15 Thread Jonas Devlieghere via lldb-commits


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

[Lldb-commits] [lldb] b7dd601 - [lldb] Show module name in progress update for downloading symbols (#85342)

2024-03-15 Thread via lldb-commits


Author: Jonas Devlieghere
Date: 2024-03-15T12:34:34-07:00
New Revision: b7dd6012ebc06b6383cf1449058bf916da0eb4bc

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

LOG: [lldb] Show module name in progress update for downloading symbols (#85342)

Currently, we always show the argument passed to dsymForUUID in the
corresponding progress update. Most of the time this is a UUID, but it
can also be an absolute path. The former is pretty uninformative and the
latter needlessly noisy.

This changes the progress update to print the UUID and the module name,
if both are available. Otherwise, we print the UUID or the module name
depending on which one is available.

We now also unconditionally pass the module file spec and architecture
to DownloadObjectAndSymbolFile, while previously this was conditional on
the file existing on-disk. This should be harmless:

  - We already check that the file exists in DownloadObjectAndSymbolFile.
  - It doesn't make sense to check the filesystem for the architecutre.

rdar://124643548

Added: 


Modified: 
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index b2346c2402a81d..ae6c6d5479a198 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -3377,7 +3377,7 @@ class CommandObjectTargetModulesList : public 
CommandObjectParsed {
   case 'r': {
 size_t ref_count = 0;
 char in_shared_cache = 'Y';
-
+
 ModuleSP module_sp(module->shared_from_this());
 if (!ModuleList::ModuleIsInCache(module))
   in_shared_cache = 'N';
@@ -4508,11 +4508,8 @@ class CommandObjectTargetSymbolsAdd : public 
CommandObjectParsed {
 
 ModuleSpec module_spec;
 module_spec.GetUUID() = frame_module_sp->GetUUID();
-
-if (FileSystem::Instance().Exists(frame_module_sp->GetPlatformFileSpec())) 
{
-  module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
-  module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
-}
+module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
+module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
 
 if (!DownloadObjectAndSymbolFile(module_spec, result, flush)) {
   result.AppendError("unable to find debug symbols for the current frame");
@@ -4557,12 +4554,8 @@ class CommandObjectTargetSymbolsAdd : public 
CommandObjectParsed {
 
   ModuleSpec module_spec;
   module_spec.GetUUID() = frame_module_sp->GetUUID();
-
-  if (FileSystem::Instance().Exists(
-  frame_module_sp->GetPlatformFileSpec())) {
-module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
-module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
-  }
+  module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
+  module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
 
   bool current_frame_flush = false;
   if (DownloadObjectAndSymbolFile(module_spec, result, 
current_frame_flush))

diff  --git 
a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp 
b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
index f7df4650941a80..69595c6c14e4c0 100644
--- 
a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
+++ 
b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
@@ -1066,11 +1066,21 @@ bool 
SymbolLocatorDebugSymbols::DownloadObjectAndSymbolFile(
   command << lookup_arg;
 
   // Log and report progress.
+  std::string lookup_desc;
+  if (uuid_ptr && file_spec_ptr)
+lookup_desc =
+llvm::formatv("{0} ({1})", file_spec_ptr->GetFilename().GetString(),
+  uuid_ptr->GetAsString());
+  else if (uuid_ptr)
+lookup_desc = uuid_ptr->GetAsString();
+  else if (file_spec_ptr)
+lookup_desc = file_spec_ptr->GetFilename().GetString();
+
   Log *log = GetLog(LLDBLog::Host);
-  LLDB_LOG(log, "Calling {0} with {1} to find dSYM: {2}", dsymForUUID_exe_path,
-   lookup_arg, command.GetString());
+  LLDB_LOG(log, "Calling {0} for {1} to find dSYM: {2}", dsymForUUID_exe_path,
+   lookup_desc, command.GetString());
 
-  Progress progress("Downloading symbol file", lookup_arg);
+  Progress progress("Downloading symbol file for", lookup_desc);
 
   // Invoke dsymForUUID.
   int exit_status = -1;



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

[Lldb-commits] [lldb] [lldb] Show module name in progress update for downloading symbols (PR #85342)

2024-03-15 Thread Jonas Devlieghere via lldb-commits


https://github.com/JDevlieghere updated 
https://github.com/llvm/llvm-project/pull/85342

>From b50eb7442ab8f37659596efad19f8b30b145 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Thu, 14 Mar 2024 17:43:02 -0700
Subject: [PATCH 1/3] [lldb] Show module name in progress update for
 downloading symbols

Currently, we always show the argument passed to dsymForUUID in the
corresponding progress update. Most of the time this is a UUID, but it
can also be an absolute path. The former is pretty uninformative and the
latter needlessly noisy.

This changes the progress update to print the UUID and the module name,
if both are available. Otherwise, we print the UUID or the module name
depending on which one is available.

We now also unconditionally pass the module file spec and architecture
to DownloadObjectAndSymbolFile, while previously this was conditional on
the file existing on-disk. This should be harmless:

  - We already check that the file exists in DownloadObjectAndSymbolFile.
  - It doesn't make sense to check the filesystem for the architecutre.

rdar://124643548
---
 lldb/source/Commands/CommandObjectTarget.cpp| 17 +
 .../DebugSymbols/SymbolLocatorDebugSymbols.cpp  | 14 --
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index b2346c2402a81d..ae6c6d5479a198 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -3377,7 +3377,7 @@ class CommandObjectTargetModulesList : public 
CommandObjectParsed {
   case 'r': {
 size_t ref_count = 0;
 char in_shared_cache = 'Y';
-
+
 ModuleSP module_sp(module->shared_from_this());
 if (!ModuleList::ModuleIsInCache(module))
   in_shared_cache = 'N';
@@ -4508,11 +4508,8 @@ class CommandObjectTargetSymbolsAdd : public 
CommandObjectParsed {
 
 ModuleSpec module_spec;
 module_spec.GetUUID() = frame_module_sp->GetUUID();
-
-if (FileSystem::Instance().Exists(frame_module_sp->GetPlatformFileSpec())) 
{
-  module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
-  module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
-}
+module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
+module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
 
 if (!DownloadObjectAndSymbolFile(module_spec, result, flush)) {
   result.AppendError("unable to find debug symbols for the current frame");
@@ -4557,12 +4554,8 @@ class CommandObjectTargetSymbolsAdd : public 
CommandObjectParsed {
 
   ModuleSpec module_spec;
   module_spec.GetUUID() = frame_module_sp->GetUUID();
-
-  if (FileSystem::Instance().Exists(
-  frame_module_sp->GetPlatformFileSpec())) {
-module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
-module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
-  }
+  module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
+  module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
 
   bool current_frame_flush = false;
   if (DownloadObjectAndSymbolFile(module_spec, result, 
current_frame_flush))
diff --git 
a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp 
b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
index f7df4650941a80..acdf962a447266 100644
--- 
a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
+++ 
b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
@@ -1066,11 +1066,21 @@ bool 
SymbolLocatorDebugSymbols::DownloadObjectAndSymbolFile(
   command << lookup_arg;
 
   // Log and report progress.
+  std::string lookup_desc;
+  if (uuid_ptr && file_spec_ptr)
+lookup_desc =
+llvm::formatv("{0} ({1})", file_spec_ptr->GetFilename().GetString(),
+  uuid_ptr->GetAsString());
+  else if (uuid_ptr)
+lookup_desc = uuid_ptr->GetAsString();
+  else if (file_spec_ptr)
+lookup_desc = file_spec_ptr->GetFilename().GetString();
+
   Log *log = GetLog(LLDBLog::Host);
   LLDB_LOG(log, "Calling {0} with {1} to find dSYM: {2}", dsymForUUID_exe_path,
-   lookup_arg, command.GetString());
+   lookup_desc, command.GetString());
 
-  Progress progress("Downloading symbol file", lookup_arg);
+  Progress progress("Downloading symbol file", lookup_desc);
 
   // Invoke dsymForUUID.
   int exit_status = -1;

>From b23b1eedbf39f4d963927cbbb99a69ecb308f449 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Fri, 15 Mar 2024 12:14:53 -0700
Subject: [PATCH 2/3] Change wording

---
 .../SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp| 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp 
b/lldb/s

[Lldb-commits] [lldb] [lldb] Show module name in progress update for downloading symbols (PR #85342)

2024-03-15 Thread Adrian Prantl via lldb-commits



@@ -1066,11 +1066,21 @@ bool 
SymbolLocatorDebugSymbols::DownloadObjectAndSymbolFile(
   command << lookup_arg;
 
   // Log and report progress.
+  std::string lookup_desc;
+  if (uuid_ptr && file_spec_ptr)
+lookup_desc =
+llvm::formatv("{0} ({1})", file_spec_ptr->GetFilename().GetString(),
+  uuid_ptr->GetAsString());
+  else if (uuid_ptr)
+lookup_desc = uuid_ptr->GetAsString();
+  else if (file_spec_ptr)
+lookup_desc = file_spec_ptr->GetFilename().GetString();
+
   Log *log = GetLog(LLDBLog::Host);
   LLDB_LOG(log, "Calling {0} with {1} to find dSYM: {2}", dsymForUUID_exe_path,
-   lookup_arg, command.GetString());
+   lookup_desc, command.GetString());

adrian-prantl wrote:

Technically this log message also needs updating...

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

[Lldb-commits] [lldb] [lldb] Show module name in progress update for downloading symbols (PR #85342)

2024-03-15 Thread Adrian Prantl via lldb-commits


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

[Lldb-commits] [lldb] [lldb] Show module name in progress update for downloading symbols (PR #85342)

2024-03-15 Thread Adrian Prantl via lldb-commits


https://github.com/adrian-prantl approved this pull request.


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

[Lldb-commits] [lldb] [lldb] Show module name in progress update for downloading symbols (PR #85342)

2024-03-15 Thread Alex Langford via lldb-commits


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

+1 to Adrian's suggestion.

LGTM since you've addressed that now

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

[Lldb-commits] [lldb] [lldb] Show module name in progress update for downloading symbols (PR #85342)

2024-03-15 Thread Jonas Devlieghere via lldb-commits


https://github.com/JDevlieghere updated 
https://github.com/llvm/llvm-project/pull/85342

>From b50eb7442ab8f37659596efad19f8b30b145 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Thu, 14 Mar 2024 17:43:02 -0700
Subject: [PATCH 1/2] [lldb] Show module name in progress update for
 downloading symbols

Currently, we always show the argument passed to dsymForUUID in the
corresponding progress update. Most of the time this is a UUID, but it
can also be an absolute path. The former is pretty uninformative and the
latter needlessly noisy.

This changes the progress update to print the UUID and the module name,
if both are available. Otherwise, we print the UUID or the module name
depending on which one is available.

We now also unconditionally pass the module file spec and architecture
to DownloadObjectAndSymbolFile, while previously this was conditional on
the file existing on-disk. This should be harmless:

  - We already check that the file exists in DownloadObjectAndSymbolFile.
  - It doesn't make sense to check the filesystem for the architecutre.

rdar://124643548
---
 lldb/source/Commands/CommandObjectTarget.cpp| 17 +
 .../DebugSymbols/SymbolLocatorDebugSymbols.cpp  | 14 --
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index b2346c2402a81d..ae6c6d5479a198 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -3377,7 +3377,7 @@ class CommandObjectTargetModulesList : public 
CommandObjectParsed {
   case 'r': {
 size_t ref_count = 0;
 char in_shared_cache = 'Y';
-
+
 ModuleSP module_sp(module->shared_from_this());
 if (!ModuleList::ModuleIsInCache(module))
   in_shared_cache = 'N';
@@ -4508,11 +4508,8 @@ class CommandObjectTargetSymbolsAdd : public 
CommandObjectParsed {
 
 ModuleSpec module_spec;
 module_spec.GetUUID() = frame_module_sp->GetUUID();
-
-if (FileSystem::Instance().Exists(frame_module_sp->GetPlatformFileSpec())) 
{
-  module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
-  module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
-}
+module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
+module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
 
 if (!DownloadObjectAndSymbolFile(module_spec, result, flush)) {
   result.AppendError("unable to find debug symbols for the current frame");
@@ -4557,12 +4554,8 @@ class CommandObjectTargetSymbolsAdd : public 
CommandObjectParsed {
 
   ModuleSpec module_spec;
   module_spec.GetUUID() = frame_module_sp->GetUUID();
-
-  if (FileSystem::Instance().Exists(
-  frame_module_sp->GetPlatformFileSpec())) {
-module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
-module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
-  }
+  module_spec.GetFileSpec() = frame_module_sp->GetPlatformFileSpec();
+  module_spec.GetArchitecture() = frame_module_sp->GetArchitecture();
 
   bool current_frame_flush = false;
   if (DownloadObjectAndSymbolFile(module_spec, result, 
current_frame_flush))
diff --git 
a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp 
b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
index f7df4650941a80..acdf962a447266 100644
--- 
a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
+++ 
b/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp
@@ -1066,11 +1066,21 @@ bool 
SymbolLocatorDebugSymbols::DownloadObjectAndSymbolFile(
   command << lookup_arg;
 
   // Log and report progress.
+  std::string lookup_desc;
+  if (uuid_ptr && file_spec_ptr)
+lookup_desc =
+llvm::formatv("{0} ({1})", file_spec_ptr->GetFilename().GetString(),
+  uuid_ptr->GetAsString());
+  else if (uuid_ptr)
+lookup_desc = uuid_ptr->GetAsString();
+  else if (file_spec_ptr)
+lookup_desc = file_spec_ptr->GetFilename().GetString();
+
   Log *log = GetLog(LLDBLog::Host);
   LLDB_LOG(log, "Calling {0} with {1} to find dSYM: {2}", dsymForUUID_exe_path,
-   lookup_arg, command.GetString());
+   lookup_desc, command.GetString());
 
-  Progress progress("Downloading symbol file", lookup_arg);
+  Progress progress("Downloading symbol file", lookup_desc);
 
   // Invoke dsymForUUID.
   int exit_status = -1;

>From b23b1eedbf39f4d963927cbbb99a69ecb308f449 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Fri, 15 Mar 2024 12:14:53 -0700
Subject: [PATCH 2/2] Change wording

---
 .../SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp| 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/lldb/source/Plugins/SymbolLocator/DebugSymbols/SymbolLocatorDebugSymbols.cpp 
b/lldb/s

[Lldb-commits] [lldb] [lldb] Show module name in progress update for downloading symbols (PR #85342)

2024-03-15 Thread Greg Clayton via lldb-commits



@@ -1066,11 +1066,21 @@ bool 
SymbolLocatorDebugSymbols::DownloadObjectAndSymbolFile(
   command << lookup_arg;
 
   // Log and report progress.
+  std::string lookup_desc;
+  if (uuid_ptr && file_spec_ptr)
+lookup_desc =
+llvm::formatv("{0} ({1})", file_spec_ptr->GetFilename().GetString(),
+  uuid_ptr->GetAsString());
+  else if (uuid_ptr)
+lookup_desc = uuid_ptr->GetAsString();
+  else if (file_spec_ptr)
+lookup_desc = file_spec_ptr->GetFilename().GetString();
+
   Log *log = GetLog(LLDBLog::Host);
   LLDB_LOG(log, "Calling {0} with {1} to find dSYM: {2}", dsymForUUID_exe_path,
-   lookup_arg, command.GetString());
+   lookup_desc, command.GetString());
 
-  Progress progress("Downloading symbol file", lookup_arg);
+  Progress progress("Downloading symbol file", lookup_desc);

clayborg wrote:

I like Adrian's proposal. We could also do `"Downloading symbols for"` or 
`"Downloading debug info for"` as options.

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

[Lldb-commits] [lldb] [lldb] Show module name in progress update for downloading symbols (PR #85342)

2024-03-15 Thread Adrian Prantl via lldb-commits



@@ -1066,11 +1066,21 @@ bool 
SymbolLocatorDebugSymbols::DownloadObjectAndSymbolFile(
   command << lookup_arg;
 
   // Log and report progress.
+  std::string lookup_desc;
+  if (uuid_ptr && file_spec_ptr)
+lookup_desc =
+llvm::formatv("{0} ({1})", file_spec_ptr->GetFilename().GetString(),
+  uuid_ptr->GetAsString());
+  else if (uuid_ptr)
+lookup_desc = uuid_ptr->GetAsString();
+  else if (file_spec_ptr)
+lookup_desc = file_spec_ptr->GetFilename().GetString();
+
   Log *log = GetLog(LLDBLog::Host);
   LLDB_LOG(log, "Calling {0} with {1} to find dSYM: {2}", dsymForUUID_exe_path,
-   lookup_arg, command.GetString());
+   lookup_desc, command.GetString());
 
-  Progress progress("Downloading symbol file", lookup_arg);
+  Progress progress("Downloading symbol file", lookup_desc);

adrian-prantl wrote:

Right, this makes it sounds as if we are downloading `libfoo.dylib`. But we're 
downloading the symbol file for `libfoo.dylib`, which is, e.g., 
`libfoo.dylib.dSYM`. Hence the "for"

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

[Lldb-commits] [lldb] [lldb] Fix dwim-print to not delete non-result persistent variables (PR #85152)

2024-03-15 Thread via lldb-commits


jimingham wrote:

That's an okay approach if the only time an expression produces a result 
variable dwim-print should not delete is when a persistent variable is 
explicitly mentioned.  But anyway, since that's the case we're fixing, and 
checking as a persistent variable as a short-cut early on isn't a bad idea, it 
is fine for these purposes.

I think you need to get the persistent variables for whatever the language of 
the expression happens to be, not just get all the persistent variables. 
Otherwise, if you define a C persistent variable, then stop in a swift frame 
and do:

(lldb) p $works_in_c
10
(lldb) p $works_in_c + 10
unknown identifier "$works_in_c"

which would be confusing.


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

[Lldb-commits] [lldb] [lldb] Fix dwim-print to not delete non-result persistent variables (PR #85152)

2024-03-15 Thread Dave Lee via lldb-commits


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

[Lldb-commits] [lldb] [lldb] Fix dwim-print to not delete non-result persistent variables (PR #85152)

2024-03-15 Thread Dave Lee via lldb-commits



@@ -170,6 +170,14 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 ExpressionResults expr_result = target.EvaluateExpression(
 expr, exe_scope, valobj_sp, eval_options, &fixed_expression);
 
+auto persistent_name = valobj_sp->GetName();
+// EvaluateExpression doesn't generate a new persistent result (`$0`) when
+// the expression is already just a persistent variable (`$var`). Instead,
+// the same persistent variable is reused. Take note of when a persistent
+// result is created, to prevent unintentional deletion of a user's
+// persistent variable.
+bool did_persist_result = persistent_name != expr;

kastiglione wrote:

Jim, I have updated the PR to try the expression as a persistent variable – 
prior to expression evaluation. My thinking is, instead of evaluating first, 
then asking if a persistent variable was created, it's more straight to the 
point to first ask "is this a persistent variable".

I like the idea of trying the various kinds of variables in sequence: frame, 
target, persistent.

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

[Lldb-commits] [lldb] [lldb] Fix dwim-print to not delete non-result persistent variables (PR #85152)

2024-03-15 Thread Dave Lee via lldb-commits


https://github.com/kastiglione updated 
https://github.com/llvm/llvm-project/pull/85152

>From 970cf82fa3d64c5a4e1b3929c110b42974ef13cd Mon Sep 17 00:00:00 2001
From: Dave Lee 
Date: Wed, 13 Mar 2024 14:49:23 -0700
Subject: [PATCH 1/2] [lldb] Fix dwim-print to not delete non-result persistent
 variables

---
 lldb/source/Commands/CommandObjectDWIMPrint.cpp| 12 ++--
 lldb/test/API/commands/dwim-print/TestDWIMPrint.py | 12 
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp 
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index b183cb423111fb..5c043dfd101be6 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -170,6 +170,14 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 ExpressionResults expr_result = target.EvaluateExpression(
 expr, exe_scope, valobj_sp, eval_options, &fixed_expression);
 
+auto persistent_name = valobj_sp->GetName();
+// EvaluateExpression doesn't generate a new persistent result (`$0`) when
+// the expression is already just a persistent variable (`$var`). Instead,
+// the same persistent variable is reused. Take note of when a persistent
+// result is created, to prevent unintentional deletion of a user's
+// persistent variable.
+bool did_persist_result = persistent_name != expr;
+
 // Only mention Fix-Its if the expression evaluator applied them.
 // Compiler errors refer to the final expression after applying Fix-It(s).
 if (!fixed_expression.empty() && target.GetEnableNotifyAboutFixIts()) {
@@ -199,9 +207,9 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 }
   }
 
-  if (suppress_result)
+  if (did_persist_result && suppress_result)
 if (auto result_var_sp =
-target.GetPersistentVariable(valobj_sp->GetName())) {
+target.GetPersistentVariable(persistent_name)) {
   auto language = valobj_sp->GetPreferredDisplayLanguage();
   if (auto *persistent_state =
   target.GetPersistentExpressionStateForLanguage(language))
diff --git a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py 
b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
index 040632096c70e7..c650b1e3533e08 100644
--- a/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ b/lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -146,3 +146,15 @@ def test_void_result(self):
 self, "// break here", lldb.SBFileSpec("main.c")
 )
 self.expect("dwim-print (void)15", matching=False, 
patterns=["(?i)error"])
+
+def test_preserves_persistent_variables(self):
+"""Test dwim-print does not delete persistent variables."""
+self.build()
+lldbutil.run_to_source_breakpoint(
+self, "// break here", lldb.SBFileSpec("main.c")
+)
+self.expect("dwim-print int $i = 15")
+# Run the same expression twice and verify success. This ensures the
+# first command does not delete the persistent variable.
+for _ in range(2):
+self.expect("dwim-print $i", startstr="(int) 15")

>From 6503e22c894d9ed3f0d94d30a2165cf7f1914e47 Mon Sep 17 00:00:00 2001
From: Dave Lee 
Date: Fri, 15 Mar 2024 11:02:24 -0700
Subject: [PATCH 2/2] Try expr as a persistent variable

---
 .../Commands/CommandObjectDWIMPrint.cpp   | 24 +--
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp 
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 5c043dfd101be6..9db0c5a1e73ea1 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -23,7 +23,6 @@
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-forward.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Support/FormatVariadic.h"
 
 #include 
 
@@ -161,7 +160,16 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 }
   }
 
-  // Second, also lastly, try `expr` as a source expression to evaluate.
+  // Second, try `expr` as a persistent variable.
+  if (expr.starts_with("$"))
+if (auto var_sp = target.GetPersistentVariable(ConstString(expr)))
+  if (auto valobj_sp = var_sp->GetValueObject()) {
+valobj_sp->Dump(result.GetOutputStream(), dump_options);
+result.SetStatus(eReturnStatusSuccessFinishResult);
+return;
+  }
+
+  // Third, and lastly, try `expr` as a source expression to evaluate.
   {
 auto *exe_scope = m_exe_ctx.GetBestExecutionContextScope();
 ValueObjectSP valobj_sp;
@@ -170,14 +178,6 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
 ExpressionResults expr_result = target.EvaluateExpression(
 expr, exe_scope, valobj_sp, eval_options, &fixed_expression);
 
-auto persistent_name = valobj_sp->GetName();
-// EvaluateExpression doesn't generate a new persis

[Lldb-commits] [lldb] fd09d51 - [lldb] Add missing headers lldb/Host/Alarm.h

2024-03-15 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2024-03-15T09:55:15-07:00
New Revision: fd09d510d066583c088e4dbcf23ac0b500c5cc7a

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

LOG: [lldb] Add missing headers lldb/Host/Alarm.h

Added: 


Modified: 
lldb/include/lldb/Host/Alarm.h

Removed: 




diff  --git a/lldb/include/lldb/Host/Alarm.h b/lldb/include/lldb/Host/Alarm.h
index 7bdbc8f2b0ed74..23b1ff1af56890 100644
--- a/lldb/include/lldb/Host/Alarm.h
+++ b/lldb/include/lldb/Host/Alarm.h
@@ -13,6 +13,9 @@
 #include "lldb/lldb-types.h"
 #include "llvm/Support/Chrono.h"
 
+#include 
+#include 
+
 namespace lldb_private {
 
 /// \class Alarm abstraction that enables scheduling a callback function after 
a



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

[Lldb-commits] [lldb] [lldb] Add an Alarm class for coalescing progress reports (PR #85329)

2024-03-15 Thread Jonas Devlieghere via lldb-commits


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

[Lldb-commits] [lldb] f01a32f - [lldb] Add an Alarm class for coalescing progress reports (#85329)

2024-03-15 Thread via lldb-commits


Author: Jonas Devlieghere
Date: 2024-03-15T09:35:38-07:00
New Revision: f01a32f5c58b199edf7cd1492a20578453852f0e

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

LOG: [lldb] Add an Alarm class for coalescing progress reports (#85329)

The commit introduces a new, generic, Alarm class. The class lets you to
schedule functions (callbacks) that will execute after a predefined
timeout. Once scheduled, you can cancel and reset a callback, given the
timeout hasn't expired yet.

The alarm class worker thread that sleeps until the next timeout
expires. When the thread wakes up, it checks for all the callbacks that
have expired and calls them in order. Because the callback is called
from the worker thread, the only guarantee is that a callback is called
no sooner than the timeout. A long running callback could potentially
block the worker threads and delay other callbacks from getting called.

I intentionally kept the implementation as simple as possible while
addressing the needs for the use case of coalescing progress events as
discussed in [1]. If we want to rely on this somewhere else, we can
reassess whether we need to address this class' limitations.

[1] https://discourse.llvm.org/t/rfc-improve-lldb-progress-reporting/75717/

Added: 
lldb/include/lldb/Host/Alarm.h
lldb/source/Host/common/Alarm.cpp
lldb/unittests/Host/AlarmTest.cpp

Modified: 
lldb/source/Host/CMakeLists.txt
lldb/unittests/Host/CMakeLists.txt

Removed: 




diff  --git a/lldb/include/lldb/Host/Alarm.h b/lldb/include/lldb/Host/Alarm.h
new file mode 100644
index 00..7bdbc8f2b0ed74
--- /dev/null
+++ b/lldb/include/lldb/Host/Alarm.h
@@ -0,0 +1,112 @@
+//===-- Alarm.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_HOST_ALARM_H
+#define LLDB_HOST_ALARM_H
+
+#include "lldb/Host/HostThread.h"
+#include "lldb/lldb-types.h"
+#include "llvm/Support/Chrono.h"
+
+namespace lldb_private {
+
+/// \class Alarm abstraction that enables scheduling a callback function after 
a
+/// specified timeout. Creating an alarm for a callback returns a Handle that
+/// can be used to restart or cancel the alarm.
+class Alarm {
+public:
+  using Handle = uint64_t;
+  using Callback = std::function;
+  using TimePoint = llvm::sys::TimePoint<>;
+  using Duration = std::chrono::milliseconds;
+
+  Alarm(Duration timeout, bool run_callback_on_exit = false);
+  ~Alarm();
+
+  /// Create an alarm for the given callback. The alarm will expire and the
+  /// callback will be called after the timeout.
+  ///
+  /// \returns
+  ///   Handle which can be used to restart or cancel the alarm.
+  Handle Create(Callback callback);
+
+  /// Restart the alarm for the given Handle. The alarm will expire and the
+  /// callback will be called after the timeout.
+  ///
+  /// \returns
+  ///   True if the alarm was successfully restarted. False if there is no 
alarm
+  ///   for the given Handle or the alarm already expired.
+  bool Restart(Handle handle);
+
+  /// Cancel the alarm for the given Handle. The alarm and its handle will be
+  /// removed.
+  ///
+  /// \returns
+  ///   True if the alarm was successfully canceled and the Handle removed.
+  ///   False if there is no alarm for the given Handle or the alarm already
+  ///   expired.
+  bool Cancel(Handle handle);
+
+  static constexpr Handle INVALID_HANDLE = 0;
+
+private:
+  /// Helper functions to start, stop and check the status of the alarm thread.
+  /// @{
+  void StartAlarmThread();
+  void StopAlarmThread();
+  bool AlarmThreadRunning();
+  /// @}
+
+  /// Return an unique, monotonically increasing handle.
+  static Handle GetNextUniqueHandle();
+
+  /// Helper to compute the next time the alarm thread needs to wake up.
+  TimePoint GetNextExpiration() const;
+
+  /// Alarm entry.
+  struct Entry {
+Handle handle;
+Callback callback;
+TimePoint expiration;
+
+Entry(Callback callback, TimePoint expiration);
+bool operator==(const Entry &rhs) { return handle == rhs.handle; }
+  };
+
+  /// List of alarm entries.
+  std::vector m_entries;
+
+  /// Timeout between when an alarm is created and when it fires.
+  Duration m_timeout;
+
+  /// The alarm thread.
+  /// @{
+  HostThread m_alarm_thread;
+  lldb::thread_result_t AlarmThread();
+  /// @}
+
+  /// Synchronize access between the alarm thread and the main thread.
+  std::mutex m_alarm_mutex;
+
+  /// Condition variable used to wake up the alarm thread.
+  std::condition_variabl

[Lldb-commits] [lldb] [lldb] Add an Alarm class for coalescing progress reports (PR #85329)

2024-03-15 Thread Jonas Devlieghere via lldb-commits


https://github.com/JDevlieghere updated 
https://github.com/llvm/llvm-project/pull/85329

>From 1ee7c2ffb76f13caa2052bef5dbe4f1982c8bade Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Tue, 5 Mar 2024 22:57:43 -0800
Subject: [PATCH] [lldb] Add an Alarm class for coalescing progress reports

The commit introduces a new, generic, Alarm class. The class lets you to
schedule functions (callbacks) that will execute after a predefined
timeout. Once scheduled, you can cancel and reset a callback, given the
timeout hasn't expired yet.

The alarm class worker thread that sleeps until the next timeout
expires. When the thread wakes up, it checks for all the callbacks that
have expired and calls them in order. Because the callback is called
from the worker thread, the only guarantee is that a callback is called
no sooner than the timeout. A long running callback could potentially
block the worker threads and delay other callbacks from getting called.

I intentionally kept the implementation as simple as possible while
addressing the needs for the use case of coalescing progress events as
discussed in [1]. If we want to rely on this somewhere else, we can
reassess whether we need to address this class' limitations.

[1] https://discourse.llvm.org/t/rfc-improve-lldb-progress-reporting/75717/
---
 lldb/include/lldb/Host/Alarm.h | 112 +++
 lldb/source/Host/CMakeLists.txt|   1 +
 lldb/source/Host/common/Alarm.cpp  | 216 +
 lldb/unittests/Host/AlarmTest.cpp  | 159 +
 lldb/unittests/Host/CMakeLists.txt |   1 +
 5 files changed, 489 insertions(+)
 create mode 100644 lldb/include/lldb/Host/Alarm.h
 create mode 100644 lldb/source/Host/common/Alarm.cpp
 create mode 100644 lldb/unittests/Host/AlarmTest.cpp

diff --git a/lldb/include/lldb/Host/Alarm.h b/lldb/include/lldb/Host/Alarm.h
new file mode 100644
index 00..7bdbc8f2b0ed74
--- /dev/null
+++ b/lldb/include/lldb/Host/Alarm.h
@@ -0,0 +1,112 @@
+//===-- Alarm.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_HOST_ALARM_H
+#define LLDB_HOST_ALARM_H
+
+#include "lldb/Host/HostThread.h"
+#include "lldb/lldb-types.h"
+#include "llvm/Support/Chrono.h"
+
+namespace lldb_private {
+
+/// \class Alarm abstraction that enables scheduling a callback function after 
a
+/// specified timeout. Creating an alarm for a callback returns a Handle that
+/// can be used to restart or cancel the alarm.
+class Alarm {
+public:
+  using Handle = uint64_t;
+  using Callback = std::function;
+  using TimePoint = llvm::sys::TimePoint<>;
+  using Duration = std::chrono::milliseconds;
+
+  Alarm(Duration timeout, bool run_callback_on_exit = false);
+  ~Alarm();
+
+  /// Create an alarm for the given callback. The alarm will expire and the
+  /// callback will be called after the timeout.
+  ///
+  /// \returns
+  ///   Handle which can be used to restart or cancel the alarm.
+  Handle Create(Callback callback);
+
+  /// Restart the alarm for the given Handle. The alarm will expire and the
+  /// callback will be called after the timeout.
+  ///
+  /// \returns
+  ///   True if the alarm was successfully restarted. False if there is no 
alarm
+  ///   for the given Handle or the alarm already expired.
+  bool Restart(Handle handle);
+
+  /// Cancel the alarm for the given Handle. The alarm and its handle will be
+  /// removed.
+  ///
+  /// \returns
+  ///   True if the alarm was successfully canceled and the Handle removed.
+  ///   False if there is no alarm for the given Handle or the alarm already
+  ///   expired.
+  bool Cancel(Handle handle);
+
+  static constexpr Handle INVALID_HANDLE = 0;
+
+private:
+  /// Helper functions to start, stop and check the status of the alarm thread.
+  /// @{
+  void StartAlarmThread();
+  void StopAlarmThread();
+  bool AlarmThreadRunning();
+  /// @}
+
+  /// Return an unique, monotonically increasing handle.
+  static Handle GetNextUniqueHandle();
+
+  /// Helper to compute the next time the alarm thread needs to wake up.
+  TimePoint GetNextExpiration() const;
+
+  /// Alarm entry.
+  struct Entry {
+Handle handle;
+Callback callback;
+TimePoint expiration;
+
+Entry(Callback callback, TimePoint expiration);
+bool operator==(const Entry &rhs) { return handle == rhs.handle; }
+  };
+
+  /// List of alarm entries.
+  std::vector m_entries;
+
+  /// Timeout between when an alarm is created and when it fires.
+  Duration m_timeout;
+
+  /// The alarm thread.
+  /// @{
+  HostThread m_alarm_thread;
+  lldb::thread_result_t AlarmThread();
+  /// @}
+
+  /// Synchronize access between the alarm thread and the main thread.
+  std::mutex m_a

[Lldb-commits] [lldb] [lldb] Show module name in progress update for downloading symbols (PR #85342)

2024-03-15 Thread Jonas Devlieghere via lldb-commits



@@ -1066,11 +1066,21 @@ bool 
SymbolLocatorDebugSymbols::DownloadObjectAndSymbolFile(
   command << lookup_arg;
 
   // Log and report progress.
+  std::string lookup_desc;
+  if (uuid_ptr && file_spec_ptr)
+lookup_desc =
+llvm::formatv("{0} ({1})", file_spec_ptr->GetFilename().GetString(),
+  uuid_ptr->GetAsString());
+  else if (uuid_ptr)
+lookup_desc = uuid_ptr->GetAsString();
+  else if (file_spec_ptr)
+lookup_desc = file_spec_ptr->GetFilename().GetString();
+
   Log *log = GetLog(LLDBLog::Host);
   LLDB_LOG(log, "Calling {0} with {1} to find dSYM: {2}", dsymForUUID_exe_path,
-   lookup_arg, command.GetString());
+   lookup_desc, command.GetString());
 
-  Progress progress("Downloading symbol file", lookup_arg);
+  Progress progress("Downloading symbol file", lookup_desc);

JDevlieghere wrote:

The 3 possible status messages are:

```
Downloading symbol file: libfoo.dylib (----)
Downloading symbol file: ----
Downloading symbol file: libfoo.dylib
```

For when we have both the file and UUID, only the UUID or only the file. 

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

[Lldb-commits] [lldb] [lldb] Fix dwim-print to not delete non-result persistent variables (PR #85152)

2024-03-15 Thread Dave Lee via lldb-commits


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

[Lldb-commits] [lldb] [lldb] Show module name in progress update for downloading symbols (PR #85342)

2024-03-15 Thread Adrian Prantl via lldb-commits



@@ -1066,11 +1066,21 @@ bool 
SymbolLocatorDebugSymbols::DownloadObjectAndSymbolFile(
   command << lookup_arg;
 
   // Log and report progress.
+  std::string lookup_desc;
+  if (uuid_ptr && file_spec_ptr)
+lookup_desc =
+llvm::formatv("{0} ({1})", file_spec_ptr->GetFilename().GetString(),
+  uuid_ptr->GetAsString());
+  else if (uuid_ptr)
+lookup_desc = uuid_ptr->GetAsString();
+  else if (file_spec_ptr)
+lookup_desc = file_spec_ptr->GetFilename().GetString();
+
   Log *log = GetLog(LLDBLog::Host);
   LLDB_LOG(log, "Calling {0} with {1} to find dSYM: {2}", dsymForUUID_exe_path,
-   lookup_arg, command.GetString());
+   lookup_desc, command.GetString());
 
-  Progress progress("Downloading symbol file", lookup_arg);
+  Progress progress("Downloading symbol file", lookup_desc);

adrian-prantl wrote:

I think this needs to say `Downloading symbol file for` now, because otherwise 
it sounds like we're downloading `file_spec`?

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

[Lldb-commits] [lldb] [lldb] Show module name in progress update for downloading symbols (PR #85342)

2024-03-15 Thread Adrian Prantl via lldb-commits


https://github.com/adrian-prantl requested changes to this pull request.


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

[Lldb-commits] [lldb] [lldb] Show module name in progress update for downloading symbols (PR #85342)

2024-03-15 Thread Adrian Prantl via lldb-commits


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

[Lldb-commits] [lldb] [lldb] Add an Alarm class for coalescing progress reports (PR #85329)

2024-03-15 Thread Adrian Prantl via lldb-commits



@@ -0,0 +1,193 @@
+//===-- Alarm.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/Host/Alarm.h"
+#include "lldb/Host/ThreadLauncher.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+Alarm::Alarm(Duration timeout, bool run_callback_on_exit)
+: m_timeout(timeout), m_run_callbacks_on_exit(run_callback_on_exit) {
+  StartAlarmThread();
+}
+
+Alarm::~Alarm() { StopAlarmThread(); }
+
+Alarm::Handle Alarm::Create(std::function callback) {
+  // Gracefully deal with the unlikely event that the alarm thread failed to
+  // launch.
+  if (!AlarmThreadRunning())
+return INVALID_HANDLE;
+
+  // Compute the next expiration before we take the lock. This ensures that
+  // waiting on the lock doesn't eat into the timeout.
+  const TimePoint expiration = GetNextExpiration();
+
+  Handle handle = INVALID_HANDLE;
+
+  {
+std::lock_guard alarm_guard(m_alarm_mutex);
+
+// Create a new unique entry and remember its handle.
+m_entries.emplace_back(callback, expiration);
+handle = m_entries.back().handle;
+
+// Tell the alarm thread we need to recompute the next alarm.
+m_recompute_next_alarm = true;
+  }
+
+  m_alarm_cv.notify_one();
+  return handle;
+}
+
+bool Alarm::Restart(Handle handle) {
+  // Gracefully deal with the unlikely event that the alarm thread failed to
+  // launch.
+  if (!AlarmThreadRunning())
+return false;
+
+  // Compute the next expiration before we take the lock. This ensures that
+  // waiting on the lock doesn't eat into the timeout.
+  const TimePoint expiration = GetNextExpiration();
+
+  {
+std::lock_guard alarm_guard(m_alarm_mutex);
+
+// Find the entry corresponding to the given handle.
+const auto it =
+std::find_if(m_entries.begin(), m_entries.end(),
+ [handle](Entry &entry) { return entry.handle == handle; 
});
+if (it == m_entries.end())
+  return false;
+
+// Update the expiration.
+it->expiration = expiration;
+
+// Tell the alarm thread we need to recompute the next alarm.
+m_recompute_next_alarm = true;
+  }
+
+  m_alarm_cv.notify_one();
+  return true;
+}
+
+bool Alarm::Cancel(Handle handle) {
+  // Gracefully deal with the unlikely event that the alarm thread failed to
+  // launch.
+  if (!AlarmThreadRunning())
+return false;
+
+  {
+std::lock_guard alarm_guard(m_alarm_mutex);
+
+const auto it =
+std::find_if(m_entries.begin(), m_entries.end(),
+ [handle](Entry &entry) { return entry.handle == handle; 
});
+
+if (it == m_entries.end())
+  return false;
+
+m_entries.erase(it);
+  }
+
+  // No need to notify the alarm thread. This only affects the alarm thread if
+  // we removed the entry that corresponds to the next alarm. If that's the
+  // case, the thread will wake up as scheduled, find no expired events, and
+  // recompute the next alarm time.
+  return true;
+}
+
+Alarm::Entry::Entry(Alarm::Callback callback, Alarm::TimePoint expiration)
+: handle(Alarm::GetNextUniqueHandle()), callback(std::move(callback)),
+  expiration(std::move(expiration)) {}
+
+void Alarm::StartAlarmThread() {
+  if (!m_alarm_thread.IsJoinable()) {
+llvm::Expected alarm_thread = ThreadLauncher::LaunchThread(
+"lldb.debugger.alarm-thread", [this] { return AlarmThread(); },
+8 * 1024 * 1024); // Use larger 8MB stack for this thread
+if (alarm_thread) {
+  m_alarm_thread = *alarm_thread;
+} else {
+  LLDB_LOG_ERROR(GetLog(LLDBLog::Host), alarm_thread.takeError(),
+ "failed to launch host thread: {0}");
+}
+  }
+}
+
+void Alarm::StopAlarmThread() {
+  if (m_alarm_thread.IsJoinable()) {
+{
+  std::lock_guard alarm_guard(m_alarm_mutex);
+  m_exit = true;
+}
+m_alarm_cv.notify_one();
+m_alarm_thread.Join(nullptr);
+  }
+}
+
+bool Alarm::AlarmThreadRunning() { return m_alarm_thread.IsJoinable(); }
+
+lldb::thread_result_t Alarm::AlarmThread() {

adrian-prantl wrote:

I feel like this function could use some high-level comments above each code 
block

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

[Lldb-commits] [lldb] [lldb] Add an Alarm class for coalescing progress reports (PR #85329)

2024-03-15 Thread Adrian Prantl via lldb-commits



@@ -0,0 +1,164 @@
+//===-- AlarmTest.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/Host/Alarm.h"
+#include "gtest/gtest.h"
+
+#include 
+#include 
+
+using namespace lldb_private;
+using namespace std::chrono_literals;
+
+static constexpr auto ALARM_TIMEOUT = 500ms;
+static constexpr auto TEST_TIMEOUT = 1000ms;

adrian-prantl wrote:

We usually add a factor 10 when running under ASAN `#if 
__has_feature(address_sanitizer)`

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

[Lldb-commits] [lldb] [lldb] Add an Alarm class for coalescing progress reports (PR #85329)

2024-03-15 Thread Adrian Prantl via lldb-commits


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

[Lldb-commits] [lldb] [lldb] Add an Alarm class for coalescing progress reports (PR #85329)

2024-03-15 Thread Adrian Prantl via lldb-commits


https://github.com/adrian-prantl approved this pull request.


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

[Lldb-commits] [lldb] [lldb] Show module name in progress update for downloading symbols (PR #85342)

2024-03-15 Thread Med Ismail Bennani via lldb-commits


medismailben wrote:

LGTM!

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

57 matches

Mail list logo