[PATCH] D154838: [analyzer] Add check for null pointer passed to %p of printf family
Georgiy Lebedev via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 10 05:59:55 PDT 2023
CuriousGeorgiy added a comment.
Hi! This is my first time contributing to the LLVM project and particularly the clang static analyzer. This patch is based off https://reviews.llvm.org/D139604?id=481154. I have several questions regarding the reviewers comments (most of which I tried to address) and the patch itself:
> 4. One should be able to disable your rule in case something goes wild. So, your checker should be registered at the end of the file by applying the REGISTER_CHECKER() macro.
This check is part of the UnixAPIPortability checker, which is already registered. Do you suggest creating a separate checker for this check?
> 5. One should update the documentation and also mention the new check in the release docs to give visibility.
The current documentation for the UnixAPIPortability checker is quite generic (docs/analyazer/checkers.rst) and doesn’t mention specific checks. Should I mention this new check there?
> 1. Add tests demonstrating all possible aspects of your change.
I added a number of tests for various aspects of the new check, but I’m not sure they are sufficient and I’m not sure what needs to be covered. For instance, should I add test that the warning is not issued for functions not located in the global namespace? Should I add a test case for `nullptr`?
A couple of questions regarding the patch itself:
1. Should I cover non-standard (i.e., non ISO C standard) functions from the `printf_s` family? Should I cover non-standard functions like `dprintf`?
2. What is the right way to emit a bug report? I have studied the code base and found several patterns: in some cases, an error node is simply generated, in others a new transition is also added based on the analyzed state. In some cases an expression value is also tracked.
3. Should I try to report all possible bugs, or only report the first one found?
4. What is the correct way to update expected plists for test inputs? I tried to simply replace the existing one (unix-fns.c.plist) with the output I get from executing the first command of unix-fns.c test, but I still get the following error:
console
georgiy.lebedev at georgiy-lebedev llvm-project % build-debug/bin/llvm-lit -sv clang/test/Analysis/unix-fns.c
llvm-lit: /Users/georgiy.lebedev/Work/llvm-project/llvm/utils/lit/lit/llvm/config.py:484: note: using clang: /Users/georgiy.lebedev/Work/llvm-project/build-debug/bin/clang
llvm-lit: /Users/georgiy.lebedev/Work/llvm-project/llvm/utils/lit/lit/util.py:439: note: using SDKROOT: '/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk'
FAIL: Clang :: Analysis/unix-fns.c (1 of 1)
******************** TEST 'Clang :: Analysis/unix-fns.c' FAILED ********************
Script:
--
: 'RUN: at line 1'; /Users/georgiy.lebedev/Work/llvm-project/build-debug/bin/clang -cc1 -internal-isystem /Users/georgiy.lebedev/Work/llvm-project/build-debug/lib/clang/17/include -nostdsysteminc -analyze -analyzer-constraints=range -setup-static-analyzer -triple x86_64-apple-darwin10 -analyzer-checker=core,unix.API,osx.API,optin.portability /Users/georgiy.lebedev/Work/llvm-project/clang/test/Analysis/unix-fns.c -analyzer-output=plist -analyzer-config faux-bodies=true -fblocks -verify -o /Users/georgiy.lebedev/Work/llvm-project/build-debug/tools/clang/test/Analysis/Output/unix-fns.c.tmp.plist
: 'RUN: at line 2'; grep -Ev '^[[:space:]]*<string>.* version .*</string>[[:space:]]*$|^[[:space:]]*<string>/.*</string>[[:space:]]*$|^[[:space:]]*<string>.:.*</string>[[:space:]]*$' </Users/georgiy.lebedev/Work/llvm-project/build-debug/tools/clang/test/Analysis/Output/unix-fns.c.tmp.plist | diff -ub /Users/georgiy.lebedev/Work/llvm-project/clang/test/Analysis/Inputs/expected-plists/unix-fns.c.plist -
: 'RUN: at line 3'; mkdir -p /Users/georgiy.lebedev/Work/llvm-project/build-debug/tools/clang/test/Analysis/Output/unix-fns.c.tmp.dir
: 'RUN: at line 4'; /Users/georgiy.lebedev/Work/llvm-project/build-debug/bin/clang -cc1 -internal-isystem /Users/georgiy.lebedev/Work/llvm-project/build-debug/lib/clang/17/include -nostdsysteminc -analyze -analyzer-constraints=range -setup-static-analyzer -analyzer-checker=core,unix.API,osx.API,optin.portability -analyzer-output=html -analyzer-config faux-bodies=true -fblocks -o /Users/georgiy.lebedev/Work/llvm-project/build-debug/tools/clang/test/Analysis/Output/unix-fns.c.tmp.dir /Users/georgiy.lebedev/Work/llvm-project/clang/test/Analysis/unix-fns.c
: 'RUN: at line 5'; rm -fR /Users/georgiy.lebedev/Work/llvm-project/build-debug/tools/clang/test/Analysis/Output/unix-fns.c.tmp.dir
--
Exit Code: 1
Command Output (stdout):
--
--- /Users/georgiy.lebedev/Work/llvm-project/clang/test/Analysis/Inputs/expected-plists/unix-fns.c.plist 2023-07-08 19:58:06
+++ - 2023-07-08 19:58:20
@@ -3,7 +3,6 @@
<plist version="1.0">
<dict>
<key>clang_version</key>
-<string>clang version 17.0.0 (git at github.com:llvm/llvm-project.git 1882a4ee69b3cc2202d8d7d7b6465475f6d19886)</string>
<key>diagnostics</key>
<array>
<dict>
@@ -4065,7 +4064,6 @@
</array>
<key>files</key>
<array>
- <string>/Users/georgiy.lebedev/Work/llvm-project/clang/test/Analysis/unix-fns.c</string>
</array>
</dict>
</plist>
--
********************
********************
Failed Tests (1):
Clang :: Analysis/unix-fns.c
Testing Time: 0.79s
Failed: 1
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154838/new/
https://reviews.llvm.org/D154838
More information about the cfe-commits
mailing list