8256828: ostream::print_cr() truncates buffer in copy-through case

Reviewed-by: stuefe, matsaave
This commit is contained in:
David Holmes 2024-06-06 00:15:43 +00:00
parent 60ea17e848
commit ca9390755b
3 changed files with 317 additions and 19 deletions

View File

@ -81,45 +81,67 @@ bool outputStream::update_position(const char* s, size_t len) {
}
// Execute a vsprintf, using the given buffer if necessary.
// Return a pointer to the formatted string.
// Return a pointer to the formatted string. Optimise for
// strings without format specifiers, or only "%s". See
// comments in the header file for more details.
const char* outputStream::do_vsnprintf(char* buffer, size_t buflen,
const char* format, va_list ap,
bool add_cr,
size_t& result_len) {
assert(buflen >= 2, "buffer too small");
const char* result;
if (add_cr) buflen--;
const char* result; // The string to return. May not be the buffer.
size_t required_len = 0; // The length of buffer needed to avoid truncation
// (excluding space for the nul terminator).
if (add_cr) { // Ensure space for CR even if truncation occurs.
buflen--;
}
if (!strchr(format, '%')) {
// constant format string
result = format;
result_len = strlen(result);
if (add_cr && result_len >= buflen) result_len = buflen-1; // truncate
} else if (format[0] == '%' && format[1] == 's' && format[2] == '\0') {
if (add_cr && result_len >= buflen) { // truncate
required_len = result_len + 1;
result_len = buflen - 1;
}
} else if (strncmp(format, "%s", 3) == 0) { //(format[0] == '%' && format[1] == 's' && format[2] == '\0') {
// trivial copy-through format string
result = va_arg(ap, const char*);
result_len = strlen(result);
if (add_cr && result_len >= buflen) result_len = buflen-1; // truncate
if (add_cr && result_len >= buflen) { // truncate
required_len = result_len + 1;
result_len = buflen - 1;
}
} else {
int required_len = os::vsnprintf(buffer, buflen, format, ap);
assert(required_len >= 0, "vsnprintf encoding error");
int required_buffer_len = os::vsnprintf(buffer, buflen, format, ap);
assert(required_buffer_len >= 0, "vsnprintf encoding error");
result = buffer;
if ((size_t)required_len < buflen) {
required_len = required_buffer_len;
if (required_len < buflen) {
result_len = required_len;
} else {
DEBUG_ONLY(warning("outputStream::do_vsnprintf output truncated -- buffer length is %d bytes but %d bytes are needed.",
add_cr ? (int)buflen + 1 : (int)buflen, add_cr ? required_len + 2 : required_len + 1);)
} else { // truncation
result_len = buflen - 1;
}
}
if (add_cr) {
if (result != buffer) {
if (result != buffer) { // Need to copy to add CR
memcpy(buffer, result, result_len);
result = buffer;
} else {
required_len++;
}
buffer[result_len++] = '\n';
buffer[result_len] = 0;
}
#ifdef ASSERT
if (required_len > result_len) {
warning("outputStream::do_vsnprintf output truncated -- buffer length is " SIZE_FORMAT
" bytes but " SIZE_FORMAT " bytes are needed.",
add_cr ? buflen + 1 : buflen, required_len + 1);
}
#endif
return result;
}

View File

@ -41,7 +41,8 @@ DEBUG_ONLY(class ResourceMark;)
// we may use jio_printf:
// jio_fprintf(defaultStream::output_stream(), "Message");
// This allows for redirection via -XX:+DisplayVMOutputToStdout and
// -XX:+DisplayVMOutputToStderr
// -XX:+DisplayVMOutputToStderr.
class outputStream : public CHeapObjBase {
private:
NONCOPYABLE(outputStream);
@ -56,6 +57,23 @@ class outputStream : public CHeapObjBase {
// Returns whether a newline was seen or not
bool update_position(const char* s, size_t len);
// Processes the given format string and the supplied arguments
// to produce a formatted string in the supplied buffer. Returns
// the formatted string (in the buffer). If the formatted string
// would be longer than the buffer, it is truncated.
//
// If the format string is a plain string (no format specifiers)
// or is exactly "%s" to print a supplied argument string, then
// the buffer is ignored, and we return the string directly.
// However, if `add_cr` is true then we have to copy the string
// into the buffer, which risks truncation if the string is too long.
//
// The `result_len` reference is always set to the length of the returned string.
//
// If add_cr is true then the cr will always be placed in the buffer (buffer minimum size is 2).
//
// In a debug build, if truncation occurs a VM warning is issued.
static const char* do_vsnprintf(char* buffer, size_t buflen,
const char* format, va_list ap,
bool add_cr,
@ -69,6 +87,8 @@ class outputStream : public CHeapObjBase {
void do_vsnprintf_and_write(const char* format, va_list ap, bool add_cr) ATTRIBUTE_PRINTF(2, 0);
public:
class TestSupport; // Unit test support
// creation
outputStream();
outputStream(bool has_time_stamps);
@ -91,14 +111,18 @@ class outputStream : public CHeapObjBase {
void set_position(int pos) { _position = pos; }
// printing
// Note that (v)print_cr forces the use of internal buffering to allow
// appending of the "cr". This can lead to truncation if the buffer is
// too small.
void print(const char* format, ...) ATTRIBUTE_PRINTF(2, 3);
void print_cr(const char* format, ...) ATTRIBUTE_PRINTF(2, 3);
void vprint(const char *format, va_list argptr) ATTRIBUTE_PRINTF(2, 0);
void vprint_cr(const char* format, va_list argptr) ATTRIBUTE_PRINTF(2, 0);
void print_raw(const char* str) { write(str, strlen(str)); }
void print_raw(const char* str, size_t len) { write(str, len); }
void print_raw_cr(const char* str) { write(str, strlen(str)); cr(); }
void print_raw_cr(const char* str, size_t len){ write(str, len); cr(); }
void print_raw(const char* str) { write(str, strlen(str)); }
void print_raw(const char* str, size_t len) { write(str, len); }
void print_raw_cr(const char* str) { write(str, strlen(str)); cr(); }
void print_raw_cr(const char* str, size_t len){ write(str, len); cr(); }
void print_data(void* data, size_t len, bool with_ascii, bool rel_addr=true);
void put(char ch);
void sp(int count = 1);

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019 SAP SE. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
@ -27,6 +27,7 @@
#include "memory/resourceArea.hpp"
#include "runtime/os.hpp"
#include "utilities/globalDefinitions.hpp"
#include "utilities/defaultStream.hpp"
#include "utilities/ostream.hpp"
#include "unittest.hpp"
@ -123,6 +124,257 @@ TEST_VM(ostream, bufferedStream_dynamic_large) {
*/
// Test helper for do_vsnprintf
class outputStream::TestSupport : AllStatic {
// Shared constants and variables for all subtests.
static const size_t buflen = 11;
static char buffer[buflen];
static const size_t max_len = buflen - 1;
static size_t result_len;
static const char* result;
static void reset() {
result_len = 0;
result = nullptr;
buffer[0] = '\0';
}
static const char* test(char* buf, size_t len, bool add_cr,
size_t& rlen, const char* format, ...) {
va_list ap;
va_start(ap, format);
const char* res = do_vsnprintf(buf, len, format, ap, add_cr, rlen);
va_end(ap);
return res;
}
public:
// Case set 1: constant string with no format specifiers
static void test_constant_string() {
reset();
// Case 1-1: no cr, no truncation, excess capacity
{
const char* str = "012345678";
size_t initial_len = strlen(str);
ASSERT_TRUE(initial_len < max_len);
result = test(buffer, buflen, false, result_len, str);
ASSERT_EQ(result, str);
ASSERT_EQ(strlen(result), result_len);
}
reset();
// Case 1-2: no cr, no truncation, exact capacity
{
const char* str = "0123456789";
size_t initial_len = strlen(str);
ASSERT_EQ(initial_len, max_len);
result = test(buffer, buflen, false, result_len, str);
ASSERT_EQ(result, str);
ASSERT_EQ(strlen(result), result_len);
}
reset();
// Case 1-3: no cr, no truncation, exceeds capacity
{
const char* str = "0123456789A";
size_t initial_len = strlen(str);
ASSERT_TRUE(initial_len > max_len);
result = test(buffer, buflen, false, result_len, str);
ASSERT_EQ(result, str);
ASSERT_EQ(strlen(result), result_len);
ASSERT_EQ(result_len, initial_len);
}
reset();
// Case 1-4: add cr, no truncation, excess capacity
{
const char* str = "01234567";
size_t initial_len = strlen(str);
ASSERT_TRUE(initial_len < max_len);
result = test(buffer, buflen, true, result_len, str);
ASSERT_EQ(result, buffer);
ASSERT_EQ(strlen(result), result_len);
ASSERT_EQ(result_len, initial_len + 1);
ASSERT_TRUE(result_len <= max_len);
}
reset();
// Case 1-5: add cr, no truncation, exact capacity
{
const char* str = "012345678";
size_t initial_len = strlen(str);
ASSERT_TRUE(initial_len < max_len);
result = test(buffer, buflen, true, result_len, str);
ASSERT_EQ(result, buffer);
ASSERT_EQ(strlen(result), result_len);
ASSERT_EQ(result_len, initial_len + 1);
ASSERT_TRUE(result_len <= max_len);
}
reset();
// Case 1-6: add cr, truncation
{
const char* str = "0123456789";
size_t initial_len = strlen(str);
ASSERT_EQ(initial_len, max_len);
::printf("Truncation warning expected: requires %d\n", (int)(initial_len + 1 + 1));
result = test(buffer, buflen, true, result_len, str);
ASSERT_EQ(result, buffer);
ASSERT_EQ(strlen(result), result_len);
ASSERT_EQ(result_len, initial_len);
ASSERT_TRUE(result_len <= max_len);
}
}
// Case set 2: "%s" string
static void test_percent_s_string() {
reset();
// Case 2-1: no cr, no truncation, excess capacity
{
const char* str = "012345678";
size_t initial_len = strlen(str);
ASSERT_TRUE(initial_len < max_len);
result = test(buffer, buflen, false, result_len, "%s", str);
ASSERT_EQ(result, str);
ASSERT_EQ(strlen(result), result_len);
}
reset();
// Case 2-2: no cr, no truncation, exact capacity
{
const char* str = "0123456789";
size_t initial_len = strlen(str);
ASSERT_EQ(initial_len, max_len);
result = test(buffer, buflen, false, result_len, "%s", str);
ASSERT_EQ(result, str);
ASSERT_EQ(strlen(result), result_len);
}
reset();
// Case 2-3: no cr, no truncation, exceeds capacity
{
const char* str = "0123456789A";
size_t initial_len = strlen(str);
ASSERT_TRUE(initial_len > max_len);
result = test(buffer, buflen, false, result_len, "%s", str);
ASSERT_EQ(result, str);
ASSERT_EQ(strlen(result), result_len);
ASSERT_EQ(result_len, initial_len);
}
reset();
// Case 2-4: add cr, no truncation, excess capacity
{
const char* str = "01234567";
size_t initial_len = strlen(str);
ASSERT_TRUE(initial_len < max_len);
result = test(buffer, buflen, true, result_len, "%s", str);
ASSERT_EQ(result, buffer);
ASSERT_EQ(strlen(result), result_len);
ASSERT_EQ(result_len, initial_len + 1);
ASSERT_TRUE(result_len <= max_len);
}
reset();
// Case 2-5: add cr, no truncation, exact capacity
{
const char* str = "012345678";
size_t initial_len = strlen(str);
ASSERT_TRUE(initial_len < max_len);
result = test(buffer, buflen, true, result_len, "%s", str);
ASSERT_EQ(result, buffer);
ASSERT_EQ(strlen(result), result_len);
ASSERT_EQ(result_len, initial_len + 1);
ASSERT_TRUE(result_len <= max_len);
}
reset();
// Case 2-6: add cr, truncation
{
const char* str = "0123456789";
size_t initial_len = strlen(str);
ASSERT_EQ(initial_len, max_len);
::printf("Truncation warning expected: requires %d\n", (int)(initial_len + 1 + 1));
result = test(buffer, buflen, true, result_len, "%s", str);
ASSERT_EQ(result, buffer);
ASSERT_EQ(strlen(result), result_len);
ASSERT_EQ(result_len, initial_len);
ASSERT_TRUE(result_len <= max_len);
}
}
// Case set 3: " %s" string - the space means we avoid the pass-through optimization and use vsnprintf
static void test_general_string() {
reset();
// Case 3-1: no cr, no truncation, excess capacity
{
const char* str = "01234567";
size_t initial_len = strlen(str) + 1;
ASSERT_TRUE(initial_len < max_len);
result = test(buffer, buflen, false, result_len, " %s", str);
ASSERT_EQ(result, buffer);
ASSERT_EQ(strlen(result), result_len);
}
reset();
// Case 3-2: no cr, no truncation, exact capacity
{
const char* str = "012345678";
size_t initial_len = strlen(str) + 1;
ASSERT_EQ(initial_len, max_len);
result = test(buffer, buflen, false, result_len, " %s", str);
ASSERT_EQ(result, buffer);
ASSERT_EQ(strlen(result), result_len);
}
reset();
// Case 3-3: no cr, truncation
{
const char* str = "0123456789";
size_t initial_len = strlen(str) + 1;
ASSERT_TRUE(initial_len > max_len);
::printf("Truncation warning expected: requires %d\n", (int)(initial_len + 1));
result = test(buffer, buflen, false, result_len, " %s", str);
ASSERT_EQ(result, buffer);
ASSERT_EQ(strlen(result), result_len);
}
reset();
// Case 3-4: add cr, no truncation, excess capacity
{
const char* str = "0123456";
size_t initial_len = strlen(str) + 1;
ASSERT_TRUE(initial_len < max_len);
result = test(buffer, buflen, true, result_len, " %s", str);
ASSERT_EQ(result, buffer);
ASSERT_EQ(strlen(result), result_len);
ASSERT_EQ(result_len, initial_len + 1);
ASSERT_TRUE(result_len <= max_len);
}
reset();
// Case 3-5: add cr, no truncation, exact capacity
{
const char* str = "01234567";
size_t initial_len = strlen(str) + 1;
ASSERT_TRUE(initial_len < max_len);
result = test(buffer, buflen, true, result_len, " %s", str);
ASSERT_EQ(result, buffer);
ASSERT_EQ(strlen(result), result_len);
ASSERT_EQ(result_len, initial_len + 1);
ASSERT_TRUE(result_len <= max_len);
}
reset();
// Case 3-6: add cr, truncation
{
const char* str = "012345678";
size_t initial_len = strlen(str) + 1;
ASSERT_EQ(initial_len, max_len);
::printf("Truncation warning expected: requires %d\n", (int)(initial_len + 1 + 1));
result = test(buffer, buflen, true, result_len, " %s", str);
ASSERT_EQ(result, buffer);
ASSERT_EQ(strlen(result), result_len);
ASSERT_EQ(result_len, initial_len);
ASSERT_TRUE(result_len <= max_len);
}
}
};
const size_t outputStream::TestSupport::max_len;
char outputStream::TestSupport::buffer[outputStream::TestSupport::buflen];
size_t outputStream::TestSupport::result_len = 0;
const char* outputStream::TestSupport::result = nullptr;
TEST_VM(ostream, do_vsnprintf_buffering) {
outputStream::TestSupport::test_constant_string();
outputStream::TestSupport::test_percent_s_string();
outputStream::TestSupport::test_general_string();
}