git-compat-util: add NOT_CONSTANT macro and use it in atfork_prepare()

Our hope is that the number of code paths that falsely trigger warnings with the -Wunreachable-code compilation option are small, and they can be worked around case-by-case basis, like we just did in the previous commit. If we need such a workaround a bit more often, however, we may benefit from a more generic and descriptive facility that helps document the cases we need such workarounds. Side note: if we need the workaround all over the place, it simply means -Wunreachable-code is not a good tool for us to save engineering effort to catch mistakes. We are still exploring if it helps us, so let's assume that it is not the case. Introduce NOT_CONSTANT() macro, with which, the developer can tell the compiler: Do not optimize this expression out, because, despite whatever you are told by the system headers, this expression should *not* be treated as a constant. and use it as a replacement for the workaround we used that was somewhat specific to the sigfillset case. If the compiler already knows that the call to sigfillset() cannot fail on a particular platform it is compiling for and declares that the if() condition would not hold, it is plausible that the next version of the compiler may learn that sigfillset() that never fails would not touch errno and decide that in this sequence: errno = 0; sigfillset(&all) if (errno) die_errno("sigfillset"); the if() statement will never trigger. Marking that the value returned by sigfillset() cannot be a constant would document our intention better and would not break with such a new version of compiler that is even more "clever". With the marco, the above sequence can be rewritten: if (NOT_CONSTANT(sigfillset(&all))) die_errno("sigfillset"); which looks almost like other innocuous annotations we have, e.g. UNUSED. Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Mar 17, 2025 at 16:53 UTC 82e79c63642c3e9815aab3433d9979afd173d07d
5 files changed +18 -7
Makefile
+1
@@ -985,6 +985,7 @@ LIB_OBJS += compat/nonblock.o
985 LIB_OBJS += compat/obstack.o
986 LIB_OBJS += compat/terminal.o
987 LIB_OBJS += compat/zlib-uncompress2.o
988 +LIB_OBJS += compiler-tricks/not-constant.o
989 LIB_OBJS += config.o
990 LIB_OBJS += connect.o
991 LIB_OBJS += connected.o
compiler-tricks/not-constant.c new
+2
@@ -0,0 +1,2 @@
1 +#include <git-compat-util.h>
2 +int false_but_the_compiler_does_not_know_it_;
git-compat-util.h
+9
@@ -1593,4 +1593,13 @@ static inline void *container_of_or_null_offset(void *ptr, size_t offset)
1593 ((uintptr_t)&(ptr)->member - (uintptr_t)(ptr))
1594 #endif /* !__GNUC__ */
1595
1596 +/*
1597 + * Prevent an overly clever compiler from optimizing an expression
1598 + * out, triggering a false positive when building with the
1599 + * -Wunreachable-code option. false_but_the_compiler_does_not_know_it_
1600 + * is defined in a compilation unit separate from where the macro is
1601 + * used, initialized to 0, and never modified.
1602 + */
1603 +#define NOT_CONSTANT(expr) ((expr) || false_but_the_compiler_does_not_know_it_)
1604 +extern int false_but_the_compiler_does_not_know_it_;
1605 #endif
meson.build
+1
@@ -249,6 +249,7 @@ libgit_sources = [
249 'compat/obstack.c',
250 'compat/terminal.c',
251 'compat/zlib-uncompress2.c',
252 + 'compiler-tricks/not-constant.c',
253 'config.c',
254 'connect.c',
255 'connected.c',
run-command.c
+5 -7
@@ -516,14 +516,12 @@ static void atfork_prepare(struct atfork_state *as)
516 sigset_t all;
517
518 /*
519 - * Do not use the return value of sigfillset(). It is transparently 0
520 - * on some platforms, meaning a clever compiler may complain that
521 - * the conditional body is dead code. Instead, check for error via
522 - * errno, which outsmarts the compiler.
519 + * POSIX says sigfillset() can fail, but an overly clever
520 + * compiler can see through the header files and decide
521 + * it cannot fail on a particular platform it is compiling for,
522 + * triggering -Wunreachable-code false positive.
523 */
524 - errno = 0;
525 - sigfillset(&all);
526 - if (errno)
524 + if (NOT_CONSTANT(sigfillset(&all)))
525 die_errno("sigfillset");
526 #ifdef NO_PTHREADS
527 if (sigprocmask(SIG_SETMASK, &all, &as->old))