RFR: 8303805: [REDO] JDK-8302189 and JDK-8302799

Kim Barrett kbarrett at openjdk.org
Tue Mar 28 00:06:23 UTC 2023


Please review this redo of the following changes:
8302189: Mark assertion failures noreturn
8302799: Refactor Debugging variable usage for noreturn crash reporting

The original change tripped over a bug the handling of noreturn and
__builtin_unreachable by Xcode12 (so probably generic clang12 and perhaps
earlier).  It seems to have been fixed in Xcode13 (and probably other uses of
clang13), but we're not yet ready to require that version.

As such, we've taken the original change and kludged around the problem for
now.  The kludge should be removed once we require clang13+.

As a reminder, the original change that was backed out did the following:
https://github.com/openjdk/jdk/pull/12845

(1) Changed how assertions were suppressed when running the various debugging
commands defined in debug.cpp.  The failure reporting functions used to just
return if the Debugging variable was set (by a debug command invocation).
Now, debug command invocation establishes a DebuggingContext object which
causes asserts to bypass the check entirely.

(2) Added [[noreturn]] attributes to error reporting functions declared in
debug.hpp.

This change consists of
(1) Reapply the change from PR#12845.
(2) A minor improvement to the DebuggingContext destructor.
(3) Replace uses of [[noreturn]] with new ATTRIBUTE_NORETURN macro (the kludge).

The ATTRIBUTE_NORETURN macro expands to `[[noreturn]]` unless the compiler is
clang and its version is less than 13, which it expands to nothing.

The replaced [[noreturn]] uses include both those added by JDK-8302189 and
those added by the earlier JDK-8302798.

This is problematic in the long run.  It prevents the removal of otherwise
obsolete and unreachable code following a call to a noreturn function.  We
have such code in various places because these functions were previously not
noreturn (and with this change still aren't for clang12 and prior).  It may
also require the inclusion of such code in future changes.  This may result in
change testing on Linux and/or Windows missing problems that will only show up
when testing on a clang-using platform.

We're really going to want to require a minimum of clang13 sooner rather than
later.  Unfortunately, we've run into other problems with Xcode13 and Xcode14,
and the aix-ppc maintainers have only just begun testing xlclang++ 17.1.  (The
version they have been using is based on clang3!)  Hence this kludge for now,
to permit use of gcc12 (JDK-8294031).

Testing:
mach5 tier1-7.

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

Commit messages:
 - attribute macro
 - tidy destructor
 - redo from PR12845

Changes: https://git.openjdk.org/jdk/pull/13199/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=13199&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8303805
  Stats: 232 lines in 12 files changed: 180 ins; 19 del; 33 mod
  Patch: https://git.openjdk.org/jdk/pull/13199.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/13199/head:pull/13199

PR: https://git.openjdk.org/jdk/pull/13199



More information about the build-dev mailing list