CDRIVER-5549 Address -fsanitize=function and -Wcast-function-type-strict warnings

[eramongodb]

Summary

Resolves CDRIVER-5549. Verified by this patch.

Recommend reviewing this PR by-commit.

This PR identifies and addresses ~20 UBSan runtime errors (exact number depends on the server version and topology) and 154 instances of -Wcast-function-type-strict warnings that were locally observed when building the C Driver with Clang 17. Some instances may still be unaccounted for due to sensitivity to specific build configurations, but this PR aims to provide a template for addressing such instances in the future should they become a problem (note: this PR does not add Clang 17+ test coverage to the EVG config).

Description

Clang 17 extends the UBSan check "function", usually enabled for C++ code, to C code as well. This check diagnoses the following clause from the C99 draft:

If the function is defined with a type that is not compatible with the type (of the expression) pointed to by the expression that denotes the called function, the behavior is undefined.

Many C codebases, including our own, violates this clause, often through the following commonly-seen pattern:

// Target functions to invoke.
void foo (A *a);
void bar (B *b);

// A "common" function pointer type for generic code.
typedef void (*fn_t)(void *);

void invoke (fn_t fn, void *ptr) {
  fn (ptr); // <-- `fn` is invoked as `void (void *)`.
}

void example (A *a, B *b) {
  // Pass target functions via "common" function pointer type.
  invoke (&foo, a);
  invoke (&bar, b);
}

Unfortunately, this pattern is undefined behavior. The type of *fn when invoked as fn (ptr) is void (void *), which is not compatible with the type of the actual functions being invoked: void (A *) and void (B *) respectively. The convertibility of individual object pointers of type T* to/from void* does not extend to the parameters of a function type.

Therefore, it is necessary to ensure that the type of the function being invoked is always compatible with the type of the function in the invocation expression. Identifying such cases can be assisted by the -Wcast-function-type-strict Clang warning, which warns whenever a function is cast to/from an incompatible function type.

The solution usually involved one of three kinds of common patterns:

• Change the parameter type(s) to void * for consistency with the "common" function pointer type and cast back to the object pointer type within the function body. (e.g. TestFuncWC and TestFunc callbacks, TS pool callbacks, etc.).
• If unable to change the target function's type, use an intermediate function whose type is consistent with the "common" function pointer type. (e.g. bson_destroy_vp, _apm_func + _apm_func_defn, etc.)
• If unable to use any compatible function pointer type, pass the target function via an object pointer (void *) instead. (e.g. TestFnCtx (introduced in CDRIVER-3632 Address pedantic warnings for function pointer conversions #850), make_cursor_helper_t, etc.)

As an exception to the above, the mongoc_set_t const-for-each interface needed to be extended, as a pattern that reuses a common implementation that does not violate function type compatibility could not be identified.

-Wcast-function-type-strict warnings pertaining to external libraries are explicitly suppressed or silenced, as there is little we can do about it ourselves (e.g. OpenSSL/Cyrus callback registration APIs).

As a drive-by fix (not quite related), a benign UBSan runtime error emitted by the utf8proc library is suppressed by a new .ubsan-suppressions file. Details are documented within the suppressions file itself.

[kevinAlbs]

The efforts to reduce UBSan and warnings are much appreciated. I expect this will ease the eventual upgrade of clang used in UBSan tasks.

LGTM with one suggested change:

Compiling with -Wcast-function-type-strict locally (on "Homebrew clang version 18.1.8") identifies:

mongoc-util.c:922:48: error: cast from 'size_t (*)(void)' (aka 'unsigned long (*)(void)') to 'uint64_t (*)(void)' (aka 'unsigned long long (*)(void)') converts to incompatible function type [-Werror,-Wcast-function-type-strict]
  922 |    return _mongoc_rand_java64 (max - min + 1u, (uint64_t (*) (void)) rand) + min;
      |                                                ^~~~~~~~~~~~~~~~~~~~~~~~~~

Suggest wrapping call to rand:

static uint64_t
rand_u64 (void)
{
   return (uint64_t) rand ();
}

[eramongodb]

Suggest wrapping call to rand

Addressed by removing the rand parameter from _mongoc_rand_size_t entirely, given the implementation of the size_t random functions were already special-cased on sizeof(size_t) and the only use case affected by this removal is in mongoc-topology.c, which also deferred to the (special-cased) _mongoc_simple_rand_size_t generator:

mongo-c-driver/src/libmongoc/src/mongoc/mongoc-topology.c

Line 285 in fed2beb

const size_t swap_pos = _mongoc_rand_size_t (0u, idx, _mongoc_simple_rand_size_t);

[laurelxiang]

LGTM!