Skip to content
Permalink
Browse files
8214976: Warn about uses of functions replaced for portability
Reviewed-by: dholmes, tschatzl, coleenp
  • Loading branch information
Kim Barrett committed Jun 8, 2022
1 parent 024a240 commit 04f02ac6b2ce496b86642987bb7e25d21b52a5b6
Showing 11 changed files with 153 additions and 27 deletions.
@@ -415,7 +415,7 @@ char* os::map_memory_to_file_aligned(size_t size, size_t alignment, int file_des

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);
ALLOW_C_FUNCTION(::vsnprintf, 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)) {
@@ -783,11 +783,11 @@ struct hostent* os::get_host_by_name(char* name) {
}

void os::exit(int num) {
::exit(num);
ALLOW_C_FUNCTION(::exit, ::exit(num);)
}

void os::_exit(int num) {
::_exit(num);
ALLOW_C_FUNCTION(::_exit, ::_exit(num);)
}

// Builds a platform dependent Agent_OnLoad_<lib_name> function name
@@ -2003,7 +2003,7 @@ void os::abort(bool dump_core, void* siginfo, const void* context) {
LINUX_ONLY(if (DumpPrivateMappingsInCore) ClassLoader::close_jrt_image();)
::abort(); // dump core
}
::_exit(1);
os::_exit(1);
}

// Die immediately, no exit hook, no abort hook, no cleanup.
@@ -1686,7 +1686,7 @@ void os::get_summary_os_info(char* buf, size_t buflen) {
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);
ALLOW_C_FUNCTION(::vsnprintf, 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)) {
@@ -4139,9 +4139,9 @@ int os::win32::exit_process_or_thread(Ept what, int exit_code) {
if (what == EPT_THREAD) {
_endthreadex((unsigned)exit_code);
} else if (what == EPT_PROCESS) {
::exit(exit_code);
ALLOW_C_FUNCTION(::exit, ::exit(exit_code);)
} else { // EPT_PROCESS_DIE
::_exit(exit_code);
ALLOW_C_FUNCTION(::_exit, ::_exit(exit_code);)
}

// Should not reach here
@@ -39,6 +39,7 @@
#include "runtime/deoptimization.hpp"
#include "runtime/jniHandles.inline.hpp"
#include "runtime/javaCalls.hpp"
#include "runtime/os.hpp"
#include "jvmci/jniAccessMark.inline.hpp"
#include "jvmci/jvmciCompiler.hpp"
#include "jvmci/jvmciRuntime.hpp"
@@ -730,8 +731,7 @@ void JVMCIEnv::fthrow_error(const char* file, int line, const char* format, ...)
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);
JavaThread* THREAD = JavaThread::current();
if (is_hotspot()) {
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1998, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1998, 2022, 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
@@ -385,7 +385,7 @@ static void self_destruct_if_needed() {
if ((SelfDestructTimer != 0.0) && !VMError::is_error_reported() &&
(os::elapsedTime() > SelfDestructTimer * 60.0)) {
tty->print_cr("VM self-destructed");
exit(-1);
os::exit(-1);
}
}

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2022, 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
@@ -70,4 +70,25 @@
#define PRAGMA_NONNULL_IGNORED
#endif

// Support warnings for use of certain C functions, except where explicitly
// permitted.
//
// FORBID_C_FUNCTION(signature, alternative)
// - signature: the function that should not normally be used.
// - alternative: a string that may be used in a warning about a use, typically
// suggesting an alternative.
//
// ALLOW_C_FUNCTION(name, ... using statement ...)
// - name: the name of a forbidden function whose use is permitted in statement.
// - statement: a use of the otherwise forbidden function. Using a variadic
// tail allows the statement to contain non-nested commas.

#ifndef FORBID_C_FUNCTION
#define FORBID_C_FUNCTION(signature, alternative)
#endif

#ifndef ALLOW_C_FUNCTION
#define ALLOW_C_FUNCTION(name, ...) __VA_ARGS__
#endif

#endif // SHARE_UTILITIES_COMPILERWARNINGS_HPP
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2022, 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
@@ -63,4 +63,26 @@

#endif // clang/gcc version check

#if (__GNUC__ >= 9) || (__clang_major__ >= 14)

// Use "warning" attribute to detect uses of "forbidden" functions.
//
// Note: _FORTIFY_SOURCE transforms calls to certain functions into calls to
// associated "checking" functions, and that transformation seems to occur
// *before* the attribute check. We use fortification in fastdebug builds,
// so uses of functions that are both forbidden and fortified won't cause
// forbidden warnings in such builds.
#define FORBID_C_FUNCTION(signature, alternative) \
extern "C" __attribute__((__warning__(alternative))) signature;

// Disable warning attribute over the scope of the affected statement.
// The name serves only to document the intended function.
#define ALLOW_C_FUNCTION(name, ...) \
PRAGMA_DIAG_PUSH \
PRAGMA_DISABLE_GCC_WARNING("-Wattribute-warning") \
__VA_ARGS__ \
PRAGMA_DIAG_POP

#endif // gcc9+ or clang14+

#endif // SHARE_UTILITIES_COMPILERWARNINGS_GCC_HPP
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2022, 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
@@ -30,4 +30,39 @@

#define PRAGMA_DISABLE_MSVC_WARNING(num) __pragma(warning(disable : num))

// The Visual Studio implementation of FORBID_C_FUNCTION explicitly does
// nothing, because there doesn't seem to be a way to implement it for Visual
// Studio. What seems the most likely approach is to use deprecation warnings,
// but that runs into problems.
//
// (1) Declaring the function deprecated (using either __declspec(deprecated)
// or the C++14 [[deprecated]] attribute) fails with warnings like this:
// warning C4273: 'exit': inconsistent dll linkage
// It seems attributes are not simply additive with this compiler.
//
// (2) Additionally adding __declspec(dllimport) to deal with (1) fails with
// warnings like this:
// error C2375: 'vsnprintf': redefinition; different linkage
// It seems some functions in the set of interest have different linkage than
// others ("exit" is marked imported while "vsnprintf" is not, for example).
// That makes it difficult to provide a generic macro.
//
// (3) Using __pragma(deprecated(name)) fails with
// warning C4995: 'frobnicate': name was marked as #pragma deprecated
// for a *declaration* (not a use) of a 'frobnicate' function.
//
// ALLOW_C_FUNCTIONS disables deprecation warnings over the statement scope.
// Some of the functions we're interested in allowing are conditionally
// deprecated on Windows, under the control of various preprocessor defines
// such as _CRT_SECURE_NO_WARNINGS. Annotating vetted uses allows those
// warnings to catch unchecked uses.

#define FORBID_C_FUNCTION(signature, alternative)

#define ALLOW_C_FUNCTION(name, ...) \
PRAGMA_DIAG_PUSH \
PRAGMA_DISABLE_MSVC_WARNING(4996) \
__VA_ARGS__ \
PRAGMA_DIAG_POP

#endif // SHARE_UTILITIES_COMPILERWARNINGS_VISCPP_HPP
@@ -156,6 +156,19 @@ inline intptr_t p2i(const volatile void* p) {
return (intptr_t) p;
}


//----------------------------------------------------------------------------------------------------
// Forbid the use of various C library functions.
// Some of these have os:: replacements that should normally be used instead.
// Others are considered security concerns, with preferred alternatives.

FORBID_C_FUNCTION(void exit(int), "use os::exit");
FORBID_C_FUNCTION(void _exit(int), "use os::exit");
FORBID_C_FUNCTION(char* strerror(int), "use os::strerror");
FORBID_C_FUNCTION(char* strtok(char*, const char*), "use strtok_r");
FORBID_C_FUNCTION(int vsprintf(char*, const char*, va_list), "use os::vsnprintf");
FORBID_C_FUNCTION(int vsnprintf(char*, size_t, const char*, va_list), "use os::vsnprintf");

//----------------------------------------------------------------------------------------------------
// Constants

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2022, 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
@@ -38,6 +38,7 @@
#include "jni.h"
#include "unittest.hpp"

#include "runtime/os.hpp"
#include "runtime/thread.inline.hpp"

// Default value for -new-thread option: true on AIX because we run into
@@ -123,7 +124,7 @@ class JVMInitializerListener : public ::testing::EmptyTestEventListener {
int ret_val = init_jvm(_argc, _argv, false, &_jvm);
if (ret_val != 0) {
ADD_FAILURE() << "Could not initialize the JVM: " << ret_val;
exit(1);
os::exit(1);
}
}
}
@@ -245,7 +246,7 @@ static void runUnitTestsInner(int argc, char** argv) {
char* java_home = get_java_home_arg(argc, argv);
if (java_home == NULL) {
fprintf(stderr, "ERROR: You must specify a JDK to use for running the unit tests.\n");
exit(1);
os::exit(1);
}
#ifndef _WIN32
int overwrite = 1; // overwrite an eventual existing value for JAVA_HOME
@@ -294,7 +295,7 @@ static void runUnitTestsInner(int argc, char** argv) {

if (result != 0) {
fprintf(stderr, "ERROR: RUN_ALL_TESTS() failed. Error %d\n", result);
exit(2);
os::exit(2);
}

if (jvm_listener != NULL) {
@@ -323,7 +324,7 @@ static void run_in_new_thread(const args_t* args) {
hdl = CreateThread(NULL, STACK_SIZE, thread_wrapper, (void*)args, 0, NULL);
if (hdl == NULL) {
fprintf(stderr, "Failed to create main thread\n");
exit(2);
os::exit(2);
}
WaitForSingleObject(hdl, INFINITE);
}
@@ -345,12 +346,12 @@ static void run_in_new_thread(const args_t* args) {

if (pthread_create(&tid, &attr, thread_wrapper, (void*)args) != 0) {
fprintf(stderr, "Failed to create main thread\n");
exit(2);
os::exit(2);
}

if (pthread_join(tid, NULL) != 0) {
fprintf(stderr, "Failed to join main thread\n");
exit(2);
os::exit(2);
}
}

@@ -0,0 +1,31 @@
/*
* Copyright (c) 2022, 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
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

#include "precompiled.hpp"
#include "runtime/os.hpp"

extern void gtest_exit_from_child_vm(int num);

void gtest_exit_from_child_vm(int num) {
os::exit(num);
}
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2022, 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
@@ -68,6 +68,9 @@
#endif
#endif

// Wrapper around os::exit so we don't need to include os.hpp here.
extern void gtest_exit_from_child_vm(int num);

#define CONCAT(a, b) a ## b

#define TEST(category, name) GTEST_TEST(category, name)
@@ -94,7 +97,7 @@
} \
} \
fprintf(stderr, "OKIDOKI"); \
exit(0); \
gtest_exit_from_child_vm(0); \
} \
\
TEST(category, CONCAT(name, _other_vm)) { \
@@ -112,7 +115,7 @@
static void child_ ## category ## _ ## name ## _() { \
::testing::GTEST_FLAG(throw_on_failure) = true; \
test_ ## category ## _ ## name ## _(); \
exit(0); \
gtest_exit_from_child_vm(0); \
} \
\
TEST(category, CONCAT(name, _vm_assert)) { \
@@ -134,7 +137,7 @@
static void child_ ## category ## _ ## name ## _() { \
::testing::GTEST_FLAG(throw_on_failure) = true; \
test_ ## category ## _ ## name ## _(); \
exit(0); \
gtest_exit_from_child_vm(0); \
} \
\
TEST(category, CONCAT(name, _vm_assert)) { \
@@ -155,13 +158,13 @@
static void child_ ## category ## _ ## name ## _() { \
::testing::GTEST_FLAG(throw_on_failure) = true; \
test_ ## category ## _ ## name ## _(); \
exit(0); \
gtest_exit_from_child_vm(0); \
} \
\
TEST(category, CONCAT(name, _vm_assert)) { \
ASSERT_EXIT(child_ ## category ## _ ## name ## _(), \
::testing::ExitedWithCode(1), \
msg); \
msg); \
} \
\
void test_ ## category ## _ ## name ## _()

2 comments on commit 04f02ac

@openjdk-notifier
Copy link

@openjdk-notifier openjdk-notifier bot commented on 04f02ac Jun 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TheRealMDoerr
Copy link
Contributor

@TheRealMDoerr TheRealMDoerr commented on 04f02ac Jun 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Causes build warning:
os_posix.cpp:418:57: error: call to 'vsnprintf' declared with attribute warning: use os::vsnprintf [-Werror=attribute-warning]
418 | ALLOW_C_FUNCTION(::vsnprintf, int result = ::vsnprintf(buf, len, fmt, args);)
(gcc 9.4.0 and 9.3.1)

Please sign in to comment.