Repository navigation
Track buffer sizes for static global pointers - #8828
JeewoongKim wants to merge 3 commits into
Conversation
bc87761 to
d7d165a
Compare
|
Thanks for working on this. I'm not sure this approach is feasible though. |
Yes I agree, thanks for looking at Cppcheck.
I assume that is what the last test case with the I fear that the upside of this PR seems pretty small because it only targets main(). it could find problems in some small and simple synthetic test case more or less. more generic heuristics would be appreciated that could find simple bugs in all code. |
|
Thanks for all the feedback! I initially focused too narrowly on the specific reproducer in the ticket, so the solution ended up being too limited. I agree that a more general heuristic would be more useful. I'm looking into an approach that can handle simple cases across functions. |
Replace the main()-specific propagation with tracking for static global pointers that are not modified or escaped. Also handle allocation functions such as malloc in addition to new.
|
I reworked the implementation to use a more general heuristic. I started with internal-linkage globals rather than external-linkage ones, since their modifications This removes |
|
@chrchr-github, When you have a chance, could you take another look at the updated version? |
|
Test results for commit d781b29 (tools/test-my-pr.py, main compared to this PR): Test: http://ec2-16-170-140-253.eu-north-1.compute.amazonaws.com/pr-8828/ Posted automatically by the cppcheck PR test runner. +N: warnings only with this PR, -N: warnings only with main. The AI review is written by Claude and can be wrong. |
| " return a[5];\n" | ||
| "}\n"); | ||
| ASSERT_EQUALS("", errout_str()); | ||
|
|
There was a problem hiding this comment.
I assume you will warn here:
check("static int *a = new int[2];\n"
"void f1() {\n"
" delete [] a; a = new int[10];\n"
"}\n"
"int f2() {\n"
" return a[5];\n"
"}\n");
it would be questionable. We can't know size of a buffer in f2.
|
I am not sure about this PR. You extend the data flow analysis its in theory good thing but in practice I fear we didn't see much benefit. For 1000 packages there was no extra findings. And it makes valueflow a little slower. |
This PR started from ticket 14934.
The initial implementation propagated the buffer size of a global pointer into
main().Based on the review feedback, I replaced that approach with tracking for static
global pointers that are not modified or escaped within the translation unit.
Allocation functions such as
mallocare also handled in addition tonew.The original 14934 example uses an external-linkage global, which requires additional
reasoning about modification from other translation units. I therefore limited this change
to internal-linkage globals as a conservative first step rather than claiming to fix directly.