Page MenuHomePhabricator

[analyzer][NFC] Merge checkNewAllocator's paramaters into CXXAllocatorCall
ClosedPublic

Authored by Szelethus on Mar 1 2020, 4:51 PM.

Details

Summary

Party based on this thread: http://lists.llvm.org/pipermail/cfe-dev/2020-February/064754.html.

This patch merges two of CXXAllocatorCall's parameters, so that we are able to supply a CallEvent object to check::NewAllocatorCall (see the description of D75430 to see why this would be great).

One of the things mentioned by @NoQ was the following:

I think at this point we might actually do a good job sorting out this check::NewAllocator issue because we have a "separate" "Environment" to hold the other SVal, which is "objects under construction"! - so we should probably simply teach CXXAllocatorCall to extract the value from the objects-under-construction trait of the program state and we're good.

I had MallocChecker in my crosshair for now, so I admittedly threw together something as a proof of concept. Now that I know that this effort is worth pursuing though, I'll happily look for a solution better then demonstrated in this patch.

Diff Detail

Event Timeline

Szelethus created this revision.Mar 1 2020, 4:51 PM

look for a solution better then demonstrated in this patch.

I believe this solution exactly what Artem suggested, so there is nothing to feel bad about. Cool.

Charusso accepted this revision.Mar 3 2020, 3:53 AM
This revision is now accepted and ready to land.Mar 3 2020, 3:53 AM

Other than a nit looks good to me but wait for @NoQ before committing.

clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
212

Since SValBuilder has relatively little to do with call events I am kind of surprised seeing this here.

NoQ added inline comments.Mar 15 2020, 10:52 PM
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
1014

Why does this method accept a state? Can't we use our own state?

Szelethus updated this revision to Diff 254816.Apr 3 2020, 8:20 AM
Szelethus marked 2 inline comments as done.

Inlines addressed, rebase.

Hmm, upon taking a look at alignment new in previous patches, I think implementing a checker for MEM57-CPP. Avoid using default operator new for over-aligned types seems a very low hanging fruit, though probably not a common source of bugs.

NoQ accepted this revision.Apr 6 2020, 1:11 AM

Great, thanks!!

This revision was automatically updated to reflect the committed changes.