-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Description
One of the CI cron jobs got updated to clang-17 and everything went haywire:
../root/systemd/src/basic/hashmap.c:350:9: runtime error: call to function string_hash_func through pointer to incorrect function type 'void (*)(const void *, struct siphash *)'
/systemd-meson-build/../root/systemd/src/basic/hash-funcs.c:9: note: string_hash_func defined here
#0 0x7ffff5e8693c in base_bucket_hash /systemd-meson-build/../root/systemd/src/basic/hashmap.c:350:9
#1 0x7ffff5e8890c in _hashmap_contains /systemd-meson-build/../root/systemd/src/basic/hashmap.c:1393:16
#2 0x7ffff6f997a8 in hashmap_contains /systemd-meson-build/../root/systemd/src/basic/hashmap.h:183:16
#3 0x7ffff6f997a8 in unit_add_name /systemd-meson-build/../root/systemd/src/core/unit.c:270:13
#4 0x7ffff6f99354 in unit_new_for_name /systemd-meson-build/../root/systemd/src/core/unit.c:154:13
#5 0x10002ac4 in LLVMFuzzerTestOneInput /systemd-meson-build/../root/systemd/src/core/fuzz-unit-file.c:68:9
#6 0x1000388c in main /systemd-meson-build/../root/systemd/src/fuzz/fuzz-main.c:50:29
#7 0x7ffff524a968 in generic_start_main.isra.0 (/lib64/libc.so.6+0x2a968) (BuildId: 974585e36a1ee093f3ce9785541a56bd3c3ecc08)
#8 0x7ffff524ab00 in __libc_start_main (/lib64/libc.so.6+0x2ab00) (BuildId: 974585e36a1ee093f3ce9785541a56bd3c3ecc08)
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../root/systemd/src/basic/hashmap.c:350:9 in
After some debugging and reading, there's a major change in UBsan, which allows using -fsanitize=function on C code [0] (so far it was C++ only), and it's turned on by default. However, this change doesn't like the shenanigans we do in our hashmap implementation.
Specifically, this is now a problem:
#include <stdio.h>
typedef void (*fun_t)(const void *, const void*);
void foo(const char *a, const char *b) {
printf("%s; %s\n", a, b);
}
int main(void) {
fun_t fun = (fun_t) foo;
fun("a", "b");
return 0;
}# UBSAN_OPTIONS=print_stacktrace=1:print_summary=1:halt_on_error=1 ./main
main.c:12:5: runtime error: call to function (unknown) through pointer to incorrect function type 'void (*)(const void *, const void *)'
(/root/main+0x42ce28): note: (unknown) defined here
#0 0x42cebd (/root/main+0x42cebd) (BuildId: bdc69aa49f46458e294b32c22e5de7ce9193f7da)
#1 0x7fd70defb149 (/lib64/libc.so.6+0x28149) (BuildId: 9cffb80784713ce0dee846bfa36cb43dbd9b3e5a)
#2 0x7fd70defb20a (/lib64/libc.so.6+0x2820a) (BuildId: 9cffb80784713ce0dee846bfa36cb43dbd9b3e5a)
#3 0x4033e4 (/root/main+0x4033e4) (BuildId: bdc69aa49f46458e294b32c22e5de7ce9193f7da)
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior main.c:12:5 in
And we do this all over the place in the hashmap stuff. Usually, this would also throw a warning during build, but we get around this by explicitly typecasting the hash and compare functions:
systemd/src/basic/hash-funcs.h
Lines 18 to 27 in 4f3d8de
| #define _DEFINE_HASH_OPS(uq, name, type, hash_func, compare_func, free_key_func, free_value_func, scope) \ | |
| _unused_ static void (* UNIQ_T(static_hash_wrapper, uq))(const type *, struct siphash *) = hash_func; \ | |
| _unused_ static int (* UNIQ_T(static_compare_wrapper, uq))(const type *, const type *) = compare_func; \ | |
| scope const struct hash_ops name = { \ | |
| .hash = (hash_func_t) hash_func, \ | |
| .compare = (compare_func_t) compare_func, \ | |
| .free_key = free_key_func, \ | |
| .free_value = free_value_func, \ | |
| } | |
If I drop the (fun_t) typecast from the code snippet above, gcc/clang will complain:
# gcc -o main main.c
main.c: In function ‘main’:
main.c:10:17: warning: initialization of ‘fun_t’ {aka ‘void (*)(const void *, const void *)’} from incompatible pointer type ‘void (*)(const char *, const char *)’ [-Wincompatible-pointer-types]
10 | fun_t fun = foo;
| ^~~
# clang -o main main.c
main.c:10:11: error: incompatible function pointer types initializing 'fun_t' (aka 'void (*)(const void *, const void *)') with an expression of type 'void (const char *, const char *)' [-Wincompatible-function-pointer-types]
10 | fun_t fun = foo;
| ^ ~~~
1 error generated.
We're not the first project that hit this, but according to the author of the patch this is indeed an undefined behavior and should get fixed (see [1]).
I'm not really sure how to proceed next - in the worst case scenario the function sanitizer can be disabled (via -fno-sanitize=function), but I'm not sure if we wouldn't miss other stuff as well.
[0] https://reviews.llvm.org/D148827
[1] https://reviews.llvm.org/D148827#4379764