[clang] [analyzer] Fix crash on non-standard string function overloads (PR #180544)

Donát Nagy via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 9 09:54:38 PST 2026


https://github.com/NagyDonat requested changes to this pull request.

Thanks for the patch!

Your change seems to eliminate the crash described in the bug #180422, but I would suggest moving the new logic from its current location (the `identifyCall` method) to `evalStrcmpCommon`, the callback that is responsible for handling `strcmp`-like functions (which I marked with an inline comment).

This way the generic `identifyCall` would remain as simple as possible, and logic that is relevant for only a few functions is sequestered in the callback method responsible for those particular functions. (IIRC many other callbacks already start with analogous checks that validate the count and type of the arguments -- but apparently this was left out by mistake.)

Note that in addition to `strcmp` and `strcasecmp` the callback `evalStrcmpCommon` is also responsible for handling `strncmp` and `strncasecmp` (which would also benefit from a parameter type check), so you would probably need to change the "exactly two parameters" check to e.g. "at least two parameters".

On the other hand as far as I see this checker doesn't do any modeling for `strcoll` (as of now), so there is no need to validate its argument types. (I don't see any immediate reason against modeling `strcoll` in this checker – I would be open to accepting a separate PR that adds `strcoll` to the list of modeled functions and extends `evalStrcmpCommon` to also cover that locale-aware case [perhaps with yet another boolean flag].)

Also, please add a test that would reproduce the crash without your change – and produce normal behavior [no report] when this PR is applied. You could e.g. start by creating a test file `clang/test/Analysis/PR180422.c` (named after the ID of the issue that you fix) with the following contents:
```
// RUN: %clang_analyze_cc1 -verify %s -analyzer-checker=core \
// RUN:   -analyzer-checker=unix.StdCLibraryFunctions

// expected-no-diagnostics

// Intentionally non-standard declaration:
int strcmp(const char *s, char c); 

void use_nonstandard_strcmp(const char* p) {
  if (strcmp(p, '(') == 0) { 
    // ^ no-crash
  }
}
```
Here the `RUN:` lines and `expected-no-diagnostics` are "magic" comments that are recognized by our test systems (feel free to tweak the `RUN` lines if you want to pass other arguments to `clang -cc1`); while `no-crash` is just a customary comment to mark the place where a crash happened in an older version.

I wrote this test code without trying it out, so please check that it is sufficient for triggering the crash (if you execute it without applying your fix). You can run the tests of the static analyzer by building the target `check-clang-analysis` (e.g. with if you use `ninja`, then `ninja check-clang-analysis`).

If it works, then it would be probably nice to extend this test file with a second analogous test function that uses e.g. uses a nonstandard `strncasecmp` (to also cover the three-parameter situation).

https://github.com/llvm/llvm-project/pull/180544


More information about the cfe-commits mailing list