mirror of
https://github.com/neovim/neovim.git
synced 2025-10-09 19:36:40 +00:00
server.c: Fix bug in release mode (#7594)
When compiling with CMAKE_BUILD_TYPE=RelWithDebInfo, several -Wmaybe-uninitialized warnings are printed. These were thought to be false positives (#5061); there are no control paths that lead to an uninitialized value. However, when gcc is run in -O2 mode, it makes a mistake while generating the necessary logic. Specifically, for the code: ... int = 0; // Index of the server whose address equals addr. for (; i < watchers.ga_len; i++) { watcher = ((SocketWatcher **)watchers.ga_data)[i]; // <snip> } if (i >= watchers.ga_len) { ELOG("Not listening on %s", addr); return; } ... Gcc generates: ... <+98>: cmp %ebx, %ebp <+100>: jg 0x530f13 <server_stop+55> <+102>: cmp %ebp, ebx <+104>: jl 0x530f7e <server_stop+162> ... Normally, the if statement should catch the only control path where watcher is not assigned: watchers.ga_len <= 0. When compiled, the assembly lines 98 and 100 correspond to checking if i < watchers.ga_len, and the lines 102 and 104 correspond to checking if i >= watchers.ga_len. The assembly seems to compare ebp (which is watchers.ga_len) with ebx (which is i), and jump if greater; then do the same comparison and jump if less. This is where gcc makes a mistake: it flips the order of the cmp instruction. This means that the REAL behavior is first check if i < watchers.ga_len and then check if i < watchers.ga_len. Which means the code inside the if statement is NEVER executed; no combination of i and watchers.ga_len will ever trigger ELOG(). So not only is this a use of an uninitialized value if watchers.ga_len == 0 (or technically, if it's less than zero too), it also clobbers any error detection if the for loop reaches the last entry (which would normally cause i == watchers.ga_len too). This commit fixes this issue by adding a bool to keep track of whether a watcher was found during the loop. This makes gcc generate the correct code, avoiding both bugs.
This commit is contained in:

committed by
Justin M. Keyes

parent
de8b1fd1de
commit
df10714991
@@ -180,6 +180,7 @@ int server_start(const char *endpoint)
|
||||
void server_stop(char *endpoint)
|
||||
{
|
||||
SocketWatcher *watcher;
|
||||
bool watcher_found = false;
|
||||
char addr[ADDRESS_MAX_SIZE];
|
||||
|
||||
// Trim to `ADDRESS_MAX_SIZE`
|
||||
@@ -189,11 +190,12 @@ void server_stop(char *endpoint)
|
||||
for (; i < watchers.ga_len; i++) {
|
||||
watcher = ((SocketWatcher **)watchers.ga_data)[i];
|
||||
if (strcmp(addr, watcher->addr) == 0) {
|
||||
watcher_found = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (i >= watchers.ga_len) {
|
||||
if (!watcher_found) {
|
||||
ELOG("Not listening on %s", addr);
|
||||
return;
|
||||
}
|
||||
|
Reference in New Issue
Block a user