[lldb-dap] Breakpoints may not be correctly marked 'verified' after configurationDone is called

[ashgti]

I've seen TestDAP_breakpointEvents.py flake a number of times for me locally on macOS. I haven't found a root cause yet but some examples include:

https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/21926/testReport/junit/lldb-api/tools_lldb-dap_breakpoint-events/TestDAP_breakpointEvents_py/

The failures always look like:

FAIL: test_breakpoint_events (TestDAP_breakpointEvents.TestDAP_breakpointEvents)
   This test sets a breakpoint in a shared library and runs and stops
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/lldb/test/API/tools/lldb-dap/breakpoint-events/TestDAP_breakpointEvents.py", line 90, in test_breakpoint_events
    self.assertEqual(len(self.dap_server.breakpoint_events), 1)
AssertionError: 0 != 1
Config=arm64-/Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/lldb-build/bin/clang
----------------------------------------------------------------------

[llvmbot]

@llvm/issue-subscribers-lldb-dap

Author: John Harrison (ashgti)

I've seen [TestDAP_breakpointEvents.py](https://github.com/llvm/llvm-project/blob/main/lldb/test/API/tools/lldb-dap/breakpoint-events/TestDAP_breakpointEvents.py) flake a number of times for me locally on macOS. I haven't found a root cause yet but some examples include:

https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/21926/testReport/junit/lldb-api/tools_lldb-dap_breakpoint-events/TestDAP_breakpointEvents_py/

The failures always look like:

FAIL: test_breakpoint_events (TestDAP_breakpointEvents.TestDAP_breakpointEvents)
   This test sets a breakpoint in a shared library and runs and stops
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/llvm-project/lldb/test/API/tools/lldb-dap/breakpoint-events/TestDAP_breakpointEvents.py", line 90, in test_breakpoint_events
    self.assertEqual(len(self.dap_server.breakpoint_events), 1)
AssertionError: 0 != 1
Config=arm64-/Users/ec2-user/jenkins/workspace/llvm.org/as-lldb-cmake/lldb-build/bin/clang
----------------------------------------------------------------------

[ashgti]

Looking into this, I added some logging to the test and I think this is a race condition between creating the breakpoint and setting the breakpoint label.

When this fails, I'll see:

 [external] SBBreakpoint lldb::SBTarget::BreakpointCreateByLocation(const SBFileSpec &, uint32_t, uint32_t, lldb::addr_t, SBFileSpecList &) (0x16fdda950, 0x16fdd9798, 6, 0, 0, 0x16fdd97b0)
 [internal] lldb::SBBreakpoint::SBBreakpoint() (0x16fdd97a0)
 [internal] uint32_t lldb::SBFileSpecList::GetSize() const (0x16fdd97b0)
 0x0000000118875898 Broadcaster("lldb.target")::BroadcastEvent (event_sp = 0x600002e612a8 Event: broadcaster = 0x1180773f8 (lldb.target), type = 0x00000001 (breakpoint-changed), data = {bkpt: 2 type: breakpoint added}, unique=false) hijack = 0x0000000000000000
 0x1188142f0 Listener('lldb.Debugger')::AddEvent (event_sp = {0x600002e612a8})
 Target::AddBreakpoint (internal = no) => break_id = 2: file = '/Users/harjohn/Projects/llvm-project/lldb/test/API/tools/lldb-dap/breakpoint-events/main.cpp', line = 6, exact_match = 0

...

stdin/stdout.event_handler       this = 0x00000001188142F0, timeout = 1000000 us for lldb.Debugger
 GDBRemoteCommunicationClient::SendGDBStoppointTypePacket() add at addr = 0x1049d7f90
 0x0000000118875898 Broadcaster("lldb.target")::BroadcastEvent (event_sp = 0x600002e50018 Event: broadcaster = 0x1180773f8 (lldb.target), type = 0x00000001 (breakpoint-changed), data = {bkpt: 2 type: locations resolved}, unique=false) hijack = 0x0000000000000000
 0x1188142f0 Listener('lldb.Debugger')::AddEvent (event_sp = {0x600002e50018})
stdin/stdout.event_handler       0x1188142f0 'lldb.Debugger' Listener::FindNextEventInternal(broadcaster=0x0, event_type_mask=0x00000000, remove=1) event 0x600002e50018

...

stdin/stdout.event_handler       [external] static bool lldb::SBBreakpoint::EventIsBreakpointEvent(const lldb::SBEvent &) (0x170092e70)
stdin/stdout.event_handler       [external] static BreakpointEventType lldb::SBBreakpoint::GetBreakpointEventTypeFromEvent(const SBEvent &) (0x170092e70)
 Added location (skipped prologue: no): 2.1: 
  module = /Users/harjohn/Projects/lldb-build/lldb-test-build.noindex/tools/lldb-dap/breakpoint-events/TestDAP_breakpointEvents.test_breakpoint_events/a.out
  compile unit = main.cpp
  function = main
  location = /Users/harjohn/Projects/llvm-project/lldb/test/API/tools/lldb-dap/breakpoint-events/main.cpp:6:3
  address = 0x00000001049d7f90
  resolved = true
  hardware = false
  hit count = 0   

...

 [external] bool lldb::SBBreakpoint::AddName(const char *) (0x1181e9408, "dap")
stdin/stdout.event_handler       [external] lldb::SBBreakpoint::SBBreakpoint(const SBBreakpoint &) (0x170092e00, 0x170092db0)
 [internal] SBError lldb::SBBreakpoint::AddNameWithErrorHandling(const char *) (0x1181e9408, "dap")
stdin/stdout.event_handler       [external] bool lldb::SBBreakpoint::MatchesName(const char *) (0x170092e00, "dap")
 [internal] lldb::SBError::SBError() (0x16fdd96b0)

Which shows that we created the breakpoint, triggered the breakpoint added and breakpoint location resolved then finally added the breakpoint name.

Here is how we create the breakpoint:

1. dap.target.BreakpointCreateByLocation

   llvm-project/lldb/tools/lldb-dap/SourceBreakpoint.cpp

   Line 37 in 1a626e6

   bp = dap.target.BreakpointCreateByLocation(source_path.str().c_str(), line,

2. Then call into the

   llvm-project/lldb/tools/lldb-dap/Breakpoint.cpp

   Line 77 in 1a626e6

   bp.AddName(GetBreakpointLabel());

   to set the name

I'm not sure of the best approach to fix this at the moment. We'd need to make the creation + addName more atomic, but they're already pretty close in terms of code flow. There is 1 if statement between them checking if we should set the log message. I feel like there is a more basic issue here in terms of flow. I tried seeing if setting SBDebugger::SetAsync would affect this, but I still see the issue.

@JDevlieghere Any ideas on managing the sequencing of these events? I wonder if we could re-evaluate the use of the "dap" name on the breakpoint.

[ashgti]

Closing this item in favor of #131242