warning: -Wnull-dereference with ngtcp2 + gcc 13.1.0

[vszakats]

I did this

Built curl with gcc 13.1.0 and got this warning:

cd /my/curl/bld-cmake-gcc-x64-shared/lib && /usr/local/bin/x86_64-w64-mingw32-gcc -DBUILDING_LIBCURL -DCURL_HIDDEN_SYMBOLS -DHAVE_CONFIG_H -DUNICODE -D_UNICODE -Dlibcurl_EXPORTS -I/my/curl/include -I/my/brotli/x64-ucrt/usr/include -I/my/zstd/x64-ucrt/usr/include -I/my/nghttp2/x64-ucrt/usr/include -I/my/ngtcp2/x64-ucrt/usr/include -I/my/nghttp3/x64-ucrt/usr/include -I/my/libssh2/x64-ucrt/usr/include -I/my/curl/bld-cmake-gcc-x64-shared/lib/../include -I/my/curl/lib/.. -I/my/curl/lib/../include -I/my/curl/bld-cmake-gcc-x64-shared/lib/.. -I/my/curl/lib -I/my/curl/bld-cmake-gcc-x64-shared/lib -isystem /my/quictls/x64-ucrt/usr/include -isystem /my/zlib/x64-ucrt/usr/include -m64  -fno-ident  -D_UCRT  -DHAVE_SSL_SET0_WBIO -DHAS_ALPN -DNGHTTP2_STATICLIB -DNGHTTP3_STATICLIB -DNGTCP2_STATICLIB -DUSE_MANUAL=1 -m64  -specs=/my/gcc-specs-ucrt -static-libgcc  -lucrt  -W -Wall -pedantic -Wbad-function-cast -Wconversion -Winline -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wno-long-long -Wno-multichar -Wpointer-arith -Wshadow -Wsign-compare -Wundef -Wunused -Wwrite-strings -Wcast-align -Wdeclaration-after-statement -Wempty-body -Wendif-labels -Wfloat-equal -Wignored-qualifiers -Wno-format-nonliteral -Wno-sign-conversion -Wno-system-headers -Wstrict-prototypes -Wtype-limits -Wvla -Wmissing-parameter-type -Wold-style-declaration -Wstrict-aliasing=3 -Wno-pedantic-ms-format -Wformat=2 -Warray-bounds=2 -ftree-vrp -Wduplicated-cond -Wnull-dereference -fdelete-null-pointer-checks -Wshift-negative-value -Wshift-overflow=2 -Walloc-zero -Wduplicated-branches -Wformat-overflow=2 -Wformat-truncation=1 -Wrestrict -Warith-conversion -Wdouble-promotion -Wenum-conversion -Wunused-const-variable -O3 -DNDEBUG -fvisibility=hidden -municode -MD -MT lib/CMakeFiles/libcurl.dir/vquic/curl_ngtcp2.c.obj -MF CMakeFiles/libcurl.dir/vquic/curl_ngtcp2.c.obj.d -o CMakeFiles/libcurl.dir/vquic/curl_ngtcp2.c.obj -c /my/curl/lib/vquic/curl_ngtcp2.c
In function 'recv_closed_stream',
    inlined from 'cf_ngtcp2_recv' at /my/curl/lib/vquic/curl_ngtcp2.c:1416:15:
/my/curl/lib/vquic/curl_ngtcp2.c:1322:12: warning: potential null pointer dereference [-Wnull-dereference]
 1322 |   if(stream->reset) {
      |      ~~~~~~^~~~~~~
/my/curl/lib/vquic/curl_ngtcp2.c:1322:12: warning: potential null pointer dereference [-Wnull-dereference]

I expected the following

No warning.

curl/libcurl version

master @ 446061e

operating system

Cross-building for Windows.

[bagder]

This looks similar to warnings you easily get with gcc's -fanalyzer option: I think this one happens because on line 1317:

  struct stream_ctx *stream = H3_STREAM_CTX(data);

... and that macro checks for data being NULL and if so it will return NULL and then there would be a NULL dereference. Of course that will only happen if data is NULL when this function is called.

There's also an assert there that would trigger if stream ever was NULL there in a debug build.

It's not immediately obvious to me how to silence this warning.

[bagder]

@icing can you give a shot?

[vszakats]

It slips through CI because all ngtcp2 builds have debug enabled, and in those cases the assert catches it. This silences the warning for non-debug builds:

--- a/lib/vquic/curl_ngtcp2.c
+++ b/lib/vquic/curl_ngtcp2.c
@@ -1319,6 +1319,8 @@ static ssize_t recv_closed_stream(struct Curl_cfilter *cf,
 
   (void)cf;
   DEBUGASSERT(stream);
+  if(!stream)
+    goto out;
   if(stream->reset) {
     failf(data,
           "HTTP/3 stream %" PRId64 " reset by server", stream->id);

[icing]

Pushed #11151 as fix for this. The warning case never happend as this static function was only called in a context where stream had already been checked. Fixing this by making stream a parameter.