The DAP on vscode uses a JavaScript number for identifiers while the
Visual Studio version uses a C# Int for identifiers. lldb-vscode is
bit shifting identifiers 32 bits to form a unique ID. Change this to
a 16 bit shift to allow the data to fit into 32 bits.
Details
Diff Detail
- Build Status
Buildable 24633 Build 24632: arc lint + arc unit
Event Timeline
There should be quite a few more things switched to 32bit values to account for this. I've added that to my tasks. Putting this up just because this breaks functionality on Visual Studio.
This won't work when we have many frames. If you get a stack trace that blows the stack, you can easily have 300K stack frames. Thread index ID can get quite high too. There are also no lifetimes on the frame IDs we use, so we never get told to free a frame ID when the IDE is done with one. So we either have to switch to using a map that can map between frame ID and lldb::SBFrame or let the map just grow and grow, or something else. This isn't sufficient.
@clayborg We can either store a dap-frame-to-lldb-frame map as well as a lldb-frame-to-dap-frame map or we can truncate the thread count down to a much smaller count of bits. I think the former is an ugly idea to just let a pair of gigantic maps grow endlessly over the lifetime of a program. E.G. for debugging some game that could be running for a long duration. For the latter, I haven't seen a thousand thread program before and if you're using a thousand threads you probably have much bigger problems than your GUI debugger. How about a split with 10 or so bits for the thread ID and the remaining 22 bits for the frame ID?
As per the breakpoint situation, I think we could do something similar with the breakpoint ID taking 10 bits and the BreakpointLocation ID taking 22 bits since a user likely isn't going to provide 1000 chosen breakpoints anyways.
2^19 = 524288
2^13 = 8192
Not many OSs can get a half million stack frames without running out of stack. So how about 19 bits for frame ID and 13 for the thread ID?
As per the breakpoint situation, I think we could do something similar with the breakpoint ID taking 10 bits and the BreakpointLocation ID taking 22 bits since a user likely isn't going to provide 1000 chosen breakpoints anyways.
Sounds good
tools/lldb-vscode/LLDBUtils.cpp | ||
---|---|---|
68–70 | It would be nice to make all encoding and decoding into functions here. I would suggest also making: #define THREAD_INDEX_SHIFT 19 int64_t MakeVSCodeFrameID(lldb::SBFrame &frame) { return (int64_t)(frame.GetThread().GetIndexID() << THREAD_INDEX_SHIFT | frame.GetFrameID()); } uint32_t GetLLDBThreadIndexID(int64_t frame_id) { return frame_id >> THREAD_INDEX_SHIFT; } uint32_t GetLLDBFrameIndex(int64_t frame_id) { return frame_id & ((1u << THREAD_INDEX_SHIFT) - 1); } | |
tools/lldb-vscode/VSCode.cpp | ||
311 | lldb::SBThread thread = process.GetThreadByIndexID(GetLLDBThreadIndexID(frame_id)); | |
313 | return thread.GetFrameAtIndex(GetLLDBFrameIndex(frame_id)); |
Am I missing some line of code or does lldb-vscode not ever map from the DAP breakpoint ID back to the LLDB BreakpointLocation and Breakpoint IDs? I wrote the equivalent functions but there doesn't seem to be anywhere that maps in the inverse direction.
Yeah, it is not ever mapped back. I believe the ID is sent back later, but it isn't used right now. When you have breakpoints set in a source file by file and line, they send one breakpoint request with multiple lines, and when a breakpoint is removed, they just send another packet with all source file and line breakpoints and you are expected to just "figure it out". Kind of lame, but it is what we have.
tools/lldb-vscode/LLDBUtils.cpp | ||
---|---|---|
68 | "static constexpr" if we aren't going to use a #define statement | |
68–69 | We should rename this to MakeDAPFrameID to match other functions. |
Just rename MakeVSCodeFrameID to MakeDAPFrameID, or rename all MakeDAPXXX functions you added to be MakeVSCodeXXX.
"static constexpr" if we aren't going to use a #define statement