-
Notifications
You must be signed in to change notification settings - Fork 30
Commit 7631345
committed
type-context: disable clang-tidy UndefinedBinaryOperatorResult error
Reported by fanquake <[email protected]>
bitcoin/bitcoin#31802 (comment)
https://cirrus-ci.com/task/6552721135763456
https://api.cirrus-ci.com/v1/task/6552721135763456/logs/ci.log
Error is spurious and comes from kj/async-inl.h and should be suppressed in the
next version of capnproto capnproto/capnproto#2334
The error is a clang-analyzer false positive that comes from ABI-specific code
in Cap'n Proto that gets the starting function address (that can be passed to
addr2line) from a lambda or function object. This code calls a helper to get
the starting function address from a pointer-to-member-function, which in this
case is the the operator() member function. That code handles pointers to
virtual member functions, so it checks if the pointer is virtual by testing its
low-order bit, and if set, assumes the first bytes of the object are a vtable
pointer, and does pointer arithmetic with the vtable address. Clang-tidy
complains about this because it does not know the vtable address is valid,
assuming incorrectly it is a "garbage value".
This change turns off the UndefinedBinaryOperatorResult altogether instead of
suppressing this one instance because clang-tidy incorrectly considers this
error to come from "main file" of the translation unit (see
https://clang.llvm.org/extra/clang-tidy/, https://stackoverflow.com/a/47611238,
https://reviews.llvm.org/D26418). So it is not suppressed even though the
header is included via -isystem and clang-tidy --dump-config shows
"SystemHeaders: false". It is also not suppressed when exclude patterns are
added to the .clang-tidy configuration like:
HeaderFilterRegex: '.*'
ExcludeHeaderFilterRegex: '.*/include/kj/async-inl\.h$'
This has no effect because ExcludeHeaderFilterRegex does not override the
isInMainFile condition (https://github.com/llvm/llvm-project/pull/91400/files).
Adding NOLINT to the getLocalServer() line in types-context.h at the boundary
between libmultiprocess and Cap'n Proto code also does not suppress the error.
It does suppress clang-tidy "note:" lines below the NOLINT point in the call
stack, making the error messages shorter, but the only way of suppressing the
error completely seems to be either adding NOLINT inside the Cap'n Proto
header, which requires a patch, or adding it to all top-level callers of the
getLocalServer() function in .cpp files, which seems impractical and overbroad,
and I didn't attempt.
Complete error output is:
/nix/store/46kiq9naswgbqfc14kc9nxcbgd0rv0m2-capnproto-1.1.0/include/kj/async-inl.h:609:37: error: The left operand of '+' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult,-warnings-as-errors]
609 | return *(void**)(*(char**)obj + voff);
| ^
build/test/mp/test/foo.capnp.proxy-server.c++:93:12: note: Calling 'serverInvoke<mp::ProxyServer<mp::test::messages::FooFn>, capnp::CallContext<mp::test::messages::FooFn::CallParams, mp::test::messages::FooFn::CallResults>, mp::ServerField<0, mp::Accessor<mp::foo_fields::Context, 17>, mp::ServerRet<mp::Accessor<mp::foo_fields::Result, 2>, mp::ServerCall>>>'
93 | return serverInvoke(*this, call_context, MakeServerField<0, Accessor<foo_fields::Context, FIELD_IN | FIELD_BOXED>>(Make<ServerRet, Accessor<foo_fields::Result, FIELD_OUT>>(ServerCall())));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/mp/proxy-types.h:739:16: note: Calling 'ReplaceVoid<(lambda at include/mp/proxy-types.h:739:28), (lambda at include/mp/proxy-types.h:740:13)>'
739 | return ReplaceVoid([&]() { return fn.invoke(server_context, ArgList()); },
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
740 | [&]() { return kj::Promise<CallContext>(kj::mv(call_context)); })
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/mp/proxy-types.h:700:19: note: 'is_same_v' is false
700 | if constexpr (std::is_same_v<decltype(fn()), void>) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/mp/proxy-types.h:700:5: note: Taking false branch
700 | if constexpr (std::is_same_v<decltype(fn()), void>) {
| ^
include/mp/proxy-types.h:704:16: note: Calling 'operator()'
704 | return fn();
| ^~~~
include/mp/proxy-types.h:739:43: note: Calling 'ServerField::invoke'
739 | return ReplaceVoid([&]() { return fn.invoke(server_context, ArgList()); },
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/mp/proxy-types.h:563:16: note: Calling 'PassField<mp::Accessor<mp::foo_fields::Context, 17>, mp::ServerInvokeContext<mp::ProxyServer<mp::test::messages::FooFn>, capnp::CallContext<mp::test::messages::FooFn::CallParams, mp::test::messages::FooFn::CallResults>>, mp::ServerRet<mp::Accessor<mp::foo_fields::Result, 2>, mp::ServerCall>, mp::TypeList<>>'
563 | return PassField<Accessor>(Priority<2>(),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
564 | typename Split<argc, ArgTypes>::First(),
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
565 | server_context,
| ~~~~~~~~~~~~~~~
566 | this->parent(),
| ~~~~~~~~~~~~~~~
567 | typename Split<argc, ArgTypes>::Second(),
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
568 | std::forward<Args>(args)...);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/mp/type-context.h:151:12: note: Calling 'CapabilityServerSet::getLocalServer'
151 | return server.m_context.connection->m_threads.getLocalServer(thread_client)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/nix/store/46kiq9naswgbqfc14kc9nxcbgd0rv0m2-capnproto-1.1.0/include/capnp/capability.h:1274:10: note: Calling 'Promise::then'
1274 | return getLocalServerInternal(client)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1275 | .then([](void* server) -> kj::Maybe<typename T::Server&> {
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1276 | if (server == nullptr) {
| ~~~~~~~~~~~~~~~~~~~~~~~~
1277 | return nullptr;
| ~~~~~~~~~~~~~~~
1278 | } else {
| ~~~~~~~~
1279 | return *reinterpret_cast<typename T::Server*>(server);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1280 | }
| ~
1281 | });
| ~~
/nix/store/46kiq9naswgbqfc14kc9nxcbgd0rv0m2-capnproto-1.1.0/include/kj/async-inl.h:1295:32: note: Calling 'GetFunctorStartAddress::apply'
1295 | void* continuationTracePtr = _::GetFunctorStartAddress<_::FixVoid<T>&&>::apply(func);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/nix/store/46kiq9naswgbqfc14kc9nxcbgd0rv0m2-capnproto-1.1.0/include/kj/async-inl.h:677:12: note: Calling 'PtmfHelper::apply'
677 | return PtmfHelper::from<ReturnType, Decay<Func>, ParamTypes...>(
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
678 | &Decay<Func>::operator()).apply(&func);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/nix/store/46kiq9naswgbqfc14kc9nxcbgd0rv0m2-capnproto-1.1.0/include/kj/async-inl.h:606:9: note: Assuming the condition is true
606 | if (voff & 1) {
| ^~~~~~~~
/nix/store/46kiq9naswgbqfc14kc9nxcbgd0rv0m2-capnproto-1.1.0/include/kj/async-inl.h:606:5: note: Taking true branch
606 | if (voff & 1) {
| ^
/nix/store/46kiq9naswgbqfc14kc9nxcbgd0rv0m2-capnproto-1.1.0/include/kj/async-inl.h:609:37: note: The left operand of '+' is a garbage value
609 | return *(void**)(*(char**)obj + voff);
| ~~~~~~~~~~~~ ^1 parent 4896e7f commit 7631345Copy full SHA for 7631345
Expand file treeCollapse file tree
1 file changed
+3
-2
lines changed+3-2Lines changed: 3 additions & 2 deletions
Original file line number | Diff line number | Diff line change | |
---|---|---|---|
| |||
26 | 26 |
| |
27 | 27 |
| |
28 | 28 |
| |
29 |
| - | |
30 |
| - | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
31 | 32 |
| |
32 | 33 |
| |
33 | 34 |
| |
|
0 commit comments