fix off-by-one truncation detection in VADoRawLog and DoRawLog - #2125
fix off-by-one truncation detection in VADoRawLog and DoRawLog#2125nabhan06 wants to merge 4 commits into
Conversation
| kExpectedDeathOutput); | ||
| } | ||
|
|
||
| TEST(RawLoggingDeathTest, TruncationMarkerAtExactBufferBoundary) { |
There was a problem hiding this comment.
This test passes without the other changes. Is there a bug here that's reproducible at head?
There was a problem hiding this comment.
Yeah, the bug is real at head. With the zero-length prefix hook the whole 3000-byte buffer is free, so an exactly-3000-char message makes vsnprintf return 3000 == size. The old n > *size check reads that as a clean fit, so the buffer gets 2999 'x' plus the embedded NUL and is flushed with no (message truncated) and no trailing newline, which lets the next raw log line concatenate onto it. n >= *size is what reserves room for the marker.
The reason the old test passed without the fix is that it leaned on EXPECT_DEATH_IF_SUPPORTED, which no-ops when death tests aren't run in a given config, so the log statement never actually executed. I've reworked it to not depend on death tests at all: it dups STDERR_FILENO onto a pipe, logs at ERROR, and asserts on the captured bytes. That version fails at head (no marker, no newline) and passes with the fix. Verified both directions locally.
There was a problem hiding this comment.
This test still passes without the fix. It might have to do with your recent changes to avoid the prefix hook, but it makes me wonder how reproducible this if w/o using internal APIs
The death test passed at head where death tests are skipped, so it did not guard the fix. Capture STDERR_FILENO through a pipe with a zero-length prefix hook and assert the marker and trailing newline directly; this fails at head and passes with the fix on any platform where raw logging is supported.
treat n == size as truncation in VADoRawLog and DoRawLog: vsnprintf writes only size-1 chars at that boundary, so an exactly-buffer-filling raw log message was losing its last byte and being emitted without the
(message truncated)marker or trailing newline, letting a following line concatenate onto it.