1. 4
  1.  

    1. 2

      The article has some problems. I need to look at the actual test code they used but I’m not at my laptop so I can’t see exactly what they’re doing.

      The problem with runtime enablement of asserts is it forces objects to exist, including potential escape and capture, that impacts the ability to optimize code even when the runtime flag is disabled. There are many environments where debug (or generally assertion enabled) increase memory usage because additional storage and work is required to support assertions even if they’re turned off.

      I’m hesitant to say much more without looking at what they actually built/changed/tested but I want to also call out that llvm_unreachable() is an inline function that turns into __builtin_unreachable() which is a no-op/ub not an actual function call.

      1. 1

        Yeah I was looking at clang’s builtins the other day and __builtin_unreachable() struck me as very dangerous. I recall clang often compiles code following undefined behaviour to nothing, zero instructions, which has sometimes led to execution running off the end of one function into whichever function happens to be next in memory. Since __builtin_unreachable() is defined to be undefined behaviour, that’s probably not a good choice for a debug assert! However I think more recent versions of clang are more likely to emit a trap in these situations, but I wouldn’t rely on it.

        A better choice would be one of __builtin_trap() or __builtin_debugtrap() or __builtin_verbose_trap().

        1. 2

          Yuuuuuup.

          __builtin_unreachable() is a footgun, there have been silent codegen bugs in llvm and clang because of people believing llvm_unreachable() would be a termination/error in release builds but because it becomes UB it not only does nothing but also acts as “proof” that checks must be false.

          1. 1

            Since __builtin_unreachable() is defined to be undefined behaviour, that’s probably not a good choice for a debug assert!

            Why not? It doesn’t unconditionally get defined to __builtin_unreachable(), only when the assertions are disabled. When they’re enabled, they’re proper traps.

            (Of course, you can argue about whether it should always trap, but that’s not - I think - the point you’re making here.)

            Indeed, LLVM has:

            LLVM_UNREACHABLE_OPTIMIZE:BOOL This flag controls the behavior of llvm_unreachable() in release build (when assertions are disabled in general). >When ON (default) then llvm_unreachable() is considered “undefined behavior” and optimized as such. When OFF >it is instead replaced with a guaranteed “trap”.

            Specifically, it’s defined as:

            /// Marks that the current location is not supposed to be reachable.
            /// In !NDEBUG builds, prints the message and location info to stderr.
            /// In NDEBUG builds, if the platform does not support a builtin unreachable
            /// then we call an internal LLVM runtime function. Otherwise the behavior is
            /// controlled by the CMake flag
            ///   -DLLVM_UNREACHABLE_OPTIMIZE
            /// * When "ON" (default) llvm_unreachable() becomes an optimizer hint
            ///   that the current location is not supposed to be reachable: the hint
            ///   turns such code path into undefined behavior.  On compilers that don't
            ///   support such hints, prints a reduced message instead and aborts the
            ///   program.
            /// * When "OFF", a builtin_trap is emitted instead of an
            //    optimizer hint or printing a reduced message.
            ///
            /// Use this instead of assert(0). It conveys intent more clearly, suppresses
            /// diagnostics for unreachable code paths, and allows compilers to omit
            /// unnecessary code.
            #ifndef NDEBUG
            #define llvm_unreachable(msg) \
              ::llvm::llvm_unreachable_internal(msg, __FILE__, __LINE__)
            #elif !defined(LLVM_BUILTIN_UNREACHABLE)
            #define llvm_unreachable(msg) ::llvm::llvm_unreachable_internal()
            #elif LLVM_UNREACHABLE_OPTIMIZE
            #define llvm_unreachable(msg) LLVM_BUILTIN_UNREACHABLE
            #else
            #define llvm_unreachable(msg)                                                  \
              do {                                                                         \
                LLVM_BUILTIN_TRAP;                                                         \
                LLVM_BUILTIN_UNREACHABLE;                                                  \
              } while (false)
            #endif
            
            1. 1

              It doesn’t unconditionally get defined to __builtin_unreachable(), only when the assertions are disabled. When they’re enabled, they’re proper traps.

              Are you describing LLVM’s normal assertion machinery here? Because your description doesn’t match my reading of the alternative machinery described in the article.

              1. 1

                Right: LLVM gets it right, and the article’s doesn’t. But given the article was doing it for purposes of measurement, it seems reasonable there.