unix.StdCLibraryFunctions analysis regression

[avlouis]

There seems to be a regression in the unix.StdCLibraryFunctions analysis check.
Found below is code for a small example. Running clang --analyze sendto.c produces warnings such as the following:

sendto.c:20:3: warning: The 1st argument to 'sendto' is -1 but should be >= 0 [unix.StdCLibraryFunctions]
   20 |   sendto(sockfd, NULL, 0, 0, NULL, 0);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This happens for versions 19.1.2 and 19.1.3 but not 18.1.8. All acquired via this Github project.

It seems the analysis pass believes some_function_outside_tu can set sockfd to -1 as removing the call to it or adding if(0 > sockfd) return 1; between it and the call to sendto resolves this warning. some_function_outside_tu should not be able to modify sockfd since the implementation is in a different translation unit and sockfd is static.

Here is the code for sendto.c:

#include <arpa/inet.h>
#include <sys/socket.h>
#include <stddef.h>

static int sockfd = -1;

void some_function_outside_tu(); // declared here, but implementation
				 // is outside this translation unit

int main(int argc, const char* argv[]) {
  (void)argc; (void)argv;

  sockfd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
  if(0 > sockfd) {
    return 1;
  }

  some_function_outside_tu(); // this causes issues
  sendto(sockfd, NULL, 0, 0, NULL, 0);
}

[llvmbot]

@llvm/issue-subscribers-clang-static-analyzer

Author: Andrew V. Louis (avlouis)

There seems to be a regression in the `unix.StdCLibraryFunctions` analysis check. Found below is code for a small example. Running `clang --analyze sendto.c` produces warnings such as the following: ``` sendto.c:20:3: warning: The 1st argument to 'sendto' is -1 but should be >= 0 [unix.StdCLibraryFunctions] 20 | sendto(sockfd, NULL, 0, 0, NULL, 0); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ```

This happens for versions 19.1.2 and 19.1.3 but not 18.1.8. All acquired via this Github project.

It seems the analysis pass believes some_function_outside_tu can set sockfd to -1 as removing the call to it or adding if(0 > sockfd) return 1; between it and the call to sendto resolves this warning. some_function_outside_tu should not be able to modify sockfd since the implementation is in a different translation unit and sockfd is static.

Here is the code for sendto.c:

#include <arpa/inet.h>
#include <sys/socket.h>
#include <stddef.h>

static int sockfd = -1;

void some_function_outside_tu(); // declared here, but implementation
				 // is outside this translation unit

int main(int argc, const char* argv[]) {
  (void)argc; (void)argv;

  sockfd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
  if(0 > sockfd) {
    return 1;
  }

  some_function_outside_tu(); // this causes issues
  sendto(sockfd, NULL, 0, 0, NULL, 0);
}

[steakhal]

Hi,
In this simplified case you are right. The address of that static global never escapes this translation unit. However, in general, if its address would have been taken and assigned to something, or passed to an opaque function call then even these globals should escape.

So in short, even static globals are subject to invalidation for opaque fn calls. Internally, I don't think we treat statics any differently, as we don't have a more sophisticated "address taken" analysis.

Speaking of the appearing issue, I'll have a look to see what commit it bisects to to have better context.

[avlouis]

Agreed yes if the address escapes the translation unit, then opaque function calls can invalidate analysis done in this translation unit. I think a check to see if the address is never taken could help reduce false positives.