[Lldb-commits] [lldb] [lldb] [debugserver] Handle interrupted reads correctly (PR #84872)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jason Molenda (jasonmolenda)


Changes

The first half of this patch is a long-standing annoyance, if I attach to 
debugserver with lldb while it is waiting for an lldb connection, the syscall 
is interrupted and it doesn't retry, debugserver exits immediately.

The second half is a request from another tool that is communicating with 
debugserver, that we retry reads on our sockets in the same way.  I haven't dug 
in to the details of how they're communicating that this is necessary, but the 
best I've been able to find reading the POSIX API docs, this is fine.

rdar://117113298

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


1 Files Affected:

- (modified) lldb/tools/debugserver/source/RNBSocket.cpp (+13-3) 


``diff
diff --git a/lldb/tools/debugserver/source/RNBSocket.cpp 
b/lldb/tools/debugserver/source/RNBSocket.cpp
index 1282ea221625a0..fc55dbf2e1f1b0 100644
--- a/lldb/tools/debugserver/source/RNBSocket.cpp
+++ b/lldb/tools/debugserver/source/RNBSocket.cpp
@@ -120,8 +120,13 @@ rnb_err_t RNBSocket::Listen(const char *listen_host, 
uint16_t port,
   while (!accept_connection) {
 
 struct kevent event_list[4];
-int num_events =
-kevent(queue_id, events.data(), events.size(), event_list, 4, NULL);
+int num_events;
+do {
+  errno = 0;
+  num_events =
+  kevent(queue_id, events.data(), events.size(), event_list, 4, NULL);
+} while (num_events == -1 &&
+ (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR));
 
 if (num_events < 0) {
   err.SetError(errno, DNBError::MachKernel);
@@ -291,7 +296,12 @@ rnb_err_t RNBSocket::Read(std::string ) {
   // DNBLogThreadedIf(LOG_RNB_COMM, "%8u RNBSocket::%s calling read()",
   // (uint32_t)m_timer.ElapsedMicroSeconds(true), __FUNCTION__);
   DNBError err;
-  ssize_t bytesread = read(m_fd, buf, sizeof(buf));
+  ssize_t bytesread;
+  do {
+errno = 0;
+bytesread = read(m_fd, buf, sizeof(buf));
+  } while (bytesread == -1 &&
+   (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR));
   if (bytesread <= 0)
 err.SetError(errno, DNBError::POSIX);
   else

``




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

[Lldb-commits] [lldb] [lldb] [debugserver] Handle interrupted reads correctly (PR #84872)

[Jason Molenda via lldb-commits]

https://github.com/jasonmolenda created 
https://github.com/llvm/llvm-project/pull/84872

The first half of this patch is a long-standing annoyance, if I attach to 
debugserver with lldb while it is waiting for an lldb connection, the syscall 
is interrupted and it doesn't retry, debugserver exits immediately.

The second half is a request from another tool that is communicating with 
debugserver, that we retry reads on our sockets in the same way.  I haven't dug 
in to the details of how they're communicating that this is necessary, but the 
best I've been able to find reading the POSIX API docs, this is fine.

rdar://117113298

>From b2ed63b985c2509594ef647f8258a0854ff404a7 Mon Sep 17 00:00:00 2001
From: Jason Molenda 
Date: Mon, 11 Mar 2024 22:19:49 -0700
Subject: [PATCH] [lldb] [debugserver] Handle interrupted reads correctly

The first half of this patch is a long-standing annoyance,
if I attach to debugserver with lldb while it is waiting
for an lldb connection, the syscall is interrupted and it
doesn't retry, debugserver exits immediately.

The second half is a request from another tool that is
communicating with debugserver, that we retry reads on
our sockets in the same way.  I haven't dug in to the
details of how they're communicating that this is necessary,
but the best I've been able to find reading the POSIX API
docs, this is fine.

rdar://117113298
---
 lldb/tools/debugserver/source/RNBSocket.cpp | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/lldb/tools/debugserver/source/RNBSocket.cpp 
b/lldb/tools/debugserver/source/RNBSocket.cpp
index 1282ea221625a0..fc55dbf2e1f1b0 100644
--- a/lldb/tools/debugserver/source/RNBSocket.cpp
+++ b/lldb/tools/debugserver/source/RNBSocket.cpp
@@ -120,8 +120,13 @@ rnb_err_t RNBSocket::Listen(const char *listen_host, 
uint16_t port,
   while (!accept_connection) {
 
 struct kevent event_list[4];
-int num_events =
-kevent(queue_id, events.data(), events.size(), event_list, 4, NULL);
+int num_events;
+do {
+  errno = 0;
+  num_events =
+  kevent(queue_id, events.data(), events.size(), event_list, 4, NULL);
+} while (num_events == -1 &&
+ (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR));
 
 if (num_events < 0) {
   err.SetError(errno, DNBError::MachKernel);
@@ -291,7 +296,12 @@ rnb_err_t RNBSocket::Read(std::string ) {
   // DNBLogThreadedIf(LOG_RNB_COMM, "%8u RNBSocket::%s calling read()",
   // (uint32_t)m_timer.ElapsedMicroSeconds(true), __FUNCTION__);
   DNBError err;
-  ssize_t bytesread = read(m_fd, buf, sizeof(buf));
+  ssize_t bytesread;
+  do {
+errno = 0;
+bytesread = read(m_fd, buf, sizeof(buf));
+  } while (bytesread == -1 &&
+   (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR));
   if (bytesread <= 0)
 err.SetError(errno, DNBError::POSIX);
   else

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

[Lldb-commits] [lldb] [lldb] Fix build break on windows (PR #84863)

[Luke Weiler via lldb-commits]

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

[Lldb-commits] [lldb] Fix build break on windows (PR #84863)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Luke Weiler (lwmaia)


Changes

This is a one line fix for a Windows specific (I believe) build break.

The build failure looks like this:
`D:\a\_work\1\s\lldb\source\Symbol\Symtab.cpp(128): error C2440: 
'function-style-cast': cannot convert from 'lldb_private::ConstString' 
to 'llvm::StringRef'
D:\a\_work\1\s\lldb\source\Symbol\Symtab.cpp(128): note: 
'llvm::StringRef::StringRef': ambiguous call to overloaded function
D:\a\_work\1\s\llvm\include\llvm/ADT/StringRef.h(840): note: could be 
'llvm::StringRef::StringRef(llvm::StringRef )'
D:\a\_work\1\s\llvm\include\llvm/ADT/StringRef.h(104): note: or   
'llvm::StringRef::StringRef(std::string_view)'
D:\a\_work\1\s\lldb\source\Symbol\Symtab.cpp(128): note: while trying to match 
the argument list '(lldb_private::ConstString)'
D:\a\_work\1\s\lldb\source\Symbol\Symtab.cpp(128): error C2672: 
'std::multimapllvm::StringRef,const lldb_private::Symbol 
*,std::lessllvm::StringRef,std::allocatorstd::pairconst 
llvm::StringRef,const lldb_private::Symbol *::emplace': no matching 
overloaded function found
C:\Program Files\Microsoft Visual 
Studio\2022\Enterprise\VC\Tools\MSVC\14.37.32822\include\map(557): note: could 
be 
'std::_Tree_iteratorstd::_Tree_valstd::_Tree_simple_typesstd::pairconst
 llvm::StringRef,const lldb_private::Symbol * 
std::multimapllvm::StringRef,const lldb_private::Symbol 
*,std::lessllvm::StringRef,std::allocatorstd::pairconst 
llvm::StringRef,const lldb_private::Symbol *::emplace(_Valty 
...)'
`

The StringRef constructor here is intended to take a ConstString object, which 
I assume is implicitly converted to a std::string_view by compilers other than 
Visual Studio's. To fix the VS build I made the StringRef initialization more 
explicit, as you can see in the diff.

This is my first time contributing to LLVM, please let me know if I can add any 
details/clarifications :)

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


1 Files Affected:

- (modified) lldb/source/Symbol/Symtab.cpp (+1-1) 


``diff
diff --git a/lldb/source/Symbol/Symtab.cpp b/lldb/source/Symbol/Symtab.cpp
index c63bbe94fece0e..5b5bf5c3f6f8c7 100644
--- a/lldb/source/Symbol/Symtab.cpp
+++ b/lldb/source/Symbol/Symtab.cpp
@@ -125,7 +125,7 @@ void Symtab::Dump(Stream *s, Target *target, SortOrder 
sort_order,
 
   std::multimap name_map;
   for (const Symbol  : m_symbols)
-name_map.emplace(llvm::StringRef(symbol.GetName()), );
+name_map.emplace(symbol.GetName().GetStringRef(), );
 
   for (const auto _to_symbol : name_map) {
 const Symbol *symbol = name_to_symbol.second;

``




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

[Lldb-commits] [lldb] Fix build break on windows (PR #84863)

[via lldb-commits]

github-actions[bot] wrote:



Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this 
page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using `@` followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from 
other developers.

If you have further questions, they may be answered by the [LLVM GitHub User 
Guide](https://llvm.org/docs/GitHub.html).

You can also ask questions in a comment on this PR, on the [LLVM 
Discord](https://discord.com/invite/xS7Z362) or on the 
[forums](https://discourse.llvm.org/).

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

[Lldb-commits] [lldb] Fix build break on windows (PR #84863)

[Luke Weiler via lldb-commits]

https://github.com/lwmaia created 
https://github.com/llvm/llvm-project/pull/84863

This is a one line fix for a Windows specific (I believe) build break.

The build failure looks like this:
`D:\a\_work\1\s\lldb\source\Symbol\Symtab.cpp(128): error C2440: 
'': cannot convert from 'lldb_private::ConstString' to 
'llvm::StringRef'
D:\a\_work\1\s\lldb\source\Symbol\Symtab.cpp(128): note: 
'llvm::StringRef::StringRef': ambiguous call to overloaded function
D:\a\_work\1\s\llvm\include\llvm/ADT/StringRef.h(840): note: could be 
'llvm::StringRef::StringRef(llvm::StringRef &&)'
D:\a\_work\1\s\llvm\include\llvm/ADT/StringRef.h(104): note: or   
'llvm::StringRef::StringRef(std::string_view)'
D:\a\_work\1\s\lldb\source\Symbol\Symtab.cpp(128): note: while trying to match 
the argument list '(lldb_private::ConstString)'
D:\a\_work\1\s\lldb\source\Symbol\Symtab.cpp(128): error C2672: 
'std::multimap,std::allocator>>::emplace': no matching 
overloaded function found
C:\Program Files\Microsoft Visual 
Studio\2022\Enterprise\VC\Tools\MSVC\14.37.32822\include\map(557): note: could 
be 'std::_Tree_iterator>>> 
std::multimap,std::allocator>>::emplace(_Valty &&...)'
`

The StringRef constructor here is intended to take a ConstString object, which 
I assume is implicitly converted to a std::string_view by compilers other than 
Visual Studio's. To fix the VS build I made the StringRef initialization more 
explicit, as you can see in the diff.

This is my first time contributing to LLVM, please let me know if I can add any 
details/clarifications :)

>From b4df918d0ba02f9092f42ca7668788977fbd7a3e Mon Sep 17 00:00:00 2001
From: Luke Weiler 
Date: Mon, 11 Mar 2024 19:32:30 -0700
Subject: [PATCH] fix build break on windows

---
 lldb/source/Symbol/Symtab.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/source/Symbol/Symtab.cpp b/lldb/source/Symbol/Symtab.cpp
index c63bbe94fece0e..5b5bf5c3f6f8c7 100644
--- a/lldb/source/Symbol/Symtab.cpp
+++ b/lldb/source/Symbol/Symtab.cpp
@@ -125,7 +125,7 @@ void Symtab::Dump(Stream *s, Target *target, SortOrder 
sort_order,
 
   std::multimap name_map;
   for (const Symbol  : m_symbols)
-name_map.emplace(llvm::StringRef(symbol.GetName()), );
+name_map.emplace(symbol.GetName().GetStringRef(), );
 
   for (const auto _to_symbol : name_map) {
 const Symbol *symbol = name_to_symbol.second;

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

[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)

[Jordan Rupprecht via lldb-commits]

https://github.com/rupprecht updated 
https://github.com/llvm/llvm-project/pull/84860

>From b5e04eb49c89c8c654305c6ba5db5e2f237bccec Mon Sep 17 00:00:00 2001
From: Jordan Rupprecht 
Date: Mon, 11 Mar 2024 11:23:59 -0700
Subject: [PATCH 1/2] [lldb][test] Add `pexpect` category for tests that
 `import pexpect`

- Add pexpect category
- Replace skipIfWindows with this
- Make categories apply to test classes too
---
 .../Python/lldbsuite/test/decorators.py   |  4 
 lldb/packages/Python/lldbsuite/test/dotest.py | 20 +++
 .../Python/lldbsuite/test/lldbpexpect.py  |  2 +-
 .../Python/lldbsuite/test/test_categories.py  |  1 +
 .../Python/lldbsuite/test/test_result.py  | 10 +-
 .../expression/TestExpressionCmd.py   |  5 +
 .../expression/TestRepeatedExprs.py   |  5 +
 .../TestFrameVariableResponse.py  |  5 +
 .../benchmarks/startup/TestStartupDelays.py   |  5 +
 .../benchmarks/stepping/TestSteppingSpeed.py  |  5 +
 .../TestCompileRunToBreakpointTurnaround.py   |  5 +
 .../API/terminal/TestSTTYBeforeAndAfter.py|  2 +-
 12 files changed, 34 insertions(+), 35 deletions(-)

diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py 
b/lldb/packages/Python/lldbsuite/test/decorators.py
index b691f82b90652c..8e13aa6a13882f 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -409,10 +409,6 @@ def add_test_categories(cat):
 cat = test_categories.validate(cat, True)
 
 def impl(func):
-if isinstance(func, type) and issubclass(func, unittest.TestCase):
-raise Exception(
-"@add_test_categories can only be used to decorate a test 
method"
-)
 try:
 if hasattr(func, "categories"):
 cat.extend(func.categories)
diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index 291d7bad5c0897..268c02e6b49dde 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -914,6 +914,25 @@ def checkForkVForkSupport():
 configuration.skip_categories.append("fork")
 
 
+def checkPexpectSupport():
+from lldbsuite.test import lldbplatformutil
+
+platform = lldbplatformutil.getPlatform()
+
+# llvm.org/pr22274: need a pexpect replacement for windows
+if platform in ["windows"]:
+if configuration.verbose:
+print("pexpect tests will be skipped because of unsupported 
platform")
+configuration.skip_categories.append("pexpect")
+elif not configuration.shouldSkipBecauseOfCategories(["pexpect"]):
+try:
+import pexpect
+except:
+print(
+"Warning: pexpect is not installed, but pexpect tests are not 
being skipped."
+)
+
+
 def run_suite():
 # On MacOS X, check to make sure that domain for com.apple.DebugSymbols 
defaults
 # does not exist before proceeding to running the test suite.
@@ -1013,6 +1032,7 @@ def run_suite():
 checkDebugServerSupport()
 checkObjcSupport()
 checkForkVForkSupport()
+checkPexpectSupport()
 
 skipped_categories_list = ", ".join(configuration.skip_categories)
 print(
diff --git a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py 
b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
index 9d216d90307473..998a080565b6b3 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -10,7 +10,7 @@
 
 
 @skipIfRemote
-@skipIfWindows  # llvm.org/pr22274: need a pexpect replacement for windows
+@add_test_categories(["pexpect"])
 class PExpectTest(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 PROMPT = "(lldb) "
diff --git a/lldb/packages/Python/lldbsuite/test/test_categories.py 
b/lldb/packages/Python/lldbsuite/test/test_categories.py
index 3f8de175e29df3..036bda9c957d11 100644
--- a/lldb/packages/Python/lldbsuite/test/test_categories.py
+++ b/lldb/packages/Python/lldbsuite/test/test_categories.py
@@ -33,6 +33,7 @@
 "lldb-server": "Tests related to lldb-server",
 "lldb-dap": "Tests for the Debug Adaptor Protocol with lldb-dap",
 "llgs": "Tests for the gdb-server functionality of lldb-server",
+"pexpect": "Tests requiring the pexpect library to be available",
 "objc": "Tests related to the Objective-C programming language support",
 "pyapi": "Tests related to the Python API",
 "std-module": "Tests related to importing the std module",
diff --git a/lldb/packages/Python/lldbsuite/test/test_result.py 
b/lldb/packages/Python/lldbsuite/test/test_result.py
index 20365f53a67541..cda67ae27d4674 100644
--- a/lldb/packages/Python/lldbsuite/test/test_result.py
+++ b/lldb/packages/Python/lldbsuite/test/test_result.py
@@ -148,9 +148,11 @@ def getCategoriesForTest(self, test):
 Gets all the categories for the currently running test 

[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)

[Jordan Rupprecht via lldb-commits]

@@ -914,6 +914,25 @@ def checkForkVForkSupport():
 configuration.skip_categories.append("fork")
 
 
+def checkPexpectSupport():
+from lldbsuite.test import lldbplatformutil
+
+platform = lldbplatformutil.getPlatform()
+
+# llvm.org/pr22274: need a pexpect replacement for windows
+if platform in ["windows"]:
+if configuration.verbose:
+print("pexpect tests will be skipped because of unsupported 
platform")
+configuration.skip_categories.append("pexpect")
+elif not configuration.shouldSkipBecauseOfCategories(["pexpect"]):
+try:
+import pexpect
+except:
+print(
+"Warning: pexpect is not installed, but pexpect tests are not 
being skipped."

rupprecht wrote:

Note: instead of just printing a warning, we could make this also add `pexpect` 
to `skip_categories` as above. However, that would lead to a false notion that 
`pexpect` tests are running and passing. Someone who is running these tests may 
not be aware that `pexpect` needs to be installed, or maybe they are aware but 
a system change made them go away. IMHO, it is safer to let the tests fail and 
make the user acknowledge that `pexpect` tests should be skipped.

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

[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)

[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 
2a3f27cce8983e5d6871b9ebb8f5e9dd91884f0c...b5e04eb49c89c8c654305c6ba5db5e2f237bccec
 lldb/packages/Python/lldbsuite/test/decorators.py 
lldb/packages/Python/lldbsuite/test/dotest.py 
lldb/packages/Python/lldbsuite/test/lldbpexpect.py 
lldb/packages/Python/lldbsuite/test/test_categories.py 
lldb/packages/Python/lldbsuite/test/test_result.py 
lldb/test/API/benchmarks/expression/TestExpressionCmd.py 
lldb/test/API/benchmarks/expression/TestRepeatedExprs.py 
lldb/test/API/benchmarks/frame_variable/TestFrameVariableResponse.py 
lldb/test/API/benchmarks/startup/TestStartupDelays.py 
lldb/test/API/benchmarks/stepping/TestSteppingSpeed.py 
lldb/test/API/benchmarks/turnaround/TestCompileRunToBreakpointTurnaround.py 
lldb/test/API/terminal/TestSTTYBeforeAndAfter.py
``





View the diff from darker here.


``diff
--- packages/Python/lldbsuite/test/test_result.py   2024-03-11 
18:23:59.00 +
+++ packages/Python/lldbsuite/test/test_result.py   2024-03-12 
01:18:26.696261 +
@@ -158,11 +158,13 @@
 
 return test_categories
 
 def hardMarkAsSkipped(self, test):
 getattr(test, test._testMethodName).__func__.__unittest_skip__ = True
-getattr(test, test._testMethodName).__func__.__unittest_skip_why__ = (
+getattr(
+test, test._testMethodName
+).__func__.__unittest_skip_why__ = (
 "test case does not fall in any category of interest for this run"
 )
 
 def checkExclusion(self, exclusion_list, name):
 if exclusion_list:

``




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

[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jordan Rupprecht (rupprecht)


Changes

Instead of directly annotating pexpect-based tests with `@skipIfWindows`, we can tag them with a new `pexpect` category. We still 
automatically skip windows behavior by adding `pexpect` to the skip category 
list if the platform is windows, but also allow non-Windows users to skip them 
by configuring cmake with `-DLLDB_TEST_USER_ARGS=--skip-category=pexpect`

As a prerequisite, remove the restriction that `@add_test_categories` 
can only apply to test cases, and we make the test runner look for categories 
on both the class and the test method.

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


12 Files Affected:

- (modified) lldb/packages/Python/lldbsuite/test/decorators.py (-4) 
- (modified) lldb/packages/Python/lldbsuite/test/dotest.py (+20) 
- (modified) lldb/packages/Python/lldbsuite/test/lldbpexpect.py (+1-1) 
- (modified) lldb/packages/Python/lldbsuite/test/test_categories.py (+1) 
- (modified) lldb/packages/Python/lldbsuite/test/test_result.py (+5-5) 
- (modified) lldb/test/API/benchmarks/expression/TestExpressionCmd.py (+1-4) 
- (modified) lldb/test/API/benchmarks/expression/TestRepeatedExprs.py (+1-4) 
- (modified) 
lldb/test/API/benchmarks/frame_variable/TestFrameVariableResponse.py (+1-4) 
- (modified) lldb/test/API/benchmarks/startup/TestStartupDelays.py (+1-4) 
- (modified) lldb/test/API/benchmarks/stepping/TestSteppingSpeed.py (+1-4) 
- (modified) 
lldb/test/API/benchmarks/turnaround/TestCompileRunToBreakpointTurnaround.py 
(+1-4) 
- (modified) lldb/test/API/terminal/TestSTTYBeforeAndAfter.py (+1-1) 


``diff
diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py 
b/lldb/packages/Python/lldbsuite/test/decorators.py
index b691f82b90652c..8e13aa6a13882f 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -409,10 +409,6 @@ def add_test_categories(cat):
 cat = test_categories.validate(cat, True)
 
 def impl(func):
-if isinstance(func, type) and issubclass(func, unittest.TestCase):
-raise Exception(
-"@add_test_categories can only be used to decorate a test 
method"
-)
 try:
 if hasattr(func, "categories"):
 cat.extend(func.categories)
diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index 291d7bad5c0897..268c02e6b49dde 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -914,6 +914,25 @@ def checkForkVForkSupport():
 configuration.skip_categories.append("fork")
 
 
+def checkPexpectSupport():
+from lldbsuite.test import lldbplatformutil
+
+platform = lldbplatformutil.getPlatform()
+
+# llvm.org/pr22274: need a pexpect replacement for windows
+if platform in ["windows"]:
+if configuration.verbose:
+print("pexpect tests will be skipped because of unsupported 
platform")
+configuration.skip_categories.append("pexpect")
+elif not configuration.shouldSkipBecauseOfCategories(["pexpect"]):
+try:
+import pexpect
+except:
+print(
+"Warning: pexpect is not installed, but pexpect tests are not 
being skipped."
+)
+
+
 def run_suite():
 # On MacOS X, check to make sure that domain for com.apple.DebugSymbols 
defaults
 # does not exist before proceeding to running the test suite.
@@ -1013,6 +1032,7 @@ def run_suite():
 checkDebugServerSupport()
 checkObjcSupport()
 checkForkVForkSupport()
+checkPexpectSupport()
 
 skipped_categories_list = ", ".join(configuration.skip_categories)
 print(
diff --git a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py 
b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
index 9d216d90307473..998a080565b6b3 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -10,7 +10,7 @@
 
 
 @skipIfRemote
-@skipIfWindows  # llvm.org/pr22274: need a pexpect replacement for windows
+@add_test_categories(["pexpect"])
 class PExpectTest(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 PROMPT = "(lldb) "
diff --git a/lldb/packages/Python/lldbsuite/test/test_categories.py 
b/lldb/packages/Python/lldbsuite/test/test_categories.py
index 3f8de175e29df3..036bda9c957d11 100644
--- a/lldb/packages/Python/lldbsuite/test/test_categories.py
+++ b/lldb/packages/Python/lldbsuite/test/test_categories.py
@@ -33,6 +33,7 @@
 "lldb-server": "Tests related to lldb-server",
 "lldb-dap": "Tests for the Debug Adaptor Protocol with lldb-dap",
 "llgs": "Tests for the gdb-server functionality of lldb-server",
+"pexpect": "Tests requiring the pexpect library to be available",
 "objc": "Tests related to the Objective-C programming language support",
 "pyapi": 

[Lldb-commits] [lldb] [lldb][test] Add `pexpect` category for tests that `import pexpect` (PR #84860)

[Jordan Rupprecht via lldb-commits]

https://github.com/rupprecht created 
https://github.com/llvm/llvm-project/pull/84860

Instead of directly annotating pexpect-based tests with `@skipIfWindows`, we 
can tag them with a new `pexpect` category. We still automatically skip windows 
behavior by adding `pexpect` to the skip category list if the platform is 
windows, but also allow non-Windows users to skip them by configuring cmake 
with `-DLLDB_TEST_USER_ARGS=--skip-category=pexpect`

As a prerequisite, remove the restriction that `@add_test_categories` can only 
apply to test cases, and we make the test runner look for categories on both 
the class and the test method.

>From b5e04eb49c89c8c654305c6ba5db5e2f237bccec Mon Sep 17 00:00:00 2001
From: Jordan Rupprecht 
Date: Mon, 11 Mar 2024 11:23:59 -0700
Subject: [PATCH] [lldb][test] Add `pexpect` category for tests that `import
 pexpect`

- Add pexpect category
- Replace skipIfWindows with this
- Make categories apply to test classes too
---
 .../Python/lldbsuite/test/decorators.py   |  4 
 lldb/packages/Python/lldbsuite/test/dotest.py | 20 +++
 .../Python/lldbsuite/test/lldbpexpect.py  |  2 +-
 .../Python/lldbsuite/test/test_categories.py  |  1 +
 .../Python/lldbsuite/test/test_result.py  | 10 +-
 .../expression/TestExpressionCmd.py   |  5 +
 .../expression/TestRepeatedExprs.py   |  5 +
 .../TestFrameVariableResponse.py  |  5 +
 .../benchmarks/startup/TestStartupDelays.py   |  5 +
 .../benchmarks/stepping/TestSteppingSpeed.py  |  5 +
 .../TestCompileRunToBreakpointTurnaround.py   |  5 +
 .../API/terminal/TestSTTYBeforeAndAfter.py|  2 +-
 12 files changed, 34 insertions(+), 35 deletions(-)

diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py 
b/lldb/packages/Python/lldbsuite/test/decorators.py
index b691f82b90652c..8e13aa6a13882f 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -409,10 +409,6 @@ def add_test_categories(cat):
 cat = test_categories.validate(cat, True)
 
 def impl(func):
-if isinstance(func, type) and issubclass(func, unittest.TestCase):
-raise Exception(
-"@add_test_categories can only be used to decorate a test 
method"
-)
 try:
 if hasattr(func, "categories"):
 cat.extend(func.categories)
diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index 291d7bad5c0897..268c02e6b49dde 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -914,6 +914,25 @@ def checkForkVForkSupport():
 configuration.skip_categories.append("fork")
 
 
+def checkPexpectSupport():
+from lldbsuite.test import lldbplatformutil
+
+platform = lldbplatformutil.getPlatform()
+
+# llvm.org/pr22274: need a pexpect replacement for windows
+if platform in ["windows"]:
+if configuration.verbose:
+print("pexpect tests will be skipped because of unsupported 
platform")
+configuration.skip_categories.append("pexpect")
+elif not configuration.shouldSkipBecauseOfCategories(["pexpect"]):
+try:
+import pexpect
+except:
+print(
+"Warning: pexpect is not installed, but pexpect tests are not 
being skipped."
+)
+
+
 def run_suite():
 # On MacOS X, check to make sure that domain for com.apple.DebugSymbols 
defaults
 # does not exist before proceeding to running the test suite.
@@ -1013,6 +1032,7 @@ def run_suite():
 checkDebugServerSupport()
 checkObjcSupport()
 checkForkVForkSupport()
+checkPexpectSupport()
 
 skipped_categories_list = ", ".join(configuration.skip_categories)
 print(
diff --git a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py 
b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
index 9d216d90307473..998a080565b6b3 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -10,7 +10,7 @@
 
 
 @skipIfRemote
-@skipIfWindows  # llvm.org/pr22274: need a pexpect replacement for windows
+@add_test_categories(["pexpect"])
 class PExpectTest(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 PROMPT = "(lldb) "
diff --git a/lldb/packages/Python/lldbsuite/test/test_categories.py 
b/lldb/packages/Python/lldbsuite/test/test_categories.py
index 3f8de175e29df3..036bda9c957d11 100644
--- a/lldb/packages/Python/lldbsuite/test/test_categories.py
+++ b/lldb/packages/Python/lldbsuite/test/test_categories.py
@@ -33,6 +33,7 @@
 "lldb-server": "Tests related to lldb-server",
 "lldb-dap": "Tests for the Debug Adaptor Protocol with lldb-dap",
 "llgs": "Tests for the gdb-server functionality of lldb-server",
+"pexpect": "Tests requiring the pexpect library to be available",
 "objc": "Tests related to the 

[Lldb-commits] [lldb] [lldb] Implement coalescing of disjoint progress events (PR #84854)

[Chelsea Cassanova 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;
+static constexpr bool RUN_CALLBACKS_ON_EXIT = true;
+
+// Enable strict checking of the ALARM_TIMEOUTs. This should only be enabled 
for
+// development and never during automated testing where scheduling and
+// ALARM_TIMEOUTs are unpredictable.
+#define STRICT_ALARM_TIMEOUT 1
+
+#if STRICT_ALARM_TIMEOUT
+static constexpr auto EPSILON = 10ms;
+#endif
+
+TEST(AlarmTest, Create) {
+  std::mutex m;
+
+  std::vector callbacks_actual;
+  std::vector callbacks_expected;
+
+  Alarm alarm(ALARM_TIMEOUT, RUN_CALLBACKS_ON_EXIT);
+
+  // Create 5 alarms some time apart.
+  for (size_t i = 0; i < 5; ++i) {
+callbacks_actual.emplace_back();
+callbacks_expected.emplace_back(std::chrono::system_clock::now() +
+ALARM_TIMEOUT);
+
+alarm.Create([_actual, , i]() {
+  std::lock_guard guard(m);
+  callbacks_actual[i] = std::chrono::system_clock::now();
+});
+
+std::this_thread::sleep_for(ALARM_TIMEOUT / 5);
+  }
+
+  // Leave plenty of time for all the alarms to fire.
+  std::this_thread::sleep_for(TEST_TIMEOUT);
+
+  // Make sure all the alarms around the expected time.
+  for (size_t i = 0; i < 5; ++i) {
+EXPECT_GE(callbacks_actual[i], callbacks_expected[i]);
+#if STRICT_ALARM_TIMEOUT
+EXPECT_LE(callbacks_actual[i], callbacks_expected[i] + EPSILON);
+#endif
+  }
+}
+
+TEST(AlarmTest, Exit) {
+  std::mutex m;
+
+  std::vector handles;
+  std::vector callbacks;
+
+  {
+Alarm alarm(ALARM_TIMEOUT, RUN_CALLBACKS_ON_EXIT);
+
+// Create 5 alarms.
+for (size_t i = 0; i < 5; ++i) {
+  callbacks.emplace_back(false);
+
+  handles.push_back(alarm.Create([, , i]() {
+std::lock_guard guard(m);
+callbacks[i] = true;
+  }));
+}
+
+// Let the alarm go out of scope before any alarm had a chance to fire.
+  }
+
+  // Make sure none of the first 4 alarms fired.
+  for (bool callback : callbacks)
+EXPECT_TRUE(callback);

chelcassanova wrote:

Just wondering, why do we check that the first 4 alarms didn't fire but not the 
final one here?

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

[Lldb-commits] [lldb] [lldb] Implement coalescing of disjoint progress events (PR #84854)

[Chelsea Cassanova 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;
+static constexpr bool RUN_CALLBACKS_ON_EXIT = true;
+
+// Enable strict checking of the ALARM_TIMEOUTs. This should only be enabled 
for
+// development and never during automated testing where scheduling and
+// ALARM_TIMEOUTs are unpredictable.
+#define STRICT_ALARM_TIMEOUT 1
+
+#if STRICT_ALARM_TIMEOUT
+static constexpr auto EPSILON = 10ms;
+#endif
+
+TEST(AlarmTest, Create) {
+  std::mutex m;
+
+  std::vector callbacks_actual;
+  std::vector callbacks_expected;
+
+  Alarm alarm(ALARM_TIMEOUT, RUN_CALLBACKS_ON_EXIT);
+
+  // Create 5 alarms some time apart.
+  for (size_t i = 0; i < 5; ++i) {
+callbacks_actual.emplace_back();
+callbacks_expected.emplace_back(std::chrono::system_clock::now() +
+ALARM_TIMEOUT);
+
+alarm.Create([_actual, , i]() {
+  std::lock_guard guard(m);
+  callbacks_actual[i] = std::chrono::system_clock::now();
+});
+
+std::this_thread::sleep_for(ALARM_TIMEOUT / 5);
+  }
+
+  // Leave plenty of time for all the alarms to fire.
+  std::this_thread::sleep_for(TEST_TIMEOUT);
+
+  // Make sure all the alarms around the expected time.

chelcassanova wrote:

nit: Missing word or am I just reading this wrong?

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

[Lldb-commits] [lldb] [lldb] Implement coalescing of disjoint progress events (PR #84854)

[Chelsea Cassanova via lldb-commits]

@@ -210,3 +211,37 @@ TEST_F(ProgressReportTest, TestOverlappingEvents) {
   // initial report.
   EXPECT_EQ(data->GetID(), expected_progress_id);
 }
+
+TEST_F(ProgressReportTest, TestProgressManagerDisjointReports) {
+  ListenerSP listener_sp =
+  CreateListenerFor(Debugger::eBroadcastBitProgressCategory);
+  EventSP event_sp;
+  const ProgressEventData *data;
+  uint64_t expected_progress_id;
+
+  { Progress progress("Coalesced report 1", "Starting report 1"); }
+  { Progress progress("Coalesced report 1", "Starting report 2"); }
+  { Progress progress("Coalesced report 1", "Starting report 3"); }
+
+  ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+  data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+  expected_progress_id = data->GetID();
+
+  EXPECT_EQ(data->GetDetails(), "");
+  EXPECT_FALSE(data->IsFinite());
+  EXPECT_FALSE(data->GetCompleted());
+  EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
+  EXPECT_EQ(data->GetMessage(), "Coalesced report 1");
+
+  ASSERT_TRUE(listener_sp->GetEvent(event_sp, TIMEOUT));
+  data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
+
+  EXPECT_EQ(data->GetID(), expected_progress_id);
+  EXPECT_EQ(data->GetDetails(), "");
+  EXPECT_FALSE(data->IsFinite());
+  EXPECT_TRUE(data->GetCompleted());
+  EXPECT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
+  EXPECT_EQ(data->GetMessage(), "Coalesced report 1");
+
+  ASSERT_FALSE(listener_sp->GetEvent(event_sp, TIMEOUT));

chelcassanova wrote:

Good point to try popping another event from the stack and expect that to not 
work, this was something I was going to add to the original progress manager 
test but I thought would be overkill  .

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

[Lldb-commits] [lldb] [lldb] Implement coalescing of disjoint progress events (PR #84854)

[Chelsea Cassanova via lldb-commits]

@@ -0,0 +1,88 @@
+//===-- 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 {
+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();
+
+  Handle Create(Callback callback);
+  bool Restart(Handle handle);
+  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();
+
+  TimePoint GetNextExpiration() const;
+
+  /// Alarm entry.
+  struct Entry {
+Handle handle;
+Callback callback;
+TimePoint expiration;
+
+Entry(Callback callback, TimePoint expiration);
+bool operator==(const Entry ) { 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_variable m_alarm_cv;
+
+  /// Flag to signal the alarm thread that something changed and we need to
+  // recompute the next alarm.

chelcassanova wrote:

nit: missing third slash for doxygen?

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

[Lldb-commits] [lldb] [lldb] Implement coalescing of disjoint progress events (PR #84854)

[Jonas Devlieghere via lldb-commits]

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

>From 3aae84b8caaf904c11b1dab620893620a0fa1c85 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Tue, 5 Mar 2024 22:57:43 -0800
Subject: [PATCH 1/2] [lldb] Add an Alarm class

Add an Alarm class which allows scheduling callbacks after a specific
timeout has elapsed.
---
 lldb/include/lldb/Host/Alarm.h |  88 +
 lldb/source/Host/CMakeLists.txt|   1 +
 lldb/source/Host/common/Alarm.cpp  | 193 +
 lldb/unittests/Host/AlarmTest.cpp  | 164 
 lldb/unittests/Host/CMakeLists.txt |   1 +
 5 files changed, 447 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..8b4180fbb32835
--- /dev/null
+++ b/lldb/include/lldb/Host/Alarm.h
@@ -0,0 +1,88 @@
+//===-- 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 {
+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();
+
+  Handle Create(Callback callback);
+  bool Restart(Handle handle);
+  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();
+
+  TimePoint GetNextExpiration() const;
+
+  /// Alarm entry.
+  struct Entry {
+Handle handle;
+Callback callback;
+TimePoint expiration;
+
+Entry(Callback callback, TimePoint expiration);
+bool operator==(const Entry ) { 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_variable m_alarm_cv;
+
+  /// Flag to signal the alarm thread that something changed and we need to
+  // recompute the next alarm.
+  bool m_recompute_next_alarm = false;
+
+  /// Flag to signal the alarm thread to exit.
+  bool m_exit = false;
+
+  /// Flag to signal we should run all callbacks on exit.
+  bool m_run_callbacks_on_exit = false;
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_HOST_ALARM_H
diff --git a/lldb/source/Host/CMakeLists.txt b/lldb/source/Host/CMakeLists.txt
index fe6e539f758fda..c2e091ee8555b7 100644
--- a/lldb/source/Host/CMakeLists.txt
+++ b/lldb/source/Host/CMakeLists.txt
@@ -13,6 +13,7 @@ macro(add_host_subdirectory group)
 endmacro()
 
 add_host_subdirectory(common
+  common/Alarm.cpp
   common/FileAction.cpp
   common/FileCache.cpp
   common/File.cpp
diff --git a/lldb/source/Host/common/Alarm.cpp 
b/lldb/source/Host/common/Alarm.cpp
new file mode 100644
index 00..b9366522210361
--- /dev/null
+++ b/lldb/source/Host/common/Alarm.cpp
@@ -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 

[Lldb-commits] [lldb] [lldb] Implement coalescing of disjoint progress events (PR #84854)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)


Changes

This implements coalescing of progress events using a timeout, as discussed in 
the RFC on Discourse [1]. This PR consists of two commits which, depending on 
the feedback, I may split up into two PRs. For now, I think it's easier to 
review this as a whole. 

1. The first commit introduces a new generic `Alarm` class. The class lets you 
to schedule a function (callback) to be executed after a given timeout expires. 
You can cancel and reset a callback before its corresponding timeout expires. 
It achieves this with the help of a worker thread that sleeps until the next 
timeout expires. The only guarantee it provides is that your function is called 
no sooner than the requested timeout. Because the callback is called directly 
from the worker thread, a long running callback could potentially block the 
worker thread. I intentionally kept the implementation as simple as possible 
while addressing the needs for the `ProgressManager` use case. If we want to 
rely on this somewhere else, we can reassess whether we need to address those 
limitations.  
2. The second commit uses the Alarm class to coalesce progress events. To recap 
the Discourse discussion, when multiple progress events with the same title 
execute in close succession, they get broadcast as one to 
`eBroadcastBitProgressCategory`. The `ProgressManager` keeps track of the 
in-flight progress events and when the refcount hits zero, the Alarm class is 
used to schedule broadcasting the event. If a new progress event comes in 
before the alarm fires, the alarm is reset (and the process repeats when the 
new progress event ends). If no new event comes in before the timeout expires, 
the progress event is broadcast. 

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

---

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


9 Files Affected:

- (modified) lldb/include/lldb/Core/Progress.h (+17-4) 
- (added) lldb/include/lldb/Host/Alarm.h (+88) 
- (modified) lldb/source/Core/Progress.cpp (+89-25) 
- (modified) lldb/source/Host/CMakeLists.txt (+1) 
- (added) lldb/source/Host/common/Alarm.cpp (+193) 
- (modified) lldb/source/Initialization/SystemInitializerCommon.cpp (+3) 
- (modified) lldb/unittests/Core/ProgressReportTest.cpp (+54-2) 
- (added) lldb/unittests/Host/AlarmTest.cpp (+164) 
- (modified) lldb/unittests/Host/CMakeLists.txt (+1) 


``diff
diff --git a/lldb/include/lldb/Core/Progress.h 
b/lldb/include/lldb/Core/Progress.h
index c38f6dd0a140ed..67d96b01e67c4a 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -9,6 +9,7 @@
 #ifndef LLDB_CORE_PROGRESS_H
 #define LLDB_CORE_PROGRESS_H
 
+#include "lldb/Host/Alarm.h"
 #include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/StringMap.h"
@@ -146,9 +147,12 @@ class ProgressManager {
   void Increment(const Progress::ProgressData &);
   void Decrement(const Progress::ProgressData &);
 
+  static void Initialize();
+  static void Terminate();
+  static bool Enabled();
   static ProgressManager ();
 
-private:
+protected:
   enum class EventType {
 Begin,
 End,
@@ -156,9 +160,18 @@ class ProgressManager {
   static void ReportProgress(const Progress::ProgressData _data,
  EventType type);
 
-  llvm::StringMap>
-  m_progress_category_map;
-  std::mutex m_progress_map_mutex;
+  static std::optional ();
+
+  void Expire(llvm::StringRef key);
+  struct Entry {
+uint64_t refcount = 0;
+Progress::ProgressData data;
+Alarm::Handle handle = Alarm::INVALID_HANDLE;
+  };
+
+  Alarm m_alarm;
+  llvm::StringMap m_entries;
+  std::mutex m_entries_mutex;
 };
 
 } // namespace lldb_private
diff --git a/lldb/include/lldb/Host/Alarm.h b/lldb/include/lldb/Host/Alarm.h
new file mode 100644
index 00..8b4180fbb32835
--- /dev/null
+++ b/lldb/include/lldb/Host/Alarm.h
@@ -0,0 +1,88 @@
+//===-- 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 {
+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();
+
+  Handle Create(Callback callback);
+  bool Restart(Handle handle);
+  bool Cancel(Handle handle);
+
+  static 

[Lldb-commits] [lldb] [lldb] Implement coalescing of disjoint progress events (PR #84854)

[Jonas Devlieghere via lldb-commits]

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

This implements coalescing of progress events using a timeout, as discussed in 
the RFC on Discourse [1]. This PR consists of two commits which, depending on 
the feedback, I may split up into two PRs. For now, I think it's easier to 
review this as a whole. 

1. The first commit introduces a new generic `Alarm` class. The class lets you 
to schedule a function (callback) to be executed after a given timeout expires. 
You can cancel and reset a callback before its corresponding timeout expires. 
It achieves this with the help of a worker thread that sleeps until the next 
timeout expires. The only guarantee it provides is that your function is called 
no sooner than the requested timeout. Because the callback is called directly 
from the worker thread, a long running callback could potentially block the 
worker thread. I intentionally kept the implementation as simple as possible 
while addressing the needs for the `ProgressManager` use case. If we want to 
rely on this somewhere else, we can reassess whether we need to address those 
limitations.  
2. The second commit uses the Alarm class to coalesce progress events. To recap 
the Discourse discussion, when multiple progress events with the same title 
execute in close succession, they get broadcast as one to 
`eBroadcastBitProgressCategory`. The `ProgressManager` keeps track of the 
in-flight progress events and when the refcount hits zero, the Alarm class is 
used to schedule broadcasting the event. If a new progress event comes in 
before the alarm fires, the alarm is reset (and the process repeats when the 
new progress event ends). If no new event comes in before the timeout expires, 
the progress event is broadcast. 

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

>From 3aae84b8caaf904c11b1dab620893620a0fa1c85 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Tue, 5 Mar 2024 22:57:43 -0800
Subject: [PATCH 1/2] [lldb] Add an Alarm class

Add an Alarm class which allows scheduling callbacks after a specific
timeout has elapsed.
---
 lldb/include/lldb/Host/Alarm.h |  88 +
 lldb/source/Host/CMakeLists.txt|   1 +
 lldb/source/Host/common/Alarm.cpp  | 193 +
 lldb/unittests/Host/AlarmTest.cpp  | 164 
 lldb/unittests/Host/CMakeLists.txt |   1 +
 5 files changed, 447 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..8b4180fbb32835
--- /dev/null
+++ b/lldb/include/lldb/Host/Alarm.h
@@ -0,0 +1,88 @@
+//===-- 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 {
+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();
+
+  Handle Create(Callback callback);
+  bool Restart(Handle handle);
+  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();
+
+  TimePoint GetNextExpiration() const;
+
+  /// Alarm entry.
+  struct Entry {
+Handle handle;
+Callback callback;
+TimePoint expiration;
+
+Entry(Callback callback, TimePoint expiration);
+bool operator==(const Entry ) { 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_variable m_alarm_cv;
+
+  /// Flag to signal the alarm thread that something changed and we need to
+  // recompute the next alarm.
+  bool m_recompute_next_alarm 

[Lldb-commits] [lldb] Make ValueObject::Cast work for casts from smaller to larger structs in the cases where this currently can work. (PR #84588)

[via lldb-commits]

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

[Lldb-commits] [lldb] 3707c54 - Make ValueObject::Cast work for casts from smaller to larger structs in the cases where this currently can work. (#84588)

[via lldb-commits]

Author: jimingham
Date: 2024-03-11T14:13:37-07:00
New Revision: 3707c540d23a5684a1c37b0f7e41c1d8ed7f1f8a

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

LOG: Make ValueObject::Cast work for casts from smaller to larger structs in 
the cases where this currently can work. (#84588)

The ValueObjectConstResult classes that back expression result variables
play a complicated game with where the data for their values is stored.
They try to make it appear as though they are still tied to the memory
in the target into which their value was written when the expression is
run, but they also keep a copy in the Host which they use after the
value is made (expression results are "history values" so that's how we
make sure they have "the value at the time of the expression".)

However, that means that if you ask them to cast themselves to a value
bigger than their original size, they don't have a way to get more
memory for that purpose. The same thing is true of ValueObjects backed
by DataExtractors, the data extractors don't know how to get more data
than they were made with in general.

The only place where we actually ask ValueObjects to sample outside
their captured bounds is when you do ValueObject::Cast from one
structure type to a bigger structure type. In
https://reviews.llvm.org/D153657 I handled this by just disallowing
casts from one structure value to a larger one. My reasoning at the time
was that the use case for this was to support discriminator based C
inheritance schemes, and you can't directly cast values in C, only
pointers, so this was not a natural way to handle those types. It seemed
logical that since you would have had to start with pointers in the
implementation, that's how you would write your lldb introspection code
as well.

Famous last words...

Turns out there are some heavy users of the SB API's who were relying on
this working, and this is a behavior change, so this patch makes this
work in the cases where it used to work before, while still disallowing
the cases we don't know how to support.

Note that if you had done this Cast operation before with either
expression results or value objects from data extractors, lldb would not
have returned the correct results, so the cases this patch outlaws are
ones that actually produce invalid results. So nobody should be using
Cast in these cases, or if they were, this patch will point out the bug
they hadn't yet noticed.

Added: 


Modified: 
lldb/source/Core/ValueObject.cpp
lldb/test/API/python_api/value/TestValueAPI.py
lldb/test/API/python_api/value/main.c

Removed: 




diff  --git a/lldb/source/Core/ValueObject.cpp 
b/lldb/source/Core/ValueObject.cpp
index d813044d02ff5f..f39bd07a255366 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -2744,8 +2744,19 @@ ValueObjectSP ValueObject::DoCast(const CompilerType 
_type) {
 
 ValueObjectSP ValueObject::Cast(const CompilerType _type) {
   // Only allow casts if the original type is equal or larger than the cast
-  // type.  We don't know how to fetch more data for all the ConstResult types,
-  // so we can't guarantee this will work:
+  // type, unless we know this is a load address.  Getting the size wrong for
+  // a host side storage could leak lldb memory, so we absolutely want to 
+  // prevent that.  We may not always get the right value, for instance if we
+  // have an expression result value that's copied into a storage location in
+  // the target may not have copied enough memory.  I'm not trying to fix that
+  // here, I'm just making Cast from a smaller to a larger possible in all the
+  // cases where that doesn't risk making a Value out of random lldb memory.
+  // You have to check the ValueObject's Value for the address types, since
+  // ValueObjects that use live addresses will tell you they fetch data from 
the
+  // live address, but once they are made, they actually don't.
+  // FIXME: Can we make ValueObject's with a live address fetch "more data" 
from
+  // the live address if it is still valid?
+
   Status error;
   CompilerType my_type = GetCompilerType();
 
@@ -2753,9 +2764,10 @@ ValueObjectSP ValueObject::Cast(const CompilerType 
_type) {
   = ExecutionContext(GetExecutionContextRef())
   .GetBestExecutionContextScope();
   if (compiler_type.GetByteSize(exe_scope)
-  <= GetCompilerType().GetByteSize(exe_scope)) {
+  <= GetCompilerType().GetByteSize(exe_scope) 
+  || m_value.GetValueType() == Value::ValueType::LoadAddress)
 return DoCast(compiler_type);
-  }
+
   error.SetErrorString("Can only cast to a type that is equal to or smaller "
"than the orignal type.");
 

diff  --git a/lldb/test/API/python_api/value/TestValueAPI.py 

[Lldb-commits] [lldb] Report back errors in GetNumChildren() (PR #84265)

[Adrian Prantl via lldb-commits]

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

[Lldb-commits] [lldb] 6462ead - Report back errors in GetNumChildren() (#84265)

[via lldb-commits]

Author: Adrian Prantl
Date: 2024-03-11T13:04:56-07:00
New Revision: 6462eadbd316aed1b1074ed73bcaf1698886bba1

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

LOG: Report back errors in GetNumChildren() (#84265)

This is a proof-of-concept patch that illustrates how to use the
Expected return values to surface rich error messages all the way up
to the ValueObjectPrinter.

This is the final patch in the series that includes
https://github.com/llvm/llvm-project/pull/83501 and
https://github.com/llvm/llvm-project/pull/84219

Added: 
lldb/test/API/functionalities/valobj_errors/Makefile
lldb/test/API/functionalities/valobj_errors/TestValueObjectErrors.py
lldb/test/API/functionalities/valobj_errors/hidden.c
lldb/test/API/functionalities/valobj_errors/main.c

Modified: 
lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
lldb/source/Core/ValueObjectVariable.cpp
lldb/source/DataFormatters/ValueObjectPrinter.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Symbol/CompilerType.cpp
lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_declaration-with-children.s
lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-missing-signature.test

Removed: 




diff  --git a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h 
b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
index fe46321c3186cf..32b101a2f9843c 100644
--- a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
+++ b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
@@ -127,7 +127,7 @@ class ValueObjectPrinter {
   void PrintChild(lldb::ValueObjectSP child_sp,
   const DumpValueObjectOptions::PointerDepth _ptr_depth);
 
-  uint32_t GetMaxNumChildrenToPrint(bool _dotdotdot);
+  llvm::Expected GetMaxNumChildrenToPrint(bool _dotdotdot);
 
   void
   PrintChildren(bool value_printed, bool summary_printed,

diff  --git a/lldb/source/Core/ValueObjectVariable.cpp 
b/lldb/source/Core/ValueObjectVariable.cpp
index fb29c22c0ab5af..67d71c90a959d5 100644
--- a/lldb/source/Core/ValueObjectVariable.cpp
+++ b/lldb/source/Core/ValueObjectVariable.cpp
@@ -99,7 +99,8 @@ ValueObjectVariable::CalculateNumChildren(uint32_t max) {
   CompilerType type(GetCompilerType());
 
   if (!type.IsValid())
-return 0;
+return llvm::make_error("invalid type",
+   llvm::inconvertibleErrorCode());
 
   ExecutionContext exe_ctx(GetExecutionContextRef());
   const bool omit_empty_base_classes = true;

diff  --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp 
b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index b853199e878c95..bbdc2a99815706 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -621,13 +621,17 @@ void ValueObjectPrinter::PrintChild(
   }
 }
 
-uint32_t ValueObjectPrinter::GetMaxNumChildrenToPrint(bool _dotdotdot) {
+llvm::Expected
+ValueObjectPrinter::GetMaxNumChildrenToPrint(bool _dotdotdot) {
   ValueObject _valobj = GetValueObjectForChildrenGeneration();
 
   if (m_options.m_pointer_as_array)
 return m_options.m_pointer_as_array.m_element_count;
 
-  uint32_t num_children = synth_valobj.GetNumChildrenIgnoringErrors();
+  auto num_children_or_err = synth_valobj.GetNumChildren();
+  if (!num_children_or_err)
+return num_children_or_err;
+  uint32_t num_children = *num_children_or_err;
   print_dotdotdot = false;
   if (num_children) {
 const size_t max_num_children = GetMostSpecializedValue()
@@ -704,7 +708,12 @@ void ValueObjectPrinter::PrintChildren(
   ValueObject _valobj = GetValueObjectForChildrenGeneration();
 
   bool print_dotdotdot = false;
-  size_t num_children = GetMaxNumChildrenToPrint(print_dotdotdot);
+  auto num_children_or_err = GetMaxNumChildrenToPrint(print_dotdotdot);
+  if (!num_children_or_err) {
+*m_stream << " <" << llvm::toString(num_children_or_err.takeError()) << 
'>';
+return;
+  }
+  uint32_t num_children = *num_children_or_err;
   if (num_children) {
 bool any_children_printed = false;
 
@@ -753,7 +762,12 @@ bool ValueObjectPrinter::PrintChildrenOneLiner(bool 
hide_names) {
   ValueObject _valobj = GetValueObjectForChildrenGeneration();
 
   bool print_dotdotdot = false;
-  size_t num_children = GetMaxNumChildrenToPrint(print_dotdotdot);
+  auto num_children_or_err = GetMaxNumChildrenToPrint(print_dotdotdot);
+  if (!num_children_or_err) {
+*m_stream << '<' << llvm::toString(num_children_or_err.takeError()) << '>';
+return true;
+  }
+  uint32_t num_children = *num_children_or_err;
 
   if (num_children) {
 m_stream->PutChar('(');

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 

[Lldb-commits] [lldb] Report back errors in GetNumChildren() (PR #84265)

[Adrian Prantl via lldb-commits]

https://github.com/adrian-prantl updated 
https://github.com/llvm/llvm-project/pull/84265

>From 18812772780abee17dd106e0e8f8c78ab8d4dbf1 Mon Sep 17 00:00:00 2001
From: Adrian Prantl 
Date: Fri, 8 Mar 2024 14:46:13 -0800
Subject: [PATCH] Report back errors in GetNumChildren()

This is a proof-of-concept patch that illustrates how to use the
Expected return values to surface rich error messages all the way up
to the ValueObjectPrinter.
---
 .../lldb/DataFormatters/ValueObjectPrinter.h  |  2 +-
 lldb/source/Core/ValueObjectVariable.cpp  |  3 ++-
 .../DataFormatters/ValueObjectPrinter.cpp | 22 +++
 .../TypeSystem/Clang/TypeSystemClang.cpp  |  9 +---
 lldb/source/Symbol/CompilerType.cpp   |  3 ++-
 .../functionalities/valobj_errors/Makefile|  9 
 .../valobj_errors/TestValueObjectErrors.py| 14 
 .../functionalities/valobj_errors/hidden.c|  4 
 .../API/functionalities/valobj_errors/main.c  |  9 
 .../x86/DW_AT_declaration-with-children.s |  2 +-
 .../x86/debug-types-missing-signature.test|  2 +-
 11 files changed, 67 insertions(+), 12 deletions(-)
 create mode 100644 lldb/test/API/functionalities/valobj_errors/Makefile
 create mode 100644 
lldb/test/API/functionalities/valobj_errors/TestValueObjectErrors.py
 create mode 100644 lldb/test/API/functionalities/valobj_errors/hidden.c
 create mode 100644 lldb/test/API/functionalities/valobj_errors/main.c

diff --git a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h 
b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
index fe46321c3186cf..32b101a2f9843c 100644
--- a/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
+++ b/lldb/include/lldb/DataFormatters/ValueObjectPrinter.h
@@ -127,7 +127,7 @@ class ValueObjectPrinter {
   void PrintChild(lldb::ValueObjectSP child_sp,
   const DumpValueObjectOptions::PointerDepth _ptr_depth);
 
-  uint32_t GetMaxNumChildrenToPrint(bool _dotdotdot);
+  llvm::Expected GetMaxNumChildrenToPrint(bool _dotdotdot);
 
   void
   PrintChildren(bool value_printed, bool summary_printed,
diff --git a/lldb/source/Core/ValueObjectVariable.cpp 
b/lldb/source/Core/ValueObjectVariable.cpp
index fb29c22c0ab5af..67d71c90a959d5 100644
--- a/lldb/source/Core/ValueObjectVariable.cpp
+++ b/lldb/source/Core/ValueObjectVariable.cpp
@@ -99,7 +99,8 @@ ValueObjectVariable::CalculateNumChildren(uint32_t max) {
   CompilerType type(GetCompilerType());
 
   if (!type.IsValid())
-return 0;
+return llvm::make_error("invalid type",
+   llvm::inconvertibleErrorCode());
 
   ExecutionContext exe_ctx(GetExecutionContextRef());
   const bool omit_empty_base_classes = true;
diff --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp 
b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index b853199e878c95..bbdc2a99815706 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -621,13 +621,17 @@ void ValueObjectPrinter::PrintChild(
   }
 }
 
-uint32_t ValueObjectPrinter::GetMaxNumChildrenToPrint(bool _dotdotdot) {
+llvm::Expected
+ValueObjectPrinter::GetMaxNumChildrenToPrint(bool _dotdotdot) {
   ValueObject _valobj = GetValueObjectForChildrenGeneration();
 
   if (m_options.m_pointer_as_array)
 return m_options.m_pointer_as_array.m_element_count;
 
-  uint32_t num_children = synth_valobj.GetNumChildrenIgnoringErrors();
+  auto num_children_or_err = synth_valobj.GetNumChildren();
+  if (!num_children_or_err)
+return num_children_or_err;
+  uint32_t num_children = *num_children_or_err;
   print_dotdotdot = false;
   if (num_children) {
 const size_t max_num_children = GetMostSpecializedValue()
@@ -704,7 +708,12 @@ void ValueObjectPrinter::PrintChildren(
   ValueObject _valobj = GetValueObjectForChildrenGeneration();
 
   bool print_dotdotdot = false;
-  size_t num_children = GetMaxNumChildrenToPrint(print_dotdotdot);
+  auto num_children_or_err = GetMaxNumChildrenToPrint(print_dotdotdot);
+  if (!num_children_or_err) {
+*m_stream << " <" << llvm::toString(num_children_or_err.takeError()) << 
'>';
+return;
+  }
+  uint32_t num_children = *num_children_or_err;
   if (num_children) {
 bool any_children_printed = false;
 
@@ -753,7 +762,12 @@ bool ValueObjectPrinter::PrintChildrenOneLiner(bool 
hide_names) {
   ValueObject _valobj = GetValueObjectForChildrenGeneration();
 
   bool print_dotdotdot = false;
-  size_t num_children = GetMaxNumChildrenToPrint(print_dotdotdot);
+  auto num_children_or_err = GetMaxNumChildrenToPrint(print_dotdotdot);
+  if (!num_children_or_err) {
+*m_stream << '<' << llvm::toString(num_children_or_err.takeError()) << '>';
+return true;
+  }
+  uint32_t num_children = *num_children_or_err;
 
   if (num_children) {
 m_stream->PutChar('(');
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 

[Lldb-commits] [lldb] Turn off instruction flow control annotations by default (PR #84607)

[via lldb-commits]

PiJoules wrote:

Hi, I suspect this led to the test failure we're seeing at 
https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8753741062398610017/overview.

```
Script:
--
/b/s/w/ir/x/w/lldb_install/python3/bin/python3 
/b/s/w/ir/x/w/llvm-llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS 
--env ARCHIVER=/b/s/w/ir/x/w/cipd/bin/llvm-ar --env 
OBJCOPY=/b/s/w/ir/x/w/cipd/bin/llvm-objcopy --env 
LLVM_LIBS_DIR=/b/s/w/ir/x/w/llvm_build/./lib --env 
LLVM_INCLUDE_DIR=/b/s/w/ir/x/w/llvm_build/include --env 
LLVM_TOOLS_DIR=/b/s/w/ir/x/w/llvm_build/./bin --libcxx-include-dir 
/b/s/w/ir/x/w/llvm_build/include/c++/v1 --libcxx-include-target-dir 
/b/s/w/ir/x/w/llvm_build/include/x86_64-unknown-linux-gnu/c++/v1 
--libcxx-library-dir /b/s/w/ir/x/w/llvm_build/./lib/x86_64-unknown-linux-gnu 
--arch x86_64 --build-dir /b/s/w/ir/x/w/llvm_build/lldb-test-build.noindex 
--lldb-module-cache-dir 
/b/s/w/ir/x/w/llvm_build/lldb-test-build.noindex/module-cache-lldb/lldb-api 
--clang-module-cache-dir 
/b/s/w/ir/x/w/llvm_build/lldb-test-build.noindex/module-cache-clang/lldb-api 
--executable /b/s/w/ir/x/w/llvm_build/./bin/lldb --compiler 
/b/s/w/ir/x/w/llvm_build/./bin/clang --dsymutil 
/b/s/w/ir/x/w/llvm_build/./bin/dsymutil --llvm-tools-dir 
/b/s/w/ir/x/w/llvm_build/./bin --lldb-libs-dir /b/s/w/ir/x/w/llvm_build/./lib 
/b/s/w/ir/x/w/llvm-llvm-project/lldb/test/API/functionalities/data-formatter/builtin-formats
 -p TestBuiltinFormats.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 19.0.0git (https://llvm.googlesource.com/a/llvm-project revision 
8467457afc61d70e881c9817ace26356ef757733)
  clang revision 8467457afc61d70e881c9817ace26356ef757733
  llvm revision 8467457afc61d70e881c9817ace26356ef757733
Skipping the following test categories: ['dsym', 'gmodules', 'debugserver', 
'objc']

--
Command Output (stderr):
--
PASS: LLDB (/b/s/w/ir/x/w/llvm_build/bin/clang-x86_64) :: test 
(TestBuiltinFormats.TestCase.test)
PASS: LLDB (/b/s/w/ir/x/w/llvm_build/bin/clang-x86_64) :: testAllPlatforms 
(TestBuiltinFormats.TestCase.testAllPlatforms)
FAIL: LLDB (/b/s/w/ir/x/w/llvm_build/bin/clang-x86_64) :: test_instruction 
(TestBuiltinFormats.TestCase.test_instruction)
PASS: LLDB (/b/s/w/ir/x/w/llvm_build/bin/clang-x86_64) :: test_pointer 
(TestBuiltinFormats.TestCase.test_pointer)
==
FAIL: test_instruction (TestBuiltinFormats.TestCase.test_instruction)
--
Traceback (most recent call last):
  File 
"/b/s/w/ir/x/w/llvm-llvm-project/lldb/packages/Python/lldbsuite/test/decorators.py",
 line 150, in wrapper
return func(*args, **kwargs)
   ^
  File 
"/b/s/w/ir/x/w/llvm-llvm-project/lldb/packages/Python/lldbsuite/test/decorators.py",
 line 450, in wrapper
return func(self, *args, **kwargs)
   ^^^
  File 
"/b/s/w/ir/x/w/llvm-llvm-project/lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py",
 line 310, in test_instruction
self.assertIn(
AssertionError: '  addq   0xa(%rdi), %r8\n' not found in '(int) $0 = addq   
0xa(%rdi), %r8\n'
Config=x86_64-/b/s/w/ir/x/w/llvm_build/bin/clang
--
Ran 4 tests in 0.541s

FAILED (failures=1)
--
```

Could you take a look and send out a fix or revert? Thanks.

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

[Lldb-commits] [lldb] [LLDB] ASanLibsanitizers Use `sanitizers_address_on_report` breakpoint (PR #84583)

[Usama Hameed via lldb-commits]

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

[Lldb-commits] [lldb] 08a9207 - [LLDB] ASanLibsanitizers Use `sanitizers_address_on_report` breakpoint (#84583)

[via lldb-commits]

Author: Usama Hameed
Date: 2024-03-11T11:57:53-07:00
New Revision: 08a9207f947b8b022d70f8ee7eeeda7acc6aac76

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

LOG: [LLDB] ASanLibsanitizers Use `sanitizers_address_on_report` breakpoint 
(#84583)

symbol

This patch puts the default breakpoint on the
sanitizers_address_on_report symbol, and uses the old symbol as a backup
if the default case is not found

rdar://123911522

Added: 


Modified: 

lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
lldb/source/Plugins/InstrumentationRuntime/Utility/ReportRetriever.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
 
b/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
index d84cd36d7ce17b..cd91f4a6ff1bc2 100644
--- 
a/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
+++ 
b/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
@@ -90,9 +90,16 @@ void InstrumentationRuntimeASanLibsanitizers::Activate() {
   if (!process_sp)
 return;
 
+  lldb::ModuleSP module_sp = GetRuntimeModuleSP();
+
   Breakpoint *breakpoint = ReportRetriever::SetupBreakpoint(
-  GetRuntimeModuleSP(), process_sp,
-  ConstString("_Z22raise_sanitizers_error23sanitizer_error_context"));
+  module_sp, process_sp, ConstString("sanitizers_address_on_report"));
+
+  if (!breakpoint) {
+breakpoint = ReportRetriever::SetupBreakpoint(
+module_sp, process_sp,
+ConstString("_Z22raise_sanitizers_error23sanitizer_error_context"));
+  }
 
   if (!breakpoint)
 return;

diff  --git 
a/lldb/source/Plugins/InstrumentationRuntime/Utility/ReportRetriever.cpp 
b/lldb/source/Plugins/InstrumentationRuntime/Utility/ReportRetriever.cpp
index ff58c4cababae3..298b63bc716fcd 100644
--- a/lldb/source/Plugins/InstrumentationRuntime/Utility/ReportRetriever.cpp
+++ b/lldb/source/Plugins/InstrumentationRuntime/Utility/ReportRetriever.cpp
@@ -219,6 +219,7 @@ bool ReportRetriever::NotifyBreakpointHit(ProcessSP 
process_sp,
   return true; // Return true to stop the target
 }
 
+// FIXME: Setup the breakpoint using a less fragile SPI. rdar://124399066
 Breakpoint *ReportRetriever::SetupBreakpoint(ModuleSP module_sp,
  ProcessSP process_sp,
  ConstString symbol_name) {



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

[Lldb-commits] [lldb] [LLDB] ASanLibsanitizers Use `sanitizers_address_on_report` breakpoint (PR #84583)

[Usama Hameed via lldb-commits]

https://github.com/usama54321 updated 
https://github.com/llvm/llvm-project/pull/84583

>From 3b16e506778ce0a869e19b41d13836b4c3cf26ca Mon Sep 17 00:00:00 2001
From: usama 
Date: Mon, 11 Mar 2024 11:39:24 -0700
Subject: [PATCH] [LLDB] ASanLibsanitizers Use `sanitizers_address_on_report`
 breakpoint symbol

This patch puts the default breakpoint on the
sanitizers_address_on_report symbol, and uses the old symbol as a
backup if the default case is not found

rdar://123911522
---
 .../InstrumentationRuntimeASanLibsanitizers.cpp   | 11 +--
 .../Utility/ReportRetriever.cpp   |  1 +
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git 
a/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
 
b/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
index d84cd36d7ce17b..cd91f4a6ff1bc2 100644
--- 
a/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
+++ 
b/lldb/source/Plugins/InstrumentationRuntime/ASanLibsanitizers/InstrumentationRuntimeASanLibsanitizers.cpp
@@ -90,9 +90,16 @@ void InstrumentationRuntimeASanLibsanitizers::Activate() {
   if (!process_sp)
 return;
 
+  lldb::ModuleSP module_sp = GetRuntimeModuleSP();
+
   Breakpoint *breakpoint = ReportRetriever::SetupBreakpoint(
-  GetRuntimeModuleSP(), process_sp,
-  ConstString("_Z22raise_sanitizers_error23sanitizer_error_context"));
+  module_sp, process_sp, ConstString("sanitizers_address_on_report"));
+
+  if (!breakpoint) {
+breakpoint = ReportRetriever::SetupBreakpoint(
+module_sp, process_sp,
+ConstString("_Z22raise_sanitizers_error23sanitizer_error_context"));
+  }
 
   if (!breakpoint)
 return;
diff --git 
a/lldb/source/Plugins/InstrumentationRuntime/Utility/ReportRetriever.cpp 
b/lldb/source/Plugins/InstrumentationRuntime/Utility/ReportRetriever.cpp
index ff58c4cababae3..298b63bc716fcd 100644
--- a/lldb/source/Plugins/InstrumentationRuntime/Utility/ReportRetriever.cpp
+++ b/lldb/source/Plugins/InstrumentationRuntime/Utility/ReportRetriever.cpp
@@ -219,6 +219,7 @@ bool ReportRetriever::NotifyBreakpointHit(ProcessSP 
process_sp,
   return true; // Return true to stop the target
 }
 
+// FIXME: Setup the breakpoint using a less fragile SPI. rdar://124399066
 Breakpoint *ReportRetriever::SetupBreakpoint(ModuleSP module_sp,
  ProcessSP process_sp,
  ConstString symbol_name) {

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

[Lldb-commits] [lldb] Turn off instruction flow control annotations by default (PR #84607)

[Jason Molenda via lldb-commits]

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

[Lldb-commits] [lldb] bdbad0d - Turn off instruction flow control annotations by default (#84607)

[via lldb-commits]

Author: Jason Molenda
Date: 2024-03-11T10:21:07-07:00
New Revision: bdbad0d07bb600301cb324e87a6be37ca4af591a

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

LOG: Turn off instruction flow control annotations by default (#84607)

Walter Erquinigo added optional instruction annotations for x86
instructions in 2022 for the `thread trace dump instruction` command,
and code to DisassemblerLLVMC to add annotations for instructions that
change flow control, v. https://reviews.llvm.org/D128477

This was added as an option to `disassemble`, and the trace dump command
enables it by default, but several other instruction dumpers were
changed to display them by default as well. These are only implemented
for Intel instructions, so our disassembly on other targets ends up
looking like

```
(lldb) x/5i 0x186e4
0x186e4: 0xa9be6ffc   unknown stpx28, x27, [sp, #-0x20]!
0x186e8: 0xa9017bfd   unknown stpx29, x30, [sp, #0x10]
0x186ec: 0x910043fd   unknown addx29, sp, #0x10
0x186f0: 0xd11843ff   unknown subsp, sp, #0x610
0x186f4: 0x910c63e8   unknown addx8, sp, #0x318
```

instead of `disassemble`'s output style of

```
lldb`main:
lldb[0x186e4] <+0>:  stpx28, x27, [sp, #-0x20]!
lldb[0x186e8] <+4>:  stpx29, x30, [sp, #0x10]
lldb[0x186ec] <+8>:  addx29, sp, #0x10
lldb[0x186f0] <+12>: subsp, sp, #0x610
lldb[0x186f4] <+16>: addx8, sp, #0x318
```

Adding symbolic annotations for assembly instructions is something I'm
interested in too, because we may have users investigating a crash or
apparent-incorrect behavior who must debug optimized assembly and they
may not be familiar with the ISA they're using, so short of flipping
through a many-thousand-page PDF to understand each instruction, they're
lost. They don't write assembly or work at that level, but to understand
a bug, they have to understand what the instructions are actually doing.

But the annotations that exist today don't move us forward much on that
front - I'd argue that the flow control instructions on Intel are not
hard to understand from their names, but that might just be my personal
bias. Much trickier instructions exist in any event.

Displaying this information by default for all targets when we only have
one class of instructions on one target is not a good default.

Also, in 2011 when Greg implemented the `memory read -f i` (aka `x/i`)
command
```
commit 5009f9d5010a7e34ae15f962dac8505ea11a8716
Author: Greg Clayton 
Date:   Thu Oct 27 17:55:14 2011 +
[...]
eFormatInstruction will print out disassembly with bytes and it will use the
current target's architecture. The format character for this is "i" (which
used to be being used for the integer format, but the integer format also 
has
"d", so we gave the "i" format to disassembly), the long format is
"instruction".
```

he had DumpDataExtractor's DumpInstructions print the bytes of the
instruction -- that's the first field we see above for the `x/5i` after
the address -- and this is only useful for people who are debugging the
disassembler itself, I would argue. I don't want this displayed by
default either.

tl;dr this patch removes both fields from `memory read -f -i` and I
think this is the right call today. While I'm really interested in
instruction annotation, I don't think `x/i` is the right place to have
it enabled by default unless it's really compelling on at least some of
our major targets.

Added: 


Modified: 
lldb/source/Core/DumpDataExtractor.cpp
lldb/source/Expression/IRExecutionUnit.cpp

lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp

Removed: 




diff  --git a/lldb/source/Core/DumpDataExtractor.cpp 
b/lldb/source/Core/DumpDataExtractor.cpp
index 986c9a181919ee..826edd7bab046e 100644
--- a/lldb/source/Core/DumpDataExtractor.cpp
+++ b/lldb/source/Core/DumpDataExtractor.cpp
@@ -150,8 +150,8 @@ static lldb::offset_t DumpInstructions(const DataExtractor 
, Stream *s,
   if (bytes_consumed) {
 offset += bytes_consumed;
 const bool show_address = base_addr != LLDB_INVALID_ADDRESS;
-const bool show_bytes = true;
-const bool show_control_flow_kind = true;
+const bool show_bytes = false;
+const bool show_control_flow_kind = false;
 ExecutionContext exe_ctx;
 exe_scope->CalculateExecutionContext(exe_ctx);
 disassembler_sp->GetInstructionList().Dump(

diff  --git a/lldb/source/Expression/IRExecutionUnit.cpp 
b/lldb/source/Expression/IRExecutionUnit.cpp
index 0682746e448e30..e4e131d70d4319 100644
--- a/lldb/source/Expression/IRExecutionUnit.cpp
+++ b/lldb/source/Expression/IRExecutionUnit.cpp
@@ -201,7 +201,7 @@ Status 

[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)

[Daniil Kovalev via lldb-commits]

kovdan01 wrote:

> As this plugin seems related to backtrace perhaps the test should at least 
> run `bt` and check for the first line of the output.

I had similar thoughts, but I'm not sure if placing the test in 
`lldb/test/Shell/Commands` as `command-backtrace-missing-x86.test` is a nice 
idea. The issue occurs during loading the core dump (or, when debugging an 
executable, when executing "run" command). So, running `bt` might be misleading 
- a one might think that we test an absence of nullptr dereference during `bt`, 
but we want to test that the core dump is at least loaded properly since with 
the issue present we don't even get to the point where we can run `bt`.

I would be glad to here other people's thoughts on that - I'm personally not 
sure which testing approach is less evil here.

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

[Lldb-commits] [lldb] Turn off instruction flow control annotations by default (PR #84607)

[Greg Clayton via lldb-commits]

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


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

[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)

[Daniil Kovalev via lldb-commits]

@@ -0,0 +1,10 @@
+# UNSUPPORTED: x86

kovdan01 wrote:

Yes, I've checked that with `X86` in `LLVM_TARGETS_TO_BUILD`, the test becomes 
unsupported. So, it looks like it's currently OK

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

[Lldb-commits] [lldb] [lldb] Save the edited line before clearing it in Editline::PrintAsync (PR #84154)

[via lldb-commits]

github-actions[bot] wrote:



@karzanWang Congratulations on having your first Pull Request (PR) merged into 
the LLVM Project!

Your changes will be combined with recent changes from other authors, then 
tested
by our [build bots](https://lab.llvm.org/buildbot/). If there is a problem with 
a build, you may recieve a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail 
[here](https://llvm.org/docs/MyFirstTypoFix.html#myfirsttypofix-issues-after-landing-your-pr).

If your change does cause a problem, it may be reverted, or you can revert it 
yourself.
This is a normal part of [LLVM 
development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy).
 You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are 
working as expected, well done!


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

[Lldb-commits] [lldb] [lldb] Save the edited line before clearing it in Editline::PrintAsync (PR #84154)

[Jonas Devlieghere via lldb-commits]

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

[Lldb-commits] [lldb] 501bc10 - [lldb] Save the edited line before clearing it in Editline::PrintAsync (#84154)

[via lldb-commits]

Author: karzan
Date: 2024-03-11T10:07:12-07:00
New Revision: 501bc101c04675969ab673b247f2a58fa72bd09e

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

LOG: [lldb] Save the edited line before clearing it in Editline::PrintAsync 
(#84154)

If the `m_editor_status` is `EditorStatus::Editing`, PrintAsync clears
the currently edited line. In some situations, the edited line is not
saved. After the stream flushes, PrintAsync tries to display the unsaved
line, causing the loss of the edited line.

The issue arose while I was debugging REPRLRun in
[Fuzzilli](https://github.com/googleprojectzero/fuzzilli). I started
LLDB and attempted to set a breakpoint in libreprl-posix.c. I entered
`breakpoint set -f lib` and used the "tab" key for command completion.
After completion, the edited line was flushed, leaving a blank line.

Added: 


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

Removed: 




diff  --git a/lldb/source/Host/common/Editline.cpp 
b/lldb/source/Host/common/Editline.cpp
index e66271e8a6ee99..ed61aecc23b9b0 100644
--- a/lldb/source/Host/common/Editline.cpp
+++ b/lldb/source/Host/common/Editline.cpp
@@ -1597,6 +1597,7 @@ bool Editline::GetLines(int first_line_number, StringList 
,
 void Editline::PrintAsync(Stream *stream, const char *s, size_t len) {
   std::lock_guard guard(m_output_mutex);
   if (m_editor_status == EditorStatus::Editing) {
+SaveEditedLine();
 MoveCursor(CursorLocation::EditingCursor, CursorLocation::BlockStart);
 fprintf(m_output_file, ANSI_CLEAR_BELOW);
   }



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

[Lldb-commits] [lldb] [lldb] Save the edited line before clearing it in Editline::PrintAsync (PR #84154)

[Jonas Devlieghere via lldb-commits]

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

Coincidentally, I ran into the same issue a few weeks ago when I was messing 
around with a different way of displaying the progress reports. LGTM!

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

[Lldb-commits] [lldb] Report back errors in GetNumChildren() (PR #84265)

[Jonas Devlieghere via lldb-commits]

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


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

[Lldb-commits] [lldb] Make ValueObject::Cast work for casts from smaller to larger structs in the cases where this currently can work. (PR #84588)

[Jonas Devlieghere via lldb-commits]

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

LGTM!

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

[Lldb-commits] [lldb] [LLDB][doc] Updates build instructions. (PR #84630)

[Mark de Wever via lldb-commits]

mordante wrote:

> Looks good. I see the PR test for "Test documentation build" is failing, but 
> that's an issue with the bot and pexpect.

Is this a known issue and is somebody already working on fixing this?

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

[Lldb-commits] [lldb] Turn off instruction flow control annotations by default (PR #84607)

[Jonas Devlieghere via lldb-commits]

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

LGTM

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

[Lldb-commits] [lldb] [LLDB] ASanLibsanitizers Use `sanitizers_address_on_report` breakpoint (PR #84583)

[Jonas Devlieghere via lldb-commits]

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


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

[Lldb-commits] [lldb] [LLDB][doc] Updates build instructions. (PR #84630)

[Mark de Wever via lldb-commits]

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

[Lldb-commits] [lldb] 9a9aa41 - [LLDB][doc] Updates build instructions. (#84630)

[via lldb-commits]

Author: Mark de Wever
Date: 2024-03-11T17:45:48+01:00
New Revision: 9a9aa41dea83039154601082b1aa2c56e35a5a17

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

LOG: [LLDB][doc] Updates build instructions. (#84630)

Recently building libc++ requires building libunwind too. This updates
the LLDB instructions.

I noticed this recently and it was separately filed as
https://github.com/llvm/llvm-project/issues/84053

Added: 


Modified: 
lldb/docs/resources/build.rst

Removed: 




diff  --git a/lldb/docs/resources/build.rst b/lldb/docs/resources/build.rst
index 995273a97b653a..09d3d15a940836 100644
--- a/lldb/docs/resources/build.rst
+++ b/lldb/docs/resources/build.rst
@@ -331,7 +331,7 @@ macOS
 ^
 
 On macOS the LLDB test suite requires libc++. Either add
-``LLVM_ENABLE_RUNTIMES="libcxx;libcxxabi"`` or disable the test suite with
+``LLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind"`` or disable the test 
suite with
 ``LLDB_INCLUDE_TESTS=OFF``. Further useful options:
 
 * ``LLDB_BUILD_FRAMEWORK:BOOL``: Builds the LLDB.framework.
@@ -370,7 +370,7 @@ LLVM `_):
   $ cmake -B /path/to/lldb-build -G Ninja \
   -C /path/to/llvm-project/lldb/cmake/caches/Apple-lldb-macOS.cmake \
   -DLLVM_ENABLE_PROJECTS="clang;lldb" \
-  -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi" \
+  -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
   llvm-project/llvm
 
   $ DESTDIR=/path/to/lldb-install ninja -C /path/to/lldb-build check-lldb 
install-distribution
@@ -386,7 +386,7 @@ Build LLDB standalone for development with Xcode:
   $ cmake -B /path/to/llvm-build -G Ninja \
   -C /path/to/llvm-project/lldb/cmake/caches/Apple-lldb-base.cmake \
   -DLLVM_ENABLE_PROJECTS="clang" \
-  -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi" \
+  -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
   llvm-project/llvm
   $ ninja -C /path/to/llvm-build
 



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

[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)

[David Spickett via lldb-commits]

@@ -0,0 +1,10 @@
+# UNSUPPORTED: x86

DavidSpickett wrote:

Actually, I'm wrong.

We get the configured targets from llvm-config and I think that adds "X86" as a 
feature, target- is about what the default triple of llvm is.

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

[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)

[David Spickett via lldb-commits]

@@ -0,0 +1,10 @@
+# UNSUPPORTED: x86

DavidSpickett wrote:

This should be:
```
UNSUPPORTED: target-x86_64
```
Which I'm pretty sure is lldb's equivalent of `x86-registered-target`. Which is 
whether the lldb has that target built into it, vs. whether it's running on x86.

You see a lot of requires: native && target-x86-64 for this reason.

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

[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)

[David Spickett via lldb-commits]

DavidSpickett wrote:

The spirit of the test is fine but I think we can avoid adding another core 
file.

As this plugin seems related to backtrace perhaps the test should at least run 
`bt` and check for the first line of the output. Which means we can justify 
putting the test in `lldb/test/Shell/Commands` as 
`command-backtrace-missing-x86.test`.

Now we need a corefile, the closest ones are in `./Register/Core/Inputs/`, so 
it's a bit unorthodox, but you could load 
`%p/../Register/Core/Inputs/x86-64-linux.core` and save having to copy it.

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

[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)

[Daniil Kovalev via lldb-commits]

kovdan01 wrote:

> Can this code be hit when using an x86 core file? Then you could write a 
> shell test thatis `UNSUPPORTED: x86-registered-target` (whatever the proper 
> syntax is) and assert that it does not crash.

@DavidSpickett The issue is present when loading an x86 core files via `lldb 
--core core` (nothing else, just running this command). I've implemented shell 
test and put that into a new directory Core - see 
bd9bb0a5d73d7532f885222df900c6f6406c7473. If there is a more applicable place 
for that - would be glad to see suggestions

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

[Lldb-commits] [lldb] e77f5fe - [lldb][Docs] Add libxml2 to apt install command

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2024-03-11T11:41:56Z
New Revision: e77f5fe889909df3508fd929f2636a0ac211877a

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

LOG: [lldb][Docs] Add libxml2 to apt install command

Added: 


Modified: 
lldb/docs/resources/build.rst

Removed: 




diff  --git a/lldb/docs/resources/build.rst b/lldb/docs/resources/build.rst
index 5f4d35ced6236c..995273a97b653a 100644
--- a/lldb/docs/resources/build.rst
+++ b/lldb/docs/resources/build.rst
@@ -73,7 +73,7 @@ commands below.
 ::
 
   $ yum install libedit-devel libxml2-devel ncurses-devel python-devel swig
-  $ sudo apt-get install build-essential swig python3-dev libedit-dev 
libncurses5-dev
+  $ sudo apt-get install build-essential swig python3-dev libedit-dev 
libncurses5-dev libxml2-dev
   $ pkg install swig python libxml2
   $ pkgin install swig python36 cmake ninja-build
   $ brew install swig cmake ninja



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