Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8214976: Warn about uses of functions replaced for portability #8982

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -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); \
Copy link
Contributor

@coleenp coleenp Jun 8, 2022

Choose a reason for hiding this comment

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

Are the backslashes lined up here? Can you line them up, if not? Thanks for the comment why this function.

Copy link
Author

@kimbarrett kimbarrett Jun 8, 2022

Choose a reason for hiding this comment

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

Fixed.

} \
\
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,7 +158,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)) { \