[Lldb-commits] [PATCH] D142341: [LLDB][NFC] Fix valobj_sp null pointer checks in lldb/source/Plugins/Language/

2023-02-17 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione added inline comments.



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:225
   if (valobj_sp)
+SyntheticChildrenFrontEnd(*valobj_sp);
 Update();

Michael137 wrote:
> Michael137 wrote:
> > Michael137 wrote:
> > > this won't initialise the parent constructor though will it? Just creates 
> > > a temporary and immediately destructs it? You might need to put it back 
> > > into the initialiser list and use a ternary
> > Oh but `SyntheticChildrenFrontEnd` seems to always take a reference. The 
> > interfaces seem to be a little mismatched
> Perhaps the null-check in the constructor is just redundant and should 
> actually be an assert.
> 
> @jingham might know more about the assumptions here
@xgupta reiterating Michael's point, I think this change results in 
mis-construction. Have you run the test suite, I would expect some failures 
with this change.

I agree that this should be an assert. The other option would be to change the 
constructor's signature, using a `lldb::ValueObject &` instead of 
`lldb::ValueObjectSP`, which puts the onus on callers to handle any null-ness. 
Are we aware of any callers passing null?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142341

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

[Lldb-commits] [PATCH] D142341: [LLDB][NFC] Fix valobj_sp null pointer checks in lldb/source/Plugins/Language/

2023-02-17 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione added inline comments.



Comment at: lldb/source/Plugins/Language/ObjC/NSSet.cpp:673-675
   if (valobj_sp)
+SyntheticChildrenFrontEnd(*valobj_sp);
 Update();

by the way, these changes are missing `{}` around the `if` body.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142341

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

[Lldb-commits] [PATCH] D144230: [lldb] Make persisting result variables configurable

2023-02-17 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione updated this revision to Diff 498387.
kastiglione added a comment.

Add tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144230

Files:
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectExpression.h
  lldb/source/Commands/Options.td
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/test/API/commands/dwim-print/TestDWIMPrint.py
  lldb/test/API/commands/expression/persistent_result/Makefile
  lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
  lldb/test/API/commands/expression/persistent_result/main.c

Index: lldb/test/API/commands/expression/persistent_result/main.c
===
--- /dev/null
+++ lldb/test/API/commands/expression/persistent_result/main.c
@@ -0,0 +1,4 @@
+int main() {
+  int i = 30;
+  return 0; // break here
+}
Index: lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
@@ -0,0 +1,37 @@
+"""
+Test controlling `expression` result variables are persistent.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+def setUp(self):
+TestBase.setUp(self)
+self.build()
+lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c"))
+
+def test_enable_persistent_result(self):
+"""Test explicitly enabling result variables persistence."""
+self.expect("expression --persistent-result on -- i", substrs=["(int) $0 = 30"])
+# Verify the lifetime of $0 extends beyond the `expression` it was created in.
+self.expect("expression $0", substrs=["(int) $0 = 30"])
+
+def test_disable_persistent_result(self):
+"""Test explicitly disabling persistent result variables."""
+self.expect("expression --persistent-result off -- i", substrs=["(int) 30"])
+# Verify a persistent result was not silently created.
+self.expect("expression $0", error=True)
+
+def test_expression_persists_result(self):
+"""Test `expression`'s default behavior is to persist a result variable."""
+self.expect("expression i", substrs=["(int) $0 = 30"])
+self.expect("expression $0", substrs=["(int) $0 = 30"])
+
+def test_p_persists_result(self):
+"""Test `p` does persist a result variable."""
+self.expect("p i", substrs=["(int) $0 = 30"])
+self.expect("p $0", substrs=["(int) $0 = 30"])
Index: lldb/test/API/commands/expression/persistent_result/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/expression/persistent_result/Makefile
@@ -0,0 +1,3 @@
+OBJC_SOURCES := main.m
+
+include Makefile.rules
Index: lldb/test/API/commands/dwim-print/TestDWIMPrint.py
===
--- lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -16,7 +16,8 @@
 self.ci.HandleCommand(cmd, result)
 return result.GetOutput().rstrip()
 
-PERSISTENT_VAR = re.compile(r"\$\d+")
+VAR_IDENT_RAW = r"(?:\$\d+|\w+) = "
+VAR_IDENT = re.compile(VAR_IDENT_RAW)
 
 def _mask_persistent_var(self, string: str) -> str:
 """
@@ -24,8 +25,9 @@
 that matches any persistent result (r'\$\d+'). The returned string can
 be matched against other `expression` results.
 """
-before, after = self.PERSISTENT_VAR.split(string, maxsplit=1)
-return re.escape(before) + r"\$\d+" + re.escape(after)
+before, after = self.VAR_IDENT.split(string, maxsplit=1)
+# Support either a frame variable (\w+) or a persistent result (\$\d+).
+return re.escape(before) + self.VAR_IDENT_RAW + re.escape(after)
 
 def _expect_cmd(
 self,
@@ -51,7 +53,7 @@
 substrs = [f"note: ran `{resolved_cmd}`"]
 patterns = []
 
-if actual_cmd == "expression" and self.PERSISTENT_VAR.search(expected_output):
+if self.VAR_IDENT.search(expected_output):
 patterns.append(self._mask_persistent_var(expected_output))
 else:
 substrs.append(expected_output)
Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -425,7 +425,9 @@
 if (m_options.m_hide_pointer_value &&
 IsPointerValue(m_valobj->GetCompilerType())) {
 } else {
-  m_stream->Printf(" %s", m

[Lldb-commits] [PATCH] D144230: [lldb] Make persisting result variables configurable

2023-02-17 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione added inline comments.



Comment at: lldb/source/Commands/CommandObjectExpression.cpp:149
 
+  case 'C': {
+// 'C' for "caching", since both 'P' and 'p' for persist are taken. Both 
'R'

aprantl wrote:
> Do we expect people to actually use this flag or to just use an alias that 
> has this turned on? If the latter, maybe we should only provide a long 
> option, since the single-letter name space is precious?
Initially I was going to have only the long option, but I reconsidered because 
I don't know how frequently persistent results are used by users. In the 
future, if/when we make the persistent results opt-in (instead of opt-out), 
will people want to use persistent results sporadically? If so maybe they'd 
want a short flag to quickly opt-in.

> since the single-letter name space is precious

this is a compelling argument. I'll go with long option only to start. It's 
easier to add a short option later, than to remove it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144230

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

[Lldb-commits] [PATCH] D144230: [lldb] Make persisting result variables configurable

2023-02-17 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione updated this revision to Diff 498390.
kastiglione added a comment.

fix Makefile


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144230

Files:
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectExpression.h
  lldb/source/Commands/Options.td
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/test/API/commands/dwim-print/TestDWIMPrint.py
  lldb/test/API/commands/expression/persistent_result/Makefile
  lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
  lldb/test/API/commands/expression/persistent_result/main.c

Index: lldb/test/API/commands/expression/persistent_result/main.c
===
--- /dev/null
+++ lldb/test/API/commands/expression/persistent_result/main.c
@@ -0,0 +1,4 @@
+int main() {
+  int i = 30;
+  return 0; // break here
+}
Index: lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
@@ -0,0 +1,37 @@
+"""
+Test controlling `expression` result variables are persistent.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+def setUp(self):
+TestBase.setUp(self)
+self.build()
+lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c"))
+
+def test_enable_persistent_result(self):
+"""Test explicitly enabling result variables persistence."""
+self.expect("expression --persistent-result on -- i", substrs=["(int) $0 = 30"])
+# Verify the lifetime of $0 extends beyond the `expression` it was created in.
+self.expect("expression $0", substrs=["(int) $0 = 30"])
+
+def test_disable_persistent_result(self):
+"""Test explicitly disabling persistent result variables."""
+self.expect("expression --persistent-result off -- i", substrs=["(int) 30"])
+# Verify a persistent result was not silently created.
+self.expect("expression $0", error=True)
+
+def test_expression_persists_result(self):
+"""Test `expression`'s default behavior is to persist a result variable."""
+self.expect("expression i", substrs=["(int) $0 = 30"])
+self.expect("expression $0", substrs=["(int) $0 = 30"])
+
+def test_p_persists_result(self):
+"""Test `p` does persist a result variable."""
+self.expect("p i", substrs=["(int) $0 = 30"])
+self.expect("p $0", substrs=["(int) $0 = 30"])
Index: lldb/test/API/commands/expression/persistent_result/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/expression/persistent_result/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.m
+
+include Makefile.rules
Index: lldb/test/API/commands/dwim-print/TestDWIMPrint.py
===
--- lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -16,7 +16,8 @@
 self.ci.HandleCommand(cmd, result)
 return result.GetOutput().rstrip()
 
-PERSISTENT_VAR = re.compile(r"\$\d+")
+VAR_IDENT_RAW = r"(?:\$\d+|\w+) = "
+VAR_IDENT = re.compile(VAR_IDENT_RAW)
 
 def _mask_persistent_var(self, string: str) -> str:
 """
@@ -24,8 +25,9 @@
 that matches any persistent result (r'\$\d+'). The returned string can
 be matched against other `expression` results.
 """
-before, after = self.PERSISTENT_VAR.split(string, maxsplit=1)
-return re.escape(before) + r"\$\d+" + re.escape(after)
+before, after = self.VAR_IDENT.split(string, maxsplit=1)
+# Support either a frame variable (\w+) or a persistent result (\$\d+).
+return re.escape(before) + self.VAR_IDENT_RAW + re.escape(after)
 
 def _expect_cmd(
 self,
@@ -51,7 +53,7 @@
 substrs = [f"note: ran `{resolved_cmd}`"]
 patterns = []
 
-if actual_cmd == "expression" and self.PERSISTENT_VAR.search(expected_output):
+if self.VAR_IDENT.search(expected_output):
 patterns.append(self._mask_persistent_var(expected_output))
 else:
 substrs.append(expected_output)
Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -425,7 +425,9 @@
 if (m_options.m_hide_pointer_value &&
 IsPointerValue(m_valobj->GetCompilerType())) {
 } else {
-  m_stream->Printf(" %s", m

[Lldb-commits] [PATCH] D144230: [lldb] Make persisting result variables configurable

2023-02-17 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione updated this revision to Diff 498394.
kastiglione added a comment.

Remove short option -C, leaving only --persistent-result


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144230

Files:
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectExpression.h
  lldb/source/Commands/Options.td
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/test/API/commands/dwim-print/TestDWIMPrint.py
  lldb/test/API/commands/expression/persistent_result/Makefile
  lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
  lldb/test/API/commands/expression/persistent_result/main.c

Index: lldb/test/API/commands/expression/persistent_result/main.c
===
--- /dev/null
+++ lldb/test/API/commands/expression/persistent_result/main.c
@@ -0,0 +1,4 @@
+int main() {
+  int i = 30;
+  return 0; // break here
+}
Index: lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
@@ -0,0 +1,37 @@
+"""
+Test controlling `expression` result variables are persistent.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+def setUp(self):
+TestBase.setUp(self)
+self.build()
+lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c"))
+
+def test_enable_persistent_result(self):
+"""Test explicitly enabling result variables persistence."""
+self.expect("expression --persistent-result on -- i", substrs=["(int) $0 = 30"])
+# Verify the lifetime of $0 extends beyond the `expression` it was created in.
+self.expect("expression $0", substrs=["(int) $0 = 30"])
+
+def test_disable_persistent_result(self):
+"""Test explicitly disabling persistent result variables."""
+self.expect("expression --persistent-result off -- i", substrs=["(int) 30"])
+# Verify a persistent result was not silently created.
+self.expect("expression $0", error=True)
+
+def test_expression_persists_result(self):
+"""Test `expression`'s default behavior is to persist a result variable."""
+self.expect("expression i", substrs=["(int) $0 = 30"])
+self.expect("expression $0", substrs=["(int) $0 = 30"])
+
+def test_p_persists_result(self):
+"""Test `p` does persist a result variable."""
+self.expect("p i", substrs=["(int) $0 = 30"])
+self.expect("p $0", substrs=["(int) $0 = 30"])
Index: lldb/test/API/commands/expression/persistent_result/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/expression/persistent_result/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
Index: lldb/test/API/commands/dwim-print/TestDWIMPrint.py
===
--- lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -16,7 +16,8 @@
 self.ci.HandleCommand(cmd, result)
 return result.GetOutput().rstrip()
 
-PERSISTENT_VAR = re.compile(r"\$\d+")
+VAR_IDENT_RAW = r"(?:\$\d+|\w+) = "
+VAR_IDENT = re.compile(VAR_IDENT_RAW)
 
 def _mask_persistent_var(self, string: str) -> str:
 """
@@ -24,8 +25,9 @@
 that matches any persistent result (r'\$\d+'). The returned string can
 be matched against other `expression` results.
 """
-before, after = self.PERSISTENT_VAR.split(string, maxsplit=1)
-return re.escape(before) + r"\$\d+" + re.escape(after)
+before, after = self.VAR_IDENT.split(string, maxsplit=1)
+# Support either a frame variable (\w+) or a persistent result (\$\d+).
+return re.escape(before) + self.VAR_IDENT_RAW + re.escape(after)
 
 def _expect_cmd(
 self,
@@ -51,7 +53,7 @@
 substrs = [f"note: ran `{resolved_cmd}`"]
 patterns = []
 
-if actual_cmd == "expression" and self.PERSISTENT_VAR.search(expected_output):
+if self.VAR_IDENT.search(expected_output):
 patterns.append(self._mask_persistent_var(expected_output))
 else:
 substrs.append(expected_output)
Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -425,7 +425,9 @@
 if (m_options.m_hide_pointer_value &&
 IsPointerValue(m_valobj->GetCompilerType())) {
 }

[Lldb-commits] [PATCH] D142341: [LLDB][NFC] Fix valobj_sp null pointer checks in lldb/source/Plugins/Language/

2023-02-17 Thread Michael Buch via Phabricator via lldb-commits 
Michael137 added inline comments.



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:225
   if (valobj_sp)
+SyntheticChildrenFrontEnd(*valobj_sp);
 Update();

kastiglione wrote:
> Michael137 wrote:
> > Michael137 wrote:
> > > Michael137 wrote:
> > > > this won't initialise the parent constructor though will it? Just 
> > > > creates a temporary and immediately destructs it? You might need to put 
> > > > it back into the initialiser list and use a ternary
> > > Oh but `SyntheticChildrenFrontEnd` seems to always take a reference. The 
> > > interfaces seem to be a little mismatched
> > Perhaps the null-check in the constructor is just redundant and should 
> > actually be an assert.
> > 
> > @jingham might know more about the assumptions here
> @xgupta reiterating Michael's point, I think this change results in 
> mis-construction. Have you run the test suite, I would expect some failures 
> with this change.
> 
> I agree that this should be an assert. The other option would be to change 
> the constructor's signature, using a `lldb::ValueObject &` instead of 
> `lldb::ValueObjectSP`, which puts the onus on callers to handle any 
> null-ness. Are we aware of any callers passing null?
> 
> 
FYI, from offline conversation with Jim:

```
This is one of those things where you really should have a general guard 
against trying to create a FrontEnd with a null ValueObjectSP, since this isn't 
going to do any good.  But then you worry "what if one leaks through by 
accident" so you put some fallback code when that happens.  We were really 
insistent that "if we make a programming error in viewing a variable we'd 
really like to limit the damage and not crash..." So we didn't go straight to 
asserts in all these places the way clang, for instance, does.  Taking down a 
whole debug session because one variable was bizarre (maybe the DWARF was bad 
in a way that confused us...) is not acceptable, there's too much state in the 
investigations the debugger supports so we were often over cautious about these 
sort of checks.

The better solution would be to go to all the places that produce synthetic 
children and make sure that they error out early when told to format empty SP's.

Not sure how doable that is.  For most cases, the Synthetic Child Providers 
come from type matches, and an empty ValueObjectSP doesn't have a type, so it 
would never make a match.  But we do have a few "try it against anything" 
formatters now, things like the C++ std::function matchers, and the python 
functional matchers, so there are places where we might end up screwing this up.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142341

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

[Lldb-commits] [PATCH] D144237: [lldb/Plugins] Add memory writing capabilities to Scripted Process

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

LGTM


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

https://reviews.llvm.org/D144237

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

[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

2023-02-17 Thread Alexander Yermolovich via Phabricator via lldb-commits 
ayermolo added a comment.

In D138618#4133717 , @mib wrote:

> Hi @ayermolo!
>
> This patch is causing some failure on the macOS lldb bot: 
> https://green.lab.llvm.org/green/job/lldb-cmake/51257/
>
> Could you take a look ? If you don't have the time, we can revert your patch 
> until you manage to reproduce these failures.
>
> Let me know if you need help with that :)

@mib

I tried running bin/llvm-lit -sv 
/Users/ayermolo/local/llvm-project/lldb/test/API/commands/expression/anonymous-struct/TestCallUserAnonTypedef.py
 on a X86 mac and keep getting:

Command Output (stdout):


lldb version 16.0.0git (https://github.com/llvm/llvm-project.git revision 
7ad786a29e7bcd75bf3e7af2d96a0bf419faff64 
)

  clang revision 7ad786a29e7bcd75bf3e7af2d96a0bf419faff64
  llvm revision 7ad786a29e7bcd75bf3e7af2d96a0bf419faff64

-

Command Output (stderr):


libc++abi: terminating with uncaught exception of type std::__1::system_error: 
recursive_mutex lock failed: Resource temporarily unavailable
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and 
include the crash backtrace.

...
Unresolved: 1

I thought it was my config, so tried to change it as closely as in your 
buildbot, but still getting this error. Do you have any idea what is going on?

  cmake \
  -G Ninja \
  ../llvm-project/llvm \
  
-DCMAKE_C_COMPILER=/Applications/Xcode_14.2.0_14C18_fb.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang
 \
  
-DCMAKE_CXX_COMPILER=/Applications/Xcode_14.2.0_14C18_fb.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++
 \
  -DLLVM_ENABLE_ASSERTIONS:BOOL=TRUE \
  -DLLVM_TARGETS_TO_BUILD='X86' \
  -DLLVM_ENABLE_PROJECTS='clang;lldb;cross-project-tests' \
  -DLLVM_ENABLE_RUNTIMES='libcxx;libcxxabi;compiler-rt' \
  -DLLVM_LIT_ARGS='-v --time-tests --shuffle 
--xunit-xml-output=/Users/buildslave/jenkins/workspace/lldb-cmake/test/results.xml
 -v -j 6' \
  -DLLDB_BUILD_FRAMEWORK:BOOL=TRUE \
  -DLLDB_USE_SYSTEM_DEBUGSERVER=ON \
  -DLLDB_EDITLINE_USE_WCHAR=0 \
  -DLLDB_ENABLE_LIBEDIT:BOOL=TRUE \
  -DLLDB_ENABLE_CURSES:BOOL=TRUE \
  -DLLDB_ENABLE_PYTHON:BOOL=TRUE \
  -DLLDB_ENABLE_LIBXML2:BOOL=TRUE \
  -DLLDB_ENABLE_LUA:BOOL=FALSE \
  -DPython3_EXECUTABLE=/usr/bin/python3 \
  -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
  -DBUILTINS_CMAKE_ARGS=-DCOMPILER_RT_ENABLE_IOS=OFF \
  -DCMAKE_BUILD_TYPE=Release


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138618

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

[Lldb-commits] [PATCH] D144230: [lldb] Make persisting result variables configurable

2023-02-17 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl added inline comments.



Comment at: lldb/source/Commands/Options.td:389
 "not supported by the interpreter (defaults to true).">;
+  def persistent_result : Option<"persistent-result", "\\x01">, Groups<[1,2]>,
+Arg<"Boolean">,

Is that the approved way of doing long options-only? What's printed in `help 
dwim-print` now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144230

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

[Lldb-commits] [PATCH] D144114: [lldb] Add expression command options in dwim-print

2023-02-17 Thread Jim Ingham via Phabricator via lldb-commits 
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: lldb/source/Commands/CommandObjectDWIMPrint.cpp:119
 }
+
+llvm::ArrayRef

jingham wrote:
> There's a version of Append that lets you exclude and remap option sets.  
> This seems like a finer-grained version of the same thing where you just want 
> to exclude particular options by name but not mess with the option sets.  
> That seems a generally useful thing to do, so it would be better to make 
> another flavor of Append that takes an array of short character options and 
> just excludes them, than do it as a one-off here.
That would also make this code easier to read, since the exclusion would happen 
where you added it rather than way down below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144114

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

[Lldb-commits] [PATCH] D144230: [lldb] Make persisting result variables configurable

2023-02-17 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione added inline comments.



Comment at: lldb/source/Commands/Options.td:389
 "not supported by the interpreter (defaults to true).">;
+  def persistent_result : Option<"persistent-result", "\\x01">, Groups<[1,2]>,
+Arg<"Boolean">,

aprantl wrote:
> Is that the approved way of doing long options-only? What's printed in `help 
> dwim-print` now?
We don't have many long option-only flags, and those few others do it this way. 
If I recall, Jonas was the first to use this approach.

The relevant portion of `help dwim-print` (or `help expression`) is:

> --persistent-result 
> Persist expression result in a variable for subsequent use. Expression 
> results will be
> labeled with $-prefixed variables, e.g. $0, $1, etc. Defaults to true.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144230

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

[Lldb-commits] [PATCH] D144114: [lldb] Add expression command options in dwim-print

2023-02-17 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione added inline comments.



Comment at: lldb/source/Commands/CommandObjectDWIMPrint.cpp:119
 }
+
+llvm::ArrayRef

jingham wrote:
> jingham wrote:
> > There's a version of Append that lets you exclude and remap option sets.  
> > This seems like a finer-grained version of the same thing where you just 
> > want to exclude particular options by name but not mess with the option 
> > sets.  That seems a generally useful thing to do, so it would be better to 
> > make another flavor of Append that takes an array of short character 
> > options and just excludes them, than do it as a one-off here.
> That would also make this code easier to read, since the exclusion would 
> happen where you added it rather than way down below.
This approach is much better. I appreciate the helpful review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144114

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

[Lldb-commits] [PATCH] D144230: [lldb] Make persisting result variables configurable

2023-02-17 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione updated this revision to Diff 498462.
kastiglione added a comment.

Improve handling of `po`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144230

Files:
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectExpression.h
  lldb/source/Commands/Options.td
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/test/API/commands/dwim-print/TestDWIMPrint.py
  lldb/test/API/commands/expression/persistent_result/Makefile
  lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
  lldb/test/API/commands/expression/persistent_result/main.c

Index: lldb/test/API/commands/expression/persistent_result/main.c
===
--- /dev/null
+++ lldb/test/API/commands/expression/persistent_result/main.c
@@ -0,0 +1,4 @@
+int main() {
+  int i = 30;
+  return 0; // break here
+}
Index: lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
@@ -0,0 +1,37 @@
+"""
+Test controlling `expression` result variables are persistent.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+def setUp(self):
+TestBase.setUp(self)
+self.build()
+lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c"))
+
+def test_enable_persistent_result(self):
+"""Test explicitly enabling result variables persistence."""
+self.expect("expression --persistent-result on -- i", substrs=["(int) $0 = 30"])
+# Verify the lifetime of $0 extends beyond the `expression` it was created in.
+self.expect("expression $0", substrs=["(int) $0 = 30"])
+
+def test_disable_persistent_result(self):
+"""Test explicitly disabling persistent result variables."""
+self.expect("expression --persistent-result off -- i", substrs=["(int) 30"])
+# Verify a persistent result was not silently created.
+self.expect("expression $0", error=True)
+
+def test_expression_persists_result(self):
+"""Test `expression`'s default behavior is to persist a result variable."""
+self.expect("expression i", substrs=["(int) $0 = 30"])
+self.expect("expression $0", substrs=["(int) $0 = 30"])
+
+def test_p_persists_result(self):
+"""Test `p` does persist a result variable."""
+self.expect("p i", substrs=["(int) $0 = 30"])
+self.expect("p $0", substrs=["(int) $0 = 30"])
Index: lldb/test/API/commands/expression/persistent_result/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/expression/persistent_result/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
Index: lldb/test/API/commands/dwim-print/TestDWIMPrint.py
===
--- lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -16,7 +16,8 @@
 self.ci.HandleCommand(cmd, result)
 return result.GetOutput().rstrip()
 
-PERSISTENT_VAR = re.compile(r"\$\d+")
+VAR_IDENT_RAW = r"(?:\$\d+|\w+) = "
+VAR_IDENT = re.compile(VAR_IDENT_RAW)
 
 def _mask_persistent_var(self, string: str) -> str:
 """
@@ -24,8 +25,9 @@
 that matches any persistent result (r'\$\d+'). The returned string can
 be matched against other `expression` results.
 """
-before, after = self.PERSISTENT_VAR.split(string, maxsplit=1)
-return re.escape(before) + r"\$\d+" + re.escape(after)
+before, after = self.VAR_IDENT.split(string, maxsplit=1)
+# Support either a frame variable (\w+) or a persistent result (\$\d+).
+return re.escape(before) + self.VAR_IDENT_RAW + re.escape(after)
 
 def _expect_cmd(
 self,
@@ -51,7 +53,7 @@
 substrs = [f"note: ran `{resolved_cmd}`"]
 patterns = []
 
-if actual_cmd == "expression" and self.PERSISTENT_VAR.search(expected_output):
+if self.VAR_IDENT.search(expected_output):
 patterns.append(self._mask_persistent_var(expected_output))
 else:
 substrs.append(expected_output)
Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -425,7 +425,9 @@
 if (m_options.m_hide_pointer_value &&
 IsPointerValue(m_valobj->GetCompilerType())) {
 } else {
-  m_stream->Pri

[Lldb-commits] [PATCH] D144230: [lldb] Make persisting result variables configurable

2023-02-17 Thread Adrian Prantl via Phabricator via lldb-commits 
aprantl accepted this revision.
aprantl added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Commands/Options.td:389
 "not supported by the interpreter (defaults to true).">;
+  def persistent_result : Option<"persistent-result", "\\x01">, Groups<[1,2]>,
+Arg<"Boolean">,

kastiglione wrote:
> aprantl wrote:
> > Is that the approved way of doing long options-only? What's printed in 
> > `help dwim-print` now?
> We don't have many long option-only flags, and those few others do it this 
> way. If I recall, Jonas was the first to use this approach.
> 
> The relevant portion of `help dwim-print` (or `help expression`) is:
> 
> > --persistent-result 
> > Persist expression result in a variable for subsequent use. Expression 
> > results will be
> > labeled with $-prefixed variables, e.g. $0, $1, etc. Defaults to true.
> 
Okay, thanks for checking!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144230

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

[Lldb-commits] [PATCH] D144238: [lldb] StructuredData should not truncate uint64_t values

2023-02-17 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord updated this revision to Diff 498479.
bulbazord added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144238

Files:
  lldb/source/Utility/StructuredData.cpp
  lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py


Index: lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
===
--- lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -9,6 +9,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
+import json
 
 class TestStructuredDataAPI(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
@@ -19,8 +20,15 @@
 def structured_data_api_test(self):
 error = lldb.SBError()
 s = lldb.SBStream()
-s.Print(
-
"{\"key_dict\":{\"key_string\":\"STRING\",\"key_int\":3,\"key_float\":2.99,\"key_bool\":true,\"key_array\":[\"23\",\"arr\"]}}")
+
+dict_str = json.dumps(
+{"key_dict":
+ {"key_string":"STRING",
+  "key_uint":0x,
+  "key_float":2.99,
+  "key_bool":True,
+  "key_array":["23","arr"]}})
+s.Print(dict_str)
 example = lldb.SBStructuredData()
 
 # Check SetFromJSON API for dictionaries, integers, floating point
@@ -49,7 +57,7 @@
 self.string_struct_test(dict_struct)
 
 # Tests for integer data type
-self.int_struct_test(dict_struct)
+self.uint_struct_test(dict_struct)
 
 # Tests for floating point data type
 self.double_struct_test(dict_struct)
@@ -110,25 +118,27 @@
 str(output) +
 " returned for a string object")
 
-def int_struct_test(self, dict_struct):
-# Check a valid SBStructuredData containing an 'integer' by
-int_struct = lldb.SBStructuredData()
-int_struct = dict_struct.GetValueForKey("key_int")
-if not int_struct.IsValid():
+def uint_struct_test(self, dict_struct):
+# Check a valid SBStructuredData containing an unsigned integer.
+# We intentionally make this larger than what an int64_t can hold but
+# still small enough to fit a uint64_t
+uint_struct = lldb.SBStructuredData()
+uint_struct = dict_struct.GetValueForKey("key_uint")
+if not uint_struct.IsValid():
 self.fail("A valid object should have been returned")
 
 # Check Type API
-if not int_struct.GetType() == lldb.eStructuredDataTypeInteger:
-self.fail("Wrong type returned: " + str(int_struct.GetType()))
+if not uint_struct.GetType() == lldb.eStructuredDataTypeInteger:
+self.fail("Wrong type returned: " + str(uint_struct.GetType()))
 
 # Check API returning 'integer' value
-output = int_struct.GetIntegerValue()
-if not output == 3:
+output = uint_struct.GetIntegerValue()
+if not output == 0x:
 self.fail("wrong output: " + str(output))
 
 # Calling wrong API on a SBStructuredData
 # (e.g. getting a string value from an integer type structure)
-output = int_struct.GetStringValue(25)
+output = uint_struct.GetStringValue(25)
 if output:
 self.fail(
 "Valid string " +
Index: lldb/source/Utility/StructuredData.cpp
===
--- lldb/source/Utility/StructuredData.cpp
+++ lldb/source/Utility/StructuredData.cpp
@@ -68,6 +68,9 @@
   if (auto b = value.getAsBoolean())
 return std::make_shared(*b);
 
+  if (auto u = value.getAsUINT64())
+return std::make_shared(*u);
+
   if (auto i = value.getAsInteger())
 return std::make_shared(*i);
 


Index: lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
===
--- lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -9,6 +9,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
+import json
 
 class TestStructuredDataAPI(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
@@ -19,8 +20,15 @@
 def structured_data_api_test(self):
 error = lldb.SBError()
 s = lldb.SBStream()
-s.Print(
-"{\"key_dict\":{\"key_string\":\"STRING\",\"key_int\":3,\"key_float\":2.99,\"key_bool\":true,\"key_array\":[\"23\",\"arr\"]}}")
+
+dict_str = json.dumps(
+{"key_dict":
+ {"key_string":"STRING",
+  "key_uint":0x,
+  "key_float":2.99,
+  "key_bool":True,
+  "key_array":["23","ar

[Lldb-commits] [lldb] 2f88c07 - [lldb] StructuredData should not truncate uint64_t values

2023-02-17 Thread Alex Langford via lldb-commits 

Author: Alex Langford
Date: 2023-02-17T12:39:49-08:00
New Revision: 2f88c07cf820cff829dec5906d298fc7147af8dd

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

LOG: [lldb] StructuredData should not truncate uint64_t values

In json::Value, getAsInteger returns an optional and getAsNumber
returns an optional. If a value is larger than what an int64_t
can hold but smaller than what a uint64_t can hold, the getAsInteger
function will fail but the getAsNumber will succeed. However, the value
shouldn't be interpreted as a double.

rdar://105556974

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

Added: 


Modified: 
lldb/source/Utility/StructuredData.cpp
lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py

Removed: 




diff  --git a/lldb/source/Utility/StructuredData.cpp 
b/lldb/source/Utility/StructuredData.cpp
index acc09289e6b98..0bf617e02873f 100644
--- a/lldb/source/Utility/StructuredData.cpp
+++ b/lldb/source/Utility/StructuredData.cpp
@@ -68,6 +68,9 @@ static StructuredData::ObjectSP ParseJSONValue(json::Value 
&value) {
   if (auto b = value.getAsBoolean())
 return std::make_shared(*b);
 
+  if (auto u = value.getAsUINT64())
+return std::make_shared(*u);
+
   if (auto i = value.getAsInteger())
 return std::make_shared(*i);
 

diff  --git 
a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py 
b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
index 7bbc0fed2648d..0ab14c0d95441 100644
--- a/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ b/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -9,6 +9,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
+import json
 
 class TestStructuredDataAPI(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
@@ -19,8 +20,15 @@ def test(self):
 def structured_data_api_test(self):
 error = lldb.SBError()
 s = lldb.SBStream()
-s.Print(
-
"{\"key_dict\":{\"key_string\":\"STRING\",\"key_int\":3,\"key_float\":2.99,\"key_bool\":true,\"key_array\":[\"23\",\"arr\"]}}")
+
+dict_str = json.dumps(
+{"key_dict":
+ {"key_string":"STRING",
+  "key_uint":0x,
+  "key_float":2.99,
+  "key_bool":True,
+  "key_array":["23","arr"]}})
+s.Print(dict_str)
 example = lldb.SBStructuredData()
 
 # Check SetFromJSON API for dictionaries, integers, floating point
@@ -49,7 +57,7 @@ def structured_data_api_test(self):
 self.string_struct_test(dict_struct)
 
 # Tests for integer data type
-self.int_struct_test(dict_struct)
+self.uint_struct_test(dict_struct)
 
 # Tests for floating point data type
 self.double_struct_test(dict_struct)
@@ -110,25 +118,27 @@ def string_struct_test(self, dict_struct):
 str(output) +
 " returned for a string object")
 
-def int_struct_test(self, dict_struct):
-# Check a valid SBStructuredData containing an 'integer' by
-int_struct = lldb.SBStructuredData()
-int_struct = dict_struct.GetValueForKey("key_int")
-if not int_struct.IsValid():
+def uint_struct_test(self, dict_struct):
+# Check a valid SBStructuredData containing an unsigned integer.
+# We intentionally make this larger than what an int64_t can hold but
+# still small enough to fit a uint64_t
+uint_struct = lldb.SBStructuredData()
+uint_struct = dict_struct.GetValueForKey("key_uint")
+if not uint_struct.IsValid():
 self.fail("A valid object should have been returned")
 
 # Check Type API
-if not int_struct.GetType() == lldb.eStructuredDataTypeInteger:
-self.fail("Wrong type returned: " + str(int_struct.GetType()))
+if not uint_struct.GetType() == lldb.eStructuredDataTypeInteger:
+self.fail("Wrong type returned: " + str(uint_struct.GetType()))
 
 # Check API returning 'integer' value
-output = int_struct.GetIntegerValue()
-if not output == 3:
+output = uint_struct.GetIntegerValue()
+if not output == 0x:
 self.fail("wrong output: " + str(output))
 
 # Calling wrong API on a SBStructuredData
 # (e.g. getting a string value from an integer type structure)
-output = int_struct.GetStringValue(25)
+output = uint_struct.GetStringValue(25)
 if output:
 self.fail(
 "Valid string " +



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

[Lldb-commits] [PATCH] D144238: [lldb] StructuredData should not truncate uint64_t values

2023-02-17 Thread Alex Langford via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2f88c07cf820: [lldb] StructuredData should not truncate 
uint64_t values (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144238

Files:
  lldb/source/Utility/StructuredData.cpp
  lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py


Index: lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
===
--- lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -9,6 +9,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
+import json
 
 class TestStructuredDataAPI(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
@@ -19,8 +20,15 @@
 def structured_data_api_test(self):
 error = lldb.SBError()
 s = lldb.SBStream()
-s.Print(
-
"{\"key_dict\":{\"key_string\":\"STRING\",\"key_int\":3,\"key_float\":2.99,\"key_bool\":true,\"key_array\":[\"23\",\"arr\"]}}")
+
+dict_str = json.dumps(
+{"key_dict":
+ {"key_string":"STRING",
+  "key_uint":0x,
+  "key_float":2.99,
+  "key_bool":True,
+  "key_array":["23","arr"]}})
+s.Print(dict_str)
 example = lldb.SBStructuredData()
 
 # Check SetFromJSON API for dictionaries, integers, floating point
@@ -49,7 +57,7 @@
 self.string_struct_test(dict_struct)
 
 # Tests for integer data type
-self.int_struct_test(dict_struct)
+self.uint_struct_test(dict_struct)
 
 # Tests for floating point data type
 self.double_struct_test(dict_struct)
@@ -110,25 +118,27 @@
 str(output) +
 " returned for a string object")
 
-def int_struct_test(self, dict_struct):
-# Check a valid SBStructuredData containing an 'integer' by
-int_struct = lldb.SBStructuredData()
-int_struct = dict_struct.GetValueForKey("key_int")
-if not int_struct.IsValid():
+def uint_struct_test(self, dict_struct):
+# Check a valid SBStructuredData containing an unsigned integer.
+# We intentionally make this larger than what an int64_t can hold but
+# still small enough to fit a uint64_t
+uint_struct = lldb.SBStructuredData()
+uint_struct = dict_struct.GetValueForKey("key_uint")
+if not uint_struct.IsValid():
 self.fail("A valid object should have been returned")
 
 # Check Type API
-if not int_struct.GetType() == lldb.eStructuredDataTypeInteger:
-self.fail("Wrong type returned: " + str(int_struct.GetType()))
+if not uint_struct.GetType() == lldb.eStructuredDataTypeInteger:
+self.fail("Wrong type returned: " + str(uint_struct.GetType()))
 
 # Check API returning 'integer' value
-output = int_struct.GetIntegerValue()
-if not output == 3:
+output = uint_struct.GetIntegerValue()
+if not output == 0x:
 self.fail("wrong output: " + str(output))
 
 # Calling wrong API on a SBStructuredData
 # (e.g. getting a string value from an integer type structure)
-output = int_struct.GetStringValue(25)
+output = uint_struct.GetStringValue(25)
 if output:
 self.fail(
 "Valid string " +
Index: lldb/source/Utility/StructuredData.cpp
===
--- lldb/source/Utility/StructuredData.cpp
+++ lldb/source/Utility/StructuredData.cpp
@@ -68,6 +68,9 @@
   if (auto b = value.getAsBoolean())
 return std::make_shared(*b);
 
+  if (auto u = value.getAsUINT64())
+return std::make_shared(*u);
+
   if (auto i = value.getAsInteger())
 return std::make_shared(*i);
 


Index: lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
===
--- lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
+++ lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py
@@ -9,6 +9,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
+import json
 
 class TestStructuredDataAPI(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
@@ -19,8 +20,15 @@
 def structured_data_api_test(self):
 error = lldb.SBError()
 s = lldb.SBStream()
-s.Print(
-"{\"key_dict\":{\"key_string\":\"STRING\",\"key_int\":3,\"key_float\":2.99,\"key_bool\":true,\"key_array\":[\"23\",\"arr\"]}}")
+
+dict_str = json.dumps(
+{"key_dict":
+ {"key_string":"STRING",
+

[Lldb-commits] [PATCH] D143104: [lldb/Plugins] Add Attach capabilities to ScriptedProcess

2023-02-17 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord added a comment.

I'm not sure how much this matters but the fact that 
`ScriptedMetadata::IsValid` (and the bool operator overload) relies on the 
class name being empty is a little strange to me. I feel like you could get 
yourself into a strange situation where you set the ScriptedMetadata class name 
to "" and then ask for it right back and instead of giving you "" it'll give 
you nullptr (or None in python). This also "invalidates" the ScriptedMetadata. 
Not a hard blocker, just a little strange to me.




Comment at: lldb/source/API/SBAttachInfo.cpp:273-278
+  ScriptedMetadataSP metadata_sp = m_opaque_sp->GetScriptedMetadata();
 
-  m_opaque_sp->SetScriptedProcessClassName(class_name);
+  if (!metadata_sp)
+metadata_sp = std::make_shared(class_name, nullptr);
+
+  m_opaque_sp->SetScriptedMetadata(metadata_sp);

Maybe I'm misunderstanding but it seems like this implementation doesn't allow 
you to actually change the class name more than once? If that's not 
intentional, then we should be creating a new ScriptedMetadata and calling 
`SetScriptedMetadata` every time. If it is intentional, why?




Comment at: lldb/source/API/SBAttachInfo.cpp:312-317
+  ScriptedMetadataSP metadata_sp = m_opaque_sp->GetScriptedMetadata();
+
+  if (!metadata_sp)
+metadata_sp = std::make_shared("", dict_sp);
+
+  m_opaque_sp->SetScriptedMetadata(metadata_sp);

Same as above: This doesn't actually let you change the arguments if we already 
have an existing ScriptedMetadata. If its already set, we're just calling 
`SetScriptedMetadata` with what already exists and discarding the argument.



Comment at: lldb/source/API/SBAttachInfo.cpp:272
 void SBAttachInfo::SetScriptedProcessClassName(const char *class_name) {
   LLDB_INSTRUMENT_VA(this, class_name);
+  ScriptedMetadataSP metadata_sp = m_opaque_sp->GetScriptedMetadata();

JDevlieghere wrote:
> Newline after `LLDB_INSTRUMENT_VA`
There needs to be a newline after `LLDB_INSTRUMENT_VA` here I believe.



Comment at: lldb/source/API/SBLaunchInfo.cpp:348
   LLDB_INSTRUMENT_VA(this, class_name);
-
-  m_opaque_sp->SetScriptedProcessClassName(class_name);
+  ScriptedMetadataSP metadata_sp = m_opaque_sp->GetScriptedMetadata();
+  StructuredData::DictionarySP dict_sp =

Needs to be a newline after `LLDB_INSTRUMENT_VA`



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:203-208
+  SetPrivateState(eStateRunning);
+
+  if (error.Fail())
+return error;
+
+  SetPrivateState(eStateStopped);

I'm not sure I understand what the point of setting the state to `Running` is 
and only setting it to `Stopped` if the attach failed? Should we be mucking 
with state at all if the attach failed?


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

https://reviews.llvm.org/D143104

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

[Lldb-commits] [PATCH] D143104: [lldb/Plugins] Add Attach capabilities to ScriptedProcess

2023-02-17 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib added inline comments.



Comment at: lldb/source/API/SBAttachInfo.cpp:273-278
+  ScriptedMetadataSP metadata_sp = m_opaque_sp->GetScriptedMetadata();
 
-  m_opaque_sp->SetScriptedProcessClassName(class_name);
+  if (!metadata_sp)
+metadata_sp = std::make_shared(class_name, nullptr);
+
+  m_opaque_sp->SetScriptedMetadata(metadata_sp);

bulbazord wrote:
> Maybe I'm misunderstanding but it seems like this implementation doesn't 
> allow you to actually change the class name more than once? If that's not 
> intentional, then we should be creating a new ScriptedMetadata and calling 
> `SetScriptedMetadata` every time. If it is intentional, why?
> 
Good catch! I forgot to check-in some code --'



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:203-208
+  SetPrivateState(eStateRunning);
+
+  if (error.Fail())
+return error;
+
+  SetPrivateState(eStateStopped);

bulbazord wrote:
> I'm not sure I understand what the point of setting the state to `Running` is 
> and only setting it to `Stopped` if the attach failed? Should we be mucking 
> with state at all if the attach failed?
I guess we can do it subsequently


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

https://reviews.llvm.org/D143104

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

[Lldb-commits] [PATCH] D144311: [debugserver] Add one additional sleep before attaching after waiting

2023-02-17 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord created this revision.
bulbazord added a reviewer: jasonmolenda.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

It's possible for debugserver to attach to a process during the handoff
between /usr/lib/dyld and the dyld in the shared cache. When that
happens, we may end up in a state where there is no dyld in the process
and our debugging session is doomed. To make that scenario a lot less
likely, we can insert a sleep right before attaching after waiting to
find the right pid.

rdar://105513180


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144311

Files:
  lldb/tools/debugserver/source/DNB.cpp


Index: lldb/tools/debugserver/source/DNB.cpp
===
--- lldb/tools/debugserver/source/DNB.cpp
+++ lldb/tools/debugserver/source/DNB.cpp
@@ -793,6 +793,14 @@
   if (waitfor_pid != INVALID_NUB_PROCESS) {
 DNBLogThreadedIf(LOG_PROCESS, "Attaching to %s with pid %i...\n",
  waitfor_process_name, waitfor_pid);
+// In some cases, we attempt to attach during the transition from
+// /usr/lib/dyld to the dyld in the shared cache. If that happens, we may
+// end up in a state where there is no dyld in the process and from there
+// the debugging session is doomed.
+// In an attempt to make this scenario much less likely, we sleep
+// for an additional `waitfor_interval` number of microseconds before
+// attaching.
+::usleep(waitfor_interval);
 waitfor_pid = DNBProcessAttach(waitfor_pid, timeout_abstime,
ctx->GetIgnoredExceptions(), err_str, 
err_len);


Index: lldb/tools/debugserver/source/DNB.cpp
===
--- lldb/tools/debugserver/source/DNB.cpp
+++ lldb/tools/debugserver/source/DNB.cpp
@@ -793,6 +793,14 @@
   if (waitfor_pid != INVALID_NUB_PROCESS) {
 DNBLogThreadedIf(LOG_PROCESS, "Attaching to %s with pid %i...\n",
  waitfor_process_name, waitfor_pid);
+// In some cases, we attempt to attach during the transition from
+// /usr/lib/dyld to the dyld in the shared cache. If that happens, we may
+// end up in a state where there is no dyld in the process and from there
+// the debugging session is doomed.
+// In an attempt to make this scenario much less likely, we sleep
+// for an additional `waitfor_interval` number of microseconds before
+// attaching.
+::usleep(waitfor_interval);
 waitfor_pid = DNBProcessAttach(waitfor_pid, timeout_abstime,
ctx->GetIgnoredExceptions(), err_str, 
err_len);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D144230: [lldb] Make persisting result variables configurable

2023-02-17 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione updated this revision to Diff 498536.
kastiglione added a comment.

Fix composition of --persistent-result and -O/-v


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144230

Files:
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectExpression.h
  lldb/source/Commands/Options.td
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/test/API/commands/dwim-print/TestDWIMPrint.py
  lldb/test/API/commands/expression/persistent_result/Makefile
  lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
  lldb/test/API/commands/expression/persistent_result/main.c

Index: lldb/test/API/commands/expression/persistent_result/main.c
===
--- /dev/null
+++ lldb/test/API/commands/expression/persistent_result/main.c
@@ -0,0 +1,4 @@
+int main() {
+  int i = 30;
+  return 0; // break here
+}
Index: lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
@@ -0,0 +1,37 @@
+"""
+Test controlling `expression` result variables are persistent.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+def setUp(self):
+TestBase.setUp(self)
+self.build()
+lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c"))
+
+def test_enable_persistent_result(self):
+"""Test explicitly enabling result variables persistence."""
+self.expect("expression --persistent-result on -- i", substrs=["(int) $0 = 30"])
+# Verify the lifetime of $0 extends beyond the `expression` it was created in.
+self.expect("expression $0", substrs=["(int) $0 = 30"])
+
+def test_disable_persistent_result(self):
+"""Test explicitly disabling persistent result variables."""
+self.expect("expression --persistent-result off -- i", substrs=["(int) 30"])
+# Verify a persistent result was not silently created.
+self.expect("expression $0", error=True)
+
+def test_expression_persists_result(self):
+"""Test `expression`'s default behavior is to persist a result variable."""
+self.expect("expression i", substrs=["(int) $0 = 30"])
+self.expect("expression $0", substrs=["(int) $0 = 30"])
+
+def test_p_persists_result(self):
+"""Test `p` does persist a result variable."""
+self.expect("p i", substrs=["(int) $0 = 30"])
+self.expect("p $0", substrs=["(int) $0 = 30"])
Index: lldb/test/API/commands/expression/persistent_result/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/expression/persistent_result/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
Index: lldb/test/API/commands/dwim-print/TestDWIMPrint.py
===
--- lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -16,7 +16,8 @@
 self.ci.HandleCommand(cmd, result)
 return result.GetOutput().rstrip()
 
-PERSISTENT_VAR = re.compile(r"\$\d+")
+VAR_IDENT_RAW = r"(?:\$\d+|\w+) = "
+VAR_IDENT = re.compile(VAR_IDENT_RAW)
 
 def _mask_persistent_var(self, string: str) -> str:
 """
@@ -24,8 +25,9 @@
 that matches any persistent result (r'\$\d+'). The returned string can
 be matched against other `expression` results.
 """
-before, after = self.PERSISTENT_VAR.split(string, maxsplit=1)
-return re.escape(before) + r"\$\d+" + re.escape(after)
+before, after = self.VAR_IDENT.split(string, maxsplit=1)
+# Support either a frame variable (\w+) or a persistent result (\$\d+).
+return re.escape(before) + self.VAR_IDENT_RAW + re.escape(after)
 
 def _expect_cmd(
 self,
@@ -51,7 +53,7 @@
 substrs = [f"note: ran `{resolved_cmd}`"]
 patterns = []
 
-if actual_cmd == "expression" and self.PERSISTENT_VAR.search(expected_output):
+if self.VAR_IDENT.search(expected_output):
 patterns.append(self._mask_persistent_var(expected_output))
 else:
 substrs.append(expected_output)
Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -425,7 +425,9 @@
 if (m_options.m_hide_pointer_value &&
 IsPointerValue(m_valobj->GetCompilerType())) {
 } else {

[Lldb-commits] [lldb] 920b46e - [lldb] Add expression command options in dwim-print

2023-02-17 Thread Dave Lee via lldb-commits 

Author: Dave Lee
Date: 2023-02-17T17:50:08-08:00
New Revision: 920b46e108b23454e6827ed0fa5e0a69fcb4a6e6

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

LOG: [lldb] Add expression command options in dwim-print

Adopt `expression`'s options in `dwim-print`.

This is primarily added to support the `--language`/`-l` flag.

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

Added: 


Modified: 
lldb/include/lldb/Interpreter/Options.h
lldb/source/Commands/CommandObjectDWIMPrint.cpp
lldb/source/Commands/CommandObjectDWIMPrint.h
lldb/source/Commands/CommandObjectExpression.cpp
lldb/source/Commands/CommandObjectExpression.h
lldb/source/Interpreter/Options.cpp
lldb/test/API/commands/dwim-print/TestDWIMPrint.py

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/Options.h 
b/lldb/include/lldb/Interpreter/Options.h
index a952d5f78df9e..bf74927cf99db 100644
--- a/lldb/include/lldb/Interpreter/Options.h
+++ b/lldb/include/lldb/Interpreter/Options.h
@@ -20,6 +20,7 @@
 #include "lldb/lldb-private.h"
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace lldb_private {
 
@@ -290,6 +291,21 @@ class OptionGroupOptions : public Options {
   /// copying the option definition.
   void Append(OptionGroup *group, uint32_t src_mask, uint32_t dst_mask);
 
+  /// Append selected options from a OptionGroup class.
+  ///
+  /// Append the subset of options from \a group, where the "long_option" value
+  /// is _not_ in \a exclude_long_options.
+  ///
+  /// \param[in] group
+  /// A group of options to take option values from and copy their
+  /// definitions into this class.
+  ///
+  /// \param[in] exclude_long_options
+  /// A set of long option strings which indicate which option values 
values
+  /// to limit from \a group.
+  void Append(OptionGroup *group,
+  llvm::ArrayRef exclude_long_options);
+
   void Finalize();
 
   bool DidFinalize() { return m_did_finalize; }

diff  --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp 
b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 81da150aa2ddd..28c78af73dcec 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -36,6 +36,8 @@ 
CommandObjectDWIMPrint::CommandObjectDWIMPrint(CommandInterpreter &interpreter)
 OptionGroupFormat::OPTION_GROUP_FORMAT |
 OptionGroupFormat::OPTION_GROUP_GDB_FMT,
 LLDB_OPT_SET_1);
+  StringRef exclude_expr_options[] = {"debug", "top-level"};
+  m_option_group.Append(&m_expr_options, exclude_expr_options);
   m_option_group.Append(&m_varobj_options, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1);
   m_option_group.Finalize();
 }
@@ -57,11 +59,11 @@ bool CommandObjectDWIMPrint::DoExecute(StringRef command,
   m_cmd_name);
 return false;
   }
+
   auto verbosity = GetDebugger().GetDWIMPrintVerbosity();
 
   DumpValueObjectOptions dump_options = m_varobj_options.GetAsDumpOptions(
-  eLanguageRuntimeDescriptionDisplayVerbosityFull,
-  m_format_options.GetFormat());
+  m_expr_options.m_verbosity, m_format_options.GetFormat());
 
   // First, try `expr` as the name of a frame variable.
   if (StackFrame *frame = m_exe_ctx.GetFramePtr()) {
@@ -87,9 +89,12 @@ bool CommandObjectDWIMPrint::DoExecute(StringRef command,
 Target &target = target_ptr ? *target_ptr : GetDummyTarget();
 
 auto *exe_scope = m_exe_ctx.GetBestExecutionContextScope();
+const EvaluateExpressionOptions eval_options =
+m_expr_options.GetEvaluateExpressionOptions(target, m_varobj_options);
 ValueObjectSP valobj_sp;
-if (target.EvaluateExpression(expr, exe_scope, valobj_sp) ==
-eExpressionCompleted) {
+ExpressionResults expr_result =
+target.EvaluateExpression(expr, exe_scope, valobj_sp, eval_options);
+if (expr_result == eExpressionCompleted) {
   if (verbosity != eDWIMPrintVerbosityNone) {
 StringRef flags;
 if (args.HasArgs())

diff  --git a/lldb/source/Commands/CommandObjectDWIMPrint.h 
b/lldb/source/Commands/CommandObjectDWIMPrint.h
index 94daa85958b68..8a14a6c7bb531 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.h
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.h
@@ -9,6 +9,7 @@
 #ifndef LLDB_SOURCE_COMMANDS_COMMANDOBJECTDWIMPRINT_H
 #define LLDB_SOURCE_COMMANDS_COMMANDOBJECTDWIMPRINT_H
 
+#include "CommandObjectExpression.h"
 #include "lldb/Interpreter/CommandObject.h"
 #include "lldb/Interpreter/OptionGroupFormat.h"
 #include "lldb/Interpreter/OptionGroupValueObjectDisplay.h"
@@ -42,6 +43,7 @@ class CommandObjectDWIMPrint : public CommandObjectRaw {
   OptionGroupOptions m_option_group;
   OptionGroupForm

[Lldb-commits] [PATCH] D144114: [lldb] Add expression command options in dwim-print

2023-02-17 Thread Dave Lee via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG920b46e108b2: [lldb] Add expression command options in 
dwim-print (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144114

Files:
  lldb/include/lldb/Interpreter/Options.h
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Commands/CommandObjectDWIMPrint.h
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectExpression.h
  lldb/source/Interpreter/Options.cpp
  lldb/test/API/commands/dwim-print/TestDWIMPrint.py

Index: lldb/test/API/commands/dwim-print/TestDWIMPrint.py
===
--- lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -103,3 +103,10 @@
 lldbutil.run_to_name_breakpoint(self, "main")
 self._expect_cmd(f"dwim-print -T -- argc", "frame variable")
 self._expect_cmd(f"dwim-print -T -- argc + 1", "expression")
+
+def test_expression_language(self):
+"""Test that the language flag doesn't affect the choice of command."""
+self.build()
+lldbutil.run_to_name_breakpoint(self, "main")
+self._expect_cmd(f"dwim-print -l c++ -- argc", "frame variable")
+self._expect_cmd(f"dwim-print -l c++ -- argc + 1", "expression")
Index: lldb/source/Interpreter/Options.cpp
===
--- lldb/source/Interpreter/Options.cpp
+++ lldb/source/Interpreter/Options.cpp
@@ -781,6 +781,19 @@
   }
 }
 
+void OptionGroupOptions::Append(
+OptionGroup *group, llvm::ArrayRef exclude_long_options) {
+  auto group_option_defs = group->GetDefinitions();
+  for (uint32_t i = 0; i < group_option_defs.size(); ++i) {
+const auto &definition = group_option_defs[i];
+if (llvm::is_contained(exclude_long_options, definition.long_option))
+  continue;
+
+m_option_infos.push_back(OptionInfo(group, i));
+m_option_defs.push_back(definition);
+  }
+}
+
 void OptionGroupOptions::Finalize() {
   m_did_finalize = true;
 }
Index: lldb/source/Commands/CommandObjectExpression.h
===
--- lldb/source/Commands/CommandObjectExpression.h
+++ lldb/source/Commands/CommandObjectExpression.h
@@ -35,6 +35,12 @@
 
 void OptionParsingStarting(ExecutionContext *execution_context) override;
 
+/// Return the appropriate expression options used for evaluating the
+/// expression in the given target.
+EvaluateExpressionOptions GetEvaluateExpressionOptions(
+const Target &target,
+const OptionGroupValueObjectDisplay &display_opts);
+
 bool top_level;
 bool unwind_on_error;
 bool ignore_breakpoints;
@@ -67,10 +73,6 @@
 
   bool DoExecute(llvm::StringRef command, CommandReturnObject &result) override;
 
-  /// Return the appropriate expression options used for evaluating the
-  /// expression in the given target.
-  EvaluateExpressionOptions GetEvalOptions(const Target &target);
-
   /// Evaluates the given expression.
   /// \param output_stream The stream to which the evaluation result will be
   ///  printed.
Index: lldb/source/Commands/CommandObjectExpression.cpp
===
--- lldb/source/Commands/CommandObjectExpression.cpp
+++ lldb/source/Commands/CommandObjectExpression.cpp
@@ -181,6 +181,48 @@
   return llvm::ArrayRef(g_expression_options);
 }
 
+EvaluateExpressionOptions
+CommandObjectExpression::CommandOptions::GetEvaluateExpressionOptions(
+const Target &target, const OptionGroupValueObjectDisplay &display_opts) {
+  EvaluateExpressionOptions options;
+  options.SetCoerceToId(display_opts.use_objc);
+  if (m_verbosity == eLanguageRuntimeDescriptionDisplayVerbosityCompact)
+options.SetSuppressPersistentResult(display_opts.use_objc);
+  options.SetUnwindOnError(unwind_on_error);
+  options.SetIgnoreBreakpoints(ignore_breakpoints);
+  options.SetKeepInMemory(true);
+  options.SetUseDynamic(display_opts.use_dynamic);
+  options.SetTryAllThreads(try_all_threads);
+  options.SetDebug(debug);
+  options.SetLanguage(language);
+  options.SetExecutionPolicy(
+  allow_jit ? EvaluateExpressionOptions::default_execution_policy
+: lldb_private::eExecutionPolicyNever);
+
+  bool auto_apply_fixits;
+  if (this->auto_apply_fixits == eLazyBoolCalculate)
+auto_apply_fixits = target.GetEnableAutoApplyFixIts();
+  else
+auto_apply_fixits = this->auto_apply_fixits == eLazyBoolYes;
+
+  options.SetAutoApplyFixIts(auto_apply_fixits);
+  options.SetRetriesWithFixIts(target.GetNumberOfRetriesWithFixits());
+
+  if (top_level)
+options.SetExecutionPolicy(eExecutionPolicyTopLevel);
+
+  // If there is any chance we are going to stop and want to see what went
+  /

[Lldb-commits] [lldb] 63c77bf - [lldb] Make persisting result variables configurable

2023-02-17 Thread Dave Lee via lldb-commits 

Author: Dave Lee
Date: 2023-02-17T17:50:43-08:00
New Revision: 63c77bf71d80b24df377fc45c80bfa1904ee849e

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

LOG: [lldb] Make persisting result variables configurable

Context: The `expression` command uses artificial variables to store the 
expression
result. This result variable is unconditionally kept around after the 
expression command
has completed. These variables are known as persistent results. These are the 
variables
`$0`, `$1`, etc, that are displayed when running `p` or `expression`.

This change allows users to control whether result variables are persisted, by
introducing a `--persistent-result` flag.

This change keeps the current default behavior, persistent results are created 
by
default. This change gives users the ability to opt-out by re-aliasing `p`. For 
example:

```
command unalias p
command alias p expression --persistent-result false --
```

For consistency, this flag is also adopted by `dwim-print`. Of note, if asked,
`dwim-print` will create a persistent result even for frame variables.

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

Added: 
lldb/test/API/commands/expression/persistent_result/Makefile
lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
lldb/test/API/commands/expression/persistent_result/main.c

Modified: 
lldb/source/Commands/CommandObjectDWIMPrint.cpp
lldb/source/Commands/CommandObjectExpression.cpp
lldb/source/Commands/CommandObjectExpression.h
lldb/source/Commands/Options.td
lldb/source/DataFormatters/ValueObjectPrinter.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 28c78af73dcec..a348f416af22f 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -62,13 +62,24 @@ bool CommandObjectDWIMPrint::DoExecute(StringRef command,
 
   auto verbosity = GetDebugger().GetDWIMPrintVerbosity();
 
+  Target *target_ptr = m_exe_ctx.GetTargetPtr();
+  // Fallback to the dummy target, which can allow for expression evaluation.
+  Target &target = target_ptr ? *target_ptr : GetDummyTarget();
+
+  const EvaluateExpressionOptions eval_options =
+  m_expr_options.GetEvaluateExpressionOptions(target, m_varobj_options);
+
   DumpValueObjectOptions dump_options = m_varobj_options.GetAsDumpOptions(
   m_expr_options.m_verbosity, m_format_options.GetFormat());
+  dump_options.SetHideName(eval_options.GetSuppressPersistentResult());
 
   // First, try `expr` as the name of a frame variable.
   if (StackFrame *frame = m_exe_ctx.GetFramePtr()) {
 auto valobj_sp = frame->FindVariable(ConstString(expr));
 if (valobj_sp && valobj_sp->GetError().Success()) {
+  if (!eval_options.GetSuppressPersistentResult())
+valobj_sp = valobj_sp->Persist();
+
   if (verbosity == eDWIMPrintVerbosityFull) {
 StringRef flags;
 if (args.HasArgs())
@@ -76,6 +87,7 @@ bool CommandObjectDWIMPrint::DoExecute(StringRef command,
 result.AppendMessageWithFormatv("note: ran `frame variable {0}{1}`",
 flags, expr);
   }
+
   valobj_sp->Dump(result.GetOutputStream(), dump_options);
   result.SetStatus(eReturnStatusSuccessFinishResult);
   return true;
@@ -84,13 +96,7 @@ bool CommandObjectDWIMPrint::DoExecute(StringRef command,
 
   // Second, also lastly, try `expr` as a source expression to evaluate.
   {
-Target *target_ptr = m_exe_ctx.GetTargetPtr();
-// Fallback to the dummy target, which can allow for expression evaluation.
-Target &target = target_ptr ? *target_ptr : GetDummyTarget();
-
 auto *exe_scope = m_exe_ctx.GetBestExecutionContextScope();
-const EvaluateExpressionOptions eval_options =
-m_expr_options.GetEvaluateExpressionOptions(target, m_varobj_options);
 ValueObjectSP valobj_sp;
 ExpressionResults expr_result =
 target.EvaluateExpression(expr, exe_scope, valobj_sp, eval_options);
@@ -102,6 +108,7 @@ bool CommandObjectDWIMPrint::DoExecute(StringRef command,
 result.AppendMessageWithFormatv("note: ran `expression {0}{1}`", flags,
 expr);
   }
+
   valobj_sp->Dump(result.GetOutputStream(), dump_options);
   result.SetStatus(eReturnStatusSuccessFinishResult);
   return true;

diff  --git a/lldb/source/Commands/CommandObjectExpression.cpp 
b/lldb/source/Commands/CommandObjectExpression.cpp
index f576729b76d97..63b92363369df 100644
--- a/lldb/source/Commands/CommandObjectExpression.cpp
+++ b/lldb/source/Commands/CommandObjectExpression.cpp
@@ -146,6 +146,19

[Lldb-commits] [PATCH] D144230: [lldb] Make persisting result variables configurable

2023-02-17 Thread Dave Lee via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG63c77bf71d80: [lldb] Make persisting result variables 
configurable (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144230

Files:
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectExpression.h
  lldb/source/Commands/Options.td
  lldb/source/DataFormatters/ValueObjectPrinter.cpp
  lldb/test/API/commands/dwim-print/TestDWIMPrint.py
  lldb/test/API/commands/expression/persistent_result/Makefile
  lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
  lldb/test/API/commands/expression/persistent_result/main.c

Index: lldb/test/API/commands/expression/persistent_result/main.c
===
--- /dev/null
+++ lldb/test/API/commands/expression/persistent_result/main.c
@@ -0,0 +1,4 @@
+int main() {
+  int i = 30;
+  return 0; // break here
+}
Index: lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/persistent_result/TestPersistentResult.py
@@ -0,0 +1,37 @@
+"""
+Test controlling `expression` result variables are persistent.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+def setUp(self):
+TestBase.setUp(self)
+self.build()
+lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c"))
+
+def test_enable_persistent_result(self):
+"""Test explicitly enabling result variables persistence."""
+self.expect("expression --persistent-result on -- i", substrs=["(int) $0 = 30"])
+# Verify the lifetime of $0 extends beyond the `expression` it was created in.
+self.expect("expression $0", substrs=["(int) $0 = 30"])
+
+def test_disable_persistent_result(self):
+"""Test explicitly disabling persistent result variables."""
+self.expect("expression --persistent-result off -- i", substrs=["(int) 30"])
+# Verify a persistent result was not silently created.
+self.expect("expression $0", error=True)
+
+def test_expression_persists_result(self):
+"""Test `expression`'s default behavior is to persist a result variable."""
+self.expect("expression i", substrs=["(int) $0 = 30"])
+self.expect("expression $0", substrs=["(int) $0 = 30"])
+
+def test_p_persists_result(self):
+"""Test `p` does persist a result variable."""
+self.expect("p i", substrs=["(int) $0 = 30"])
+self.expect("p $0", substrs=["(int) $0 = 30"])
Index: lldb/test/API/commands/expression/persistent_result/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/expression/persistent_result/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
Index: lldb/test/API/commands/dwim-print/TestDWIMPrint.py
===
--- lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -16,7 +16,8 @@
 self.ci.HandleCommand(cmd, result)
 return result.GetOutput().rstrip()
 
-PERSISTENT_VAR = re.compile(r"\$\d+")
+VAR_IDENT_RAW = r"(?:\$\d+|\w+) = "
+VAR_IDENT = re.compile(VAR_IDENT_RAW)
 
 def _mask_persistent_var(self, string: str) -> str:
 """
@@ -24,8 +25,9 @@
 that matches any persistent result (r'\$\d+'). The returned string can
 be matched against other `expression` results.
 """
-before, after = self.PERSISTENT_VAR.split(string, maxsplit=1)
-return re.escape(before) + r"\$\d+" + re.escape(after)
+before, after = self.VAR_IDENT.split(string, maxsplit=1)
+# Support either a frame variable (\w+) or a persistent result (\$\d+).
+return re.escape(before) + self.VAR_IDENT_RAW + re.escape(after)
 
 def _expect_cmd(
 self,
@@ -51,7 +53,7 @@
 substrs = [f"note: ran `{resolved_cmd}`"]
 patterns = []
 
-if actual_cmd == "expression" and self.PERSISTENT_VAR.search(expected_output):
+if self.VAR_IDENT.search(expected_output):
 patterns.append(self._mask_persistent_var(expected_output))
 else:
 substrs.append(expected_output)
Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -425,7 +425,9 @@
 if