8196882: VS2017 Hotspot Defined vsnprintf Function Causes C2084 Already Defined Compilation Error
Summary: Add os::vsnprintf and os::snprintf.
Reviewed-by: lfoltan, stuefe, mlarsson
--- a/src/hotspot/os/posix/os_posix.cpp Tue Feb 13 15:48:23 2018 +0100
+++ b/src/hotspot/os/posix/os_posix.cpp Tue Feb 27 18:17:57 2018 -0500
@@ -331,8 +331,15 @@
return aligned_base;
}
-int os::log_vsnprintf(char* buf, size_t len, const char* fmt, va_list args) {
- return vsnprintf(buf, len, fmt, args);
+int os::vsnprintf(char* buf, size_t len, const char* fmt, va_list args) {
+ // All supported POSIX platforms provide C99 semantics.
+ int result = ::vsnprintf(buf, len, fmt, args);
+ // If an encoding error occurred (result < 0) then it's not clear
+ // whether the buffer is NUL terminated, so ensure it is.
+ if ((result < 0) && (len > 0)) {
+ buf[len - 1] = '\0';
+ }
+ return result;
}
int os::get_fileno(FILE* fp) {
--- a/src/hotspot/os/windows/os_windows.cpp Tue Feb 13 15:48:23 2018 +0100
+++ b/src/hotspot/os/windows/os_windows.cpp Tue Feb 27 18:17:57 2018 -0500
@@ -1494,13 +1494,39 @@
if (nl != NULL) *nl = '\0';
}
-int os::log_vsnprintf(char* buf, size_t len, const char* fmt, va_list args) {
- int ret = vsnprintf(buf, len, fmt, args);
- // Get the correct buffer size if buf is too small
- if (ret < 0) {
- return _vscprintf(fmt, args);
- }
- return ret;
+int os::vsnprintf(char* buf, size_t len, const char* fmt, va_list args) {
+#if _MSC_VER >= 1900
+ // Starting with Visual Studio 2015, vsnprint is C99 compliant.
+ int result = ::vsnprintf(buf, len, fmt, args);
+ // If an encoding error occurred (result < 0) then it's not clear
+ // whether the buffer is NUL terminated, so ensure it is.
+ if ((result < 0) && (len > 0)) {
+ buf[len - 1] = '\0';
+ }
+ return result;
+#else
+ // Before Visual Studio 2015, vsnprintf is not C99 compliant, so use
+ // _vsnprintf, whose behavior seems to be *mostly* consistent across
+ // versions. However, when len == 0, avoid _vsnprintf too, and just
+ // go straight to _vscprintf. The output is going to be truncated in
+ // that case, except in the unusual case of empty output. More
+ // importantly, the documentation for various versions of Visual Studio
+ // are inconsistent about the behavior of _vsnprintf when len == 0,
+ // including it possibly being an error.
+ int result = -1;
+ if (len > 0) {
+ result = _vsnprintf(buf, len, fmt, args);
+ // If output (including NUL terminator) is truncated, the buffer
+ // won't be NUL terminated. Add the trailing NUL specified by C99.
+ if ((result < 0) || (result >= len)) {
+ buf[len - 1] = '\0';
+ }
+ }
+ if (result < 0) {
+ result = _vscprintf(fmt, args);
+ }
+ return result;
+#endif // _MSC_VER dispatch
}
static inline time_t get_mtime(const char* filename) {
--- a/src/hotspot/share/logging/logMessageBuffer.cpp Tue Feb 13 15:48:23 2018 +0100
+++ b/src/hotspot/share/logging/logMessageBuffer.cpp Tue Feb 27 18:17:57 2018 -0500
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -110,7 +110,7 @@
va_list copy;
va_copy(copy, args);
- written += (size_t)os::log_vsnprintf(current_buffer_position, remaining_buffer_length, fmt, copy) + 1;
+ written += (size_t)os::vsnprintf(current_buffer_position, remaining_buffer_length, fmt, copy) + 1;
va_end(copy);
if (written > _message_buffer_capacity - _message_buffer_size) {
assert(attempts == 0, "Second attempt should always have a sufficiently large buffer (resized to fit).");
--- a/src/hotspot/share/logging/logTagSet.cpp Tue Feb 13 15:48:23 2018 +0100
+++ b/src/hotspot/share/logging/logTagSet.cpp Tue Feb 27 18:17:57 2018 -0500
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -118,17 +118,17 @@
// Check that string fits in buffer; resize buffer if necessary
int ret;
if (prefix_len < vwrite_buffer_size) {
- ret = os::log_vsnprintf(buf + prefix_len, sizeof(buf) - prefix_len, fmt, args);
+ ret = os::vsnprintf(buf + prefix_len, sizeof(buf) - prefix_len, fmt, args);
} else {
// Buffer too small. Just call printf to find out the length for realloc below.
- ret = os::log_vsnprintf(buf, sizeof(buf), fmt, args);
+ ret = os::vsnprintf(buf, sizeof(buf), fmt, args);
}
assert(ret >= 0, "Log message buffer issue");
if ((size_t)ret >= sizeof(buf)) {
size_t newbuf_len = prefix_len + ret + 1;
char* newbuf = NEW_C_HEAP_ARRAY(char, newbuf_len, mtLogging);
prefix_len = _write_prefix(newbuf, newbuf_len);
- ret = os::log_vsnprintf(newbuf + prefix_len, newbuf_len - prefix_len, fmt, saved_args);
+ ret = os::vsnprintf(newbuf + prefix_len, newbuf_len - prefix_len, fmt, saved_args);
assert(ret >= 0, "Log message buffer issue");
log(level, newbuf);
FREE_C_HEAP_ARRAY(char, newbuf);
--- a/src/hotspot/share/oops/generateOopMap.cpp Tue Feb 13 15:48:23 2018 +0100
+++ b/src/hotspot/share/oops/generateOopMap.cpp Tue Feb 27 18:17:57 2018 -0500
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1997, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -23,7 +23,6 @@
*/
#include "precompiled.hpp"
-#include "jvm.h"
#include "interpreter/bytecodeStream.hpp"
#include "logging/log.hpp"
#include "logging/logStream.hpp"
@@ -33,6 +32,7 @@
#include "oops/symbol.hpp"
#include "runtime/handles.inline.hpp"
#include "runtime/java.hpp"
+#include "runtime/os.hpp"
#include "runtime/relocator.hpp"
#include "runtime/timerTrace.hpp"
#include "utilities/bitMap.inline.hpp"
@@ -2151,10 +2151,10 @@
void GenerateOopMap::error_work(const char *format, va_list ap) {
_got_error = true;
char msg_buffer[512];
- vsnprintf(msg_buffer, sizeof(msg_buffer), format, ap);
+ os::vsnprintf(msg_buffer, sizeof(msg_buffer), format, ap);
// Append method name
char msg_buffer2[512];
- jio_snprintf(msg_buffer2, sizeof(msg_buffer2), "%s in method %s", msg_buffer, method()->name()->as_C_string());
+ os::snprintf(msg_buffer2, sizeof(msg_buffer2), "%s in method %s", msg_buffer, method()->name()->as_C_string());
if (Thread::current()->can_call_java()) {
_exception = Exceptions::new_exception(Thread::current(),
vmSymbols::java_lang_LinkageError(), msg_buffer2);
--- a/src/hotspot/share/prims/jvm.cpp Tue Feb 13 15:48:23 2018 +0100
+++ b/src/hotspot/share/prims/jvm.cpp Tue Feb 27 18:17:57 2018 -0500
@@ -2670,23 +2670,19 @@
ATTRIBUTE_PRINTF(3, 0)
int jio_vsnprintf(char *str, size_t count, const char *fmt, va_list args) {
- // see bug 4399518, 4417214
+ // Reject count values that are negative signed values converted to
+ // unsigned; see bug 4399518, 4417214
if ((intptr_t)count <= 0) return -1;
- int result = vsnprintf(str, count, fmt, args);
- // Note: on truncation vsnprintf(3) on Unix returns numbers of
- // characters which would have been written had the buffer been large
- // enough; on Windows, it returns -1. We handle both cases here and
- // always return -1, and perform null termination.
- if ((result > 0 && (size_t)result >= count) || result == -1) {
- str[count - 1] = '\0';
+ int result = os::vsnprintf(str, count, fmt, args);
+ if (result > 0 && (size_t)result >= count) {
result = -1;
}
return result;
}
-ATTRIBUTE_PRINTF(3, 0)
+ATTRIBUTE_PRINTF(3, 4)
int jio_snprintf(char *str, size_t count, const char *fmt, ...) {
va_list args;
int len;
@@ -2696,7 +2692,7 @@
return len;
}
-ATTRIBUTE_PRINTF(2,3)
+ATTRIBUTE_PRINTF(2, 3)
int jio_fprintf(FILE* f, const char *fmt, ...) {
int len;
va_list args;
--- a/src/hotspot/share/runtime/os.cpp Tue Feb 13 15:48:23 2018 +0100
+++ b/src/hotspot/share/runtime/os.cpp Tue Feb 27 18:17:57 2018 -0500
@@ -105,6 +105,14 @@
#endif
}
+int os::snprintf(char* buf, size_t len, const char* fmt, ...) {
+ va_list args;
+ va_start(args, fmt);
+ int result = os::vsnprintf(buf, len, fmt, args);
+ va_end(args);
+ return result;
+}
+
// Fill in buffer with current local time as an ISO-8601 string.
// E.g., yyyy-mm-ddThh:mm:ss-zzzz.
// Returns buffer, or NULL if it failed.
--- a/src/hotspot/share/runtime/os.hpp Tue Feb 13 15:48:23 2018 +0100
+++ b/src/hotspot/share/runtime/os.hpp Tue Feb 27 18:17:57 2018 -0500
@@ -639,8 +639,10 @@
static void *find_agent_function(AgentLibrary *agent_lib, bool check_lib,
const char *syms[], size_t syms_len);
- // Write to stream
- static int log_vsnprintf(char* buf, size_t len, const char* fmt, va_list args) ATTRIBUTE_PRINTF(3, 0);
+ // Provide C99 compliant versions of these functions, since some versions
+ // of some platforms don't.
+ static int vsnprintf(char* buf, size_t len, const char* fmt, va_list args) ATTRIBUTE_PRINTF(3, 0);
+ static int snprintf(char* buf, size_t len, const char* fmt, ...) ATTRIBUTE_PRINTF(3, 4);
// Get host name in buffer provided
static bool get_host_name(char* buf, size_t buflen);
--- a/src/hotspot/share/utilities/exceptions.cpp Tue Feb 13 15:48:23 2018 +0100
+++ b/src/hotspot/share/utilities/exceptions.cpp Tue Feb 27 18:17:57 2018 -0500
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1998, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2018, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -33,6 +33,7 @@
#include "runtime/init.hpp"
#include "runtime/java.hpp"
#include "runtime/javaCalls.hpp"
+#include "runtime/os.hpp"
#include "runtime/thread.inline.hpp"
#include "runtime/threadCritical.hpp"
#include "utilities/events.hpp"
@@ -239,8 +240,7 @@
va_list ap;
va_start(ap, format);
char msg[max_msg_size];
- vsnprintf(msg, max_msg_size, format, ap);
- msg[max_msg_size-1] = '\0';
+ os::vsnprintf(msg, max_msg_size, format, ap);
va_end(ap);
_throw_msg(thread, file, line, h_name, msg);
}
--- a/src/hotspot/share/utilities/globalDefinitions_visCPP.hpp Tue Feb 13 15:48:23 2018 +0100
+++ b/src/hotspot/share/utilities/globalDefinitions_visCPP.hpp Tue Feb 27 18:17:57 2018 -0500
@@ -147,14 +147,6 @@
#pragma warning( disable : 4996 ) // unsafe string functions. Same as define _CRT_SECURE_NO_WARNINGS/_CRT_SECURE_NO_DEPRICATE
#endif
-inline int vsnprintf(char* buf, size_t count, const char* fmt, va_list argptr) {
- // If number of characters written == count, Windows doesn't write a
- // terminating NULL, so we do it ourselves.
- int ret = _vsnprintf(buf, count, fmt, argptr);
- if (count > 0) buf[count-1] = '\0';
- return ret;
-}
-
// Portability macros
#define PRAGMA_INTERFACE
#define PRAGMA_IMPLEMENTATION
--- a/src/hotspot/share/utilities/ostream.cpp Tue Feb 13 15:48:23 2018 +0100
+++ b/src/hotspot/share/utilities/ostream.cpp Tue Feb 27 18:17:57 2018 -0500
@@ -96,19 +96,14 @@
result_len = strlen(result);
if (add_cr && result_len >= buflen) result_len = buflen-1; // truncate
} else {
- // Handle truncation:
- // posix: upon truncation, vsnprintf returns number of bytes which
- // would have been written (excluding terminating zero) had the buffer
- // been large enough
- // windows: upon truncation, vsnprintf returns -1
- const int written = vsnprintf(buffer, buflen, format, ap);
+ int written = os::vsnprintf(buffer, buflen, format, ap);
+ assert(written >= 0, "vsnprintf encoding error");
result = buffer;
- if (written < (int) buflen && written >= 0) {
+ if ((size_t)written < buflen) {
result_len = written;
} else {
DEBUG_ONLY(warning("increase O_BUFLEN in ostream.hpp -- output truncated");)
result_len = buflen - 1;
- buffer[result_len] = 0;
}
}
if (add_cr) {
--- a/test/hotspot/gtest/runtime/test_os.cpp Tue Feb 13 15:48:23 2018 +0100
+++ b/test/hotspot/gtest/runtime/test_os.cpp Tue Feb 27 18:17:57 2018 -0500
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -22,7 +22,9 @@
*/
#include "precompiled.hpp"
+#include "memory/resourceArea.hpp"
#include "runtime/os.hpp"
+#include "utilities/ostream.hpp"
#include "unittest.hpp"
static size_t small_page_size() {
@@ -150,3 +152,118 @@
os::page_size_for_region_aligned(region_size, 0); // should assert
}
#endif
+
+//////////////////////////////////////////////////////////////////////////////
+// Test os::vsnprintf and friends.
+
+static void check_snprintf_result(int expected, size_t limit, int actual, bool expect_count) {
+ if (expect_count || ((size_t)expected < limit)) {
+ ASSERT_EQ(expected, actual);
+ } else {
+ ASSERT_GT(0, actual);
+ }
+}
+
+// PrintFn is expected to be int (*)(char*, size_t, const char*, ...).
+// But jio_snprintf is a C-linkage function with that signature, which
+// has a different type on some platforms (like Solaris).
+template<typename PrintFn>
+static void test_snprintf(PrintFn pf, bool expect_count) {
+ const char expected[] = "abcdefghijklmnopqrstuvwxyz";
+ const int expected_len = sizeof(expected) - 1;
+ const size_t padding_size = 10;
+ char buffer[2 * (sizeof(expected) + padding_size)];
+ char check_buffer[sizeof(buffer)];
+ const char check_char = '1'; // Something not in expected.
+ memset(check_buffer, check_char, sizeof(check_buffer));
+ const size_t sizes_to_test[] = {
+ sizeof(buffer) - padding_size, // Fits, with plenty of space to spare.
+ sizeof(buffer)/2, // Fits, with space to spare.
+ sizeof(buffer)/4, // Doesn't fit.
+ sizeof(expected) + padding_size + 1, // Fits, with a little room to spare
+ sizeof(expected) + padding_size, // Fits exactly.
+ sizeof(expected) + padding_size - 1, // Doesn't quite fit.
+ 2, // One char + terminating NUL.
+ 1, // Only space for terminating NUL.
+ 0 }; // No space at all.
+ for (unsigned i = 0; i < ARRAY_SIZE(sizes_to_test); ++i) {
+ memset(buffer, check_char, sizeof(buffer)); // To catch stray writes.
+ size_t test_size = sizes_to_test[i];
+ ResourceMark rm;
+ stringStream s;
+ s.print("test_size: " SIZE_FORMAT, test_size);
+ SCOPED_TRACE(s.as_string());
+ size_t prefix_size = padding_size;
+ guarantee(test_size <= (sizeof(buffer) - prefix_size), "invariant");
+ size_t write_size = MIN2(sizeof(expected), test_size);
+ size_t suffix_size = sizeof(buffer) - prefix_size - write_size;
+ char* write_start = buffer + prefix_size;
+ char* write_end = write_start + write_size;
+
+ int result = pf(write_start, test_size, "%s", expected);
+
+ check_snprintf_result(expected_len, test_size, result, expect_count);
+
+ // Verify expected output.
+ if (test_size > 0) {
+ ASSERT_EQ(0, strncmp(write_start, expected, write_size - 1));
+ // Verify terminating NUL of output.
+ ASSERT_EQ('\0', write_start[write_size - 1]);
+ } else {
+ guarantee(test_size == 0, "invariant");
+ guarantee(write_size == 0, "invariant");
+ guarantee(prefix_size + suffix_size == sizeof(buffer), "invariant");
+ guarantee(write_start == write_end, "invariant");
+ }
+
+ // Verify no scribbling on prefix or suffix.
+ ASSERT_EQ(0, strncmp(buffer, check_buffer, prefix_size));
+ ASSERT_EQ(0, strncmp(write_end, check_buffer, suffix_size));
+ }
+
+ // Special case of 0-length buffer with empty (except for terminator) output.
+ check_snprintf_result(0, 0, pf(NULL, 0, "%s", ""), expect_count);
+ check_snprintf_result(0, 0, pf(NULL, 0, ""), expect_count);
+}
+
+// This is probably equivalent to os::snprintf, but we're being
+// explicit about what we're testing here.
+static int vsnprintf_wrapper(char* buf, size_t len, const char* fmt, ...) {
+ va_list args;
+ va_start(args, fmt);
+ int result = os::vsnprintf(buf, len, fmt, args);
+ va_end(args);
+ return result;
+}
+
+TEST(os, vsnprintf) {
+ test_snprintf(vsnprintf_wrapper, true);
+}
+
+TEST(os, snprintf) {
+ test_snprintf(os::snprintf, true);
+}
+
+// These are declared in jvm.h; test here, with related functions.
+extern "C" {
+int jio_vsnprintf(char*, size_t, const char*, va_list);
+int jio_snprintf(char*, size_t, const char*, ...);
+}
+
+// This is probably equivalent to jio_snprintf, but we're being
+// explicit about what we're testing here.
+static int jio_vsnprintf_wrapper(char* buf, size_t len, const char* fmt, ...) {
+ va_list args;
+ va_start(args, fmt);
+ int result = jio_vsnprintf(buf, len, fmt, args);
+ va_end(args);
+ return result;
+}
+
+TEST(os, jio_vsnprintf) {
+ test_snprintf(jio_vsnprintf_wrapper, false);
+}
+
+TEST(os, jio_snprintf) {
+ test_snprintf(jio_snprintf, false);
+}