[all-commits] [llvm/llvm-project] 79e804: [lldb] Improve isolation between Process plugins a...

[Felipe de Azevedo Piovezan via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 79e804b478aafdb9f543c66c1cc9cca6908d6b8f
      https://github.com/llvm/llvm-project/commit/79e804b478aafdb9f543c66c1cc9cca6908d6b8f
  Author: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
  Date:   2025-02-03 (Mon, 03 Feb 2025)

  Changed paths:
    M lldb/source/Breakpoint/BreakpointSite.cpp
    M lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
    A lldb/unittests/OperatingSystem/CMakeLists.txt
    A lldb/unittests/OperatingSystem/OperatingSystemPlugin.h
    A lldb/unittests/OperatingSystem/TestThreadSpecificBreakpoints.cpp

  Log Message:
  -----------
  [lldb] Improve isolation between Process plugins and OS plugins (#125302)

Generally speaking, process plugins (e.g. ProcessGDBRemote) should not
be aware of OS plugin threads. However, ProcessGDBRemote attempts to
check for the existence of OS threads when calculating stop info. When
OS threads are present, it sets the stop info directly on the OS plugin
thread and leaves the ThreadGDBRemote without a StopInfo.

This is problematic for a few reasons:

1. No other process plugins do this, as they shouldn't. They should set
the stop info for their own process threads, and let the abstractions
built on top propagate StopInfos.

2. This conflicts with the expectations of ThreadMemory, which checks
for the backing threads's info, and then attempts to propagate it (in
the future, it should probably ask the plugin itself too...). We see
this happening in the code below. The `if` condition will not trigger,
because `backing_stop_info_sp` will be null (remember, ProcessGDB remote
is ignoring its own threads), and then this method returns false.

```
bool ThreadMemory::CalculateStopInfo() {
...
  lldb::StopInfoSP backing_stop_info_sp(
      m_backing_thread_sp->GetPrivateStopInfo());
  if (backing_stop_info_sp &&
      backing_stop_info_sp->IsValidForOperatingSystemThread(*this)) {
    backing_stop_info_sp->SetThread(shared_from_this());
```

```
Thread::GetPrivateStopInfo
...
        if (!CalculateStopInfo())
          SetStopInfo(StopInfoSP());
```

To solve this, we change ProcessGDB remote so that it does the
principled thing: it now only sets the stop info of its own threads.
This change by itself breaks the tests TestPythonOSPlugin.py and
TestOSPluginStepping.py and probably explains why ProcessGDB had
originally "violated" this isolation of layers.

To make this work, BreakpointSites must be aware of BackingThreads when
answering the question: "Is this breakpoint valid for this thread?".
Why? Breakpoints are created on top of the OS threads (that's what the
user sees), but breakpoints are hit by process threads. In the presence
of OS threads, a TID-specific breakpoint is valid for a process thread
if it is backing an OS thread with that TID.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications