diff --git a/src/os/posix/vm/os_posix.cpp b/src/os/posix/vm/os_posix.cpp index d1bf84f6a88f95f3c5286ee0df902e9d46645c5a..c301898a2db79b5630236db89d07a4dbbc947d1f 100644 --- a/src/os/posix/vm/os_posix.cpp +++ b/src/os/posix/vm/os_posix.cpp @@ -1,5 +1,5 @@ /* -* Copyright (c) 1999, 2015, Oracle and/or its affiliates. All rights reserved. +* Copyright (c) 1999, 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 @@ -166,6 +166,16 @@ char* os::reserve_memory_aligned(size_t size, size_t alignment) { return aligned_base; } +int os::vsnprintf(char* buf, size_t len, const char* fmt, va_list args) { + 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; +} + void os::Posix::print_load_average(outputStream* st) { st->print("load average:"); double loadavg[3]; diff --git a/src/os/windows/vm/os_windows.cpp b/src/os/windows/vm/os_windows.cpp index f30423903dbe8b1e7c5355785b0ef72bdfefae55..4f550c90f76c22862f810c3f1bb222dd2d18b796 100644 --- a/src/os/windows/vm/os_windows.cpp +++ b/src/os/windows/vm/os_windows.cpp @@ -1837,6 +1837,42 @@ void os::print_siginfo(outputStream *st, void *siginfo) { st->cr(); } + +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 >= (int) len)) { + buf[len - 1] = '\0'; + } + } + if (result < 0) { + result = _vscprintf(fmt, args); + } + return result; +#endif // _MSC_VER dispatch +} + void os::print_signal_handlers(outputStream* st, char* buf, size_t buflen) { // do nothing } diff --git a/src/share/vm/gc_implementation/g1/g1GCPhaseTimes.cpp b/src/share/vm/gc_implementation/g1/g1GCPhaseTimes.cpp index af0bdb90e480d24ae29f3feff7e5d78293800d0e..d43976dd679902ab9357825e04a5a7ecefdc85c3 100644 --- a/src/share/vm/gc_implementation/g1/g1GCPhaseTimes.cpp +++ b/src/share/vm/gc_implementation/g1/g1GCPhaseTimes.cpp @@ -41,13 +41,13 @@ private: int _cur; void vappend(const char* format, va_list ap) ATTRIBUTE_PRINTF(2, 0) { - int res = vsnprintf(&_buffer[_cur], BUFFER_LEN - _cur, format, ap); - if (res != -1) { - _cur += res; - } else { + int res = os::vsnprintf(&_buffer[_cur], BUFFER_LEN - _cur, format, ap); + if (res > BUFFER_LEN) { DEBUG_ONLY(warning("buffer too small in LineBuffer");) _buffer[BUFFER_LEN -1] = 0; _cur = BUFFER_LEN; // vsnprintf above should not add to _buffer if we are called again + } else if (res != -1) { + _cur += res; } } diff --git a/src/share/vm/oops/generateOopMap.cpp b/src/share/vm/oops/generateOopMap.cpp index 2207e8c814e7c387897096d920a48edfc94b7a4e..bb951f8b00e4497b00694edbe4160fe5d5315116 100644 --- a/src/share/vm/oops/generateOopMap.cpp +++ b/src/share/vm/oops/generateOopMap.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2013, 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 @@ -29,6 +29,7 @@ #include "oops/symbol.hpp" #include "runtime/handles.inline.hpp" #include "runtime/java.hpp" +#include "runtime/os.hpp" #include "runtime/relocator.hpp" #include "utilities/bitMap.inline.hpp" #include "prims/methodHandles.hpp" @@ -2133,10 +2134,10 @@ void GenerateOopMap::compute_map(TRAPS) { 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()); _exception = Exceptions::new_exception(Thread::current(), vmSymbols::java_lang_LinkageError(), msg_buffer2); } @@ -2154,7 +2155,7 @@ void GenerateOopMap::verify_error(const char *format, ...) { _got_error = true; // Append method name char msg_buffer2[512]; - jio_snprintf(msg_buffer2, sizeof(msg_buffer2), "%s in method %s", msg, + os::snprintf(msg_buffer2, sizeof(msg_buffer2), "%s in method %s", msg, method()->name()->as_C_string()); _exception = Exceptions::new_exception(Thread::current(), vmSymbols::java_lang_LinkageError(), msg_buffer2); diff --git a/src/share/vm/prims/jni.cpp b/src/share/vm/prims/jni.cpp index 6a6ab9366ed0eb77ff18592742c968ee280f3dd6..51b453ee531b835f4de93fb62e5da595472715c6 100644 --- a/src/share/vm/prims/jni.cpp +++ b/src/share/vm/prims/jni.cpp @@ -33,6 +33,7 @@ #include "classfile/vmSymbols.hpp" #include "interpreter/linkResolver.hpp" #include "utilities/macros.hpp" +#include "utilities/ostream.hpp" #if INCLUDE_ALL_GCS #include "gc_implementation/g1/g1SATBCardTableModRefBS.hpp" #endif // INCLUDE_ALL_GCS @@ -5127,6 +5128,7 @@ void execute_internal_vm_tests() { run_unit_test(GuardedMemory::test_guarded_memory()); run_unit_test(AltHashing::test_alt_hash()); run_unit_test(test_loggc_filename()); + run_unit_test(test_snprintf()); run_unit_test(TestNewSize_test()); run_unit_test(TestKlass_test()); run_unit_test(Test_linked_list()); diff --git a/src/share/vm/prims/jvm.cpp b/src/share/vm/prims/jvm.cpp index 07c36de811b38f1d77184da62934c91419753a54..f346153d8f2dcd7e1f0e89d711fb2089d579c6db 100644 --- a/src/share/vm/prims/jvm.cpp +++ b/src/share/vm/prims/jvm.cpp @@ -2915,16 +2915,12 @@ extern "C" { 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 number 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; } diff --git a/src/share/vm/runtime/os.cpp b/src/share/vm/runtime/os.cpp index 17b5ddc2cb0258891f3f7ee9e09bd73de31f903c..8cf2ed9946d6df3abb6c187228c847a52da31d92 100644 --- a/src/share/vm/runtime/os.cpp +++ b/src/share/vm/runtime/os.cpp @@ -108,6 +108,14 @@ static time_t get_timezone(const struct tm* time_struct) { #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. diff --git a/src/share/vm/runtime/os.hpp b/src/share/vm/runtime/os.hpp index 97787f22a9d8dc67ae25c88bbbf4ddacd96e5db4..92553ab474f65f1231bfb0374d56c4f5c1312a90 100644 --- a/src/share/vm/runtime/os.hpp +++ b/src/share/vm/runtime/os.hpp @@ -616,6 +616,9 @@ class os: AllStatic { static void *find_agent_function(AgentLibrary *agent_lib, bool check_lib, const char *syms[], size_t syms_len); + 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); + // Print out system information; they are called by fatal error handler. // Output format may be different on different platforms. static void print_os_info(outputStream* st); diff --git a/src/share/vm/utilities/exceptions.cpp b/src/share/vm/utilities/exceptions.cpp index 188e09730c3340201be4ad7a3ccfc7aa4cf56507..db4a17029f1e5e8c4b4f43a2297580d34265d1db 100644 --- a/src/share/vm/utilities/exceptions.cpp +++ b/src/share/vm/utilities/exceptions.cpp @@ -30,6 +30,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" @@ -246,8 +247,7 @@ void Exceptions::fthrow(Thread* thread, const char* file, int line, Symbol* h_na 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); } diff --git a/src/share/vm/utilities/globalDefinitions_visCPP.hpp b/src/share/vm/utilities/globalDefinitions_visCPP.hpp index 46e8f9e8e937104c12ff927bb3bab32db5959f8b..21adbdf3803c08118647bf22904aed7b2c2de754 100644 --- a/src/share/vm/utilities/globalDefinitions_visCPP.hpp +++ b/src/share/vm/utilities/globalDefinitions_visCPP.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2013, 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 @@ -187,14 +187,6 @@ const jlong max_jlong = CONST64(0x7fffffffffffffff); #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 diff --git a/src/share/vm/utilities/ostream.cpp b/src/share/vm/utilities/ostream.cpp index 44ce683de636a914239ae473713c39b8b1a7398e..1b00f829a718b475a6d9dfcb4ef702ca3ef01709 100644 --- a/src/share/vm/utilities/ostream.cpp +++ b/src/share/vm/utilities/ostream.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2014, 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 @@ -27,6 +27,7 @@ #include "gc_implementation/shared/gcId.hpp" #include "oops/oop.inline.hpp" #include "runtime/arguments.hpp" +#include "runtime/os.hpp" #include "utilities/defaultStream.hpp" #include "utilities/ostream.hpp" #include "utilities/top.hpp" @@ -89,6 +90,8 @@ 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--; if (!strchr(format, '%')) { @@ -101,18 +104,20 @@ const char* outputStream::do_vsnprintf(char* buffer, size_t buflen, result = va_arg(ap, const char*); result_len = strlen(result); if (add_cr && result_len >= buflen) result_len = buflen-1; // truncate - } else if (vsnprintf(buffer, buflen, format, ap) >= 0) { - result = buffer; - result_len = strlen(result); } else { - DEBUG_ONLY(warning("increase O_BUFLEN in ostream.hpp -- output truncated");) + int written = os::vsnprintf(buffer, buflen, format, ap); + assert(written >= 0, "vsnprintf encoding error"); result = buffer; - result_len = buflen - 1; - buffer[result_len] = 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; + } } if (add_cr) { if (result != buffer) { - strncpy(buffer, result, buflen); + memcpy(buffer, result, result_len); result = buffer; } buffer[result_len++] = '\n'; @@ -578,6 +583,117 @@ void test_loggc_filename() { assert(o_result == NULL, err_msg("Too long file name after pid expansion should return NULL, but got '%s'", o_result)); } } + +////////////////////////////////////////////////////////////////////////////// +// Test os::vsnprintf and friends. + +void check_snprintf_result(int expected, size_t limit, int actual, bool expect_count) { + if (expect_count || ((size_t)expected < limit)) { + assert(expected == actual, "snprintf result not expected value"); + } else { + // Make this check more permissive for jdk8u, don't assert that actual == 0. + // e.g. jio_vsnprintf_wrapper and jio_snprintf return -1 when expected >= limit + if (expected >= (int) limit) { + assert(actual == -1, "snprintf result should be -1 for expected >= limit"); + } else { + assert(actual > 0, "snprintf result should be >0 for expected < limit"); + } + } +} + +// 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 +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); + 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(0 == strncmp(write_start, expected, write_size - 1), "strncmp failure"); + // Verify terminating NUL of output. + assert('\0' == write_start[write_size - 1], "null terminator failure"); + } 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(0 == strncmp(buffer, check_buffer, prefix_size), "prefix scribble"); + assert(0 == strncmp(write_end, check_buffer, suffix_size), "suffix scribble"); + } + + // 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; +} + +// 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; +} + +void test_snprintf() { + test_snprintf(vsnprintf_wrapper, true); + test_snprintf(os::snprintf, true); + test_snprintf(jio_vsnprintf_wrapper, false); // jio_vsnprintf returns -1 on error including exceeding buffer size + test_snprintf(jio_snprintf, false); // jio_snprintf calls jio_vsnprintf +} #endif // PRODUCT fileStream::fileStream(const char* file_name) { diff --git a/src/share/vm/utilities/ostream.hpp b/src/share/vm/utilities/ostream.hpp index 356961989331db593cb4aedac12ce5e01e31ea95..b4e7378cf52aa899a009f7e6d117376301954991 100644 --- a/src/share/vm/utilities/ostream.hpp +++ b/src/share/vm/utilities/ostream.hpp @@ -258,6 +258,7 @@ class gcLogFileStream : public fileStream { #ifndef PRODUCT // unit test for checking -Xloggc: parsing result void test_loggc_filename(); +void test_snprintf(); #endif void ostream_init();