Created attachment 58546 [details] minimal reproducer Reproducer here also: https://godbolt.org/z/zYaanP1aE Tested on GCC 13, GCC 14, and latest Compiler Explorer trunk of GCC 15. Attached is a function from a doubly-linked list. The lines of interest are: ------------------------------------------------------------------------------ 21 struct list *result = calloc(1, sizeof(*result)); 22 if (!result) 23 error(EXIT_FAILURE,errno,"%s:%d %s()",__FILE__,__LINE__,__func__); 24 25 result->destroy = destroy; ------------------------------------------------------------------------------ As you can see, I used error() with EXIT_FAILURE as first argument. The C standard (since ANSI C) defines that zero or EXIT_SUCCESS mean success, therefore EXIT_FAILURE must logically be non-zero. As per the error(3) manual, that call will never return: ------------------------------------------------------------------------------ If status has a nonzero value, then error() calls exit(3) to terminate the program using the given value as the exit status; otherwise it returns after printing the error message. ------------------------------------------------------------------------------ However, the analyzer doesn't catch this. It seems to think the call to error() will return to the program and do a possible NULL pointer dereference on the next line. The error from the analyzer is: ------------------------------------------------------------------------------ <source>: In function 'list_create': <source>:25:25: warning: dereference of NULL 'result' [CWE-476] [-Wanalyzer-null-dereference] 25 | result->destroy = destroy; | ~~~~~~~~~~~~~~~~^~~~~~~~~ 'list_create': events 1-3 21 | struct list *result = calloc(1, sizeof(*result)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~ | | | (1) allocated here 22 | if (!result) | ~ | | | (2) assuming 'result' is NULL | (3) following 'true' branch (when 'result' is NULL)... ─>─┐ | │ 'list_create': event 4 | │ |┌─────────────────────────────────────────────────────────────────────┘ 23 |│ error(EXIT_FAILURE,errno,"%s:%d %s()",__FILE__,__LINE__,__func__); |│ ^~~~~ |│ | |└──────────────────────────────────>(4) ...to here 'list_create': event 5 25 | result->destroy = destroy; | ~~~~~~~~~~~~~~~~^~~~~~~~~ | | | (5) ⚠️ dereference of NULL 'result' ------------------------------------------------------------------------------ Using built-in specs. COLLECT_GCC=/opt/compiler-explorer/gcc-snapshot/bin/gcc Target: x86_64-linux-gnu Configured with: ../gcc-trunk-20240630/configure --prefix=/opt/compiler-explorer/gcc-build/staging --enable-libstdcxx-backtrace=yes --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu --disable-bootstrap --enable-multiarch --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --enable-clocale=gnu --enable-languages=c,c++,fortran,ada,objc,obj-c++,go,d,rust,m2 --enable-ld=yes --enable-gold=yes --enable-libstdcxx-debug --enable-libstdcxx-time=yes --enable-linker-build-id --enable-lto --enable-plugins --enable-threads=posix --with-pkgversion=Compiler-Explorer-Build-gcc-1bcfed4c52bb2410ea71bf6e4d46026e18461f84-binutils-2.42 Thread model: posix Supported LTO compression algorithms: zlib gcc version 15.0.0 20240630 (experimental) (Compiler-Explorer-Build-gcc-1bcfed4c52bb2410ea71bf6e4d46026e18461f84-binutils-2.42) COLLECT_GCC_OPTIONS='-fdiagnostics-color=always' '-g' '-o' '/app/output.s' '-S' '-c' '-g3' '-O0' '-std=c11' '-Wall' '-Wextra' '-Wpedantic' '-fanalyzer' '-v' '-mtune=generic' '-march=x86-64' '-dumpdir' '/app/' /opt/compiler-explorer/gcc-trunk-20240630/bin/../libexec/gcc/x86_64-linux-gnu/15.0.0/cc1 -quiet -v -imultiarch x86_64-linux-gnu -iprefix /opt/compiler-explorer/gcc-trunk-20240630/bin/../lib/gcc/x86_64-linux-gnu/15.0.0/ -dD <source> -quiet -dumpdir /app/ -dumpbase output.c -dumpbase-ext .c -mtune=generic -march=x86-64 -g -g3 -O0 -Wall -Wextra -Wpedantic -std=c11 -version -fdiagnostics-color=always -fanalyzer -o /app/output.s GNU C11 (Compiler-Explorer-Build-gcc-1bcfed4c52bb2410ea71bf6e4d46026e18461f84-binutils-2.42) version 15.0.0 20240630 (experimental) (x86_64-linux-gnu) compiled by GNU C version 11.4.0, GMP version 6.2.1, MPFR version 4.1.0, MPC version 1.2.1, isl version isl-0.24-GMP GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096 ignoring nonexistent directory "/opt/compiler-explorer/gcc-trunk-20240630/bin/../lib/gcc/x86_64-linux-gnu/15.0.0/../../../../x86_64-linux-gnu/include" ignoring duplicate directory "/opt/compiler-explorer/gcc-trunk-20240630/bin/../lib/gcc/../../lib/gcc/x86_64-linux-gnu/15.0.0/include" ignoring nonexistent directory "/usr/local/include/x86_64-linux-gnu" ignoring duplicate directory "/opt/compiler-explorer/gcc-trunk-20240630/bin/../lib/gcc/../../lib/gcc/x86_64-linux-gnu/15.0.0/include-fixed/x86_64-linux-gnu" ignoring duplicate directory "/opt/compiler-explorer/gcc-trunk-20240630/bin/../lib/gcc/../../lib/gcc/x86_64-linux-gnu/15.0.0/include-fixed" ignoring nonexistent directory "/opt/compiler-explorer/gcc-trunk-20240630/bin/../lib/gcc/../../lib/gcc/x86_64-linux-gnu/15.0.0/../../../../x86_64-linux-gnu/include" #include "..." search starts here: #include <...> search starts here: /opt/compiler-explorer/gcc-trunk-20240630/bin/../lib/gcc/x86_64-linux-gnu/15.0.0/include /opt/compiler-explorer/gcc-trunk-20240630/bin/../lib/gcc/x86_64-linux-gnu/15.0.0/include-fixed/x86_64-linux-gnu /opt/compiler-explorer/gcc-trunk-20240630/bin/../lib/gcc/x86_64-linux-gnu/15.0.0/include-fixed /usr/local/include /opt/compiler-explorer/gcc-trunk-20240630/bin/../lib/gcc/../../include /usr/include/x86_64-linux-gnu /usr/include End of search list. Compiler executable checksum: 4447695849b4e58f4635c547043f0c6c
error() isn't a standard function thus GCC cannot guess its behavior. I.e. a different libc may provide error() with a different semantic, and the programmer can provide a customized version of error() because it's not in the reserved namespace. Thus I cannot see what we can do here...
You could add __builtin_unreachable() after the call to error in this case and the warning goes away. error is a glibc extension too. Maybe analyzer could add support for it but I suspect it is low priority since there are other things that can be improved for analyzer (maybe something like -fanalyzer-enable-gnu-extensions could be added for all of the non-standard ones, -fanalyzer-enable-freebsd-extensions, etc. could be added too). Anyways confirmed for now.
Thank you for the speedy response, confirmation, and workaround. I did not think to check if error() was standard or not. Good point. No problems on prioritisation or even disregarding this. When behaviour depends on the implementation, then an annotation like __builtin_unreachable() actually seems like the correct solution.
The analyzer *does* try to handle error() and error_at_line() from GNU's non-standard <error.h>; see: https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=gcc/analyzer/kf.cc;h=4213b89ac9fb4ff11994cf2c35f15a281be3b024;hb=HEAD#l496 In particular it tries to "know" that arg0 == 0 means "doesn't return". So something's going wrong with that; I'll take a look.
glib'c bits/error.h has: /* If we know the function will never return make sure the compiler realizes that, too. */ __extern_always_inline void error (int __status, int __errnum, const char *__format, ...) { if (__builtin_constant_p (__status) && __status != 0) __error_noreturn (__status, __errnum, __format, __va_arg_pack ()); else __error_alias (__status, __errnum, __format, __va_arg_pack ()); } and similar for error_at_line. However, looking at GENERIC (via -fdump-tree-original=stderr) https://godbolt.org/z/dqWr1G97s I see that this is coming out of the C front end as: ;; Function error (null) ;; enabled by -tree-original { if (0) { __error_noreturn (__status, __errnum, __format, __builtin_va_arg_pack ()); } else { __error_alias (__status, __errnum, __format, __builtin_va_arg_pack ()); } } before any of GCC's inlining logic gets to work on it. Hence "test" quickly gets optimized down to just: __error_alias (__status_2(D), __errnum_3(D), __format_4(D), __builtin_va_arg_pack ()); and this is what is inlined into the callsite of "error". Hence by the time the analyzer sees it we have a call to "__error_alias" which the analyzer/kf.cc doesn't recognize, and conservatively assumes could return. So there are arguably three issues here: (a) should gcc keep the __builtin_constant_p around longer so that it's still around when the inliner gets a chance to work on it? (I don't know if this is a good idea or not) (b) should glibc's header be rewritten (perhaps losing the __builtin_constant_p?) (c) the analyzer's special-casing for error and error_at_line should also recognize __error_alias and __error_at_line_alias (probably an easy workaround)
Posted to mailing lists: https://gcc.gnu.org/pipermail/gcc/2024-July/244257.html https://sourceware.org/pipermail/libc-alpha/2024-July/157942.html
As noted by Jakub, it works fine at -O1 and above: https://godbolt.org/z/GrPv3rjoj I'll fix it in the analyzer for the -O0 case.
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>: https://gcc.gnu.org/g:a6fdb1a2a2906103afd70fa68cf7c45e896b8fbb commit r15-1845-ga6fdb1a2a2906103afd70fa68cf7c45e896b8fbb Author: David Malcolm <dmalcolm@redhat.com> Date: Thu Jul 4 14:44:51 2024 -0400 analyzer: handle <error.h> at -O0 [PR115724] At -O0, glibc's: __extern_always_inline void error (int __status, int __errnum, const char *__format, ...) { if (__builtin_constant_p (__status) && __status != 0) __error_noreturn (__status, __errnum, __format, __builtin_va_arg_pack ()); else __error_alias (__status, __errnum, __format, __builtin_va_arg_pack ()); } becomes just: __extern_always_inline void error (int __status, int __errnum, const char *__format, ...) { if (0) __error_noreturn (__status, __errnum, __format, __builtin_va_arg_pack ()); else __error_alias (__status, __errnum, __format, __builtin_va_arg_pack ()); } and thus calls to "error" are calls to "__error_alias" by the time -fanalyzer "sees" them. Handle them with more special-casing in kf.cc. gcc/analyzer/ChangeLog: PR analyzer/115724 * kf.cc (register_known_functions): Add __error_alias and __error_at_line_alias. gcc/testsuite/ChangeLog: PR analyzer/115724 * c-c++-common/analyzer/error-pr115724.c: New test. Signed-off-by: David Malcolm <dmalcolm@redhat.com>
Fixed by the above patch for trunk for gcc 15. Handling of "error" was added in r11-7333-g5ee4ba031dd9fc to fix PR analyzer/99196 so keeping this open to track backporting to earlier branches.
The releases/gcc-14 branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>: https://gcc.gnu.org/g:0f26f4f76961cdc7ebf7f07bec0b370fd1a04972 commit r14-10956-g0f26f4f76961cdc7ebf7f07bec0b370fd1a04972 Author: David Malcolm <dmalcolm@redhat.com> Date: Thu Jul 4 14:44:51 2024 -0400 analyzer: handle <error.h> at -O0 [PR115724] At -O0, glibc's: __extern_always_inline void error (int __status, int __errnum, const char *__format, ...) { if (__builtin_constant_p (__status) && __status != 0) __error_noreturn (__status, __errnum, __format, __builtin_va_arg_pack ()); else __error_alias (__status, __errnum, __format, __builtin_va_arg_pack ()); } becomes just: __extern_always_inline void error (int __status, int __errnum, const char *__format, ...) { if (0) __error_noreturn (__status, __errnum, __format, __builtin_va_arg_pack ()); else __error_alias (__status, __errnum, __format, __builtin_va_arg_pack ()); } and thus calls to "error" are calls to "__error_alias" by the time -fanalyzer "sees" them. Handle them with more special-casing in kf.cc. gcc/analyzer/ChangeLog: PR analyzer/115724 * kf.cc (register_known_functions): Add __error_alias and __error_at_line_alias. gcc/testsuite/ChangeLog: PR analyzer/115724 * c-c++-common/analyzer/error-pr115724.c: New test. Signed-off-by: David Malcolm <dmalcolm@redhat.com> (cherry picked from commit a6fdb1a2a2906103afd70fa68cf7c45e896b8fbb)