Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8332632

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

XMLWordPrintable

    • 21

      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());
      ```

      `ciEnv` functions:
      ```
      bool failing() const { return _failure_reason.get() != nullptr; }
      const char* failure_reason() const { return _failure_reason.get(); }
      ```
      So the code above is:
      ```
          if (_failure_reason.get() == nullptr && !task->is_success()) {
            assert(_failure_reason.get() != nullptr, "expect failure reason");
            assert(false, "compiler should always document failure: %s", ci_env.failure_reason());
      ```

      The condition `_failure_reason.get() != nullptr` is always false in the assert because we can get to it only if `_failure_reason.get() == nullptr`. So it will be triggered. The second assert is redundant.

      The code above should be changed to:
      ```
         if (!ci_env.failing() && !task->is_success()) {
            assert(ci_env.failure_reason() != nullptr, "compiler should always document failure");
            // The compiler elected, without comment, not to register a result.
      ```

      `assert(false, "compiler should always document failure: %s", ci_env.failure_reason()); ` has UB because `ci_env.failure_reason()` is `nullptr`.

            eastigeevich Evgeny Astigeevich
            eastigeevich Evgeny Astigeevich
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated: