Adds memory allocation analysis functions#1041
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1041 +/- ##
==========================================
+ Coverage 86.62% 86.92% +0.29%
==========================================
Files 23 23
Lines 5946 6028 +82
==========================================
+ Hits 5151 5240 +89
+ Misses 795 788 -7
... and 2 files with indirect coverage changes
🚀 New features to boost your workflow:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| return llvm::isa_and_nonnull<clang::FunctionProtoType>(T); | ||
| } | ||
|
|
||
| static AllocType handleNew(const clang::CXXNewExpr* CNE) { |
There was a problem hiding this comment.
warning: no header providing "Cpp::AllocType" is directly included [misc-include-cleaner]
(T);
^| } | ||
|
|
||
| static AllocType handleCall(const clang::CallExpr* CE) { | ||
| if (auto* FD = CE->getDirectCallee()) { |
There was a problem hiding this comment.
warning: 'auto *FD' can be declared as 'const auto *FD' [readability-qualified-auto]
| if (auto* FD = CE->getDirectCallee()) { | |
| * CE) {const |
| return it->second; | ||
| return AllocType::None; | ||
| } else { | ||
| // FIXME: BindingDecl, NonTypeTemplateParmDecl are not handled |
There was a problem hiding this comment.
warning: do not use 'else' after 'return' [llvm-else-after-return]
else {
^this fix will not be applied because it overlaps with another fix
| } | ||
| }; | ||
| RetStmtVisitor Visitor; | ||
| Visitor.TraverseStmt(const_cast<CompoundStmt*>(CS)); |
There was a problem hiding this comment.
warning: do not use const_cast to remove const qualifier [cppcoreguidelines-pro-type-const-cast]
isitor;
^| } | ||
| }; | ||
| VarVisitor Visitor; | ||
| Visitor.TraverseStmt(const_cast<CompoundStmt*>(CS)); |
There was a problem hiding this comment.
warning: do not use const_cast to remove const qualifier [cppcoreguidelines-pro-type-const-cast]
isitor;
^
keremsahn
force-pushed
the
alloc-detect
branch
2 times, most recently
from
6aed8ff to
9b5d77a
Compare
keremsahn
force-pushed
the
alloc-detect
branch
3 times, most recently
from
75948c0 to
4f9b70f
Compare
keremsahn
force-pushed
the
alloc-detect
branch
2 times, most recently
from
1d246fc to
207c373
Compare
Vipul-Cariappa
left a comment
Vipul-Cariappa
left a comment
There was a problem hiding this comment.
Minor code quality related comments:
Vipul-Cariappa
left a comment
Vipul-Cariappa
left a comment
There was a problem hiding this comment.
LGTM! Small formatting thing. Please rebase so that I can merge.
I have a question:
What happens if we analyse the following:
int* heap_alloc_tail_recr_sum(int n, int acc = 0) {
if (n)
return heap_alloc_tail_recr_sum(n - 1, acc + n);
return new int(acc);
}If this does not work, please open an issue for it once this PR is merged.
| if (const auto* CE = dyn_cast<CallExpr>(finExpr)) { | ||
| return handleCall(CE); | ||
| } |
There was a problem hiding this comment.
| if (const auto* CE = dyn_cast<CallExpr>(finExpr)) { | |
| return handleCall(CE); | |
| } | |
| if (const auto* CE = dyn_cast<CallExpr>(finExpr)) | |
| return handleCall(CE); |
keremsahn
force-pushed
the
alloc-detect
branch
2 times, most recently
from
38da0c7 to
6e87954
Compare
|
|
||
| bool VisitBinaryOperator(clang::BinaryOperator* BO) { | ||
| if (BO->getOpcode() != BO_Assign) | ||
| return true; |
There was a problem hiding this comment.
Can we add coverage for this line?
There was a problem hiding this comment.
Hi Vassil, I added one check the fourth commit. Our first assumption was analyzed function does not leak memory, but i wrote the test this way to point the issue so maybe I can fix it later.
There was a problem hiding this comment.
Can we add a comment capturing that intent?
There was a problem hiding this comment.
Actually I added a FIXME up to the
TESTAC(27, NewArr);Furthermore, I will open some issues I want to work on when the PR is merged, I can open for this one too. No worries
keremsahn
changed the title
keremsahn
changed the title
3 participants
Does not support detection for multiple branches, that is another milestone will be done after discussed.