> 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.
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 four additional commits since the last revision: - Merge branch 'master' into redo-noreturn - attribute macro - tidy destructor - redo from PR12845 ------------- Changes: - all: https://git.openjdk.org/jdk/pull/13199/files - new: https://git.openjdk.org/jdk/pull/13199/files/4cb2fa14..e0f3d3a3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=13199&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=13199&range=00-01 Stats: 8977 lines in 243 files changed: 5930 ins; 1903 del; 1144 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