From 421b2c2bc1aab887522f010a7baabfc140c7927e Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Tue, 27 Oct 2026 23:26:10 +0100 Subject: [PATCH 1/2] fix(profiler): encode debug unwind event sizes as varints --- ddprof-lib/src/main/cpp/flightRecorder.cpp | 24 +++-- ddprof-lib/src/main/cpp/flightRecorder.h | 4 + .../src/test/cpp/unwindFailureEncoding_ut.cpp | 88 +++++++++++++++++++ 3 files changed, 109 insertions(+), 7 deletions(-) create mode 100644 ddprof-lib/src/test/cpp/unwindFailureEncoding_ut.cpp diff --git a/ddprof-lib/src/main/cpp/flightRecorder.cpp b/ddprof-lib/src/main/cpp/flightRecorder.cpp index 6a1964b740..cb1acc0a0e 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.cpp +++ b/ddprof-lib/src/main/cpp/flightRecorder.cpp @@ -2314,19 +2314,29 @@ void Recording::writeCounters(Buffer *buf) { } } +#ifdef DEBUG +void Recording::writeUnwindFailureEvent(Buffer *buf, u64 ticks, const char *kind, + const char *name, u64 count) { + // A 255-byte name plus the other fields needs a multi-byte event size. + int start = buf->skip(MAX_VAR32_LENGTH); + buf->putVar64(T_UNWIND_FAILURE); + buf->putVar64(ticks); + buf->putUtf8(kind); + buf->putUtf8(name); + buf->putVar64(count); + buf->putVar32(start, buf->offset() - start); +} +#endif + void Recording::writeUnwindFailures(Buffer *buf) { #ifdef DEBUG static UnwindFailures failures; UnwindStats::collectAndReset(failures); failures.forEach([&](UnwindFailureKind kind, const char *name, u64 count) { - int start = buf->skip(1); - buf->putVar64(T_UNWIND_FAILURE); - buf->putVar64(_start_ticks); - buf->putUtf8((kind & UNWIND_FAILURE_STUB) ? "stub" : "other"); - buf->putUtf8(name); - buf->putVar64(count); - writeEventSizePrefix(buf, start); + writeUnwindFailureEvent(buf, _start_ticks, + (kind & UNWIND_FAILURE_STUB) ? "stub" : "other", + name, count); flushIfNeeded(buf); }); #endif // DEBUG diff --git a/ddprof-lib/src/main/cpp/flightRecorder.h b/ddprof-lib/src/main/cpp/flightRecorder.h index c393472823..8b8de10a95 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.h +++ b/ddprof-lib/src/main/cpp/flightRecorder.h @@ -280,6 +280,10 @@ class Recording { // the encoded count can never diverge from what actually gets serialized. static size_t countSerializableChildren( const std::vector &children, int depth); +#ifdef DEBUG + static void writeUnwindFailureEvent(Buffer *buf, u64 ticks, const char *kind, + const char *name, u64 count); +#endif // Encodes the stack-pool record header. A Java entry root can establish // completeness only when the walker has not already reported missing frames. diff --git a/ddprof-lib/src/test/cpp/unwindFailureEncoding_ut.cpp b/ddprof-lib/src/test/cpp/unwindFailureEncoding_ut.cpp new file mode 100644 index 0000000000..f18e01986a --- /dev/null +++ b/ddprof-lib/src/test/cpp/unwindFailureEncoding_ut.cpp @@ -0,0 +1,88 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include +#include +#include "flightRecorder.h" +#include "unwindStats.h" + +#ifdef DEBUG +class RecordingTestAccessor { +public: + static void writeFailure(Buffer* buffer, const char* kind, const char* name, u64 count) { + Recording::writeUnwindFailureEvent(buffer, 123456789, kind, name, count); + } +}; + +// JFR varints use seven bits per byte, with all eight bits used in byte nine. +static u64 readVar(const Buffer& buffer, int& offset) { + u64 result = 0; + for (int i = 0; i < 9; i++) { + if (offset >= buffer.offset()) throw std::runtime_error("Truncated varint"); + unsigned char byte = buffer.data()[offset++]; + result |= (u64)(i == 8 ? byte : byte & 0x7f) << (i * 7); + if (i == 8 || (byte & 0x80) == 0) return result; + } + return result; +} + +static std::string readString(const Buffer& buffer, int& offset) { + if (offset >= buffer.offset() || buffer.data()[offset++] != 3) { + throw std::runtime_error("Expected UTF-8 string"); + } + u64 length = readVar(buffer, offset); + if (length > (u64)(buffer.offset() - offset)) throw std::runtime_error("Truncated string"); + std::string value(buffer.data() + offset, (size_t)length); + offset += (int)length; + return value; +} + +class UnwindFailureEncodingTest : public ::testing::TestWithParam {}; + +TEST_P(UnwindFailureEncodingTest, PreservesEventBoundariesAndDiagnosticFields) { + size_t length = GetParam(); + // Keep an assertion in the old writer from aborting the whole test binary. + ASSERT_EXIT({ + Buffer buffer; + buffer.skip(13); + std::string name(length, 'x'); + UnwindFailures failures; + failures.record(UNWIND_FAILURE_STUB, name.c_str()); + failures.forEach([&](UnwindFailureKind, const char* stored_name, u64 count) { + RecordingTestAccessor::writeFailure(&buffer, "stub", stored_name, count); + }); + int next = buffer.offset(); + RecordingTestAccessor::writeFailure(&buffer, "other", "control", ~0ULL); + + int offset = 13; + u64 event_size = readVar(buffer, offset); + EXPECT_EQ((u64)(next - 13), event_size); + EXPECT_EQ((u64)T_UNWIND_FAILURE, readVar(buffer, offset)); + EXPECT_EQ(123456789u, readVar(buffer, offset)); + EXPECT_EQ("stub", readString(buffer, offset)); + EXPECT_EQ(name.substr(0, MAX_NAME_LENGTH - 1), readString(buffer, offset)); + EXPECT_EQ(1u, readVar(buffer, offset)); + EXPECT_EQ(next, offset); + + EXPECT_EQ((u64)(buffer.offset() - next), readVar(buffer, offset)); + EXPECT_EQ((u64)T_UNWIND_FAILURE, readVar(buffer, offset)); + EXPECT_EQ(123456789u, readVar(buffer, offset)); + EXPECT_EQ("other", readString(buffer, offset)); + EXPECT_EQ("control", readString(buffer, offset)); + EXPECT_EQ(~0ULL, readVar(buffer, offset)); + EXPECT_EQ(buffer.offset(), offset); + _exit(::testing::Test::HasFailure() ? 1 : 0); + }, ::testing::ExitedWithCode(0), ""); +} + +INSTANTIATE_TEST_SUITE_P(NameLengths, UnwindFailureEncodingTest, + ::testing::Values(32, 110, 127, 230, 255, 512)); +#else +TEST(UnwindFailureEncodingTest, RequiresDebugInstrumentation) { + GTEST_SKIP() << "Unwind-failure diagnostics require DEBUG instrumentation"; +} +#endif From d1b347f655126090921d5839f00bc1c7ea54f04b Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Thu, 29 Oct 2026 14:47:06 +0100 Subject: [PATCH 2/2] docs(jfr): document debug unwind-failure event encoding --- doc/architecture/JfrIntegration.md | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/doc/architecture/JfrIntegration.md b/doc/architecture/JfrIntegration.md index c44844b2c6..7d16aa9cc0 100644 --- a/doc/architecture/JfrIntegration.md +++ b/doc/architecture/JfrIntegration.md @@ -100,9 +100,9 @@ an atomic add to `_bytes_written` and a reset (`flightRecorder.cpp:1360-1366`). There is no background writer: the thread that fills a buffer past the threshold, which can be a signal handler, writes it out. -Fixed-size events call `flushIfNeeded()` after writing. Events whose size -depends on input reserve room first by flushing against a reduced limit, for -example `RECORDING_BUFFER_LIMIT - length` for settings +Fixed-size events call `flushIfNeeded()` after writing. Larger variable-size +events reserve room first by flushing against a reduced limit, for example +`RECORDING_BUFFER_LIMIT - length` for settings (`flightRecorder.cpp:1584`) or the worst-case chain size for `datadog.ReferenceChain` (`flightRecorder.cpp:2558-2600`, `flightRecorder.h:48-81`). @@ -115,12 +115,21 @@ in the order the metadata declares them. Integers are LEB128 - **Sample events** (`recordExecutionSample()`, `recordAllocation()`, ...) reserve a 1-byte size with `skip(1)` and patch it with `writeEventSizePrefix()`, which asserts the event is shorter than - `MAX_JFR_EVENT_SIZE` (256) (`flightRecorder.cpp:2355-2376`, - `flightRecorder.h:43`). + `MAX_JFR_EVENT_SIZE` (256) (`flightRecorder.cpp:2370-2374`, + `flightRecorder.h:44`). - **Variable-size events** (settings, native libraries, OS/JVM info, - `profiler.Log`, the metadata and constant-pool events) reserve 5 bytes and - patch them with the fixed-width form `putVar32(offset, v)`, which always - writes 5 LEB128 bytes (`buffers.h:182-189`, `flightRecorder.cpp:1582-1597`). + `profiler.Log`, debug unwind failures, the metadata and constant-pool events) + reserve 5 bytes and patch them with the fixed-width form + `putVar32(offset, v)`, which always writes 5 LEB128 bytes + (`buffers.h:182-189`, `flightRecorder.cpp:1582-1597`). + +`datadog.UnwindFailure` events are emitted only in `DEBUG` builds. +`Recording::writeUnwindFailureEvent()` reserves `MAX_VAR32_LENGTH` (5) bytes +for the size prefix, followed by the event type ID, start ticks, kind, name +and count. The size includes the prefix itself. `UnwindFailures` truncates +stored names to `MAX_NAME_LENGTH - 1` (255) bytes, so an event can require a +multi-byte size. `writeUnwindFailures()` calls `flushIfNeeded()` after each +bounded event (`flightRecorder.cpp:2317-2343`, `unwindStats.h:76-85`). Stack-sampled events refer to their stack by the 64-bit trace ID returned by `CallTraceStorage::put()`, to their thread by the OS TID, and to thread state @@ -128,9 +137,9 @@ and execution mode by small enum IDs. All of these are constant-pool references, resolved when the chunk is finished (see [Constant Pools](#constant-pools)). Most sample events end with the tracing context: span ID, local-root span ID, and one varint per configured context -attribute (`writeCurrentContext()`, `flightRecorder.cpp:2339-2353`). Liveness +attribute (`writeCurrentContext()`, `flightRecorder.cpp:2354-2368`). Liveness events write the context snapshot taken when the allocation was tracked -(`writeContextSnapshot()`, `flightRecorder.cpp:2330-2337`, +(`writeContextSnapshot()`, `flightRecorder.cpp:2345-2352`, `livenessTracker.cpp:2411`). ### Writers outside `recordEvent()`