mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-06-05 10:42:13 +02:00
Merge bitcoin/bitcoin#34934: fuzz: exercise ForNode/ForEachNode callbacks in connman fuzz harness
371eac8069fuzz: exercise ForNode/ForEachNode callbacks in connman fuzz harness (frankomosh) Pull request description: Track inserted node IDs and sometimes reuse them in `ForNode()` so the successful lookup path is exercised more reliably. Replace no-op callbacks with lightweight CNode accessor calls to make `ForEachNode()` and `ForNode()` cover previously unreached callback code paths. This addresses feedback from https://github.com/bitcoin/bitcoin/pull/34830#issuecomment-4074732710 where it was noted that the callbacks had "neither the return type checked nor its side-effect”. Coverage reports from the connman fuzz corpus, before and after the change: - [Before](https://frankomosh.github.io/fuzz-coverage/connman-callback-coverage/before/index.html) - [After](https://frankomosh.github.io/fuzz-coverage/connman-callback-coverage/after/index.html) `diff cov_show_before.txt cov_show_after.txt` filtered to `ForNode`/`ForEachNode`/`IsFullOutboundConn`/`ConnectionTypeAsString`: **`IsFullOutboundConn` — `net.h:786-788`** ```diff - 786| 0| bool IsFullOutboundConn() const { - 787| 0| return m_conn_type == ConnectionType::OUTBOUND_FULL_RELAY; - 788| 0| } + 786| 1.13M| bool IsFullOutboundConn() const { + 787| 1.13M| return m_conn_type == ConnectionType::OUTBOUND_FULL_RELAY; + 788| 1.13M| } ``` **`ConnectionTypeAsString` — `net.h:967`** ```diff - 967| 0| std::string ConnectionTypeAsString() const { return ::ConnectionTypeAsString(m_conn_type); } + 967| 1.11M| std::string ConnectionTypeAsString() const { return ::ConnectionTypeAsString(m_conn_type); } ``` **`ForNode` — `net.cpp:4126-4131`** ```diff - 4126| 1.08k| if(pnode->GetId() == id) { - | Branch (4126:12): [True: 0, False: 1.08k] - 4127| 0| found = pnode; - 4131| 39| return found != nullptr && NodeFullyConnected(found) && func(found); - ^0 ^0 + 4126| 602| if(pnode->GetId() == id) { + | Branch (4126:12): [True: 1, False: 601] + 4127| 1| found = pnode; + 4131| 28| return found != nullptr && NodeFullyConnected(found) && func(found); + ^1 ^1 ``` **`ForEachNode` — `net.h:1270-1271`** ```diff - 1270| 1.13M| if (NodeFullyConnected(node)) - | Branch (1270:17): [True: 0, False: 1.13M] - 1271| 0| func(node); + 1270| 1.11M| if (NodeFullyConnected(node)) + | Branch (1270:17): [True: 1.11M, False: 0] + 1271| 1.11M| func(node); ``` Two previously uncovered functions (`IsFullOutboundConn`, `ConnectionTypeAsString`) are now exercised through the iteration callbacks. `ForNode` finds matching nodes. ACKs for top commit: nervana21: tACK371eac8069maflcko: lgtm ACK371eac8069Tree-SHA512: 3587c021b16e38ca252676a21b66c5383ab2bd3eec9073e61e9a93db7ef84a94ce5a0c037ac512483680cafabb44103b86df0893e8a9b1bf63b8383bd54f4641
This commit is contained in:
@@ -99,12 +99,14 @@ FUZZ_TARGET(connman, .init = initialize_connman)
|
||||
CNode random_node = ConsumeNode(fuzzed_data_provider);
|
||||
CSubNet random_subnet;
|
||||
std::string random_string;
|
||||
std::vector<NodeId> node_ids;
|
||||
|
||||
LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 100) {
|
||||
CNode& p2p_node{*ConsumeNodeAsUniquePtr(fuzzed_data_provider).release()};
|
||||
// Simulate post-handshake state.
|
||||
p2p_node.fSuccessfullyConnected = true;
|
||||
connman.AddTestNode(p2p_node);
|
||||
node_ids.push_back(p2p_node.GetId());
|
||||
}
|
||||
|
||||
LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) {
|
||||
@@ -141,10 +143,15 @@ FUZZ_TARGET(connman, .init = initialize_connman)
|
||||
connman.DisconnectNode(random_subnet);
|
||||
},
|
||||
[&] {
|
||||
connman.ForEachNode([](auto) {});
|
||||
},
|
||||
[&] {
|
||||
(void)connman.ForNode(fuzzed_data_provider.ConsumeIntegral<NodeId>(), [&](auto) { return fuzzed_data_provider.ConsumeBool(); });
|
||||
NodeId id = node_ids.empty() || fuzzed_data_provider.ConsumeBool()
|
||||
? fuzzed_data_provider.ConsumeIntegral<NodeId>()
|
||||
: PickValue(fuzzed_data_provider, node_ids);
|
||||
(void)connman.ForNode(id, [&](CNode* pnode) {
|
||||
(void)pnode->GetId();
|
||||
(void)pnode->IsInboundConn();
|
||||
(void)pnode->IsFullOutboundConn();
|
||||
return true;
|
||||
});
|
||||
},
|
||||
[&] {
|
||||
auto max_addresses = fuzzed_data_provider.ConsumeIntegral<size_t>();
|
||||
@@ -228,6 +235,12 @@ FUZZ_TARGET(connman, .init = initialize_connman)
|
||||
connman.SocketHandlerPublic();
|
||||
});
|
||||
}
|
||||
connman.ForEachNode([](CNode* pnode) {
|
||||
(void)pnode->GetId();
|
||||
(void)pnode->IsInboundConn();
|
||||
(void)pnode->IsFullOutboundConn();
|
||||
(void)pnode->ConnectionTypeAsString();
|
||||
});
|
||||
(void)connman.GetAddedNodeInfo(fuzzed_data_provider.ConsumeBool());
|
||||
(void)connman.GetExtraFullOutboundCount();
|
||||
(void)connman.GetLocalServices();
|
||||
|
||||
Reference in New Issue
Block a user