RFR: 8313396: Portable implementation of FORBID_C_FUNCTION and ALLOW_C_FUNCTION [v6]

[Stefan Karlsson]

On Thu, 9 Jan 2025 10:02:15 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Please review this change to how HotSpot prevents the use of certain C library
>> functions (e.g. poisons references to those functions), while permitting a
>> subset to be used in restricted circumstances.  Reasons for poisoning a
>> function include it being considered obsolete, or a security concern, or there
>> is a HotSpot function (typically in the os:: namespace) providing similar
>> functionality that should be used instead.
>> 
>> The old mechanism, based on -Wattribute-warning and the associated attribute,
>> only worked for gcc.  (Clang's implementation differs in an important way from
>> gcc, which is the subject of a clang bug that has been open for years.  MSVC
>> doesn't provide a similar mechanism.)  It also had problems with LTO, due to a
>> gcc bug.
>> 
>> The new mechanism is based on deprecation warnings, using [[deprecated]]
>> attributes. We redeclare or forward declare the functions we want to prevent
>> use of as being deprecated.  This relies on deprecation warnings being
>> enabled, which they already are in our build configuration.  All of our
>> supported compilers support the [[deprecated]] attribute.
>> 
>> Another benefit of using deprecation warnings rather than warning attributes
>> is the time when the check is performed.  Warning attributes are checked only
>> if the function is referenced after all optimizations have been performed.
>> Deprecation is checked during initial semantic analysis.  That's better for
>> our purposes here.  (This is also part of why gcc LTO has problems with the
>> old mechanism, but not the new.)
>> 
>> Adding these redeclarations or forward declarations isn't as simple as
>> expected, due to differences between the various compilers.  We hide the
>> differences behind a set of macros, FORBID_C_FUNCTION and related macros.  See
>> the compiler-specific parts of those macros for details.
>> 
>> In some situations we need to allow references to these poisoned functions.
>> 
>> One common case is where our poisoning is visible to some 3rd party code we
>> don't want to modify.  This is typically 3rd party headers included in HotSpot
>> code, such as from Google Test or the C++ Standard Library.  For these the
>> BEGIN/END_ALLOW_FORBIDDEN_FUNCTIONS pair of macros are used demark the context
>> where such references are permitted.
>> 
>> Some of the poisoned functions are needed to implement associated HotSpot os::
>> functions, or in other similarly restricted contexts.  For these, a wrapper
>> function is provided that calls the poison...
>
> Kim Barrett has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - add permit wrapper for strdup and use in aix
>  - remove os-specific posix forwarding headers

src/hotspot/os/posix/forbiddenFunctions_posix.hpp line 29:

> 27: 
> 28: #include "utilities/compilerWarnings.hpp"
> 29: #include <stddef.h> // for size_t

Suggestion:

#include "utilities/compilerWarnings.hpp"

#include <stddef.h> // for size_t

src/hotspot/os/windows/forbiddenFunctions_windows.hpp line 29:

> 27: 
> 28: #include "utilities/compilerWarnings.hpp"
> 29: #include <stddef.h> // for size_t

Suggestion:

#include "utilities/compilerWarnings.hpp"

#include <stddef.h> // for size_t

src/hotspot/share/utilities/forbiddenFunctions.hpp line 30:

> 28: #include "utilities/compilerWarnings.hpp"
> 29: #include "utilities/macros.hpp"
> 30: #include <stdarg.h> // for va_list

Suggestion:

#include "utilities/macros.hpp"

#include <stdarg.h> // for va_list

src/hotspot/share/utilities/permitForbiddenFunctions.hpp line 70:

> 68: 
> 69: #endif // SHARE_UTILITIES_PERMITFORBIDDENFUNCTIONS_HPP
> 70: 

Suggestion:

#endif // SHARE_UTILITIES_PERMITFORBIDDENFUNCTIONS_HPP

test/hotspot/gtest/unittest.hpp line 27:

> 25: #define UNITTEST_HPP
> 26: 
> 27: #include "utilities/globalDefinitions.hpp"

Suggestion:

#include "utilities/globalDefinitions.hpp"

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/22890#discussion_r1908485156
PR Review Comment: https://git.openjdk.org/jdk/pull/22890#discussion_r1908485732
PR Review Comment: https://git.openjdk.org/jdk/pull/22890#discussion_r1908486633
PR Review Comment: https://git.openjdk.org/jdk/pull/22890#discussion_r1908487438
PR Review Comment: https://git.openjdk.org/jdk/pull/22890#discussion_r1908488569