RFR: 8347719: [REDO] Portable implementation of FORBID_C_FUNCTION and ALLOW_C_FUNCTION [v3]

[Thomas Schatzl]

On Tue, 22 Apr 2025 10:59:24 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Please review this second attempt.  It's mostly similar to the original
>> attempt:
>> https://bugs.openjdk.org/browse/JDK-8313396
>> https://github.com/openjdk/jdk/pull/22890
>> but improves the workarounds for one clang issue, and adds a workaround for
>> another clang issue
>> https://bugs.openjdk.org/browse/JDK-8347649
>> 
>> See globalDefinitions_gcc.hpp for more details about those issues and the
>> workarounds.
>> 
>> Additions to the testing done for the earlier attempt (see below)
>> 
>> mach5 tier4-5. There is an Oracle-internal build configuration in tier5 that
>> failed with the earlier attempt.
>> 
>> Local manual build and tier1 test on linux with clang.  
>> For testing on linux with clang, be aware of these issues:
>> https://bugs.openjdk.org/browse/JDK-8354316
>> https://bugs.openjdk.org/browse/JDK-8354467
>> 
>> Below is a repeat of the PR summary for the earlier attempt.  
>> 
>> ----------
>> 
>> 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 forwa...
>
> 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 five additional commits since the last revision:
> 
>  - Merge branch 'master' into redo-poisoning
>  - Merge branch 'master' into redo-poisoning
>  - note that clang bug with deprecated attribute has been fixed
>  - improve/add clang-specific workarounds
>  - apply original change

Seems good.

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

Marked as reviewed by tschatzl (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24608#pullrequestreview-2790117326