RFR: 8302262: Remove -XX:SuppressErrorAt develop option

[Kim Barrett]

On Sat, 11 Feb 2023 05:40:13 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

> While I have used it rarely, I found it useful the few times I did.
> 
> Its useful in situations where I analyzed a problem, ran into an assert, but obtaining a new build was difficult or just time-consuming - slow hardware, or a customer's machine. E.g., I used this switch when porting to AIX since rebuilding the VM took about an hour and some asserts could be waived.
> 
> Quoting from[ JDK-8302189](https://bugs.openjdk.org/browse/JDK-8302189):
> 
> > "This also addresses the problems encountered in [JDK-8294031](https://bugs.openjdk.org/browse/JDK-8294031). Because the failure reporting function used by assert is not "noreturn", the compiler is analyzing the continuation from a failed assert as potentially having the values that would result in an assertion failure (in this case, a variable having a null value). Doing so, it discovers a use of that variable in a context where a null value is UB, and so issues a warning."
> 
> I don't understand why this is not a problem for release builds. Relying on assert in this case is not sufficient anyway, you'd need to handle the nullptr in release builds too.

In the example from JDK-8294031 the value shouldn’t be null. But the existence
of the not-null assertion indicates that it was null on the failed assertion
branch of the conditional. That branch doesn’t terminate in a
noreturn/unreachable black hole. The compiler can separately treat the
post-assertion rejoin point as two different continuations, one of which
definitely has a null value (because of the lack of the black hole). And that
leads to discovering later code that is unhappy with that null value, and
warning about it. That kind of splitting and value propagation is a normal
thing that can happen for all kinds of value, not just checks for non-null.
For example, an index in-bounds assertion could run into exactly the same
issue.

In a release build there isn't a null check, so the compiler has no
information either way, and _assumes_ the code doesn't invoke UB, and doesn't
check for it either.
 
> > "This matters even for production, since some of the operations (such as `guarantee` and `ShouldNotReachHere`) are present in product builds, not just debug builds. "
> 
> I can see that. But `guarantee` is rare, so maybe a compromise would be to make `guarantee` `noreturn` but leave `assert` as it is?

Also `ShouldNotReachHere`, `fatal`, &etc. which are pretty common.

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

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