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

[Kim Barrett]

On Sat, 12 Apr 2025 13:48:21 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 forward declarations isn't as simple as
> expected, due to differences between the various compilers. We hide t...

> Just a thought, what if we used decltype instead?
> 
> ```c++
> [[deprecated("use os::malloc")]] decltype(::malloc) malloc;
> [[deprecated("use os::exit")]] decltype(::exit) exit;
> ```

First, that requires the corresponding header be included first, to provide
decltype with something to work with.  (That's part of the current clang
workaround, but not needed for other platforms.  It's not currently a huge
deal, as the set of functions we're poisoning is currently pretty small.  But
I'm not sure that's a forever plan.)

Second, `[[noreturn]]` doesn't affect the type. Though
`__attribute__((noreturn))` does affect the type. (I don't know about
`__declspec((noreturn))`.) See
https://github.com/llvm/llvm-project/issues/113511
(I think it's a mistake in the standard that `[[noreturn]]` doesn't affect the
type, just as it was a mistake that `noexcept` didn't until fixed in C++17.)

Third, I have no idea whether `__declspec((import))` affects the type, and
haven't bothered experimenting because of the above.

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

PR Comment: https://git.openjdk.org/jdk/pull/24608#issuecomment-2802358064