Adjust some id bit shifts to fit inside 32 bit integers

[lanza]

Summary

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.

[lanza]

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.

[clayborg]

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.

[lanza]

@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.

[clayborg]

In D53599#1288214, @lanza wrote:

@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?

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

[lanza]

adjust the shifts

[lanza]

update

[lanza]

formating

[clayborg]

tools/lldb-vscode/LLDBUtils.cpp

68–71

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 ↗

(On Diff #172843)

lldb::SBThread thread = process.GetThreadByIndexID(GetLLDBThreadIndexID(frame_id));

313 ↗

(On Diff #172843)

return thread.GetFrameAtIndex(GetLLDBFrameIndex(frame_id));

[lanza]

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.

[lanza]

add functions that handle the bit shifting

[lanza]

clang-format

[clayborg]

In D53599#1289691, @lanza wrote:

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.

[clayborg]

Just rename MakeVSCodeFrameID to MakeDAPFrameID, or rename all MakeDAPXXX functions you added to be MakeVSCodeXXX.

[lanza]

update

tools/lldb-vscode/JSONUtils.cpp

llvm::json::Value CreateBreakpoint(lldb::SBBreakpointLocation &bp_loc) {
   // Each breakpoint location is treated as a separate breakpoint for VS code.
   // They don't have the notion of a single breakpoint with multiple locations.
   llvm::json::Object object;
   if (!bp_loc.IsValid())
     return llvm::json::Value(std::move(object));
 
   object.try_emplace("verified", true);
   const auto bp_id = bp_loc.GetBreakpoint().GetID();
-  const auto vs_id = (int64_t)(((int64_t)bp_id << 32) | bp_loc.GetID());
+  const auto vs_id = (int64_t)((bp_id << 16) | bp_loc.GetID());
   object.try_emplace("id", vs_id);
   auto bp_addr = bp_loc.GetAddress();
   if (bp_addr.IsValid()) {
     auto line_entry = bp_addr.GetLineEntry();
     const auto line = line_entry.GetLine();
     if (line != UINT32_MAX)
       object.try_emplace("line", line);
     object.try_emplace("source", CreateSource(line_entry));

tools/lldb-vscode/LLDBUtils.cpp

bool ThreadHasStopReason(lldb::SBThread &thread) {
   case lldb::eStopReasonThreadExiting:
   case lldb::eStopReasonInvalid:
   case lldb::eStopReasonNone:
     break;
   }
   return false;
 }
 
 int64_t MakeVSCodeFrameID(lldb::SBFrame &frame) {
-  return (int64_t)frame.GetThread().GetIndexID() << 32 |
-         (int64_t)frame.GetFrameID();
+  return (int32_t)frame.GetThread().GetIndexID() << 16 |
+         (int32_t)frame.GetFrameID();
 }
 
 } // namespace lldb_vscode