[lldb-dap] Fixing a type encoding issue with dap Stopped events.

[ashgti]

Previously the type of the breakpoint id in the Stopped event was a uint64_t, however thats the wrong type for a breakpoint id, which can cause encoding issues when internal breakpoints are hit.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: John Harrison (ashgti)

Changes

Previously the type of the breakpoint id in the Stopped event was a uint64_t, however thats the wrong type for a breakpoint id, which can cause encoding issues when internal breakpoints are hit.

---

Full diff: https://github.com/llvm/llvm-project/pull/72292.diff

1 Files Affected:

• (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+3-3)

diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 2023291729762f1..2b96c4f21aeb04d 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -953,9 +953,9 @@ llvm::json::Value CreateThreadStopped(lldb::SBThread &thread,
     } else {
       body.try_emplace("reason", "breakpoint");
       char desc_str[64];
-      uint64_t bp_id = thread.GetStopReasonDataAtIndex(0);
-      uint64_t bp_loc_id = thread.GetStopReasonDataAtIndex(1);
-      snprintf(desc_str, sizeof(desc_str), "breakpoint %" PRIu64 ".%" PRIu64,
+      break_id_t bp_id = thread.GetStopReasonDataAtIndex(0);
+      break_id_t bp_loc_id = thread.GetStopReasonDataAtIndex(1);
+      snprintf(desc_str, sizeof(desc_str), "breakpoint %" PRIo32 ".%" PRIo32,
                bp_id, bp_loc_id);
       body.try_emplace("hitBreakpointIds",
                        llvm::json::Array{llvm::json::Value(bp_id)});

[walter-erquinigo]

could you change this to use formatv? This kind of printing hurts my eyes.
Isn't PRIo32 octal?

[ashgti]

Switched to llvm::vformat instead.

[clayborg]

This is indeed octal which isn't what we want. You need to use PRIi32

[ashgti]

Switched to llvm::vformat.

[DavidGoldman]

This looks like an implicit cast from uint64_t to int32_t, how safe is this?

[clayborg]

We have an abstract API to access stop reason data via uint64_t SBThread::GetStopReasonDataAtIndex(...) which returns a uint64_t, but the break_id_t is defined as typedef int32_t break_id_t;. If we want to show the breakpoint ID correctly as a negative number for internal breakpoints, then this should be ok.

[ashgti]

These are implicitly cast here https://github.com/llvm/llvm-project/blob/e823136d43c40b0a9ba6930fd285768f1b46fcb6/lldb/source/API/SBThread.cpp#L251C35-L251C48 to uint64_t, this is converting it back to the internal type. This should be okay.

[clayborg]

LGTM. I didn't mean to accept the patch when it was using PRIo32....