[Lldb-commits] [lldb] Add lldb-dap to Swift distributions (PR #88482)

[Adam Fowler via lldb-commits]

https://github.com/adam-fowler created 
https://github.com/llvm/llvm-project/pull/88482

This includes the lldb-dap executable in the MacOS and Linux distributions of 
Swift. Currently there is a commit in the Apple repo to do this for just MacOS 
https://github.com/apple/llvm-project/pull/8176. This PR extends this to both 
Linux and MacOS and brings the change upstream.

@JDevlieghere @adrian-prantl 

>From 52cb463073bbb6914a361d121d04953846e3f670 Mon Sep 17 00:00:00 2001
From: Adam Fowler 
Date: Fri, 12 Apr 2024 08:46:24 +0100
Subject: [PATCH] Add lldb-dap to Swift distributions

---
 lldb/cmake/caches/Apple-lldb-Linux.cmake | 1 +
 lldb/cmake/caches/Apple-lldb-macOS.cmake | 1 +
 2 files changed, 2 insertions(+)

diff --git a/lldb/cmake/caches/Apple-lldb-Linux.cmake 
b/lldb/cmake/caches/Apple-lldb-Linux.cmake
index bfa660d8654b7b..b936929afc42f8 100644
--- a/lldb/cmake/caches/Apple-lldb-Linux.cmake
+++ b/lldb/cmake/caches/Apple-lldb-Linux.cmake
@@ -5,6 +5,7 @@ set(LLVM_DISTRIBUTION_COMPONENTS
   lldb
   liblldb
   lldb-argdumper
+  lldb-dap
   lldb-server
   lldb-python-scripts
   CACHE STRING "")
diff --git a/lldb/cmake/caches/Apple-lldb-macOS.cmake 
b/lldb/cmake/caches/Apple-lldb-macOS.cmake
index 2aef41157bab1a..5155024ac3a026 100644
--- a/lldb/cmake/caches/Apple-lldb-macOS.cmake
+++ b/lldb/cmake/caches/Apple-lldb-macOS.cmake
@@ -22,6 +22,7 @@ set(LLVM_DISTRIBUTION_COMPONENTS
   lldb
   liblldb
   lldb-argdumper
+  lldb-dap
   darwin-debug
   debugserver
   CACHE STRING "")

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

[Lldb-commits] [lldb] Add lldb-dap to Swift distributions (PR #88482)

[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/88482
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Add lldb-dap to Swift distributions (PR #88482)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Adam Fowler (adam-fowler)


Changes

This includes the lldb-dap executable in the MacOS and Linux distributions of 
Swift. Currently there is a commit in the Apple repo to do this for just MacOS 
https://github.com/apple/llvm-project/pull/8176. This PR extends this to both 
Linux and MacOS and brings the change upstream.

@JDevlieghere @adrian-prantl 

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


2 Files Affected:

- (modified) lldb/cmake/caches/Apple-lldb-Linux.cmake (+1) 
- (modified) lldb/cmake/caches/Apple-lldb-macOS.cmake (+1) 


``diff
diff --git a/lldb/cmake/caches/Apple-lldb-Linux.cmake 
b/lldb/cmake/caches/Apple-lldb-Linux.cmake
index bfa660d8654b7b..b936929afc42f8 100644
--- a/lldb/cmake/caches/Apple-lldb-Linux.cmake
+++ b/lldb/cmake/caches/Apple-lldb-Linux.cmake
@@ -5,6 +5,7 @@ set(LLVM_DISTRIBUTION_COMPONENTS
   lldb
   liblldb
   lldb-argdumper
+  lldb-dap
   lldb-server
   lldb-python-scripts
   CACHE STRING "")
diff --git a/lldb/cmake/caches/Apple-lldb-macOS.cmake 
b/lldb/cmake/caches/Apple-lldb-macOS.cmake
index 2aef41157bab1a..5155024ac3a026 100644
--- a/lldb/cmake/caches/Apple-lldb-macOS.cmake
+++ b/lldb/cmake/caches/Apple-lldb-macOS.cmake
@@ -22,6 +22,7 @@ set(LLVM_DISTRIBUTION_COMPONENTS
   lldb
   liblldb
   lldb-argdumper
+  lldb-dap
   darwin-debug
   debugserver
   CACHE STRING "")

``




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

[Lldb-commits] [lldb] [lldb/linux] Make sure the process continues running after a detach (PR #88494)

[Pavel Labath via lldb-commits]

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

Whenever an inferior thread stops, lldb-server sends a SIGSTOP to all other 
threads in the process to force them to stop as well. If those threads stop on 
their own before they get a signal, this SIGSTOP will remain pending and be 
delivered the next time the process resumes.

Normally, this is not a problem, because lldb-server will detect this stale 
SIGSTOP and resume the process. However, if we detach from the process while it 
has these SIGSTOPs pending, they will get immediately delivered, and the 
process will remain stopped (most likely forever).

This patch fixes that by sending a SIGCONT just before detaching from the 
process. This signal cancels out any pending SIGSTOPs, and ensures it is able 
to run after we detach. It does have one somewhat unfortunate side-effect that 
in that the process's SIGCONT handler (if it has one) will get executed 
spuriously (from the process's POV).

This could be _sometimes_ avoided by tracking which threads got send a SIGSTOP, 
and whether those threads stopped due to it. From what I could tell by 
observing its behavior, this is what gdb does. I have not tried to replicate 
that behavior here because it adds a nontrivial amount of complexity and the 
result is still uncertain -- we still need to send a SIGCONT (and execute the 
handler) when any thread stops for some other reason (and leaves our SIGSTOP 
hanging). Furthermore, since SIGSTOPs don't stack, it's also possible that our 
SIGSTOP/SIGCONT combination will cancel a genuine SIGSTOP being sent to the 
debugger application (by someone else), and there is nothing we can do about 
that. For this reason I think it's simplest and most predictible to just always 
send a SIGCONT when detaching, but if it turns out this is breaking something, 
we can consider implementing something more elaborate.

One alternative I did try is to use PTRACE_INTERRUPT to suspend the threads 
instead of a SIGSTOP. PTRACE_INTERUPT requires using PTRACE_SEIZE to attach to 
the process, which also made this solution somewhat complicated, but the main 
problem with that approach is that PTRACE_INTERRUPT is not considered to be a 
signal-delivery-stop, which means it's not possible to resume it while 
injecting another signal to the inferior (which some of our tests expect to be 
able to do). This limitation could be worked around by forcing the thread into 
a signal delivery stop whenever we need to do this, but this additional 
complication is what made me think this approach is also not worthwhile.

This patch should fix (at least some of) the problems with TestConcurrentVFork, 
but I've also added a dedicated test for checking that a process keeps running 
after we detach. Although the problem I'm fixing here is linux-specific, the 
core functinoality of not stopping after a detach should function the same way 
everywhere.

>From 9e1aca01da4a4aa5039c6ac1f8320f8c7d85b8e3 Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Sat, 6 Apr 2024 17:51:12 +
Subject: [PATCH] [lldb/linux] Make sure the process continues running after a
 detach

Whenever an inferior thread stops, lldb-server sends a SIGSTOP to all
other threads in the process to force them to stop as well. If those
threads stop on their own before they get a signal, this SIGSTOP will
remain pending and be delivered the next time the process resumes.

Normally, this is not a problem, because lldb-server will detect this
stale SIGSTOP and resume the process. However, if we detach from the
process while it has these SIGSTOPs pending, they will get immediately
delivered, and the process will remain stopped (most likely forever).

This patch fixes that by sending a SIGCONT just before detaching from
the process. This signal cancels out any pending SIGSTOPs, and ensures
it is able to run after we detach. It does have one somewhat unfortunate
side-effect that in that the process's SIGCONT handler (if it has one)
will get executed spuriously (from the process's POV).

This could be _sometimes_ avoided by tracking which threads got send a
SIGSTOP, and whether those threads stopped due to it. From what I could
tell by observing its behavior, this is what gdb does. I have not tried
to replicate that behavior here because it adds a nontrivial amount of
complexity and the result is still uncertain -- we still need to send a
SIGCONT (and execute the handler) when any thread stops for some other
reason (and leaves our SIGSTOP hanging). Furthermore, since SIGSTOPs
don't stack, it's also possible that our SIGSTOP/SIGCONT combination
will cancel a genuine SIGSTOP being sent to the debugger application (by
someone else), and there is nothing we can do about that. For this
reason I think it's simplest and most predictible to just always send a
SIGCONT when detaching, but if it turns out this is breaking something,
we can consider implementing something more elaborate.

One alternative I did try is to use PTR

[Lldb-commits] [lldb] [lldb/linux] Make sure the process continues running after a detach (PR #88494)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)


Changes

Whenever an inferior thread stops, lldb-server sends a SIGSTOP to all other 
threads in the process to force them to stop as well. If those threads stop on 
their own before they get a signal, this SIGSTOP will remain pending and be 
delivered the next time the process resumes.

Normally, this is not a problem, because lldb-server will detect this stale 
SIGSTOP and resume the process. However, if we detach from the process while it 
has these SIGSTOPs pending, they will get immediately delivered, and the 
process will remain stopped (most likely forever).

This patch fixes that by sending a SIGCONT just before detaching from the 
process. This signal cancels out any pending SIGSTOPs, and ensures it is able 
to run after we detach. It does have one somewhat unfortunate side-effect that 
in that the process's SIGCONT handler (if it has one) will get executed 
spuriously (from the process's POV).

This could be _sometimes_ avoided by tracking which threads got send a SIGSTOP, 
and whether those threads stopped due to it. From what I could tell by 
observing its behavior, this is what gdb does. I have not tried to replicate 
that behavior here because it adds a nontrivial amount of complexity and the 
result is still uncertain -- we still need to send a SIGCONT (and execute the 
handler) when any thread stops for some other reason (and leaves our SIGSTOP 
hanging). Furthermore, since SIGSTOPs don't stack, it's also possible that our 
SIGSTOP/SIGCONT combination will cancel a genuine SIGSTOP being sent to the 
debugger application (by someone else), and there is nothing we can do about 
that. For this reason I think it's simplest and most predictible to just always 
send a SIGCONT when detaching, but if it turns out this is breaking something, 
we can consider implementing something more elaborate.

One alternative I did try is to use PTRACE_INTERRUPT to suspend the threads 
instead of a SIGSTOP. PTRACE_INTERUPT requires using PTRACE_SEIZE to attach to 
the process, which also made this solution somewhat complicated, but the main 
problem with that approach is that PTRACE_INTERRUPT is not considered to be a 
signal-delivery-stop, which means it's not possible to resume it while 
injecting another signal to the inferior (which some of our tests expect to be 
able to do). This limitation could be worked around by forcing the thread into 
a signal delivery stop whenever we need to do this, but this additional 
complication is what made me think this approach is also not worthwhile.

This patch should fix (at least some of) the problems with TestConcurrentVFork, 
but I've also added a dedicated test for checking that a process keeps running 
after we detach. Although the problem I'm fixing here is linux-specific, the 
core functinoality of not stopping after a detach should function the same way 
everywhere.

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


5 Files Affected:

- (modified) lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp (+4) 
- (added) lldb/test/API/commands/process/detach-resumes/Makefile (+4) 
- (added) lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py 
(+57) 
- (added) lldb/test/API/commands/process/detach-resumes/main.cpp (+46) 
- (modified) 
lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py 
(-16) 


``diff
diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp 
b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index 5d2b4b03fe60cb..59fc8726b76739 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1089,6 +1089,10 @@ Status NativeProcessLinux::Detach() {
   if (GetID() == LLDB_INVALID_PROCESS_ID)
 return error;
 
+  // Cancel out any SIGSTOPs we may have sent while stopping the process.
+  // Otherwise, the process may stop as soon as we detach from it.
+  kill(GetID(), SIGCONT);
+
   for (const auto &thread : m_threads) {
 Status e = Detach(thread->GetID());
 if (e.Fail())
diff --git a/lldb/test/API/commands/process/detach-resumes/Makefile 
b/lldb/test/API/commands/process/detach-resumes/Makefile
new file mode 100644
index 00..c46619c6623481
--- /dev/null
+++ b/lldb/test/API/commands/process/detach-resumes/Makefile
@@ -0,0 +1,4 @@
+CXX_SOURCES := main.cpp
+ENABLE_THREADS := YES
+
+include Makefile.rules
diff --git a/lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py 
b/lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py
new file mode 100644
index 00..ab2ed8a6d24c85
--- /dev/null
+++ b/lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py
@@ -0,0 +1,57 @@
+"""
+Test that the process continues running after we detach from it.
+"""
+
+
+import lldb
+import time
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from

[Lldb-commits] [lldb] [lldb/linux] Make sure the process continues running after a detach (PR #88494)

[via lldb-commits]

github-actions[bot] wrote:




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



You can test this locally with the following command:


``bash
git-clang-format --diff b88a1dd6c75754ace4abe18c8ea16a019f7b5529 
9e1aca01da4a4aa5039c6ac1f8320f8c7d85b8e3 -- 
lldb/test/API/commands/process/detach-resumes/main.cpp 
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/test/API/commands/process/detach-resumes/main.cpp 
b/lldb/test/API/commands/process/detach-resumes/main.cpp
index 728ebbff58..e8050fef2c 100644
--- a/lldb/test/API/commands/process/detach-resumes/main.cpp
+++ b/lldb/test/API/commands/process/detach-resumes/main.cpp
@@ -1,9 +1,9 @@
-#include 
-#include 
-#include 
-#include 
 #include "pseudo_barrier.h"
+#include 
+#include 
 #include 
+#include 
+#include 
 #include 
 
 pseudo_barrier_t barrier;
@@ -19,11 +19,11 @@ void tfunc() {
   break_here();
 }
 
-int main(int argc, char const *argv[])
-{
+int main(int argc, char const *argv[]) {
   lldb_enable_attach();
 
-  if (argc<3) return 1;
+  if (argc < 3)
+return 1;
 
   // Create a file to signal that this process has started up.
   std::ofstream(argv[1]).close();
@@ -35,9 +35,11 @@ int main(int argc, char const *argv[])
   // Fire up the threads and have them call break_here() simultaneously.
   pseudo_barrier_init(barrier, nthreads);
   std::vector threads;
-  for(size_t i = 0; i < nthreads; ++i) threads.emplace_back(tfunc);
+  for (size_t i = 0; i < nthreads; ++i)
+threads.emplace_back(tfunc);
 
-  for(std::thread &t: threads) t.join();
+  for (std::thread &t : threads)
+t.join();
 
   // Create the file to let the debugger know we're running.
   std::ofstream(argv[2]).close();

``




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

[Lldb-commits] [lldb] [lldb/linux] Make sure the process continues running after a detach (PR #88494)

[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 
b88a1dd6c75754ace4abe18c8ea16a019f7b5529...9e1aca01da4a4aa5039c6ac1f8320f8c7d85b8e3
 lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py 
lldb/test/API/functionalities/fork/concurrent_vfork/TestConcurrentVFork.py
``





View the diff from darker here.


``diff
--- commands/process/detach-resumes/TestDetachResumes.py2024-04-12 
09:09:28.00 +
+++ commands/process/detach-resumes/TestDetachResumes.py2024-04-12 
10:10:22.544874 +
@@ -6,10 +6,11 @@
 import lldb
 import time
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
+
 
 class DetachResumesTestCase(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
 def test_detach_resumes(self):
@@ -26,11 +27,13 @@
 # it.
 exit_file_path = lldbutil.append_to_process_working_directory(
 self, "exit_file_%d" % (int(time.time()))
 )
 
-popen = self.spawnSubprocess(self.getBuildArtifact(exe), 
[sync_file_path, exit_file_path])
+popen = self.spawnSubprocess(
+self.getBuildArtifact(exe), [sync_file_path, exit_file_path]
+)
 lldbutil.wait_for_file_on_target(self, sync_file_path)
 
 self.runCmd("process attach -p " + str(popen.pid))
 
 # Set a breakpoint at a place that will be called by multiple threads

``




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

[Lldb-commits] [lldb] [lldb/linux] Make sure the process continues running after a detach (PR #88494)

[Pavel Labath via lldb-commits]

https://github.com/labath updated 
https://github.com/llvm/llvm-project/pull/88494

>From 8a9837bd306377ec44efdb8a4ff25f0132496cc0 Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Sat, 6 Apr 2024 17:51:12 +
Subject: [PATCH] [lldb/linux] Make sure the process continues running after a
 detach

Whenever an inferior thread stops, lldb-server sends a SIGSTOP to all
other threads in the process to force them to stop as well. If those
threads stop on their own before they get a signal, this SIGSTOP will
remain pending and be delivered the next time the process resumes.

Normally, this is not a problem, because lldb-server will detect this
stale SIGSTOP and resume the process. However, if we detach from the
process while it has these SIGSTOPs pending, they will get immediately
delivered, and the process will remain stopped (most likely forever).

This patch fixes that by sending a SIGCONT just before detaching from
the process. This signal cancels out any pending SIGSTOPs, and ensures
it is able to run after we detach. It does have one somewhat unfortunate
side-effect that in that the process's SIGCONT handler (if it has one)
will get executed spuriously (from the process's POV).

This could be _sometimes_ avoided by tracking which threads got send a
SIGSTOP, and whether those threads stopped due to it. From what I could
tell by observing its behavior, this is what gdb does. I have not tried
to replicate that behavior here because it adds a nontrivial amount of
complexity and the result is still uncertain -- we still need to send a
SIGCONT (and execute the handler) when any thread stops for some other
reason (and leaves our SIGSTOP hanging). Furthermore, since SIGSTOPs
don't stack, it's also possible that our SIGSTOP/SIGCONT combination
will cancel a genuine SIGSTOP being sent to the debugger application (by
someone else), and there is nothing we can do about that. For this
reason I think it's simplest and most predictible to just always send a
SIGCONT when detaching, but if it turns out this is breaking something,
we can consider implementing something more elaborate.

One alternative I did try is to use PTRACE_INTERRUPT to suspend the
threads instead of a SIGSTOP. PTRACE_INTERUPT requires using
PTRACE_SEIZE to attach to the process, which also made this solution
somewhat complicated, but the main problem with that approach is that
PTRACE_INTERRUPT is not considered to be a signal-delivery-stop, which
means it's not possible to resume it while injecting another signal to
the inferior (which some of our tests expect to be able to do). This
limitation could be worked around by forcing the thread into a signal
delivery stop whenever we need to do this, but this additional
complication is what made me think this approach is also not worthwhile.

This patch should fix (at least some of) the problems with
TestConcurrentVFork, but I've also added a dedicated test for checking
that a process keeps running after we detach. Although the problem I'm
fixing here is linux-specific, the core functinoality of not stopping
after a detach should function the same way everywhere.
---
 .../Process/Linux/NativeProcessLinux.cpp  |  4 ++
 .../commands/process/detach-resumes/Makefile  |  4 ++
 .../detach-resumes/TestDetachResumes.py   | 57 +++
 .../commands/process/detach-resumes/main.cpp  | 48 
 .../concurrent_vfork/TestConcurrentVFork.py   | 16 --
 5 files changed, 113 insertions(+), 16 deletions(-)
 create mode 100644 lldb/test/API/commands/process/detach-resumes/Makefile
 create mode 100644 
lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py
 create mode 100644 lldb/test/API/commands/process/detach-resumes/main.cpp

diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp 
b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index 5d2b4b03fe60cb..59fc8726b76739 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1089,6 +1089,10 @@ Status NativeProcessLinux::Detach() {
   if (GetID() == LLDB_INVALID_PROCESS_ID)
 return error;
 
+  // Cancel out any SIGSTOPs we may have sent while stopping the process.
+  // Otherwise, the process may stop as soon as we detach from it.
+  kill(GetID(), SIGCONT);
+
   for (const auto &thread : m_threads) {
 Status e = Detach(thread->GetID());
 if (e.Fail())
diff --git a/lldb/test/API/commands/process/detach-resumes/Makefile 
b/lldb/test/API/commands/process/detach-resumes/Makefile
new file mode 100644
index 00..c46619c6623481
--- /dev/null
+++ b/lldb/test/API/commands/process/detach-resumes/Makefile
@@ -0,0 +1,4 @@
+CXX_SOURCES := main.cpp
+ENABLE_THREADS := YES
+
+include Makefile.rules
diff --git a/lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py 
b/lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py
new file mode 100644
index 00..ab2ed8a6d24c85
--- /dev/null
+++

[Lldb-commits] [lldb] [lldb/linux] Make sure the process continues running after a detach (PR #88494)

[Pavel Labath via lldb-commits]

https://github.com/labath updated 
https://github.com/llvm/llvm-project/pull/88494

>From c6b2c5e58321e72155b8c45cd3591487c1cafacf Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Sat, 6 Apr 2024 17:51:12 +
Subject: [PATCH] [lldb/linux] Make sure the process continues running after a
 detach

Whenever an inferior thread stops, lldb-server sends a SIGSTOP to all
other threads in the process to force them to stop as well. If those
threads stop on their own before they get a signal, this SIGSTOP will
remain pending and be delivered the next time the process resumes.

Normally, this is not a problem, because lldb-server will detect this
stale SIGSTOP and resume the process. However, if we detach from the
process while it has these SIGSTOPs pending, they will get immediately
delivered, and the process will remain stopped (most likely forever).

This patch fixes that by sending a SIGCONT just before detaching from
the process. This signal cancels out any pending SIGSTOPs, and ensures
it is able to run after we detach. It does have one somewhat unfortunate
side-effect that in that the process's SIGCONT handler (if it has one)
will get executed spuriously (from the process's POV).

This could be _sometimes_ avoided by tracking which threads got send a
SIGSTOP, and whether those threads stopped due to it. From what I could
tell by observing its behavior, this is what gdb does. I have not tried
to replicate that behavior here because it adds a nontrivial amount of
complexity and the result is still uncertain -- we still need to send a
SIGCONT (and execute the handler) when any thread stops for some other
reason (and leaves our SIGSTOP hanging). Furthermore, since SIGSTOPs
don't stack, it's also possible that our SIGSTOP/SIGCONT combination
will cancel a genuine SIGSTOP being sent to the debugger application (by
someone else), and there is nothing we can do about that. For this
reason I think it's simplest and most predictible to just always send a
SIGCONT when detaching, but if it turns out this is breaking something,
we can consider implementing something more elaborate.

One alternative I did try is to use PTRACE_INTERRUPT to suspend the
threads instead of a SIGSTOP. PTRACE_INTERUPT requires using
PTRACE_SEIZE to attach to the process, which also made this solution
somewhat complicated, but the main problem with that approach is that
PTRACE_INTERRUPT is not considered to be a signal-delivery-stop, which
means it's not possible to resume it while injecting another signal to
the inferior (which some of our tests expect to be able to do). This
limitation could be worked around by forcing the thread into a signal
delivery stop whenever we need to do this, but this additional
complication is what made me think this approach is also not worthwhile.

This patch should fix (at least some of) the problems with
TestConcurrentVFork, but I've also added a dedicated test for checking
that a process keeps running after we detach. Although the problem I'm
fixing here is linux-specific, the core functinoality of not stopping
after a detach should function the same way everywhere.
---
 .../Process/Linux/NativeProcessLinux.cpp  |  4 ++
 .../commands/process/detach-resumes/Makefile  |  4 ++
 .../detach-resumes/TestDetachResumes.py   | 59 +++
 .../commands/process/detach-resumes/main.cpp  | 48 +++
 .../concurrent_vfork/TestConcurrentVFork.py   | 16 -
 5 files changed, 115 insertions(+), 16 deletions(-)
 create mode 100644 lldb/test/API/commands/process/detach-resumes/Makefile
 create mode 100644 
lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py
 create mode 100644 lldb/test/API/commands/process/detach-resumes/main.cpp

diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp 
b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index 5d2b4b03fe60cb..59fc8726b76739 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1089,6 +1089,10 @@ Status NativeProcessLinux::Detach() {
   if (GetID() == LLDB_INVALID_PROCESS_ID)
 return error;
 
+  // Cancel out any SIGSTOPs we may have sent while stopping the process.
+  // Otherwise, the process may stop as soon as we detach from it.
+  kill(GetID(), SIGCONT);
+
   for (const auto &thread : m_threads) {
 Status e = Detach(thread->GetID());
 if (e.Fail())
diff --git a/lldb/test/API/commands/process/detach-resumes/Makefile 
b/lldb/test/API/commands/process/detach-resumes/Makefile
new file mode 100644
index 00..c46619c6623481
--- /dev/null
+++ b/lldb/test/API/commands/process/detach-resumes/Makefile
@@ -0,0 +1,4 @@
+CXX_SOURCES := main.cpp
+ENABLE_THREADS := YES
+
+include Makefile.rules
diff --git a/lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py 
b/lldb/test/API/commands/process/detach-resumes/TestDetachResumes.py
new file mode 100644
index 00..57727294ddc3d3
--- /dev/null
+++ b

[Lldb-commits] [lldb] [lldb][libc++] Adds local_t clock data formatters. (PR #88178)

[Michael Buch via lldb-commits]

@@ -1068,6 +1068,29 @@ static void 
LoadLibCxxFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
 eTypeOptionCascade,
 true);
 
+  AddCXXSummary(
+  cpp_category_sp,
+  lldb_private::formatters::LibcxxChronoLocalSecondsSummaryProvider,
+  "libc++ std::chrono::local_seconds summary provider",
+  "^std::__[[:alnum:]]+::chrono::time_point<"
+  "std::__[[:alnum:]]+::chrono::local_t, "
+  "std::__[[:alnum:]]+::chrono::durationhttps://github.com/llvm/llvm-project/pull/88178
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][libc++] Adds local_t clock data formatters. (PR #88178)

[Michael Buch via lldb-commits]

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

LGTM, thanks!

https://github.com/llvm/llvm-project/pull/88178
___
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 tests for evaluating local variables whose name clashes with Objective-C types (PR #87807)

[Michael Buch via lldb-commits]

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

>From 734e127b758b00210aa508c84d0222165c036ac4 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 5 Apr 2024 12:10:09 +0100
Subject: [PATCH 1/2] [lldb][test] Add tests for evaluating local variables
 whose name clashes with Objective-C types

Depends on https://github.com/llvm/llvm-project/pull/87767
---
 .../objc-builtin-types/TestObjCBuiltinTypes.py  | 13 +
 .../API/lang/objcxx/objc-builtin-types/main.cpp |  8 +++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git 
a/lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py 
b/lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py
index 611c7388999058..19ae2f091bdd5e 100644
--- a/lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py
+++ b/lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py
@@ -13,6 +13,7 @@ def setUp(self):
 # Find the line numbers to break inside main().
 self.main_source = "main.cpp"
 self.break_line = line_number(self.main_source, "// Set breakpoint 
here.")
+self.bar_break_line = line_number(self.main_source, "return id + 
Class")
 
 @add_test_categories(["pyapi"])
 def test_with_python_api(self):
@@ -26,6 +27,11 @@ def test_with_python_api(self):
 bpt = target.BreakpointCreateByLocation(self.main_source, 
self.break_line)
 self.assertTrue(bpt, VALID_BREAKPOINT)
 
+bar_bpt = target.BreakpointCreateByLocation(
+self.main_source, self.bar_break_line
+)
+self.assertTrue(bar_bpt, VALID_BREAKPOINT)
+
 # Now launch the process, and do not stop at entry point.
 process = target.LaunchSimple(None, None, 
self.get_process_working_directory())
 
@@ -52,3 +58,10 @@ def test_with_python_api(self):
 patterns=["\(id\) \$.* = nil"],
 )
 self.expect("expr --language C++ -- id my_id = 0; my_id", error=True)
+
+lldbutil.continue_to_breakpoint(process, bar_bpt)
+
+self.expect_expr("id", result_value="12", result_type="int")
+self.expect_expr("Class", result_value="15", result_type="int")
+self.expect("expr --language C++ -- id", error=True)
+self.expect("expr --language C++ -- Class", error=True)
diff --git a/lldb/test/API/lang/objcxx/objc-builtin-types/main.cpp 
b/lldb/test/API/lang/objcxx/objc-builtin-types/main.cpp
index 6dd8cbc6e9fef6..5b35ec0f0b8c98 100644
--- a/lldb/test/API/lang/objcxx/objc-builtin-types/main.cpp
+++ b/lldb/test/API/lang/objcxx/objc-builtin-types/main.cpp
@@ -2,8 +2,14 @@ namespace ns {
   typedef int id;
 };
 
+int bar() {
+  int id = 12;
+  int Class = 15;
+  return id + Class;
+}
+
 int main()
 {
   ns::id foo = 0;
-  return foo; // Set breakpoint here.
+  return foo + bar(); // Set breakpoint here.
 }

>From 081c72dd4d61279a999fb70e8aefd289a327de3f Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 12 Apr 2024 16:03:41 +0200
Subject: [PATCH 2/2] fixup! fix test

---
 .../lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py| 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git 
a/lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py 
b/lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py
index 19ae2f091bdd5e..3cdca31b8969bd 100644
--- a/lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py
+++ b/lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py
@@ -63,5 +63,5 @@ def test_with_python_api(self):
 
 self.expect_expr("id", result_value="12", result_type="int")
 self.expect_expr("Class", result_value="15", result_type="int")
-self.expect("expr --language C++ -- id", error=True)
-self.expect("expr --language C++ -- Class", error=True)
+self.expect("expr --language Objective-C++ -- id", error=True)
+self.expect("expr --language Objective-C++ -- Class", error=True)

___
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 tests for evaluating local variables whose name clashes with Objective-C types (PR #87807)

[Michael Buch via lldb-commits]

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

[Lldb-commits] [lldb] 9d8ccbc - [lldb][test] Add tests for evaluating local variables whose name clashes with Objective-C types (#87807)

[via lldb-commits]

Author: Michael Buch
Date: 2024-04-12T16:13:54+02:00
New Revision: 9d8ccbc1be78fb4ba0ad1c6ead2dc04b24c8

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

LOG: [lldb][test] Add tests for evaluating local variables whose name clashes 
with Objective-C types (#87807)

Depends on https://github.com/llvm/llvm-project/pull/87767

Added: 


Modified: 
lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py
lldb/test/API/lang/objcxx/objc-builtin-types/main.cpp

Removed: 




diff  --git 
a/lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py 
b/lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py
index 611c7388999058..3cdca31b8969bd 100644
--- a/lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py
+++ b/lldb/test/API/lang/objcxx/objc-builtin-types/TestObjCBuiltinTypes.py
@@ -13,6 +13,7 @@ def setUp(self):
 # Find the line numbers to break inside main().
 self.main_source = "main.cpp"
 self.break_line = line_number(self.main_source, "// Set breakpoint 
here.")
+self.bar_break_line = line_number(self.main_source, "return id + 
Class")
 
 @add_test_categories(["pyapi"])
 def test_with_python_api(self):
@@ -26,6 +27,11 @@ def test_with_python_api(self):
 bpt = target.BreakpointCreateByLocation(self.main_source, 
self.break_line)
 self.assertTrue(bpt, VALID_BREAKPOINT)
 
+bar_bpt = target.BreakpointCreateByLocation(
+self.main_source, self.bar_break_line
+)
+self.assertTrue(bar_bpt, VALID_BREAKPOINT)
+
 # Now launch the process, and do not stop at entry point.
 process = target.LaunchSimple(None, None, 
self.get_process_working_directory())
 
@@ -52,3 +58,10 @@ def test_with_python_api(self):
 patterns=["\(id\) \$.* = nil"],
 )
 self.expect("expr --language C++ -- id my_id = 0; my_id", error=True)
+
+lldbutil.continue_to_breakpoint(process, bar_bpt)
+
+self.expect_expr("id", result_value="12", result_type="int")
+self.expect_expr("Class", result_value="15", result_type="int")
+self.expect("expr --language Objective-C++ -- id", error=True)
+self.expect("expr --language Objective-C++ -- Class", error=True)

diff  --git a/lldb/test/API/lang/objcxx/objc-builtin-types/main.cpp 
b/lldb/test/API/lang/objcxx/objc-builtin-types/main.cpp
index 6dd8cbc6e9fef6..5b35ec0f0b8c98 100644
--- a/lldb/test/API/lang/objcxx/objc-builtin-types/main.cpp
+++ b/lldb/test/API/lang/objcxx/objc-builtin-types/main.cpp
@@ -2,8 +2,14 @@ namespace ns {
   typedef int id;
 };
 
+int bar() {
+  int id = 12;
+  int Class = 15;
+  return id + Class;
+}
+
 int main()
 {
   ns::id foo = 0;
-  return foo; // Set breakpoint here.
+  return foo + bar(); // Set breakpoint here.
 }



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

[Lldb-commits] [lldb] Add lldb-dap to Swift distributions (PR #88482)

[Jonas Devlieghere via lldb-commits]

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


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

[Lldb-commits] [lldb] 2c2377d - Add lldb-dap to Swift distributions (#88482)

[via lldb-commits]

Author: Adam Fowler
Date: 2024-04-12T07:39:22-07:00
New Revision: 2c2377d3a9305b86ab110a4f8390b2d16bff9510

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

LOG: Add lldb-dap to Swift distributions (#88482)

This includes the lldb-dap executable in the MacOS and Linux
distributions of Swift. Currently there is a commit in the Apple repo to
do this for just MacOS https://github.com/apple/llvm-project/pull/8176.
This PR extends this to both Linux and MacOS and brings the change
upstream.

@JDevlieghere @adrian-prantl

Added: 


Modified: 
lldb/cmake/caches/Apple-lldb-Linux.cmake
lldb/cmake/caches/Apple-lldb-macOS.cmake

Removed: 




diff  --git a/lldb/cmake/caches/Apple-lldb-Linux.cmake 
b/lldb/cmake/caches/Apple-lldb-Linux.cmake
index bfa660d8654b7b..b936929afc42f8 100644
--- a/lldb/cmake/caches/Apple-lldb-Linux.cmake
+++ b/lldb/cmake/caches/Apple-lldb-Linux.cmake
@@ -5,6 +5,7 @@ set(LLVM_DISTRIBUTION_COMPONENTS
   lldb
   liblldb
   lldb-argdumper
+  lldb-dap
   lldb-server
   lldb-python-scripts
   CACHE STRING "")

diff  --git a/lldb/cmake/caches/Apple-lldb-macOS.cmake 
b/lldb/cmake/caches/Apple-lldb-macOS.cmake
index 2aef41157bab1a..5155024ac3a026 100644
--- a/lldb/cmake/caches/Apple-lldb-macOS.cmake
+++ b/lldb/cmake/caches/Apple-lldb-macOS.cmake
@@ -22,6 +22,7 @@ set(LLVM_DISTRIBUTION_COMPONENTS
   lldb
   liblldb
   lldb-argdumper
+  lldb-dap
   darwin-debug
   debugserver
   CACHE STRING "")



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

[Lldb-commits] [lldb] Add lldb-dap to Swift distributions (PR #88482)

[Jonas Devlieghere via lldb-commits]

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

[Lldb-commits] [lldb] Add lldb-dap to Swift distributions (PR #88482)

[via lldb-commits]

github-actions[bot] wrote:



@adam-fowler 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 receive 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/88482
___
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:

@jasonmolenda A kind reminder regarding the PR - see answers to your previous 
comments above

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] 5752e31 - [lldb] fix dead lock in TypeCategoryMap.cpp (#87540)

[via lldb-commits]

Author: Vincent Belliard
Date: 2024-04-12T10:48:41-04:00
New Revision: 5752e3196bc52fdac70e3650abc703570ff6209b

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

LOG: [lldb] fix dead lock in TypeCategoryMap.cpp (#87540)

FormatManager::GetCategoryForLanguage and
FormatManager::GetCategory(can_create = true) can be called concurrently
and they both take the TypeCategory::m_map_mutex and the
FormatManager::m_language_categories_mutex but in reverse order.

On one thread, GetCategoryForLanguage takes m_language_categories_mutex
and then ends calling TypeCategoryMap::Get which takes m_map_mutex

On another thread GetCategory calls TypeCategoryMap::Add which takes
m_map_mutex and then calls FormatManager::Changed() which takes
m_language_categories_mutex

If both threads are running concurrently, we have a dead lock.

The patch releases the m_map_mutex before calling Changed which avoids
the dead lock.

-

Co-authored-by: Vincent Belliard 

Added: 


Modified: 
lldb/source/DataFormatters/TypeCategoryMap.cpp

Removed: 




diff  --git a/lldb/source/DataFormatters/TypeCategoryMap.cpp 
b/lldb/source/DataFormatters/TypeCategoryMap.cpp
index fd76bab95826af..ce2cf369b5be53 100644
--- a/lldb/source/DataFormatters/TypeCategoryMap.cpp
+++ b/lldb/source/DataFormatters/TypeCategoryMap.cpp
@@ -25,19 +25,31 @@ TypeCategoryMap::TypeCategoryMap(IFormatChangeListener *lst)
 }
 
 void TypeCategoryMap::Add(KeyType name, const TypeCategoryImplSP &entry) {
-  std::lock_guard guard(m_map_mutex);
-  m_map[name] = entry;
+  {
+std::lock_guard guard(m_map_mutex);
+m_map[name] = entry;
+  }
+  // Release the mutex to avoid a potential deadlock between
+  // TypeCategoryMap::m_map_mutex and
+  // FormatManager::m_language_categories_mutex which can be acquired in
+  // reverse order when calling FormatManager::Changed.
   if (listener)
 listener->Changed();
 }
 
 bool TypeCategoryMap::Delete(KeyType name) {
-  std::lock_guard guard(m_map_mutex);
-  MapIterator iter = m_map.find(name);
-  if (iter == m_map.end())
-return false;
-  m_map.erase(name);
-  Disable(name);
+  {
+std::lock_guard guard(m_map_mutex);
+MapIterator iter = m_map.find(name);
+if (iter == m_map.end())
+  return false;
+m_map.erase(name);
+Disable(name);
+  }
+  // Release the mutex to avoid a potential deadlock between
+  // TypeCategoryMap::m_map_mutex and
+  // FormatManager::m_language_categories_mutex which can be acquired in
+  // reverse order when calling FormatManager::Changed.
   if (listener)
 listener->Changed();
   return true;
@@ -123,9 +135,15 @@ void TypeCategoryMap::DisableAllCategories() {
 }
 
 void TypeCategoryMap::Clear() {
-  std::lock_guard guard(m_map_mutex);
-  m_map.clear();
-  m_active_categories.clear();
+  {
+std::lock_guard guard(m_map_mutex);
+m_map.clear();
+m_active_categories.clear();
+  }
+  // Release the mutex to avoid a potential deadlock between
+  // TypeCategoryMap::m_map_mutex and
+  // FormatManager::m_language_categories_mutex which can be acquired in
+  // reverse order when calling FormatManager::Changed.
   if (listener)
 listener->Changed();
 }



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

[Lldb-commits] [lldb] [lldb] fix dead lock in TypeCategoryMap.cpp (PR #87540)

[Vy Nguyen via lldb-commits]

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

[Lldb-commits] [lldb] [lldb] fix dead lock in TypeCategoryMap.cpp (PR #87540)

[via lldb-commits]

github-actions[bot] wrote:



@v-bulle 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 receive 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/87540
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [libcxx] [lldb] [libc++][CI] Tests LLDB libc++ data formatters. (PR #88312)

[Mark de Wever via lldb-commits]

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

[Lldb-commits] [libcxx] [lldb] [libc++][CI] Tests LLDB libc++ data formatters. (PR #88312)

[Mark de Wever via lldb-commits]

@@ -751,6 +751,8 @@ def setUpCommands(cls):
 "settings set symbols.enable-external-lookup false",
 # Inherit the TCC permissions from the inferior's parent.
 "settings set target.inherit-tcc true",
+# Based on 
https://discourse.llvm.org/t/running-lldb-in-a-container/76801/4
+"settings set target.disable-aslr false",

mordante wrote:

The reason for change is in the Discourse link. This probably needs to be done 
in a separate commit.

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

[Lldb-commits] [libcxx] [lldb] [libc++][CI] Tests LLDB libc++ data formatters. (PR #88312)

[Mark de Wever via lldb-commits]

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

[Lldb-commits] [libcxx] [lldb] [libc++][CI] Tests LLDB libc++ data formatters. (PR #88312)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)


Changes

This enables testing of the LLDB libc++ specific data formatters.
This is enabled in the bootstrap build since building LLDB requires Clang and 
this
is quite expensive. Adding this test changes the build time from 31 to 34 
minutes.

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


2 Files Affected:

- (modified) libcxx/utils/ci/run-buildbot (+7-3) 
- (modified) lldb/packages/Python/lldbsuite/test/lldbtest.py (+2) 


``diff
diff --git a/libcxx/utils/ci/run-buildbot b/libcxx/utils/ci/run-buildbot
index a6f3eb174308b4..e6240a829b0c73 100755
--- a/libcxx/utils/ci/run-buildbot
+++ b/libcxx/utils/ci/run-buildbot
@@ -376,18 +376,22 @@ bootstrapping-build)
   -DCMAKE_CXX_COMPILER_LAUNCHER="ccache" \
   -DCMAKE_BUILD_TYPE=Release \
   -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
-  -DLLVM_ENABLE_PROJECTS="clang" \
+  -DLLVM_ENABLE_PROJECTS="clang;lldb" \
   -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
   -DLLVM_RUNTIME_TARGETS="$(${CXX} --print-target-triple)" \
+  -DLLVM_HOST_TRIPLE="$(${CXX} --print-target-triple)" \
   -DLLVM_TARGETS_TO_BUILD="host" \
   -DRUNTIMES_BUILD_ALLOW_DARWIN=ON \
   -DLLVM_ENABLE_ASSERTIONS=ON \
   -DLLVM_LIT_ARGS="-sv --xunit-xml-output test-results.xml 
--timeout=1500 --time-tests"
 
-echo "+++ Running the libc++ and libc++abi tests"
+echo "+++ Running the LLDB libc++ data formatter tests"
+${NINJA} -vC "${BUILD_DIR}" 
check-lldb-api-functionalities-data-formatter-data-formatter-stl-libcxx
+
+echo "--- Running the libc++ and libc++abi tests"
 ${NINJA} -vC "${BUILD_DIR}" check-runtimes
 
-echo "--- Installing libc++ and libc++abi to a fake location"
+echo "+++ Installing libc++ and libc++abi to a fake location"
 ${NINJA} -vC "${BUILD_DIR}" install-runtimes
 
 ccache -s
diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index c28a78a2c4a27a..7a7afec7345707 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -751,6 +751,8 @@ def setUpCommands(cls):
 "settings set symbols.enable-external-lookup false",
 # Inherit the TCC permissions from the inferior's parent.
 "settings set target.inherit-tcc true",
+# Based on 
https://discourse.llvm.org/t/running-lldb-in-a-container/76801/4
+"settings set target.disable-aslr false",
 # Kill rather than detach from the inferior if something goes 
wrong.
 "settings set target.detach-on-error false",
 # Disable fix-its by default so that incorrect expressions in 
tests don't

``




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

[Lldb-commits] [libcxx] [lldb] [libc++][CI] Tests LLDB libc++ data formatters. (PR #88312)

[Michael Buch via lldb-commits]

Michael137 wrote:

Awesome, thanks for doing this!!

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

[Lldb-commits] [lldb] Reapply "Fix error in unrecognized register name handling for "SBFram…e.register"" (#88468)" (PR #88535)

[via lldb-commits]

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


The only change is a fix for the "register" iterator test to not rely on 
particular register names.

I mistook where the artificial "pc" register is generated.  It isn't added to 
the register list or the register sets (except on arm where that's the name of 
the actual register), so I can't use it in this test.  I instead just assert 
that the "register" generator produces the same list as flattening the register 
sets from "registers".

This reverts commit 9f14914753599f3879e4c273191959e2f1b3632c.

>From 4a335c835239edc1989a1ddd7875a6e6edd2d143 Mon Sep 17 00:00:00 2001
From: Jim Ingham 
Date: Fri, 12 Apr 2024 08:52:43 -0700
Subject: [PATCH] Reapply "Fix error in unrecognized register name handling for
 "SBFrame.register"" (#88468)" with a fix for the "register" iterator test to
 not rely on particular register names.

I mistook where the artificial "pc" register is generated.  It isn't added to 
the register
list or the register sets (except on arm where that's the name of the actual 
register), so
I can't use it in this test.  I instead just assert that the "register" 
generator produces
the same list as flattening the register sets from "registers".

This reverts commit 9f14914753599f3879e4c273191959e2f1b3632c.
---
 lldb/bindings/interface/SBFrameExtensions.i  | 12 +++-
 lldb/test/API/python_api/frame/TestFrames.py | 16 ++--
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/lldb/bindings/interface/SBFrameExtensions.i 
b/lldb/bindings/interface/SBFrameExtensions.i
index 43b22ed7a6b325..e0472280666ab9 100644
--- a/lldb/bindings/interface/SBFrameExtensions.i
+++ b/lldb/bindings/interface/SBFrameExtensions.i
@@ -44,6 +44,16 @@ STRING_EXTENSION_OUTSIDE(SBFrame)
 def __init__(self, regs):
 self.regs = regs
 
+def __iter__(self):
+return self.get_registers()
+
+def get_registers(self):
+for i in range(0,len(self.regs)):
+rs = self.regs[i]
+for j in range (0,rs.num_children):
+reg = rs.GetChildAtIndex(j)
+yield reg
+  
 def __getitem__(self, key):
 if type(key) is str:
 for i in range(0,len(self.regs)):
@@ -52,7 +62,7 @@ STRING_EXTENSION_OUTSIDE(SBFrame)
 reg = rs.GetChildAtIndex(j)
 if reg.name == key: return reg
 else:
-return lldb.SBValue()
+return SBValue()
 
 return registers_access(self.registers)
 
diff --git a/lldb/test/API/python_api/frame/TestFrames.py 
b/lldb/test/API/python_api/frame/TestFrames.py
index a82b129bc8099d..0ec12206024ab7 100644
--- a/lldb/test/API/python_api/frame/TestFrames.py
+++ b/lldb/test/API/python_api/frame/TestFrames.py
@@ -73,10 +73,12 @@ def test_get_arg_vals_for_call_stack(self):
 gpr_reg_set = lldbutil.get_GPRs(frame)
 pc_value = gpr_reg_set.GetChildMemberWithName("pc")
 self.assertTrue(pc_value, "We should have a valid PC.")
-pc_value_int = int(pc_value.GetValue(), 0)
+   
+
 # Make sure on arm targets we dont mismatch PC value on the 
basis of thumb bit.
 # Frame PC will not have thumb bit set in case of a thumb
 # instruction as PC.
+pc_value_int = int(pc_value.GetValue(), 0)
 if self.getArchitecture() in ["arm", "armv7", "armv7k"]:
 pc_value_int &= ~1
 self.assertEqual(
@@ -91,7 +93,17 @@ def test_get_arg_vals_for_call_stack(self):
 frame.GetSP(),
 "SP gotten as a value should equal frame's GetSP",
 )
-
+# Test that the "register" property's flat list matches the 
list from
+# the "registers" property that returns register sets:
+register_regs = set()
+flattened_regs = set()
+for reg_set in frame.registers:
+for reg in reg_set:
+flattened_regs.add(reg.name)
+for reg in frame.register:
+register_regs.add(reg.name)
+self.assertEqual(register_regs, flattened_regs, "register 
matches registers")
+
 print("---", file=session)
 process.Continue()
 

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

[Lldb-commits] [lldb] Reapply "Fix error in unrecognized register name handling for "SBFram…e.register"" (#88468)" (PR #88535)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: None (jimingham)


Changes


The only change is a fix for the "register" iterator test to not rely on 
particular register names.

I mistook where the artificial "pc" register is generated.  It isn't added to 
the register list or the register sets (except on arm where that's the name of 
the actual register), so I can't use it in this test.  I instead just assert 
that the "register" generator produces the same list as flattening the register 
sets from "registers".

This reverts commit 9f14914753599f3879e4c273191959e2f1b3632c.

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


2 Files Affected:

- (modified) lldb/bindings/interface/SBFrameExtensions.i (+11-1) 
- (modified) lldb/test/API/python_api/frame/TestFrames.py (+14-2) 


``diff
diff --git a/lldb/bindings/interface/SBFrameExtensions.i 
b/lldb/bindings/interface/SBFrameExtensions.i
index 43b22ed7a6b325..e0472280666ab9 100644
--- a/lldb/bindings/interface/SBFrameExtensions.i
+++ b/lldb/bindings/interface/SBFrameExtensions.i
@@ -44,6 +44,16 @@ STRING_EXTENSION_OUTSIDE(SBFrame)
 def __init__(self, regs):
 self.regs = regs
 
+def __iter__(self):
+return self.get_registers()
+
+def get_registers(self):
+for i in range(0,len(self.regs)):
+rs = self.regs[i]
+for j in range (0,rs.num_children):
+reg = rs.GetChildAtIndex(j)
+yield reg
+  
 def __getitem__(self, key):
 if type(key) is str:
 for i in range(0,len(self.regs)):
@@ -52,7 +62,7 @@ STRING_EXTENSION_OUTSIDE(SBFrame)
 reg = rs.GetChildAtIndex(j)
 if reg.name == key: return reg
 else:
-return lldb.SBValue()
+return SBValue()
 
 return registers_access(self.registers)
 
diff --git a/lldb/test/API/python_api/frame/TestFrames.py 
b/lldb/test/API/python_api/frame/TestFrames.py
index a82b129bc8099d..0ec12206024ab7 100644
--- a/lldb/test/API/python_api/frame/TestFrames.py
+++ b/lldb/test/API/python_api/frame/TestFrames.py
@@ -73,10 +73,12 @@ def test_get_arg_vals_for_call_stack(self):
 gpr_reg_set = lldbutil.get_GPRs(frame)
 pc_value = gpr_reg_set.GetChildMemberWithName("pc")
 self.assertTrue(pc_value, "We should have a valid PC.")
-pc_value_int = int(pc_value.GetValue(), 0)
+   
+
 # Make sure on arm targets we dont mismatch PC value on the 
basis of thumb bit.
 # Frame PC will not have thumb bit set in case of a thumb
 # instruction as PC.
+pc_value_int = int(pc_value.GetValue(), 0)
 if self.getArchitecture() in ["arm", "armv7", "armv7k"]:
 pc_value_int &= ~1
 self.assertEqual(
@@ -91,7 +93,17 @@ def test_get_arg_vals_for_call_stack(self):
 frame.GetSP(),
 "SP gotten as a value should equal frame's GetSP",
 )
-
+# Test that the "register" property's flat list matches the 
list from
+# the "registers" property that returns register sets:
+register_regs = set()
+flattened_regs = set()
+for reg_set in frame.registers:
+for reg in reg_set:
+flattened_regs.add(reg.name)
+for reg in frame.register:
+register_regs.add(reg.name)
+self.assertEqual(register_regs, flattened_regs, "register 
matches registers")
+
 print("---", file=session)
 process.Continue()
 

``




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

[Lldb-commits] [lldb] Reapply "Fix error in unrecognized register name handling for "SBFram…e.register"" (#88468)" (PR #88535)

[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 
e0a628715a8464e220c8ba9e9aaaf2561139198a...4a335c835239edc1989a1ddd7875a6e6edd2d143
 lldb/test/API/python_api/frame/TestFrames.py
``





View the diff from darker here.


``diff
--- TestFrames.py   2024-04-12 17:00:07.00 +
+++ TestFrames.py   2024-04-12 17:09:04.374356 +
@@ -71,12 +71,11 @@
 # but they should be valid.  Uses get_GPRs() from the lldbutil
 # module.
 gpr_reg_set = lldbutil.get_GPRs(frame)
 pc_value = gpr_reg_set.GetChildMemberWithName("pc")
 self.assertTrue(pc_value, "We should have a valid PC.")
-   
-
+
 # Make sure on arm targets we dont mismatch PC value on the 
basis of thumb bit.
 # Frame PC will not have thumb bit set in case of a thumb
 # instruction as PC.
 pc_value_int = int(pc_value.GetValue(), 0)
 if self.getArchitecture() in ["arm", "armv7", "armv7k"]:
@@ -100,12 +99,14 @@
 for reg_set in frame.registers:
 for reg in reg_set:
 flattened_regs.add(reg.name)
 for reg in frame.register:
 register_regs.add(reg.name)
-self.assertEqual(register_regs, flattened_regs, "register 
matches registers")
-
+self.assertEqual(
+register_regs, flattened_regs, "register matches registers"
+)
+
 print("---", file=session)
 process.Continue()
 
 # At this point, the inferior process should have exited.
 self.assertEqual(process.GetState(), lldb.eStateExited, PROCESS_EXITED)

``




https://github.com/llvm/llvm-project/pull/88535
___
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)

[Jason Molenda via lldb-commits]

jasonmolenda wrote:

Hi sorry @kovdan01 I missed this one in the emails.  You're using an lldb which 
was built without the `LLVM_TARGETS_TO_BUILD` including X86, and running that 
lldb on an x86 corefile, got it.  I have low confidence how well lldb will work 
in this situation, e.g. inferior function calls are obviously going to fail 
completely, and possibly not in a graceful way, but that doesn't impact 
corefiles.  I'm less thrilled about adding a 570kb corefile to the repository 
to check this combination doesn't crash the unwinder.  In 
lldb/unittest/UnwindAssembly we build the `x86` directory when 

```
if ("X86" IN_LIST LLVM_TARGETS_TO_BUILD)
  add_subdirectory(x86)
endif()
```

In Testx86AssemblyInspectionEngine.cpp we initialize llvm state in 
`Testx86AssemblyInspectionEngine::SetUpTestCase` and then run individual tests 
in the `TEST_F()` entries, creating a byte stream of prologues like 

```
  // 'int main() { }' compiled for x86_64-apple-macosx with clang
  uint8_t data[] = {
  0x55, // offset 0 -- pushq %rbp
  0x48, 0x89, 0xe5, // offset 1 -- movq %rsp, %rbp
  0x31, 0xc0,   // offset 4 -- xorl %eax, %eax
  0x5d, // offset 6 -- popq %rbp
  0xc3  // offset 7 -- retq
  };
```

and run the unwind engine on those bytes.  

Could we add a `x86-but-no-x86-target` directory, write one test to see that 
the unwind engine can run against a byte buffer like this and not crash instead 
maybe?  

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] Reapply "Fix error in unrecognized register name handling for "SBFram…e.register"" (#88468)" (PR #88535)

[Chelsea Cassanova via lldb-commits]

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

LGTM!

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

[Lldb-commits] [lldb] Reapply "Fix error in unrecognized register name handling for "SBFram…e.register"" (#88468)" (PR #88535)

[Alex Langford via lldb-commits]

@@ -91,7 +93,17 @@ def test_get_arg_vals_for_call_stack(self):
 frame.GetSP(),
 "SP gotten as a value should equal frame's GetSP",
 )
-
+# Test that the "register" property's flat list matches the 
list from
+# the "registers" property that returns register sets:
+register_regs = set()
+flattened_regs = set()
+for reg_set in frame.registers:
+for reg in reg_set:
+flattened_regs.add(reg.name)
+for reg in frame.register:
+register_regs.add(reg.name)

bulbazord wrote:

Instead of iterating over it twice, why not combine into one loop?

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

[Lldb-commits] [lldb] Reapply "Fix error in unrecognized register name handling for "SBFram…e.register"" (#88468)" (PR #88535)

[Alex Langford via lldb-commits]

@@ -44,6 +44,16 @@ STRING_EXTENSION_OUTSIDE(SBFrame)
 def __init__(self, regs):
 self.regs = regs
 
+def __iter__(self):
+return self.get_registers()
+
+def get_registers(self):
+for i in range(0,len(self.regs)):
+rs = self.regs[i]
+for j in range (0,rs.num_children):

bulbazord wrote:

The `range` expressions in this function can be simplified. Dropping the 0 
means the list of numbers automatically starts at 0:
```
for i in range(len(self.regs)):
...
for j in range(rs.num_children):
```



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

[Lldb-commits] [lldb] Reapply "Fix error in unrecognized register name handling for "SBFram…e.register"" (#88468)" (PR #88535)

[via lldb-commits]

@@ -91,7 +93,17 @@ def test_get_arg_vals_for_call_stack(self):
 frame.GetSP(),
 "SP gotten as a value should equal frame's GetSP",
 )
-
+# Test that the "register" property's flat list matches the 
list from
+# the "registers" property that returns register sets:
+register_regs = set()
+flattened_regs = set()
+for reg_set in frame.registers:
+for reg in reg_set:
+flattened_regs.add(reg.name)
+for reg in frame.register:
+register_regs.add(reg.name)

jimingham wrote:

Iterating over what twice?  One is iterating over the elements of the list that 
is the `registers` property, and the other using the generator for the 
`register` (note no final `s`) property.

`registers` is actually a poorly named property since it actually produces a 
list of `register sets`.

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

[Lldb-commits] [lldb] [lldb][libc++] Adds local_t clock data formatters. (PR #88178)

[Mark de Wever via lldb-commits]

@@ -1068,6 +1068,29 @@ static void 
LoadLibCxxFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
 eTypeOptionCascade,
 true);
 
+  AddCXXSummary(
+  cpp_category_sp,
+  lldb_private::formatters::LibcxxChronoLocalSecondsSummaryProvider,
+  "libc++ std::chrono::local_seconds summary provider",
+  "^std::__[[:alnum:]]+::chrono::time_point<"
+  "std::__[[:alnum:]]+::chrono::local_t, "
+  "std::__[[:alnum:]]+::chrono::durationhttp://eel.is/c++draft/time.syn#def:signed_integer_type_of_at_least_35_bits)>;

In libc++ this is hard coded to `long long` on all platforms
https://github.com/llvm/llvm-project/blob/main/libcxx/include/__chrono/duration.h#L287

There is some value in it to future proof it, if we ever get a platform where 
long is 64 bit and long long 128 bit.

I'll also update `LibcxxChronoSysSecondsSummaryProvider` above.

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

[Lldb-commits] [lldb] Reapply "Fix error in unrecognized register name handling for "SBFram…e.register"" (#88468)" (PR #88535)

[via lldb-commits]

@@ -44,6 +44,16 @@ STRING_EXTENSION_OUTSIDE(SBFrame)
 def __init__(self, regs):
 self.regs = regs
 
+def __iter__(self):
+return self.get_registers()
+
+def get_registers(self):
+for i in range(0,len(self.regs)):
+rs = self.regs[i]
+for j in range (0,rs.num_children):

jimingham wrote:

Unless there's some difference in behavior, I don't mind burning a few 
characters to make the range explicit.

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

[Lldb-commits] [lldb] Reapply "Fix error in unrecognized register name handling for "SBFram…e.register"" (#88468)" (PR #88535)

[via lldb-commits]

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

[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88543)

[Miro Bucko via lldb-commits]

https://github.com/mbucko created 
https://github.com/llvm/llvm-project/pull/88543

Summary:
AddMemoryList() was returning the last error status returned by ReadMemory(). 
So if an invalid memory region was read last, the function would return an 
error.

Test Plan:
./bin/llvm-lit -sv 
~/src/llvm-project/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py

Reviewers:

Subscribers:

Tasks:

Tags:

>From d1a4bce2f16b384deadaf48c17b7a2b1dbb16c29 Mon Sep 17 00:00:00 2001
From: Miro Bucko 
Date: Fri, 12 Apr 2024 09:55:46 -0700
Subject: [PATCH] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam

Summary:
AddMemoryList() was returning the last error status returned by ReadMemory(). 
So if an invalid memory region was read last, the function would return an 
error.

Test Plan:
./bin/llvm-lit -sv 
~/src/llvm-project/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py

Reviewers:

Subscribers:

Tasks:

Tags:
---
 .../Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp  | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp 
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 50d1b563f469cf..a45521d3429912 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -655,9 +655,10 @@ MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP 
&process_sp,
 const addr_t addr = core_range.range.start();
 const addr_t size = core_range.range.size();
 auto data_up = std::make_unique(size, 0);
+Status read_error;
 const size_t bytes_read =
-process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
-if (bytes_read == 0)
+process_sp->ReadMemory(addr, data_up->GetBytes(), size, read_error);
+if (read_error.Fail() || bytes_read == 0)
   continue;
 // We have a good memory region with valid bytes to store.
 LocationDescriptor memory_dump;

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

[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88543)

[Miro Bucko via lldb-commits]

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

[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88543)

[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/88543
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Reapply "Fix error in unrecognized register name handling for "SBFram…e.register"" (#88468)" (PR #88535)

[Alex Langford via lldb-commits]

@@ -44,6 +44,16 @@ STRING_EXTENSION_OUTSIDE(SBFrame)
 def __init__(self, regs):
 self.regs = regs
 
+def __iter__(self):
+return self.get_registers()
+
+def get_registers(self):
+for i in range(0,len(self.regs)):
+rs = self.regs[i]
+for j in range (0,rs.num_children):

bulbazord wrote:

Fair, explicit is good. I meant it as a suggestion originally.

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

[Lldb-commits] [lldb] Reapply "Fix error in unrecognized register name handling for "SBFram…e.register"" (#88468)" (PR #88535)

[Alex Langford via lldb-commits]

@@ -91,7 +93,17 @@ def test_get_arg_vals_for_call_stack(self):
 frame.GetSP(),
 "SP gotten as a value should equal frame's GetSP",
 )
-
+# Test that the "register" property's flat list matches the 
list from
+# the "registers" property that returns register sets:
+register_regs = set()
+flattened_regs = set()
+for reg_set in frame.registers:
+for reg in reg_set:
+flattened_regs.add(reg.name)
+for reg in frame.register:
+register_regs.add(reg.name)

bulbazord wrote:

Oh, I see now. I definitely did not notice the difference between 
`frame.register` and `frame.registers`... I wish we could change that. Maybe we 
could make `frame.register_sets` an alias for `frame.registers` and switch to 
using that?

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

[Lldb-commits] [lldb] Reapply "Fix error in unrecognized register name handling for "SBFram…e.register"" (#88468)" (PR #88535)

[Alex Langford via lldb-commits]

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

LGTM

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

[Lldb-commits] [lldb] 446d38c - [lldb] Fix test build failure

[Jan Svoboda via lldb-commits]

Author: Jan Svoboda
Date: 2024-04-12T10:48:54-07:00
New Revision: 446d38c65f72fd5d242a64182b05683577f683d3

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

LOG: [lldb] Fix test build failure

Caused by commit edd7fed9da48c0e708cce9bd4d305ae43d8bd77c

Added: 


Modified: 
lldb/unittests/Host/FileSystemTest.cpp

Removed: 




diff  --git a/lldb/unittests/Host/FileSystemTest.cpp 
b/lldb/unittests/Host/FileSystemTest.cpp
index 4ed9beff4d303c..3b5ee7c8bc2237 100644
--- a/lldb/unittests/Host/FileSystemTest.cpp
+++ b/lldb/unittests/Host/FileSystemTest.cpp
@@ -74,7 +74,7 @@ class DummyFileSystem : public vfs::FileSystem {
   }
   // Map any symlink to "/symlink".
   std::error_code getRealPath(const Twine &Path,
-  SmallVectorImpl &Output) const override {
+  SmallVectorImpl &Output) override {
 auto I = FilesAndDirs.find(Path.str());
 if (I == FilesAndDirs.end())
   return make_error_code(llvm::errc::no_such_file_or_directory);



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

[Lldb-commits] [lldb] Reapply "Fix error in unrecognized register name handling for "SBFram…e.register"" (#88468)" (PR #88535)

[via lldb-commits]

@@ -91,7 +93,17 @@ def test_get_arg_vals_for_call_stack(self):
 frame.GetSP(),
 "SP gotten as a value should equal frame's GetSP",
 )
-
+# Test that the "register" property's flat list matches the 
list from
+# the "registers" property that returns register sets:
+register_regs = set()
+flattened_regs = set()
+for reg_set in frame.registers:
+for reg in reg_set:
+flattened_regs.add(reg.name)
+for reg in frame.register:
+register_regs.add(reg.name)

jimingham wrote:

This PR is about fixing `register` not `registers`.  If you try to iterate over 
it in the current sources you get a weird error.  When I fixed just that error, 
then iterating over it spun forever, so I either had to explicitly turn off 
iterability or make some reasonable generator, which is what this patch does.  

Adding `register_sets` as another property that does the same thing as 
`registers` seems fine, but also not really a part of this PR.

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

[Lldb-commits] [lldb] 4b0beb4 - Reapply "Fix error in unrecognized register name handling for "SBFram…e.register"" (#88468)" (#88535)

[via lldb-commits]

Author: jimingham
Date: 2024-04-12T10:51:49-07:00
New Revision: 4b0beb4f5ec42aea58461df7994e2fa40f335bb6

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

LOG: Reapply "Fix error in unrecognized register name handling for 
"SBFram…e.register"" (#88468)" (#88535)

The only change is a fix for the "register" iterator test to not rely on
particular register names.

I mistook where the artificial "pc" register is generated. It isn't
added to the register list or the register sets (except on arm where
that's the name of the actual register), so I can't use it in this test.
I instead just assert that the "register" generator produces the same
list as flattening the register sets from "registers".

This reverts commit 9f14914753599f3879e4c273191959e2f1b3632c.

Added: 


Modified: 
lldb/bindings/interface/SBFrameExtensions.i
lldb/test/API/python_api/frame/TestFrames.py

Removed: 




diff  --git a/lldb/bindings/interface/SBFrameExtensions.i 
b/lldb/bindings/interface/SBFrameExtensions.i
index 43b22ed7a6b325..e0472280666ab9 100644
--- a/lldb/bindings/interface/SBFrameExtensions.i
+++ b/lldb/bindings/interface/SBFrameExtensions.i
@@ -44,6 +44,16 @@ STRING_EXTENSION_OUTSIDE(SBFrame)
 def __init__(self, regs):
 self.regs = regs
 
+def __iter__(self):
+return self.get_registers()
+
+def get_registers(self):
+for i in range(0,len(self.regs)):
+rs = self.regs[i]
+for j in range (0,rs.num_children):
+reg = rs.GetChildAtIndex(j)
+yield reg
+  
 def __getitem__(self, key):
 if type(key) is str:
 for i in range(0,len(self.regs)):
@@ -52,7 +62,7 @@ STRING_EXTENSION_OUTSIDE(SBFrame)
 reg = rs.GetChildAtIndex(j)
 if reg.name == key: return reg
 else:
-return lldb.SBValue()
+return SBValue()
 
 return registers_access(self.registers)
 

diff  --git a/lldb/test/API/python_api/frame/TestFrames.py 
b/lldb/test/API/python_api/frame/TestFrames.py
index a82b129bc8099d..0ec12206024ab7 100644
--- a/lldb/test/API/python_api/frame/TestFrames.py
+++ b/lldb/test/API/python_api/frame/TestFrames.py
@@ -73,10 +73,12 @@ def test_get_arg_vals_for_call_stack(self):
 gpr_reg_set = lldbutil.get_GPRs(frame)
 pc_value = gpr_reg_set.GetChildMemberWithName("pc")
 self.assertTrue(pc_value, "We should have a valid PC.")
-pc_value_int = int(pc_value.GetValue(), 0)
+   
+
 # Make sure on arm targets we dont mismatch PC value on the 
basis of thumb bit.
 # Frame PC will not have thumb bit set in case of a thumb
 # instruction as PC.
+pc_value_int = int(pc_value.GetValue(), 0)
 if self.getArchitecture() in ["arm", "armv7", "armv7k"]:
 pc_value_int &= ~1
 self.assertEqual(
@@ -91,7 +93,17 @@ def test_get_arg_vals_for_call_stack(self):
 frame.GetSP(),
 "SP gotten as a value should equal frame's GetSP",
 )
-
+# Test that the "register" property's flat list matches the 
list from
+# the "registers" property that returns register sets:
+register_regs = set()
+flattened_regs = set()
+for reg_set in frame.registers:
+for reg in reg_set:
+flattened_regs.add(reg.name)
+for reg in frame.register:
+register_regs.add(reg.name)
+self.assertEqual(register_regs, flattened_regs, "register 
matches registers")
+
 print("---", file=session)
 process.Continue()
 



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

[Lldb-commits] [lldb] Reapply "Fix error in unrecognized register name handling for "SBFram…e.register"" (#88468)" (PR #88535)

[via lldb-commits]

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

[Lldb-commits] [libcxx] [lldb] [libc++][CI] Tests LLDB libc++ data formatters. (PR #88312)

[Jason Molenda via lldb-commits]

jasonmolenda wrote:

The lldb change part of this PR looks good to me.

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

[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)

[Miro Bucko via lldb-commits]

https://github.com/mbucko created 
https://github.com/llvm/llvm-project/pull/88564

Summary:
AddMemoryList() was returning the last error status returned by ReadMemory(). 
So if an invalid memory region was read last, the function would return an 
error.

Test Plan:
./bin/llvm-lit -sv 
~/src/llvm-project/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py

Reviewers:
kevinfrei,clayborg 

Subscribers:

Tasks:

Tags:

>From d1a4bce2f16b384deadaf48c17b7a2b1dbb16c29 Mon Sep 17 00:00:00 2001
From: Miro Bucko 
Date: Fri, 12 Apr 2024 09:55:46 -0700
Subject: [PATCH] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam

Summary:
AddMemoryList() was returning the last error status returned by ReadMemory(). 
So if an invalid memory region was read last, the function would return an 
error.

Test Plan:
./bin/llvm-lit -sv 
~/src/llvm-project/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py

Reviewers:

Subscribers:

Tasks:

Tags:
---
 .../Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp  | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp 
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 50d1b563f469cf..a45521d3429912 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -655,9 +655,10 @@ MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP 
&process_sp,
 const addr_t addr = core_range.range.start();
 const addr_t size = core_range.range.size();
 auto data_up = std::make_unique(size, 0);
+Status read_error;
 const size_t bytes_read =
-process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
-if (bytes_read == 0)
+process_sp->ReadMemory(addr, data_up->GetBytes(), size, read_error);
+if (read_error.Fail() || bytes_read == 0)
   continue;
 // We have a good memory region with valid bytes to store.
 LocationDescriptor memory_dump;

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

[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)

[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/88564
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Miro Bucko (mbucko)


Changes

Summary:
AddMemoryList() was returning the last error status returned by ReadMemory(). 
So if an invalid memory region was read last, the function would return an 
error.

Test Plan:
./bin/llvm-lit -sv 
~/src/llvm-project/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py

Reviewers:
kevinfrei,clayborg 

Subscribers:

Tasks:

Tags:

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


1 Files Affected:

- (modified) lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp 
(+3-2) 


``diff
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp 
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 50d1b563f469cf..a45521d3429912 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -655,9 +655,10 @@ MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP 
&process_sp,
 const addr_t addr = core_range.range.start();
 const addr_t size = core_range.range.size();
 auto data_up = std::make_unique(size, 0);
+Status read_error;
 const size_t bytes_read =
-process_sp->ReadMemory(addr, data_up->GetBytes(), size, error);
-if (bytes_read == 0)
+process_sp->ReadMemory(addr, data_up->GetBytes(), size, read_error);
+if (read_error.Fail() || bytes_read == 0)
   continue;
 // We have a good memory region with valid bytes to store.
 LocationDescriptor memory_dump;

``




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

[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)

[Jason Molenda via lldb-commits]

@@ -655,9 +655,10 @@ MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP 
&process_sp,
 const addr_t addr = core_range.range.start();
 const addr_t size = core_range.range.size();
 auto data_up = std::make_unique(size, 0);
+Status read_error;

jasonmolenda wrote:

The goal is to skip memory ranges that couldn't be read, without surfacing an 
error about them, right.  I don't mind it that much, but another way to be to 
use the existing `error` Status object (which we know is state==Success at this 
point), and if our memory read does fail, we could do 
```
if (error.Fail() || bytes_read == 0) {
  error.Clear();
  continue;
}
```

To make it clear that we don't want to surface a memory read failure for a 
region to the caller.  But this is more of a style preference I think.

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

[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)

[Miro Bucko via lldb-commits]

@@ -655,9 +655,10 @@ MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP 
&process_sp,
 const addr_t addr = core_range.range.start();
 const addr_t size = core_range.range.size();
 auto data_up = std::make_unique(size, 0);
+Status read_error;

mbucko wrote:

Yes, this also works. Let me know if you'd like me to change it

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

[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)

[David Blaikie via lldb-commits]

@@ -48,15 +60,31 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames 
&debug_names) {
   return result;
 }
 
+DWARFTypeUnit *
+DebugNamesDWARFIndex::GetForeignTypeUnit(const DebugNames::Entry &entry) const 
{
+  std::optional type_sig = entry.getForeignTUTypeSignature();
+  if (type_sig)
+if (auto dwp_sp = m_debug_info.GetDwpSymbolFile())

dwblaikie wrote:

this `auto` should probably be const ref, to avoid inc/dec on the ref counted 
smart pointer's ref count

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

[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)

[David Blaikie via lldb-commits]

@@ -1726,44 +1725,59 @@ lldb::ModuleSP 
SymbolFileDWARF::GetExternalModule(ConstString name) {
   return pos->second;
 }
 
-DWARFDIE
-SymbolFileDWARF::GetDIE(const DIERef &die_ref) {
-  // This method can be called without going through the symbol vendor so we
-  // need to lock the module.
-  std::lock_guard guard(GetModuleMutex());
-
-  SymbolFileDWARF *symbol_file = nullptr;
-
+SymbolFileDWARF *SymbolFileDWARF::GetDIERefSymbolFile(const DIERef &die_ref) {
   // Anytime we get a "lldb::user_id_t" from an lldb_private::SymbolFile API we
   // must make sure we use the correct DWARF file when resolving things. On
   // MacOSX, when using SymbolFileDWARFDebugMap, we will use multiple
   // SymbolFileDWARF classes, one for each .o file. We can often end up with
   // references to other DWARF objects and we must be ready to receive a
   // "lldb::user_id_t" that specifies a DIE from another SymbolFileDWARF
   // instance.
+
   std::optional file_index = die_ref.file_index();
+
+  // If the file index matches, then we have the right SymbolFileDWARF already.
+  // This will work for both .dwo file and DWARF in .o files for mac. Also if
+  // both the file indexes are invalid, then we have a match.
+  if (GetFileIndex() == file_index)
+return this;
+
+  // If we are currently in a .dwo file and our file index doesn't match we 
need
+  // to let the base symbol file handle this.
+  SymbolFileDWARFDwo *dwo = llvm::dyn_cast_or_null(this);
+  if (dwo)
+return dwo->GetBaseSymbolFile().GetDIERefSymbolFile(die_ref);
+
   if (file_index) {
-if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile()) {
-  symbol_file = debug_map->GetSymbolFileByOSOIndex(*file_index); // OSO 
case
-  if (symbol_file)
-return symbol_file->DebugInfo().GetDIE(die_ref);
-  return DWARFDIE();
-}
+SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile();
+if (debug_map) {
+// We have a SymbolFileDWARFDebugMap, so let it find the right file
+  return debug_map->GetSymbolFileByOSOIndex(*file_index);
+} else {
+  // Handle the .dwp file case correctly
+  if (*file_index == DIERef::k_file_index_mask)
+return GetDwpSymbolFile().get(); // DWP case
 
-if (*file_index == DIERef::k_file_index_mask)
-  symbol_file = GetDwpSymbolFile().get(); // DWP case
-else
-  symbol_file = this->DebugInfo()
-.GetUnitAtIndex(*die_ref.file_index())
+  // Handle the .dwo file case correctly
+  return DebugInfo().GetUnitAtIndex(*die_ref.file_index())
 ->GetDwoSymbolFile(); // DWO case
-  } else if (die_ref.die_offset() == DW_INVALID_OFFSET) {
-return DWARFDIE();
+}
   }
+  return this;
+}
 
-  if (symbol_file)
-return symbol_file->GetDIE(die_ref);
+DWARFDIE
+SymbolFileDWARF::GetDIE(const DIERef &die_ref) {
+  if (die_ref.die_offset() == DW_INVALID_OFFSET)
+return DWARFDIE();
 
-  return DebugInfo().GetDIE(die_ref);
+  // This method can be called without going through the symbol vendor so we
+  // need to lock the module.
+  std::lock_guard guard(GetModuleMutex());
+  SymbolFileDWARF *symbol_file = GetDIERefSymbolFile(die_ref);
+  if (symbol_file)

dwblaikie wrote:

Similar feedback others have given elsewhere, roll the variable declaration 
into the `if` condition

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

[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)

[David Blaikie via lldb-commits]

@@ -58,6 +58,8 @@ class DWARFDebugInfo {
 
   const DWARFDebugAranges &GetCompileUnitAranges();
 
+  const std::shared_ptr GetDwpSymbolFile();

dwblaikie wrote:

Remove const from by-value return. (it messes with move semantics and some 
other things) - or was this meant to return by reference? - yeah, I guess this 
latter, the underlying `m_dwarf.GetDwpSymbolFile()` seems to return by const 
reference, so this function probably should too?

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

[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)

[David Blaikie via lldb-commits]

@@ -0,0 +1,91 @@
+// REQUIRES: lld
+
+// This test will make a type that will be compiled differently into two
+// different .dwo files in a type unit with the same type hash, but with
+// differing contents. I have discovered that the hash for the type unit is
+// simply based off of the typename and doesn't seem to differ when the 
contents
+// differ, so that will help us test foreign type units in the .debug_names

dwblaikie wrote:

Rather than the personal statement, could maybe just be explicit:

"Clang's type unit signature is based only on the mangled name of the type, 
regardless of the contents of the type"

(I can tell you that's how it works - that's how I implemented it - it is a 
violation of the DWARF spec (the spec says to use a content hash - though that 
content hash is of the semantic contents, not the syntactic content (eg: if the 
TU contained a type with a single int member, the spec-compliant hash would be 
the same whether it was the type DIE followed by the int DIE, or the other way 
around) - so it would still be the same despite some variations in layout, so 
the index entries still wouldn't be portable to all matched-hash definitions))

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

[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)

[David Blaikie via lldb-commits]

@@ -273,6 +301,44 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
 if (!isType(entry.tag()))
   continue;
 
+
+DWARFTypeUnit *foreign_tu = GetForeignTypeUnit(entry);
+if (foreign_tu) {
+  // If this entry represents a foreign type unit, we need to verify that
+  // the type unit that ended up in the final .dwp file is the right type
+  // unit. Type units have signatures which are the same across multiple
+  // .dwo files, but only one of those type units will end up in the .dwp
+  // file. The contents of type units for the same type can be different
+  // in different .dwo file, which means the DIE offsets might not be the
+  // same between two different type units. So we need to determine if this
+  // accelerator table matches the type unit in the .dwp file. If it 
doesn't
+  // match, then we need to ignore this accelerator table entry as the type
+  // unit that is in the .dwp file will have its own index.
+  const llvm::DWARFDebugNames::NameIndex *name_index = 
entry.getNameIndex();
+  if (name_index == nullptr)
+continue;
+  // In order to determine if the type unit that ended up in a .dwp file
+  // is valid, we need to grab the type unit and check the attribute on the
+  // type unit matches the .dwo file. For this to happen we rely on each
+  // .dwo file having its own .debug_names table with a single compile unit
+  // and multiple type units. This is the only way we can tell if a type
+  // unit came from a specific .dwo file.

dwblaikie wrote:

Sounds like this wouldn't work for a merged `.debug_names` table? Could you 
leave a FIXME/do you plan to fix this?
Oh, it also wouldn't work for any kind of LTO which could have multiple CUs in 
a single object file/dwo file.

(FYI @cmtice )

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

[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)

[David Blaikie via lldb-commits]

@@ -273,6 +301,44 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
 if (!isType(entry.tag()))
   continue;
 
+
+DWARFTypeUnit *foreign_tu = GetForeignTypeUnit(entry);
+if (foreign_tu) {
+  // If this entry represents a foreign type unit, we need to verify that
+  // the type unit that ended up in the final .dwp file is the right type
+  // unit. Type units have signatures which are the same across multiple
+  // .dwo files, but only one of those type units will end up in the .dwp
+  // file. The contents of type units for the same type can be different
+  // in different .dwo file, which means the DIE offsets might not be the
+  // same between two different type units. So we need to determine if this
+  // accelerator table matches the type unit in the .dwp file. If it 
doesn't
+  // match, then we need to ignore this accelerator table entry as the type
+  // unit that is in the .dwp file will have its own index.
+  const llvm::DWARFDebugNames::NameIndex *name_index = 
entry.getNameIndex();
+  if (name_index == nullptr)
+continue;
+  // In order to determine if the type unit that ended up in a .dwp file
+  // is valid, we need to grab the type unit and check the attribute on the
+  // type unit matches the .dwo file. For this to happen we rely on each
+  // .dwo file having its own .debug_names table with a single compile unit
+  // and multiple type units. This is the only way we can tell if a type
+  // unit came from a specific .dwo file.

dwblaikie wrote:

I think our conclusion from previous discussions was to put 
`DW_IDX_compile_unit`, in addition to the `DW_IDX_type_unit` on the entries in 
the .debug_names table - and add the CU column to the .debug_tu_index in the 
dwp file, to match these things up?

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

[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)

[David Blaikie via lldb-commits]

@@ -0,0 +1,91 @@
+// REQUIRES: lld
+
+// This test will make a type that will be compiled differently into two
+// different .dwo files in a type unit with the same type hash, but with
+// differing contents. I have discovered that the hash for the type unit is
+// simply based off of the typename and doesn't seem to differ when the 
contents
+// differ, so that will help us test foreign type units in the .debug_names
+// section of the main executable. When a DWP file is made, only one type unit
+// will be kept and the type unit that is kept has the .dwo file name that it
+// came from. When LLDB loads the foreign type units, it needs to verify that
+// any entries from foreign type units come from the right .dwo file. We test
+// this since the contents of type units are not always the same even though
+// they have the same type hash. We don't want invalid accelerator table 
entries
+// to come from one .dwo file and be used on a type unit from another since 
this
+// could cause invalid lookups to happen. LLDB knows how to track down which
+// .dwo file a type unit comes from by looking at the DW_AT_dwo_name attribute
+// in the DW_TAG_type_unit.
+
+// Now test with DWARF5
+// RUN: %clang -target x86_64-pc-linux -gdwarf-5 -gsplit-dwarf \
+// RUN:   -fdebug-types-section -gpubnames -c %s -o %t.main.o
+// RUN: %clang -target x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -DVARIANT \
+// RUN:   -fdebug-types-section -gpubnames -c %s -o %t.foo.o
+// RUN: ld.lld %t.main.o %t.foo.o -o %t
+
+// First we check when we make the .dwp file with %t.main.dwo first so it will
+// pick the type unit from %t.main.dwo. Verify we find only the types from
+// %t.main.dwo's type unit.
+// RUN: llvm-dwp %t.main.dwo %t.foo.dwo -o %t.dwp
+// RUN: %lldb \
+// RUN:   -o "type lookup IntegerType" \
+// RUN:   -o "type lookup FloatType" \
+// RUN:   -o "type lookup IntegerType" \
+// RUN:   -b %t | FileCheck %s
+// CHECK: (lldb) type lookup IntegerType
+// CHECK-NEXT: int
+// CHECK-NEXT: (lldb) type lookup FloatType
+// CHECK-NEXT: double
+// CHECK-NEXT: (lldb) type lookup IntegerType
+// CHECK-NEXT: int
+
+// Next we check when we make the .dwp file with %t.foo.dwo first so it will
+// pick the type unit from %t.main.dwo. Verify we find only the types from
+// %t.main.dwo's type unit.
+// RUN: llvm-dwp %t.foo.dwo %t.main.dwo -o %t.dwp
+// RUN: %lldb \
+// RUN:   -o "type lookup IntegerType" \
+// RUN:   -o "type lookup FloatType" \
+// RUN:   -o "type lookup IntegerType" \
+// RUN:   -b %t | FileCheck %s --check-prefix=VARIANT
+
+// VARIANT: (lldb) type lookup IntegerType
+// VARIANT-NEXT: unsigned int
+// VARIANT-NEXT: (lldb) type lookup FloatType
+// VARIANT-NEXT: float
+// VARIANT-NEXT: (lldb) type lookup IntegerType
+// VARIANT-NEXT: unsigned int
+
+
+// We need to do this so we end with a type unit in each .dwo file and that has
+// the same signature but different contents. When we make the .dwp file, then
+// one of the type units will end up in the .dwp file and we will have
+// .debug_names accelerator tables for both type units and we need to ignore
+// the type units .debug_names entries that don't match the .dwo file whose
+// copy of the type unit ends up in the final .dwp file. To do this, LLDB will
+// look at the type unit and take the DWO name attribute and make sure it
+// matches, and if it doesn't, it will ignore the accelerator table entry.
+struct CustomType {
+  // We switch the order of "FloatType" and "IntegerType" so that if we do
+  // end up reading the wrong accelerator table entry, that we would end up
+  // getting an invalid offset and not find anything, or the offset would have
+  // matched and we would find the wrong thing.

dwblaikie wrote:

Couldn't we have a difference between these type variants that's simpler - like 
one version of the struct with a single `int` member, and one version of the 
type with no members? Then do lookup of the `CustomType` and make sure the type 
we get back is the one with the `int` member? (or not, depending on the test)

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

[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)

[Alexander Yermolovich via lldb-commits]

@@ -273,6 +301,44 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
 if (!isType(entry.tag()))
   continue;
 
+
+DWARFTypeUnit *foreign_tu = GetForeignTypeUnit(entry);
+if (foreign_tu) {
+  // If this entry represents a foreign type unit, we need to verify that
+  // the type unit that ended up in the final .dwp file is the right type
+  // unit. Type units have signatures which are the same across multiple
+  // .dwo files, but only one of those type units will end up in the .dwp
+  // file. The contents of type units for the same type can be different
+  // in different .dwo file, which means the DIE offsets might not be the
+  // same between two different type units. So we need to determine if this
+  // accelerator table matches the type unit in the .dwp file. If it 
doesn't
+  // match, then we need to ignore this accelerator table entry as the type
+  // unit that is in the .dwp file will have its own index.
+  const llvm::DWARFDebugNames::NameIndex *name_index = 
entry.getNameIndex();
+  if (name_index == nullptr)
+continue;
+  // In order to determine if the type unit that ended up in a .dwp file
+  // is valid, we need to grab the type unit and check the attribute on the
+  // type unit matches the .dwo file. For this to happen we rely on each
+  // .dwo file having its own .debug_names table with a single compile unit
+  // and multiple type units. This is the only way we can tell if a type
+  // unit came from a specific .dwo file.

ayermolo wrote:

> I think our conclusion from previous discussions was to put 
> `DW_IDX_compile_unit`, in addition to the `DW_IDX_type_unit` on the entries 
> in the .debug_names table - and add the CU column to the .debug_tu_index in 
> the dwp file, to match these things up?

We went with adding comp_dir/name to type unit unit die:
https://discourse.llvm.org/t/debuginfo-dwarfv5-lld-debug-names-with-fdebug-type-sections/73445/29?u=ayermolo
A bit of a shortcut so we don't have to deal with non-standar dwp.

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

[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)

[Alexander Yermolovich via lldb-commits]

@@ -273,6 +301,44 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
 if (!isType(entry.tag()))
   continue;
 
+
+DWARFTypeUnit *foreign_tu = GetForeignTypeUnit(entry);
+if (foreign_tu) {
+  // If this entry represents a foreign type unit, we need to verify that
+  // the type unit that ended up in the final .dwp file is the right type
+  // unit. Type units have signatures which are the same across multiple
+  // .dwo files, but only one of those type units will end up in the .dwp
+  // file. The contents of type units for the same type can be different
+  // in different .dwo file, which means the DIE offsets might not be the
+  // same between two different type units. So we need to determine if this
+  // accelerator table matches the type unit in the .dwp file. If it 
doesn't
+  // match, then we need to ignore this accelerator table entry as the type
+  // unit that is in the .dwp file will have its own index.
+  const llvm::DWARFDebugNames::NameIndex *name_index = 
entry.getNameIndex();
+  if (name_index == nullptr)
+continue;
+  // In order to determine if the type unit that ended up in a .dwp file
+  // is valid, we need to grab the type unit and check the attribute on the
+  // type unit matches the .dwo file. For this to happen we rely on each
+  // .dwo file having its own .debug_names table with a single compile unit
+  // and multiple type units. This is the only way we can tell if a type
+  // unit came from a specific .dwo file.

ayermolo wrote:

> Sounds like this wouldn't work for a merged `.debug_names` table? Could you 
> leave a FIXME/do you plan to fix this? Oh, it also wouldn't work for any kind 
> of LTO which could have multiple CUs in a single object file/dwo file.
> 
> (FYI @cmtice )

Does "multiple cus" in .o/dwo actually happen? 
>From spec perspective I don't understand how this will work. Skeleton CU 
>points to .o/dwo which then has multiple cus? What does it mean?

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

[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)

[via lldb-commits]

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

Thanks for a quick fix!

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

[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)

[Jason Molenda via lldb-commits]

@@ -655,9 +655,10 @@ MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP 
&process_sp,
 const addr_t addr = core_range.range.start();
 const addr_t size = core_range.range.size();
 auto data_up = std::make_unique(size, 0);
+Status read_error;

jasonmolenda wrote:

I don't work on this plugin myself, I'm sure the way you expressed the idea is 
fine, it's little more than a style difference.

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