<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>Issue</th>
<td>
<a href=https://github.com/llvm/llvm-project/issues/104241>104241</a>
</td>
</tr>
<tr>
<th>Summary</th>
<td>
[clang][analyzer] Possible false positive from unix.BlockInCriticalSection
</td>
</tr>
<tr>
<th>Labels</th>
<td>
clang
</td>
</tr>
<tr>
<th>Assignees</th>
<td>
</td>
</tr>
<tr>
<th>Reporter</th>
<td>
hcho3
</td>
</tr>
</table>
<pre>
Clang-tidy generates a false positive for the `unix.BlockInCriticalSection` check when a thread that enters a critical section performs a `recv()` with a non-blocking socket.
Example: https://github.com/dmlc/xgboost/blob/0def8e0bae41e0eeeb4ee078e6e699ad6bef1986/src/collective/loop.cc#L88-L105
<details>
<summary>Error message</summary>
```
/workspace/include/xgboost/collective/socket.h:696:12: warning: Call to blocking function 'recv' inside of critical section [clang-analyzer-unix.BlockInCriticalSection]
696 | return recv(handle_, _buf, len, flags);
| ^
/workspace/src/collective/loop.cc:272:5: note: Calling 'Loop::Process'
272 | this->Process();
| ^~~~~~~~~~~~~~~
/workspace/src/collective/loop.cc:154:3: note: Loop condition is true. Entering loop body
154 | while (true) {
| ^
/workspace/src/collective/loop.cc:156:24: note: Calling constructor for 'unique_lock<std::mutex>'
156 | std::unique_lock lock{mu_};
| ^~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/unique_lock.h:69:2: note: Entering critical section here
69 | lock();
| ^~~~~~
/workspace/src/collective/loop.cc:156:24: note: Returning from constructor for 'unique_lock<std::mutex>'
156 | std::unique_lock lock{mu_};
| ^~~~~~~~~
/workspace/src/collective/loop.cc:167:11: note: Assuming field 'stop_' is false
167 | if (stop_) {
| ^~~~~
/workspace/src/collective/loop.cc:167:7: note: Taking false branch
167 | if (stop_) {
| ^
/workspace/src/collective/loop.cc:174:14: note: Assuming the condition is false
174 | while (!queue_.empty()) {
| ^~~~~~~~~~~~~~~
/workspace/src/collective/loop.cc:174:7: note: Loop condition is false. Execution continues on line 179
174 | while (!queue_.empty()) {
| ^
/workspace/src/collective/loop.cc:182:17: note: Calling 'Loop::ProcessQueue'
182 | auto rc = this->ProcessQueue(&qcopy);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
/workspace/src/collective/loop.cc:30:7: note: Assuming field 'stop_' is false
30 | if (stop_) {
| ^~~~~
/workspace/src/collective/loop.cc:30:3: note: Taking false branch
30 | if (stop_) {
| ^
/workspace/src/collective/loop.cc:38:10: note: Assuming the condition is true
38 | while (!qcopy.empty()) {
| ^~~~~~~~~~~~~~
/workspace/src/collective/loop.cc:38:3: note: Loop condition is true. Entering loop body
38 | while (!qcopy.empty()) {
| ^
/workspace/src/collective/loop.cc:43:29: note: Assuming 'i' is < 'n_ops'
43 | for (std::size_t i = 0; i < n_ops; ++i) {
| ^~~~~~~~~
/workspace/src/collective/loop.cc:43:5: note: Loop condition is true. Entering loop body
43 | for (std::size_t i = 0; i < n_ops; ++i) {
| ^
/workspace/src/collective/loop.cc:47:7: note: Control jumps to 'case kSleep:' at line 56
47 | switch (op.code) {
| ^
/workspace/src/collective/loop.cc:57:11: note: Execution continues on line 65
57 | break;
| ^
/workspace/src/collective/loop.cc:43:29: note: Assuming 'i' is >= 'n_ops'
43 | for (std::size_t i = 0; i < n_ops; ++i) {
| ^~~~~~~~~
/workspace/src/collective/loop.cc:43:5: note: Loop condition is false. Execution continues on line 69
43 | for (std::size_t i = 0; i < n_ops; ++i) {
| ^
/workspace/src/collective/loop.cc:70:9: note: Assuming the condition is false
70 | if (!poll.fds.empty()) {
| ^~~~~~~~~~~~~~~~~
/workspace/src/collective/loop.cc:70:5: note: Taking false branch
70 | if (!poll.fds.empty()) {
| ^
/workspace/src/collective/loop.cc:80:11: note: Assuming the condition is true
80 | CHECK(!qcopy.empty());
| ^
/workspace/include/xgboost/logging.h:144:24: note: expanded from macro 'CHECK'
144 | if (XGBOOST_EXPECT(!(cond), false)) \
| ^~~~
/workspace/include/xgboost/base.h:53:54: note: expanded from macro 'XGBOOST_EXPECT'
53 | #define XGBOOST_EXPECT(cond, ret) __builtin_expect((cond), (ret))
| ^~~~
/workspace/src/collective/loop.cc:80:5: note: Taking false branch
80 | CHECK(!qcopy.empty());
| ^
/workspace/include/xgboost/logging.h:144:3: note: expanded from macro 'CHECK'
144 | if (XGBOOST_EXPECT(!(cond), false)) \
| ^
/workspace/src/collective/loop.cc:83:5: note: Loop condition is true. Entering loop body
83 | for (std::size_t i = 0; i < n_ops; ++i) {
| ^
/workspace/src/collective/loop.cc:88:11: note: Assuming field 'sock' is non-null
88 | if (!op.sock) {
| ^~~~~~~~
/workspace/src/collective/loop.cc:88:7: note: Taking false branch
88 | if (!op.sock) {
| ^
/workspace/src/collective/loop.cc:91:9: note: Assuming the condition is false
91 | CHECK(op.sock->NonBlocking());
| ^
/workspace/include/xgboost/logging.h:144:22: note: expanded from macro 'CHECK'
144 | if (XGBOOST_EXPECT(!(cond), false)) \
| ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
/workspace/include/xgboost/base.h:53:54: note: expanded from macro 'XGBOOST_EXPECT'
53 | #define XGBOOST_EXPECT(cond, ret) __builtin_expect((cond), (ret))
| ^~~~
/workspace/src/collective/loop.cc:91:9: note: Taking false branch
91 | CHECK(op.sock->NonBlocking());
| ^
/workspace/include/xgboost/logging.h:144:3: note: expanded from macro 'CHECK'
144 | if (XGBOOST_EXPECT(!(cond), false)) \
| ^
/workspace/src/collective/loop.cc:94:7: note: Control jumps to 'case kRead:' at line 95
94 | switch (op.code) {
| ^
/workspace/src/collective/loop.cc:96:11: note: Taking true branch
96 | if (poll.CheckRead(*op.sock)) {
| ^
/workspace/src/collective/loop.cc:97:28: note: Calling 'TCPSocket::Recv'
97 | n_bytes_done = op.sock->Recv(op.ptr + op.off, op.n - op.off);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/workspace/include/xgboost/collective/socket.h:696:12: note: Call to blocking function 'recv' inside of critical section
696 | return recv(handle_, _buf, len, flags);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
--------------------------------
```
</details>
Suggested fix: Add a note in the documentation about possible false positives with non-blocking sockets.
</pre>
<img width="1px" height="1px" alt="" src="http://email.email.llvm.org/o/eJzUWsly47gZfhrogpKKBPeDDhItJal0ZTrtPsxNBZK_SIwhgE2AXuaQZ08BpLVbTUvujMNSURsBfN-_Y6FKsVIATFEwR8HdiLa6ks20yivpjTJZvExTTkU51qx4wSUIaKgGhSleU64A11IxzR4Br2WDdQUYhU4r2PNkzmX-8A-RNkyznPJ7yDWTAoUOzivIH_BTBQJTrKsGaIF1RTUGoaExXed9I6y6VriGZi2bjfnPDNBA_ohIjEhi-ntiusIUCynGmRmUiRIrmT-AnmDk3CFntnimm5oD8ma40rpWyJshskRkWTJdtdkklxtElsWG54gsn8tMSqURWWZcZogsnQLWMTgZBd8FBwAyH8CJYgghTBJahBms3SQOEVmqxvSQS84N8EdAZMmlrCd5joj3JY7HX1wn6DD1dy8tQFPGFfIW3U8YealqNxvavCBvsWga2eANKEVLQF5qBtn-edBT6PSv7itZPsnmQdU0NyiYyHlbwAG9A5i9wCrkzcIkRN7MJUZcT7QRTJTmY0o5x1rirYzXrei0g0jUaSTCTChWAJbrUx2iYJ5bS6KC8pc_oRlfspPgrpcGDpMQoyjF5mpAt43Avf4rKgoOK0RSvMratXnnIMzbmtNSGfPw5q_dmMt0g4LFWQldUJ03I5GRRmDEIKSGV3EYKSASfZGyNiblzb42MgelEIlehyUR2aLXFVNj5C22T8VvQvzPwfV-wG7gI2_m7QM2KHEuRcGsOpjCumlhgvHCuJ2hYppj4_KvgNzA78E_VYwDRiQ2bRBJMIo-RrJuYIyN-OdEm0uhdNPmWjY2vCAStYL9aGFlbMa4iS46uW9aDc_GIXaCd4Od2WC8fXKvA2x7ieabdoWiu8t62BJrVWPwMxMYytzGizhchf6YM9E-j0vRIrJ0XUSWk8npbeeGOSJz--oezphWpvcdut4VjXD2ZbNV1ol_VdDAlkGY7JHHHdOfmdvHKO-bdVEbHxq5-ZQ6HM4vjMzd3ec3U6rdWHoMeGH4KC3rlQ1-qkuKW_hhtAefrY3_9A-_5UC34Iz2YX6nXYy2STprqMirG2BdASkyVuH6Z0Vn6oSDSHQotsjfw7eNPIi4P1poYTWBTa1fenO-KMmbIqglEF2OoBb3BC-eIW_tb7kUmokWFJYCcyYAu1HyscSuoBITS2ho-vq3QbPvhjHZw01bLXGTY-TdHWe0vmGMSPgjl_XLwPR2k54851hN7_BQ7Dk9s1_onRahN9A534noCjSxsQVnmF_abL9FFh8XA8ZyjZ6v8Mgrgd9a0dxGAl8nct-gNq5wRuSIRKy3TOSl5qtYyXq_gsS-t_W-LoPG20yo2J-w0phZZ3SQN7cfU9z14c1xV2WwQz4350VLKLhVFR_Oa09P12rqJI-mUuhGcvxHu6mVmf4gEuVUAX645wA2bpIIY6q7aB-EO3r7WVY9MZ1XhqMZShZv1dD4euzBaa1yKS-FwXbsIDooFjHOGqAPHzV3Gmz9C6Ps_4kDfEx1OMQLBhQIYfKZPSIymeK88i5XcThytiN3qQwRt5acT9aFuqaEu0ZFFn0wNPXejvj9CGPn7RnG5XQc79Cmf1-k_3w7lb3Dk8-tE3FZlkyUdkLq-v7JjA-eayoKKLoJ34bmjY2SPahdFen7B5XN73-b__bb_ffV4vevi_R7hx6R2BC2sNPennp5B-l5CxlMI6MKLIfA-u0QCscQdyEp6DwVEa-AtfHiEzYdjxQ3oA3-1SprGddMrOC5hlxbvvtkEYm7R81rONOfG9dg8_8LDcr7DPZ0ZYSMP6QYin9ljruCVDxk4cOuKtnsLaQYi5bzHZ_4ZI0BEVfWk67RT0L-lYCHroBci-79sBL32vSZuAdF2atH9hjNpPtfUsz7tfhfGOrJZ3DN40Jg0PLO50sCf00WOLXBt13jk1jd_3M-SE7WDd-eQ34DWhxPIZPdxCzxh08hb5w9dnt-7hkrMbnrOH4m4dGMsRO5LZbTCvKOmBH5bBdUPzSuGgGT-I0Vze_p13u7n9kl0G_d5uQO_fF8F2Oxyl40qFUhBdg8u2fxXXMj9VqbzDw3f8q13W-U9UTg8faH9y53Dl0BvXb7dl82N-zdHm_Ffsw27BA54PFPrvOb369764gsj7fXu_t9W5agtAkq7Nmm5aKwZwg0YCZsbi5k3m5AaGrlRDPZalxLpVjG4ejwg-rOIJw5gaAm3XijYuoViZfQEUzdiHhhmCTEG1VTD9yiyAs_890iyBwnjLOAUA_8zI_CwqMjNiUO8Z3Y9YnrEsebRGGY-U7kOutsHUcORb4DG8r4hPPHzUQ25Ygp1cLUdXziuyNOM-DKnu4gxG7AI0JQcDdqpqbBOGtLhXyHM6XVrgvNNLdHQroWwR0K5q_79ii4w1_PC6IL0Rf29Udtw6cXzmAYAP3buG7kHzZHLi0hhciy5_Q4Jf8NAAD__2ObMDc">