🍒 Cherrypick upstream lldb-dap changes #10118
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…gressEvent.cpp; addressing llvm#108745 (llvm#114087) I hope it was missed unintentionally, pushing the same for the review. Ref: llvm#108745 --------- Co-authored-by: Santhosh Kumar Ellendula <[email protected]> Co-authored-by: Santhosh Kumar Ellendula <[email protected]> (cherry picked from commit 62ff85f)
This commit cleans up the includes in the `lldb-dap` subfolder. The main motivation was that I got annoyed by `clangd` always complaining about unused includes while working on lldb-dap. (cherry picked from commit 09c258e)
(cherry picked from commit 19c0a74)
…lvm#115208) Refactoring breakpoints to not use the `g_dap` reference. Instead, when a breakpoint is constructed it will be passed a DAP reference that it should use for its lifetime. This is part of a larger refactor to remove the global `g_dap` variable to allow us to create multiple DAP instances. --------- Co-authored-by: Pavel Labath <[email protected]> (cherry picked from commit b99d411)
…5551) This should fix https://lab.llvm.org/buildbot/#/builders/141/builds/3722 (cherry picked from commit 8d8d9f0)
Heterogenous lookups allow us to call find with StringRef, avoiding a temporary heap allocation of std::string. (cherry picked from commit c3c424d)
…ng in required arguments. (llvm#115561) This is part of a larger refactor to remove the global `g_dap` variable. (cherry picked from commit faaf2db)
…ead of the global instance. (llvm#115948) The refactor will unblock us for creating multiple DAP instances. (cherry picked from commit a6d299d)
The command was using the wrong publisher name ("llvm" rather than
"llvm-vs-code-extensions") resulting in the command complaining:
```
npm run vscode-uninstall
> [email protected] vscode-uninstall
> code --uninstall-extension llvm.lldb-dap
Extension 'llvm.lldb-dap' is not installed.
Make sure you use the full extension ID, including the publisher, e.g.: ms-dotnettools.csharp
```
(cherry picked from commit f715124)
I am a member of Microsoft vcpkg, due to there are new changes merged by microsoft/STL#5105, which revealed a conformance issue in `llvm`. It must add include `<chrono>` to fix this error. Compiler error with this STL change: ``` D:\b\llvm\src\org-18.1.6-e754cb1d0b.clean\lldb\tools\lldb-dap\ProgressEvent.h(79): error C2039: 'system_clock': is not a member of 'std::chrono' D:\b\llvm\src\org-18.1.6-e754cb1d0b.clean\lldb\tools\lldb-dap\ProgressEvent.cpp(134): error C3083: 'system_clock': the symbol to the left of a '::' must be a type D:\b\llvm\src\org-18.1.6-e754cb1d0b.clean\lldb\tools\lldb-dap\ProgressEvent.cpp(134): error C2039: 'now': is not a member of 'std::chrono' ``` (cherry picked from commit 72aefbb)
…lvm#123826) Recently I added SBProgress (llvm#119052), and during that original commit I tested if the progress event was sent over LLDB-DAP, and it was. However upon the suggestion of @JDevlieghere and @labath we added an external category (llvm#120171), which I did not test. This small patch wires up DAP to listen for external events by default, and adds the external category to the SBDebugger enumeration. (cherry picked from commit b9813ce)
…ject lifecycle. (llvm#122783) This moves the ownership of the threads that forward stdout/stderr to the DAP object itself to ensure that the threads are joined and that the forwarding is cleaned up when the DAP connection is disconnected. This is part of a larger refactor to allow lldb-dap to run in a listening mode and accept multiple connections. This reverts the previous revert and now that the underlying Windows issue was fixed by 3ea2b54. (cherry picked from commit 873426b)
I just noticed with these changes lldb-dap was using 200% of my CPU and root causing the issue it seems that lldb_private::Pipe::Read() (without a timeout) is using a timeout of `std::chrono::microseconds::zero()` which ends up setting the SelectHelper timeout to `now + 0` (see https://github.com/llvm/llvm-project/blob/7ceef1b1824073fcfd4724539f5942442da1a9c2/lldb/source/Host/posix/PipePosix.cpp#L314 and https://github.com/llvm/llvm-project/blob/7ceef1b1824073fcfd4724539f5942442da1a9c2/lldb/source/Utility/SelectHelper.cpp#L46) which causes SelectHelper to return immediately with a timeout error. As a result the `lldb_dap::OutputRedirector::RedirectTo()` to turn into a busy loop. Additionally, the 'read' call is waiting until the output buffer is full before returning, which prevents any partial output (see https://github.com/llvm/llvm-project/blob/7ceef1b1824073fcfd4724539f5942442da1a9c2/lldb/source/Host/posix/PipePosix.cpp#L325C9-L326C17). This is not the desired behavior for lldb-dap. Instead we want a write to the FD to result in a callback to send the DAP Output event, which mean we want partial output. To mitigate this, I'm reverting the reading operation to the previous behavior before 873426b but keeping the refactored structure and thread management. (cherry picked from commit adb9ef0)
A previous change is triggering a failure due to SOCKET not being defined in IOStream.h. Adjusting the Windows includes to correct the imports and using a more narrow import (winsock2.h vs windows.h). Also removed a stale comment. Tested this on an x86_64 wins 11 vm. This should fix https://lab.llvm.org/buildbot/#/builders/197/builds/1379 and https://lab.llvm.org/buildbot/#/builders/141/builds/5878 (cherry picked from commit 41910f7)
This commit adds support for column breakpoints to lldb-dap To do so, support for the `breakpointLocations` request was added. To find all available breakpoint positions, we iterate over the line table. The `setBreakpoints` request already forwarded the column correctly to `SBTarget::BreakpointCreateByLocation`. However, `SourceBreakpointMap` did not keep track of multiple breakpoints in the same line. To do so, the `SourceBreakpointMap` is now indexed by line+column instead of by line only. This was previously submitted as llvm#113787, but got reverted due to failures on ARM and macOS. This second attempt has less strict test case expectations. Also, I added a release note. (cherry picked from commit 8e6fa15)
(cherry picked from commit 1932ed0)
…out. (llvm#126833) If you have an lldb-dap log file you'll almost always see a final message like: ``` <-- Content-Length: 94 { "body": { "category": "stdout", "output": "\u0000\u0000" }, "event": "output", "seq": 0, "type": "event" } <-- Content-Length: 94 { "body": { "category": "stderr", "output": "\u0000\u0000" }, "event": "output", "seq": 0, "type": "event" } ``` The OutputRedirect is always writing the `"\0"` byte as a final stdout message during shutdown. Instead, I adjusted this to detect the sentinel value and break out of the read loop as if we detected EOF. --------- Co-authored-by: Pavel Labath <[email protected]> (cherry picked from commit c2e9677)
|
@swift-ci test |
|
@swift-ci test macOS |
adrian-prantl
approved these changes
Feb 26, 2025
|
@swift-ci test macOS |
|
Still failing because #10099 is missing. |
|
The automerger says it should be there. Let's try this again: @swift-ci test macOS |
Again... @swift-ci test macOS |
|
@swift-ci test macOS |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.