github / codeql

CodeQL: the libraries and queries that power security researchers around the world, as well as code scanning in GitHub Advanced Security
https://codeql.github.com
MIT License
7.51k stars 1.49k forks source link

error: expression preceding parentheses of apparent call must have (pointer-to-) function type #17009

Open flowerhack opened 1 month ago

flowerhack commented 1 month ago

Hey there,

I'm a committer for the Chromium project & we've been experimenting with building CodeQL databases of Chromium.

As of right now, when compiling Chromium, we see ~80,000 errors of the form: error: expression preceding parentheses of apparent call must have (pointer-to-) function type.

I have attempted to make a standalone reproducer for this error. However, curiously, these errors seem to vanish if the file is first preprocessed before being compiled as part of a codeql database trace-command directive.

This has caused me to wonder:

(1) Are these errors spurious or otherwise incosequential to the final database being produced? I would not be surprised if e.g. CodeQL raises an error during the preprocessor step, but still successfully compiles the code in question into the database. If that is the case, I'll be happy simply knowing that these errors can be disregarded.

(2) If these errors ARE consequential and causing the database to be incomplete, is it possible to triage & fix this bug without a standalone reproducer?

In lieu of a reproducer, I can give a few examples of the specific places this error seems to crop up:

When compiling BoringSSL:

Command: /b/s/w/ir/x/w/codeql/cpp/tools/linux64/extractor --mimic '/b/s/w/ir/x/w/src/third_party/llvm-build/Release+Asserts/bin/clang' -MMD -MF obj/third_party/boringssl/boringssl/x509spki.o.d -DDCHECK_ALWAYS_ON=1 -DUSE_UDEV -DUSE_AURA=1 -DUSE_GLIB=1 -DUSE_OZONE=1 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_GNU_SOURCE -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_EXTENSIVE '-DCR_CLANG_REVISION="llvmorg-19-init-10646-g084e2b53-57"' -DCOMPONENT_BUILD -DCR_LIBCXX_REVISION=852bc6746f45add53fec19f3a29280e69e358d44 -DTEMP_REBUILD_HACK_ASSERTION_HANDLER -DCR_SYSROOT_KEY=20230611T210420Z-2 -D_DEBUG -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DBORINGSSL_IMPLEMENTATION -DBORINGSSL_ALLOW_CXX_RUNTIME -DBORINGSSL_NO_STATIC_INITIALIZER -DOPENSSL_SMALL -DBORINGSSL_SHARED_LIBRARY -I../.. -Igen '-I../../buildtools/third_party/libc++' -I../../third_party/boringssl/src/include -fno-delete-null-pointer-checks -fno-ident -fno-strict-aliasing -fstack-protector -funwind-tables -fPIC -pthread -fcolor-diagnostics -fmerge-all-constants -fno-sized-deallocation -fcrash-diagnostics-dir=../../tools/clang/crashreports -mllvm -instcombine-lower-dbg-declare=0 -mllvm -split-threshold-for-reg-with-hint=0 -ffp-contract=off -fcomplete-member-pointers -m64 -msse3 -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -ffile-compilation-dir=. -no-canonical-prefixes -ftrivial-auto-var-init=pattern -O0 -fno-omit-frame-pointer -gdwarf-4 -g2 -gdwarf-aranges -gsplit-dwarf -ggnu-pubnames -fvisibility=hidden -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -DUNSAFE_BUFFERS_BUILD -Wall -Wno-unused-variable '-Wno-c++11-narrowing' -Wno-unused-but-set-variable -Wno-misleading-indentation -Wno-missing-field-initializers -Wno-unused-parameter -Wno-psabi -Wloop-analysis -Wno-unneeded-internal-declaration -Wno-cast-function-type -Wno-ignored-pragma-optimize -Wno-deprecated-builtins -Wno-bitfield-constant-conversion -Wno-deprecated-this-capture -Wno-invalid-offsetof -Wno-vla-extension -Wno-thread-safety-reference-return -Werror -std=c11 --sysroot=../../build/linux/debian_bullseye_amd64-sysroot -c ../../third_party/boringssl/src/crypto/x509/x509spki.c -o obj/third_party/boringssl/boringssl/x509spki.o

We see that the extractor is unhappy about various stddef includes, including but not limited to __stddef_size_t.h, __stddef_null.h, __stddef_ptrdiff_t.h, __stddef_wchar_t.h, and __stddef_offsetof.h:

Warning[extractor-c++]: In construct_text_message: "/b/s/w/ir/x/w/src/third_party/llvm-build/Release+Asserts/lib/clang/19/include/__stddef_size_t.h", line 15: error: expression preceding parentheses of apparent call must have (pointer-to-) function type
      (__has_feature(modules) && !__building_module(_Builtin_stddef))
                                  ^
...
Warning[extractor-c++]: In construct_text_message: "/b/s/w/ir/x/w/src/third_party/llvm-build/Release+Asserts/lib/clang/19/include/__stddef_null.h", line 10: error: expression preceding parentheses of apparent call must have (pointer-to-) function type
  #if !defined(NULL) || !__building_module(_Builtin_stddef)
                         ^
...
Warning[extractor-c++]: In construct_text_message: "/b/s/w/ir/x/w/src/third_party/llvm-build/Release+Asserts/lib/clang/19/include/__stddef_ptrdiff_t.h", line 15: error: expression preceding parentheses of apparent call must have (pointer-to-) function type
      (__has_feature(modules) && !__building_module(_Builtin_stddef))
                                  ^

In general it seems a lot of these errors trace back to the __has_feature preprocessor feature and the __build_module Clang builtin.

I have attached a few extractor logs that include instances of this error occurring, in case that's helpful.

If this is not sufficient information to determine a fix (or, if a reproducer would make a fix easier in any way), please do let me know, and I'll see if there's something else I can do to provide one.

Thank you!

16302.log dca2d.log f6fca.log

aibaars commented 1 month ago

Thanks for the report! I'll pass it on to the team.

jketema commented 1 month ago

Hi @flowerhack,

The problem disappears after preprocessing, because __building_module is one of clang's builtin macros, and all macros and their uses will of course be evaluated during preprocessing. So, it makes sense that the problem doesn't show up once you've preprocessed the source file.

The cause here is that we currently do not support the __building_module macro. We were aware of this problem already, and we have reported it to our frontend provider earlier this year.

Whether the problem affects your database in any serious way will depend, and might be easier for you to judge than for me. Our frontend will always set __has_feature(modules) equal to 0, and it will skip any block of code guarded by an #if where the condition includes a __building_module somewhere.

What we can do is have our frontend always define __building_module as 0, whether this is sufficient for you will depend on whether Chromium is actually using modules anywhere. Modules are something we currently do not support.

rvermeulen commented 2 weeks ago

@flowerhack do you have any further questions?