clang UBSan reports division by zero errors on unit tests

[stessaris]

Hi, I run the tests with the clang flags-fsanitize=address,undefined -fno-omit-frame-pointer, and it's reporting several division by zero errors.

E.g., the first two are:

unit_tests.c:4944:3: runtime error: division by zero
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior unit_tests.c:4944:3 
unit_tests.c:4944:3: runtime error: division by zero
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior unit_tests.c:4944:3 

I ran the tests on mcr.microsoft.com/devcontainers/cpp:ubuntu-24.04 (via devcontainers), the clang version is:

$ clang --version
Ubuntu clang version 18.1.3 (1ubuntu1)
Target: aarch64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

[JacksonAllan]

Thanks for the report, and sorry for the slow response!

I managed to trace the problem. It stems from the CC_WARN_DUPLICATE_SIDE_EFFECTS macro, which is defined as:

// In GCC and Clang, we can generate a warning if the user passes an expression that may have side effects as the first
// argument of API macros.
// If the expression could have side effects, the compiler will be unable to resolve a comparison of it with itself at
// compile time, which we can check using __builtin_constant_p.
// The warning itself is generated via a division by zero.
// This macro may produce false positives (e.g. for &our_containers[ our_func() ] where our_func is a pure function that
// always returns the same value), but that is a reasonable price to pay for more macro safety.

#ifdef __GNUC__

#define CC_WARN_DUPLICATE_SIDE_EFFECTS( cntr )                                                            \
(void)(                                                                                                   \
  "WARNING: CONTAINER ARGUMENT MAY HAVE DUPLICATED SIDE EFFECTS" &&1/__builtin_constant_p((cntr)==(cntr)) \
)                                                                                                         \

#else
#define CC_WARN_DUPLICATE_SIDE_EFFECTS( cntr ) (void)0
#endif

CC's API macros are all guaranteed to be safe with the exception of the first argument (the pointer to the target container), which might be evaluated multiple times. CC_WARN_DUPLICATE_SIDE_EFFECTS is supposed to do nothing more than generate a compiler warning in the case that the user provides an expression with side effects as this argument. Currently, this mechanism is applied inside every API macro, but I was already planning to remove it from macros that don't actually evaluate their first argument multiple times in the next update.

The problem, it seems, is that in Clang, __builtin_constant_p behaves differently to how it behaves in GCC. In GCC, __builtin_constant_p detects a comparison of a variable with itself as a compile-time constant, but in Clang it does not:

#include <assert.h>

int main( void )
{
  int x;
  _Static_assert( __builtin_constant_p( &x == &x ), "Not a compile-time constant!" ); // GCC: Passes. Clang: Passes.
  _Static_assert( __builtin_constant_p( x == x ), "Not a compile-time constant!" );   // GCC: Passes. Clang: Fails.
}

This is causing a problem in a few unit tests that pass el (a pointer to a container inside another containers), instead of e.g. &our_vec, into an API macro as the container argument. I didn't notice it before because when optimizations are turned on, Clang apparently just omits the whole duplicate-side-effects check.

Some initial experimentation suggests that __builtin_assume( ( (void)(cntr), "WARNING: CONTAINER ARGUMENT MAY HAVE DUPLICATED SIDE EFFECTS" ) ) will work for Clang. I'll do some more tests and release a patch.