[lldb][target] Add progress report for wait-attaching to process #144768

chelcassanova · 2025-06-18T17:56:01Z

This commit adds a progress report when wait-attaching to a process as well as a test for this.

llvmbot · 2025-06-18T17:56:38Z

@llvm/pr-subscribers-lldb

Author: Chelsea Cassanova (chelcassanova)

Changes

This commit adds a progress report when wait-attaching to a process as well as a test for this.

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

2 Files Affected:

(modified) lldb/source/Target/Target.cpp (+2)
(modified) lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py (+32)

diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 45a9e1196a049..8819651378791 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3546,6 +3546,8 @@ llvm::Expected<TraceSP> Target::GetTraceOrCreate() {
 }
 
 Status Target::Attach(ProcessAttachInfo &attach_info, Stream *stream) {
+  std::unique_ptr<Progress> attach_progress;
+  attach_progress = std::make_unique<Progress>("Waiting to attach to process");
   m_stats.SetLaunchOrAttachTime();
   auto state = eStateInvalid;
   auto process_sp = GetProcessSP();
diff --git a/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py b/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
index 9af53845ca1b7..b0ca3939c551b 100644
--- a/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
+++ b/lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py
@@ -2,6 +2,7 @@
 Test that we are able to broadcast and receive progress events from lldb
 """
 import lldb
+import threading
 
 import lldbsuite.test.lldbutil as lldbutil
 
@@ -16,6 +17,37 @@ def setUp(self):
             self.broadcaster, lldb.SBDebugger.eBroadcastBitProgress
         )
 
+    def test_wait_attach_progress_reporting(self):
+        """Test that progress reports for wait attaching work as intended."""
+        self.build()
+        target = self.dbg.CreateTarget(None)
+
+        # Wait attach to a process, then check to see that a progress report was created
+        # and that its message is correct for waiting to attach to a process.
+        class AttachThread(threading.Thread):
+            def __init__(self, target):
+                threading.Thread.__init__(self)
+                self.target = target
+
+            def run(self):
+                self.target.AttachToProcessWithName(
+                    lldb.SBListener(), "a.out", True, lldb.SBError()
+                )
+
+        thread = AttachThread(target)
+        thread.start()
+
+        event = lldbutil.fetch_next_event(self, self.listener, self.broadcaster)
+        progress_data = lldb.SBDebugger.GetProgressDataFromEvent(event)
+        message = progress_data.GetValueForKey("message").GetStringValue(100)
+        self.assertGreater(len(message), 0)
+        self.assertEqual(message, "Waiting to attach to process")
+
+        # Interrupt the process attach to keep the test from stalling.
+        target.process.SendAsyncInterrupt()
+
+        thread.join()
+
     def test_dwarf_symbol_loading_progress_report(self):
         """Test that we are able to fetch dwarf symbol loading progress events"""
         self.build()

medismailben

Cool! LGTM with comment addressed.

medismailben · 2025-06-18T17:57:35Z

lldb/source/Target/Target.cpp

@@ -3546,6 +3546,8 @@ llvm::Expected<TraceSP> Target::GetTraceOrCreate() {
 }

 Status Target::Attach(ProcessAttachInfo &attach_info, Stream *stream) {
+  std::unique_ptr<Progress> attach_progress;
+  attach_progress = std::make_unique<Progress>("Waiting to attach to process");


This could just be a local variable, no need to make it a unique ptr

bulbazord · 2025-06-18T18:05:05Z

lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py

+        event = lldbutil.fetch_next_event(self, self.listener, self.broadcaster)
+        progress_data = lldb.SBDebugger.GetProgressDataFromEvent(event)
+        message = progress_data.GetValueForKey("message").GetStringValue(100)
+        self.assertGreater(len(message), 0)


nit: Checking for len(message) > 0 is redundant with the assertEqual below.

This commit adds a progress report when wait-attaching to a process as well as a test for this.

chelcassanova · 2025-06-18T20:07:29Z

@bulbazord @medismailben Updated this patch to add both of your suggestions

chelcassanova · 2025-06-18T22:38:24Z

This is breaking TestCreateAfterAttach.py on Ubuntu:

======================================================================
FAIL: test_create_after_attach_dwo (TestCreateAfterAttach.CreateAfterAttachTestCase.test_create_after_attach_dwo)
   Test thread creation after process attach.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1804, in test_method
    return attrvalue(self)
           ^^^^^^^^^^^^^^^
  File "/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/decorators.py", line 149, in wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/test/API/functionalities/thread/create_after_attach/TestCreateAfterAttach.py", line 36, in test_create_after_attach
    self.runCmd("process attach -p " + str(pid))
  File "/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1005, in runCmd
    self.assertTrue(self.res.Succeeded(), msg + output)
AssertionError: False is not true : Command 'process attach -p 1474309' did not return successfully
Error output:
error: attach failed: lost connection

on the buildbots for lldb-remote-linux-ubuntu, lldb-arm-ubuntu, lldb-aarch64-ubuntu, lldb-arm-ubuntu. Reverting for now.

DavidSpickett · 2025-06-20T10:33:06Z

Let me know if you need help to reproduce this.

Surprised the x86 bot isn't on that list. That might be a clue, I think the remote target for remote-linux-ubuntu might be an Arm board.

DavidSpickett · 2025-06-20T10:49:31Z

Though I cannot reproduce the failure, so it could be to do with the high load when run on the bots. Some process being deprioritised.

labath · 2025-06-20T12:24:50Z

Well.. IIUC, this test tries to attach to a process called "a.out" -- and hopes that the process doesn't appear. If you run the test suite in parallel, you'll have any number of processes like that going around all the time. Not only will this test fail, but it will mess up the state of a random other test.

I guess you could just wait for a sufficiently unique process name...

labath · 2025-06-20T12:25:16Z

lldb/test/API/functionalities/progress_reporting/TestProgressReporting.py

@@ -16,6 +17,36 @@ def setUp(self):
            self.broadcaster, lldb.SBDebugger.eBroadcastBitProgress
        )

+    def test_wait_attach_progress_reporting(self):
+        """Test that progress reports for wait attaching work as intended."""
+        self.build()


sound like you don't actually need to build anything here..

Trying this locally, removing the self.build() line causes the test to stall out so I'm going to keep it in.

chelcassanova · 2025-06-20T19:20:22Z

@labath

Well.. IIUC, this test tries to attach to a process called "a.out" -- and hopes that the process doesn't appear.

That's pretty much it, I just need to wait-attach to some process to trigger the progress report I'm expecting and then drop the process once we've triggered that report and check that it was emitted correctly.

If you run the test suite in parallel, you'll have any number of processes like that going around all the time. Not only will this test fail, but it will mess up the state of a random other test.

Great point, I hadn't considered this. I'm also a little surprised this seems to specifically affect ARM Ubuntu here 🤔 .

I guess you could just wait for a sufficiently unique process name...

This is probably the right way to go for this, I'll update the test to do this.

@DavidSpickett

Though I cannot reproduce the failure, so it could be to do with the high load when run on the bots. Some process being deprioritised.

With this and Pavel's comment about the generic process name it's probably how this test works on the bots. I'm not sure if there's a way to preview how this test would run on the bots before actually landing (outside of running it locally or having the GH pre-commit checks), but I'll try relanding and changing the process name to wait on for now since I don't think I have another way to ensure the test would be fine on the bots.

…ess" (llvm#144810) This relands commit e0933ab. The original commit was causing the test TestCreateAfterAttach.py to fail on ARM Ubuntu bots. It's possible that this could've been happening because the test for wait-attach progress reporting is waiting on a process named "a.out" which could be too generic as multiple other tests (when run in parallel on the bots) could also be using processes named "a.out". This commit changes the wait-attach progress report test to wait on a unique process name. Original PR description: This commit adds a progress report when wait-attaching to a process as well as a test for this. Original PR link: llvm#144768

chelcassanova · 2025-06-20T22:15:13Z

PR to reland this: #145111

…ess" (llvm#144810) This relands commit e0933ab. The original commit was causing the test TestCreateAfterAttach.py to fail on ARM Ubuntu bots. It's possible that this could've been happening because the test for wait-attach progress reporting is waiting on a process named "a.out" which could be too generic as multiple other tests (when run in parallel on the bots) could also be using processes named "a.out". This commit changes the wait-attach progress report test to wait on a unique process name. Original PR description: This commit adds a progress report when wait-attaching to a process as well as a test for this. Original PR link: llvm#144768

chelcassanova requested review from bulbazord and medismailben June 18, 2025 17:56

chelcassanova requested a review from JDevlieghere as a code owner June 18, 2025 17:56

llvmbot added the lldb label Jun 18, 2025

medismailben approved these changes Jun 18, 2025

View reviewed changes

bulbazord reviewed Jun 18, 2025

View reviewed changes

[lldb][target] Add progress report for wait-attaching to process

daae7e7

This commit adds a progress report when wait-attaching to a process as well as a test for this.

chelcassanova force-pushed the progress-report-for-wait-attach branch from 211e0e2 to daae7e7 Compare June 18, 2025 20:05

JDevlieghere approved these changes Jun 18, 2025

View reviewed changes

chelcassanova merged commit 03bdc0a into llvm:main Jun 18, 2025
7 checks passed

chelcassanova deleted the progress-report-for-wait-attach branch June 18, 2025 20:52

labath reviewed Jun 20, 2025

View reviewed changes

chelcassanova mentioned this pull request Jun 20, 2025

Reland "[lldb][target] Add progress report for wait-attaching to proc… #145111

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[lldb][target] Add progress report for wait-attaching to process #144768

[lldb][target] Add progress report for wait-attaching to process #144768

Uh oh!

chelcassanova commented Jun 18, 2025

Uh oh!

llvmbot commented Jun 18, 2025

Uh oh!

medismailben left a comment

Uh oh!

medismailben Jun 18, 2025 •

edited

Loading

Uh oh!

bulbazord Jun 18, 2025

Uh oh!

chelcassanova commented Jun 18, 2025

Uh oh!

Uh oh!

chelcassanova commented Jun 18, 2025

Uh oh!

DavidSpickett commented Jun 20, 2025

Uh oh!

DavidSpickett commented Jun 20, 2025

Uh oh!

labath commented Jun 20, 2025

Uh oh!

labath Jun 20, 2025

Uh oh!

chelcassanova Jun 20, 2025

Uh oh!

chelcassanova commented Jun 20, 2025

Uh oh!

chelcassanova commented Jun 20, 2025

Uh oh!

Uh oh!

[lldb][target] Add progress report for wait-attaching to process #144768

[lldb][target] Add progress report for wait-attaching to process #144768

Uh oh!

Conversation

chelcassanova commented Jun 18, 2025

Uh oh!

llvmbot commented Jun 18, 2025

Uh oh!

medismailben left a comment

Choose a reason for hiding this comment

Uh oh!

medismailben Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bulbazord Jun 18, 2025

Choose a reason for hiding this comment

Uh oh!

chelcassanova commented Jun 18, 2025

Uh oh!

Uh oh!

chelcassanova commented Jun 18, 2025

Uh oh!

DavidSpickett commented Jun 20, 2025

Uh oh!

DavidSpickett commented Jun 20, 2025

Uh oh!

labath commented Jun 20, 2025

Uh oh!

labath Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

chelcassanova Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

chelcassanova commented Jun 20, 2025

Uh oh!

chelcassanova commented Jun 20, 2025

Uh oh!

Uh oh!

medismailben Jun 18, 2025 •

edited

Loading