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

[Coleen Phillimore]

On Thu, 9 Jan 2025 22:07:12 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 15 additional commits since the last revision:
> 
>  - Merge branch 'master' into new-poison
>  - Merge branch 'master' into new-poison
>  - remove more os-specific posix forwarding headers
>  - stefank whitespace suggestions
>  - add permit wrapper for strdup and use in aix
>  - remove os-specific posix forwarding headers
>  - aix permit patches
>  - more fixes for clang noreturn issues
>  - Merge branch 'master' into new-poison
>  - update copyrights
>  - ... and 5 more: https://git.openjdk.org/jdk/compare/3c60d213...6d49abbb

This looks good for me too.  I think you should allow GHA to run for this change.

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

Marked as reviewed by coleenp (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22890#pullrequestreview-2546569987