[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

Whisperity via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 31 02:24:27 PST 2023


whisperity added a comment.

Alright, the tests came back from the CI positively and there were no crashes on like 15 projects (about 50:50 C and C++). There are some nice results:

- In LLVM, in `LexPPMacroExpansion.cpp` `ExpandBuiltinMacro()` there is a hit for a call `Result = asctime(TM);`. The suggestion is `strftime` which is available in all C++ versions and >= C99, so this seems like a valid finding. `Result` is a `const char *` which is initialised by this call.
- In Bitcoin, **2** findings in `logging.cpp` doing `setbuf(out, nullptr);`. Not sure if these are legit findings because these seem to be only "resetting" some buffering, but the warning message is legible.
- In Postgres:
  - **2** findings in `isolationtester.c`, same `setbuf(stdout, NULL);` and `setbuf(stderr, NULL);`. `setvbuf/4` is available from C99 so this seems like a valid finding from a language version standpoint.
  - In `initdb.c`, a call to `rewind` with the comment preceding: //`/* now reprocess the file and store the lines */`//.
- In SQLite:
  - In `lemon.c`, a call to `rewind` [...]
  - In `shell.c`, **3** calls to `rewind` [...]
- In OpenSSL:
  - In `randfile.c` and `apps.c`, **2** `setbuf(fileptr, NULL);` calls [...]
- In Vim, **4** calls to `rewind()` in `xxd.c`, `tag.c`, and `term.c`.
  - The case in `term.c` is extra funny because directly following the `rewind(fd)` call there is a `while (!feof(fd)) ...`
- In MemcacheD, a `setbuf(stderr, NULL);` with the comment //`/* set stderr non-buffering ... */`//

It seems not many projects use Annex K at least from this test set. Note that we're primarily working on Linux, where Annex K isn't in the standard library. It has also been reported to not that useful <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1967.htm>, although it's good that the check uses them optionally. In general, we seem to have made a good enough framework for registering "unsafe standard APIs" later on with those stringswitches.

Regarding the performance, I did not see any meaningful slow-down. Most of the projects analysed in a few seconds or minutes, similar to other Tidy runs in the same infrastructure.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91000/new/

https://reviews.llvm.org/D91000



More information about the cfe-commits mailing list