Add CI jobs and fix clang-tidy and iwyu errors #184
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
src/mp/gen.cpp:82:13: error: function 'ForEachMethod' is within a recursive call chain [misc-no-recursion,-warnings-as-errors] 82 | static void ForEachMethod(const capnp::InterfaceSchema& interface, const std::function<void(const capnp::InterfaceSchema& interface, const capnp::InterfaceSchema::Method)>& callback)
include/mp/proxy-types.h:203:98: error: 'values' used after it was forwarded [bugprone-use-after-move,-warnings-as-errors]
203 | CustomBuildField(TypeList<LocalTypes...>(), Priority<3>(), context, std::forward<Values>(values)...,
| ^
include/mp/proxy-types.h:202:9: note: forward occurred here
202 | if (CustomHasValue(context, std::forward<Values>(values)...)) {
| ^
include/mp/proxy-types.h:333:74: error: 'arg2' used after it was forwarded [bugprone-use-after-move,-warnings-as-errors]
333 | next_fn.handleChain(std::forward<Arg1>(arg1), std::forward<Arg2>(arg2), typename S::Second(),
| ^
include/mp/proxy-types.h:332:9: note: forward occurred here
332 | handleChain(std::forward<Arg1>(arg1), std::forward<Arg2>(arg2), typename S::First());
include/mp/proxy-types.h:399:70: error: 'values' used after it was forwarded [bugprone-use-after-move,-warnings-as-errors]
399 | ParamList(), Priority<1>(), std::forward<Values>(values)..., Make<StructField, Accessor>(params));
| ^
include/mp/proxy-types.h:396:17: note: forward occurred here
396 | MaybeBuildField(std::integral_constant<bool, Accessor::in>(), ParamList(), invoke_context,
include/mp/type-number.h:53:77: error: use nullptr [modernize-use-nullptr,-warnings-as-errors]
53 | typename std::enable_if<std::is_enum<LocalType>::value>::type* enable = 0)
| ^
| nullptr
mp/include/mp/util.h:173:5: error: use '= default' to define a trivial destructor [modernize-use-equals-default,-warnings-as-errors]
173 | ~Lock() MP_RELEASE() {}
| ^ ~~
| = default;
This error should not be a real problem because code is taking an invalid reference to an empty object that has no state and could never be used. But taking the reference could technically be undefined behavior. 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 include/mp/proxy-types.h:134:5: error: Address of stack memory associated with temporary object of type '(lambda at include/mp/proxy-types.h:134:51)' is still referred to by a temporary object on the stack upon returning to the caller. This will be a dangling reference [clang-analyzer-core.StackAddressEscape,-warnings-as-errors] 134 | return ReadDestEmplace{TypeList<LocalType>(), [&](auto&&... args) -> decltype(auto) { | ^ build/test/mp/test/foo.capnp.proxy-server.c++:51:12: note: Calling 'serverInvoke<mp::ProxyServer<mp::test::messages::FooInterface>, capnp::CallContext<mp::test::messages::FooInterface::PassCustomParams, mp::test::messages::FooInterface::PassCustomResults>, mp::ServerField<1, mp::Accessor<mp::foo_fields::Arg, 17>, mp::ServerRet<mp::Accessor<mp::foo_fields::Result, 18>, mp::ServerCall>>>' 51 | return serverInvoke(*this, call_context, MakeServerField<1, Accessor<foo_fields::Arg, FIELD_IN | FIELD_BOXED>>(Make<ServerRet, Accessor<foo_fields::Result, FIELD_OUT | FIELD_BOXED>>(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 true 700 | if constexpr (std::is_same_v<decltype(fn()), void>) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/mp/proxy-types.h:700:5: note: Taking true branch 700 | if constexpr (std::is_same_v<decltype(fn()), void>) { | ^ include/mp/proxy-types.h:701:9: note: Calling 'operator()' 701 | 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::Arg, 17>, mp::test::FooCustom, mp::ServerInvokeContext<mp::ProxyServer<mp::test::messages::FooInterface>, capnp::CallContext<mp::test::messages::FooInterface::PassCustomParams, mp::test::messages::FooInterface::PassCustomResults>>, const mp::ServerRet<mp::Accessor<mp::foo_fields::Result, 18>, 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/proxy-types.h:304:5: note: Calling 'MaybeReadField<mp::TypeList<mp::test::FooCustom>, mp::InvokeContext &, mp::StructField<mp::Accessor<mp::foo_fields::Arg, 17>, const mp::test::messages::FooInterface::PassCustomParams::Reader>, mp::ReadDestEmplace<mp::test::FooCustom, (lambda at include/mp/proxy-types.h:305:83)>>' 304 | MaybeReadField(std::integral_constant<bool, Accessor::in>(), TypeList<ArgType>(), invoke_context, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 305 | Make<StructField, Accessor>(params), ReadDestEmplace(TypeList<ArgType>(), [&](auto&&... args) -> auto& { | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 306 | param.emplace(std::forward<decltype(args)>(args)...); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 307 | return *param; | ~~~~~~~~~~~~~~ 308 | })); | ~~~ include/mp/proxy-types.h:276:5: note: Calling 'ReadField<mp::test::FooCustom, mp::InvokeContext &, mp::StructField<mp::Accessor<mp::foo_fields::Arg, 17>, const mp::test::messages::FooInterface::PassCustomParams::Reader>, mp::ReadDestEmplace<mp::test::FooCustom, (lambda at include/mp/proxy-types.h:305:83)>>' 276 | ReadField(std::forward<Args>(args)...); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/mp/proxy-types.h:175:12: note: Calling 'CustomReadField<mp::StructField<mp::Accessor<mp::foo_fields::Arg, 17>, const mp::test::messages::FooInterface::PassCustomParams::Reader>, mp::ReadDestEmplace<mp::test::FooCustom, (lambda at include/mp/proxy-types.h:305:83)>>' 175 | return CustomReadField(TypeList<RemoveCvRef<LocalTypes>...>(), Priority<2>(), std::forward<Args>(args)...); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ test/mp/test/foo-types.h:51:12: note: Calling 'ReadDestEmplace::update' 51 | return read_dest.update([&](FooCustom& value) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 52 | value.v1 = ReadField(TypeList<std::string>(), invoke_context, mp::Make<mp::ValueField>(custom.getV1()), ReadDestTemp<std::string>()); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 53 | value.v2 = custom.getV2(); | ~~~~~~~~~~~~~~~~~~~~~~~~~~ 54 | }); | ~~ include/mp/proxy-types.h:112:23: note: 'is_const_v' is false 112 | if constexpr (std::is_const_v<std::remove_reference_t<std::invoke_result_t<EmplaceFn>>>) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/mp/proxy-types.h:112:9: note: Taking false branch 112 | if constexpr (std::is_const_v<std::remove_reference_t<std::invoke_result_t<EmplaceFn>>>) { | ^ include/mp/proxy-types.h:122:13: note: Calling 'operator()' 122 | update_fn(temp); | ^~~~~~~~~~~~~~~ test/mp/test/foo-types.h:52:113: note: Calling 'ReadDestTemp<std::basic_string<char>>' 52 | value.v1 = ReadField(TypeList<std::string>(), invoke_context, mp::Make<mp::ValueField>(custom.getV1()), ReadDestTemp<std::string>()); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ include/mp/proxy-types.h:134:5: note: Address of stack memory associated with temporary object of type '(lambda at include/mp/proxy-types.h:134:51)' is still referred to by a temporary object on the stack upon returning to the caller. This will be a dangling reference 134 | return ReadDestEmplace{TypeList<LocalType>(), [&](auto&&... args) -> decltype(auto) { | ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 135 | return LocalType{std::forward<decltype(args)>(args)...}; | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 136 | }}; | ~
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 include/mp/type-number.h:55:32: error: The value '0' provided to the cast expression is not in the valid range of values for 'FooEnum' [clang-analyzer-optin.core.EnumCastOutOfRange,-warnings-as-errors] 55 | return read_dest.construct(static_cast<LocalType>(input.get())); | ^ test/mp/test/foo.h:26:12: note: enum declared here 26 | enum class FooEnum : uint8_t { ONE = 1, TWO = 2, }; | ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ build/test/mp/test/foo.capnp.proxy-server.c++:63:12: note: Calling 'serverInvoke<mp::ProxyServer<mp::test::messages::FooInterface>, capnp::CallContext<mp::test::messages::FooInterface::PassEnumParams, mp::test::messages::FooInterface::PassEnumResults>, mp::ServerField<1, mp::Accessor<mp::foo_fields::Arg, 1>, mp::ServerRet<mp::Accessor<mp::foo_fields::Result, 2>, mp::ServerCall>>>' 63 | return serverInvoke(*this, call_context, MakeServerField<1, Accessor<foo_fields::Arg, FIELD_IN>>(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 true 700 | if constexpr (std::is_same_v<decltype(fn()), void>) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/mp/proxy-types.h:700:5: note: Taking true branch 700 | if constexpr (std::is_same_v<decltype(fn()), void>) { | ^ include/mp/proxy-types.h:701:9: note: Calling 'operator()' 701 | 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::Arg, 1>, mp::test::FooEnum, mp::ServerInvokeContext<mp::ProxyServer<mp::test::messages::FooInterface>, capnp::CallContext<mp::test::messages::FooInterface::PassEnumParams, mp::test::messages::FooInterface::PassEnumResults>>, const 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/proxy-types.h:304:5: note: Calling 'MaybeReadField<mp::TypeList<mp::test::FooEnum>, mp::InvokeContext &, mp::StructField<mp::Accessor<mp::foo_fields::Arg, 1>, const mp::test::messages::FooInterface::PassEnumParams::Reader>, mp::ReadDestEmplace<mp::test::FooEnum, (lambda at include/mp/proxy-types.h:305:83)>>' 304 | MaybeReadField(std::integral_constant<bool, Accessor::in>(), TypeList<ArgType>(), invoke_context, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 305 | Make<StructField, Accessor>(params), ReadDestEmplace(TypeList<ArgType>(), [&](auto&&... args) -> auto& { | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 306 | param.emplace(std::forward<decltype(args)>(args)...); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 307 | return *param; | ~~~~~~~~~~~~~~ 308 | })); | ~~~ include/mp/proxy-types.h:276:5: note: Calling 'ReadField<mp::test::FooEnum, mp::InvokeContext &, mp::StructField<mp::Accessor<mp::foo_fields::Arg, 1>, const mp::test::messages::FooInterface::PassEnumParams::Reader>, mp::ReadDestEmplace<mp::test::FooEnum, (lambda at include/mp/proxy-types.h:305:83)>>' 276 | ReadField(std::forward<Args>(args)...); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/mp/proxy-types.h:175:12: note: Calling 'CustomReadField<mp::test::FooEnum, mp::StructField<mp::Accessor<mp::foo_fields::Arg, 1>, const mp::test::messages::FooInterface::PassEnumParams::Reader>, mp::ReadDestEmplace<mp::test::FooEnum, (lambda at include/mp/proxy-types.h:305:83)>>' 175 | return CustomReadField(TypeList<RemoveCvRef<LocalTypes>...>(), Priority<2>(), std::forward<Args>(args)...); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/mp/type-number.h:55:55: note: Calling 'StructField::get' 55 | return read_dest.construct(static_cast<LocalType>(input.get())); | ^~~~~~~~~~~ include/mp/proxy-types.h:40:41: note: Calling 'Arg::get' 40 | decltype(auto) get() const { return Accessor::get(this->m_struct); } | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ build/test/mp/test/foo.capnp.proxy.h:198:82: note: Calling 'Reader::getArg' 198 | template<typename S> static auto get(S&& s) -> decltype(s.getArg()) { return s.getArg(); } | ^~~~~~~~~~ build/test/mp/test/foo.capnp.h:7042:10: note: Calling 'StructReader::getDataField' 7042 | return _reader.getDataField< ::int32_t>( | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 7043 | ::capnp::bounded<0>() * ::capnp::ELEMENTS); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /nix/store/46kiq9naswgbqfc14kc9nxcbgd0rv0m2-capnproto-1.1.0/include/capnp/layout.h:1099:7: note: Assuming the condition is false 1099 | if ((offset + ONE * ELEMENTS) * capnp::bitsPerElement<T>() <= dataSize) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /nix/store/46kiq9naswgbqfc14kc9nxcbgd0rv0m2-capnproto-1.1.0/include/capnp/layout.h:1099:3: note: Taking false branch 1099 | if ((offset + ONE * ELEMENTS) * capnp::bitsPerElement<T>() <= dataSize) { | ^ /nix/store/46kiq9naswgbqfc14kc9nxcbgd0rv0m2-capnproto-1.1.0/include/capnp/layout.h:1102:5: note: Returning zero 1102 | return static_cast<T>(0); | ^~~~~~~~~~~~~~~~~~~~~~~~ build/test/mp/test/foo.capnp.h:7042:10: note: Returning from 'StructReader::getDataField' 7042 | return _reader.getDataField< ::int32_t>( | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 7043 | ::capnp::bounded<0>() * ::capnp::ELEMENTS); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ build/test/mp/test/foo.capnp.h:7042:3: note: Returning zero 7042 | return _reader.getDataField< ::int32_t>( | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 7043 | ::capnp::bounded<0>() * ::capnp::ELEMENTS); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ build/test/mp/test/foo.capnp.proxy.h:198:82: note: Returning from 'Reader::getArg' 198 | template<typename S> static auto get(S&& s) -> decltype(s.getArg()) { return s.getArg(); } | ^~~~~~~~~~ build/test/mp/test/foo.capnp.proxy.h:198:75: note: Returning zero 198 | template<typename S> static auto get(S&& s) -> decltype(s.getArg()) { return s.getArg(); } | ^~~~~~~~~~~~~~~~~ include/mp/proxy-types.h:40:41: note: Returning from 'Arg::get' 40 | decltype(auto) get() const { return Accessor::get(this->m_struct); } | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/mp/proxy-types.h:40:34: note: Returning zero 40 | decltype(auto) get() const { return Accessor::get(this->m_struct); } | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/mp/type-number.h:55:55: note: Returning from 'StructField::get' 55 | return read_dest.construct(static_cast<LocalType>(input.get())); | ^~~~~~~~~~~ include/mp/type-number.h:55:32: note: The value '0' provided to the cast expression is not in the valid range of values for 'FooEnum' 55 | return read_dest.construct(static_cast<LocalType>(input.get())); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
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); | ~~~~~~~~~~~~ ^
|
Not sure why CI does not seem to run as part of this PR, but CI was successful in the branch behind the PR:
Maybe this PR has to be merged first before CI will run on any PRs. |
|
I'll submit a patch to cap'n proto to avoid the Ideally we'd never see this error because cap'n proto headers are included with |
maybe the "actions" has to be enabled? At least on https://github.com/bitcoin/bitcoin/actions I see the tab, but https://github.com/bitcoin-core/libmultiprocess/actions is a 404 right now. See https://github.com/bitcoin-core/libmultiprocess/settings/actions |
Thanks! That seems right. I can't change the setting there since I see "This setting has been disabled by organization administrators" but I assume if gets changed from "Disable actions" to one of the other settings this should start to work. |
|
re: #184 (comment)
Opened capnproto/capnproto#2334 with this Updated 48ecb5f -> 80f1c2e ( |
|
Actions should be available in this repository now. |
maflcko
reviewed
Jun 23, 2025
|
When I update the subtree to use this in bitcoin/bitcoin#31802 my macOS build fails: Is this expected? Other than that the code changes in 80f1c2e seem fine, with a combination of pleasing tidy and skipping some checks. I'd like to test it on top of #31802 to make sure we caught all the Pokemons though. |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
Also add IWYU to the default job, since using libc++ instead of libstdc++ causes IWYU to give slightly different recommendations. Better to comply with both sets of recommendations so IWYU is usable on more platforms.
Bitcoin tidy plugin isn't currently enabled here so they don't do anything. Suggested by maflcko <[email protected]> bitcoin-core#184 (comment)
ryanofsky
added a commit
to ryanofsky/libmultiprocess
that referenced
this pull request
Jun 23, 2025
Bitcoin tidy plugin isn't currently enabled here so they don't do anything. Suggested by maflcko <[email protected]> bitcoin-core#184 (comment)
ryanofsky
commented
Jun 23, 2025
There was a problem hiding this comment.
Updated 80f1c2e -> f15724a (pr/ci.2 -> pr/ci.3, compare, CI) expanding ci job to cover libc++ and gcc, rewriting a commit message, and making a few other tweaks.
re: #184 (comment)
When I update the subtree to use this in bitcoin/bitcoin#31802 my macOS build fails:
I misread this error and didn't realize it was coming from the bitcoin build not the libmultiprocess build, and assumed it might be caused a libc++ / libstdc++ difference (which is why I added libstdc++ ci job).
But I think this error is just caused by #160 and should be fixed by bitcoin/bitcoin@d234efe from bitcoin/bitcoin#32345
re: #184 (comment)
Actions should be available in this repository now.
Thanks I saw the setting on https://github.com/bitcoin-core/libmultiprocess/settings/actions was changed from "Disable actions" to "Allow bitcoin-core actions and reusable workflows." Though after another push it still seems like PR actions don't run here, but not sure if that is because of settings, or because no actions are defined on the master branch right now.
To experiment I did change "Allow bitcoin-core, and select non-bitcoin-core, actions and reusable workflows" and filled in "Allow specified actions and reusable workflows" list with:
- actions/checkout@*
- cachix/install-nix-action@*,
So maybe that will have an effect if there is another push.
|
Should probably add documentation about this, but with latest push it should be possible to run the two CI jobs locally on linux if nix is installed with: CI_CONFIG=ci/configs/gcc-libstdc++.sh ci/run.sh
CI_CONFIG=ci/configs/clang-libc++.sh ci/run.sh |
hebasto
reviewed
Jun 25, 2025
hebasto
reviewed
Jun 25, 2025
hebasto
reviewed
Jun 25, 2025
ryanofsky
added a commit
to ryanofsky/libmultiprocess
that referenced
this pull request
Jun 25, 2025
Bitcoin tidy plugin isn't currently enabled here so they don't do anything. Suggested by maflcko <[email protected]> bitcoin-core#184 (comment)
ryanofsky
added a commit
to ryanofsky/libmultiprocess
that referenced
this pull request
Jun 25, 2025
Suggested by maflcko <[email protected]> bitcoin-core#184 (comment)
ryanofsky
changed the title
hebasto
reviewed
Jun 25, 2025
ryanofsky
added a commit
to ryanofsky/libmultiprocess
that referenced
this pull request
Jun 25, 2025
Bitcoin tidy plugin isn't currently enabled here so they don't do anything. Suggested by maflcko <[email protected]> bitcoin-core#184 (comment)
ryanofsky
added a commit
to ryanofsky/libmultiprocess
that referenced
this pull request
Jun 25, 2025
Suggested by maflcko <[email protected]> bitcoin-core#184 (comment)
ryanofsky
added a commit
to ryanofsky/libmultiprocess
that referenced
this pull request
Jun 25, 2025
Bitcoin tidy plugin isn't currently enabled here so they don't do anything. Suggested by maflcko <[email protected]> bitcoin-core#184 (comment)
ryanofsky
added a commit
to ryanofsky/libmultiprocess
that referenced
this pull request
Jun 25, 2025
Suggested by maflcko <[email protected]> bitcoin-core#184 (comment)
hebasto
approved these changes
Jun 25, 2025
|
Thanks for the review! I pushed another update since the I'll try to avoid making more changes here and add new jobs (sanitizers) in future PRs. Updated 1bfed25 -> 757e13a ( |
|
ACK 757e13a I ran the three ci scripts on Ubuntu. We'll need to be careful when updating the subtree in bitcoin/bitcoin#32345. But debugging that should be easier with the new CI coverage added in this PR. |
|
for reference the error is: https://github.com/bitcoin-core/libmultiprocess/actions/runs/15907709886 |
|
Thanks, as mentioned #184 (review) I did try to allow these actions but probably didn't do it right. The current setting looks like:
|
|
Problem with github actions seems to be fixed. CI jobs appear at the bottom of a PR I just opened #186. I updated the list of allowed actions in settings to |
Sjors
added a commit
to Sjors/bitcoin
that referenced
this pull request
Jun 27, 2025
1ff5ad1ef9 ci: add thread sanitizer job ff5141c5df proxy-io: fix race condition ProxyClientBase cleanup handler 5975fef52c mptest: fix race condition in TestSetup constructor 8954cc0377 Merge bitcoin-core/libmultiprocess#184: Add CI jobs and fix clang-tidy and iwyu errors 757e13a755 ci: add gnu32 cross-compiled 32-bit build 15bf349000 doc: fix typo found by DrahtBot 1a598d5905 clang-tidy: drop 'bitcoin-*' check cbb1e43fdc ci: test libc++ instead of libstdc++ in one job 76313450c2 type-context: disable clang-tidy UndefinedBinaryOperatorResult error 4896e7fe51 proxy-types: fix clang-tidy EnumCastOutOfRange error 060a739269 proxy-types: fix clang-tidy StackAddressEscape error 977d721020 ci: add github actions jobs testing gcc, clang-20, clang-tidy, and iwyu 0d5f1faae5 iwyu: fix add/remove include errors 753d2b10cc util: fix clang-tidy modernize-use-equals-default error ae4f1dc2bb type-number: fix clang-tidy modernize-use-nullptr error 07a741bf69 proxy-types: fix clang-tidy bugprone-use-after-move error 3673114bc9 proxy-types: fix clang-tidy bugprone-use-after-move error 422923f384 proxy-types: fix clang-tidy bugprone-use-after-move error c6784c6ade mpgen: disable clang-tidy misc-no-recursion error c5498aa11b tidy: copy clang-tidy file from bitcoin core 258a617c1e Merge bitcoin-core/libmultiprocess#160: refactor: EventLoop locking cleanups + client disconnect exception 84cf56a0b5 test: Test disconnects during IPC calls 949573da84 Prevent IPC server crash if disconnected during IPC call 0198397580 Merge bitcoin-core/libmultiprocess#179: scripted-diff: Remove copyright year (ranges) ea38392960 Prevent EventLoop async cleanup thread early exit during shutdown 616d9a75d2 doc: Document ProxyClientBase destroy_connection option 56fff76f94 Improve IPC client disconnected exceptions 9b8ed3dc5f refactor: Add clang thread safety annotations to EventLoop 52256e730f refactor: Remove DestructorCatcher and AsyncCallable f24894794a refactor: Drop addClient/removeClient methods 2b830e558e refactor: Use EventLoopRef instead of addClient/removeClient 315ff537fb refactor: Add ProxyContext EventLoop* member 9aaeec3678 proxy-io.h: Add EventLoopRef RAII class handle addClient/removeClient refcounting f58c8d8ba2 proxy-io.h: Add more detailed EventLoop comment 5108445e5d test: Add test coverage for client & server disconnections 59030c68cb Merge bitcoin-core/libmultiprocess#181: type-function.h: Fix CustomBuildField overload 688140b1df test: Add coverage for type-function.h 8b96229da5 type-function.h: Fix CustomBuildField overload fa2ff9a668 scripted-diff: Remove copyright year (ranges) git-subtree-dir: src/ipc/libmultiprocess git-subtree-split: 1ff5ad1ef9cedd45834ee2bb522c56b3afe89c60
Sjors
added a commit
to Sjors/bitcoin
that referenced
this pull request
Jun 28, 2025
258b83cdba proxy-io: fix race conditions in disconnect callback code f01867805e proxy-io: fix race conditions in ProxyClientBase cleanup handler 6f72e105da refactor: Rename ProxyClient cleanup_it variable 3b138eca55 refactor: rename ProxyClient<Thread>::m_cleanup_it 266c7446e2 mptest: fix race condition in TestSetup constructor 54f3e3b61c ci: add thread sanitizer job c0efaa5e8c Merge bitcoin-core/libmultiprocess#187: ci: have bash scripts explicitly opt out of locale dependence. 3a6db38e56 ci: rename configs to .bash 401e0ce1d9 ci: add copyright to bash scripts e956467ae4 ci: export LC_ALL 8954cc0377 Merge bitcoin-core/libmultiprocess#184: Add CI jobs and fix clang-tidy and iwyu errors 757e13a755 ci: add gnu32 cross-compiled 32-bit build 15bf349000 doc: fix typo found by DrahtBot 1a598d5905 clang-tidy: drop 'bitcoin-*' check cbb1e43fdc ci: test libc++ instead of libstdc++ in one job 76313450c2 type-context: disable clang-tidy UndefinedBinaryOperatorResult error 4896e7fe51 proxy-types: fix clang-tidy EnumCastOutOfRange error 060a739269 proxy-types: fix clang-tidy StackAddressEscape error 977d721020 ci: add github actions jobs testing gcc, clang-20, clang-tidy, and iwyu 0d5f1faae5 iwyu: fix add/remove include errors 753d2b10cc util: fix clang-tidy modernize-use-equals-default error ae4f1dc2bb type-number: fix clang-tidy modernize-use-nullptr error 07a741bf69 proxy-types: fix clang-tidy bugprone-use-after-move error 3673114bc9 proxy-types: fix clang-tidy bugprone-use-after-move error 422923f384 proxy-types: fix clang-tidy bugprone-use-after-move error c6784c6ade mpgen: disable clang-tidy misc-no-recursion error c5498aa11b tidy: copy clang-tidy file from bitcoin core git-subtree-dir: src/ipc/libmultiprocess git-subtree-split: 258b83cdbae0cd13a98a95bc12117caf216d4993
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The main thing this change does is add a simple github actions CI job to build the project, run tests, and do checks with iwyu and clang-tidy.
It also fixes bitcoin CI warnings reported by fanquake in bitcoin/bitcoin#31802 (comment) that show up in the logs but do not cause errors, as well as a number of other warnings that do not seem to show up in bitcoin CI. Individual commit messages describe the fixes in detail.