RFR: 8332632: Redundant assert "compiler should always document failure: %s" with possible UB

[Evgeny Astigeevich]

[JDK-8303951](https://bugs.openjdk.org/browse/JDK-8303951) added the following code: https://github.com/openjdk/jdk/pull/13038/files#diff-2e74481e557cbe87170a56a6e592eea33bb59019926e1c32bebcfaf5b571bb53R2280


     if (!ci_env.failing() && !task->is_success()) {
+ assert(ci_env.failure_reason() != nullptr, "expect failure reason");
+ assert(false, "compiler should always document failure: %s", ci_env.failure_reason());


The second assert is redundant because `ci_env.failure_reason() != nullptr` is always `false`. It also has possible UB.

A compiler sees if-statement checking `!ci_env.failing() ` which, if it is true, implies `ci_env.failure_reason()` is `nullptr`.

Based on this information the compiler can optimize `assert(ci_env.failure_reason() != nullptr, "expect failure reason"); ` to
`assert(false, "expect failure reason"); `.
The compiler can optimize `assert(false, "compiler should always document failure: %s", ci_env.failure_reason()); ` to `assert(false, "compiler should always document failure: %s", nullptr); `.

So the original code would be like the following:


if (!ci_env.failing() && !task->is_success()) {
  assert(false, "expect failure reason");
  assert(false, "compiler should always document failure: %s", nullptr);
}


We have an expression where a format string is used. Format strings usually have undefined behavior if `nullptr` is passed for the character string format specifier. See `std::printf` for example.

Even the second assert is never executed, it makes the IF-block to have UB. The C++ standard says: correct C++ programs are free of undefined behavior. See https://en.cppreference.com/w/cpp/language/ub and https://en.cppreference.com/w/cpp/language/as_if

Choosing between readability and correctness, I choose correctness.

I think the one assert `assert(ci_env.failure_reason() != nullptr, "compiler should always document failure"); ` meets being self-documented and correct.

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

Commit messages:
 - 8332632: Redundant assert "compiler should always document failure: %s" with possible UB

Changes: https://git.openjdk.org/jdk/pull/19395/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19395&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8332632
  Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/19395.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19395/head:pull/19395

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