This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] CFRetainReleaseChecker: Avoid checking C++ methods with the same name.
ClosedPublic

Authored by NoQ on Aug 16 2018, 2:36 PM.

Details

Summary

CFRetainReleaseChecker is a tiny checker that verifies that arguments of CoreFoundation retain/release functions are non-null. The checker accidentally checks all functions with the respective name, not just the actual CFRetain etc, which has caused a crash.

Fix it and modernize the checker to use CallEvent/CallDescription APIs to avoid further mistakes.

rdar://problem/42433152

Diff Detail

Event Timeline

NoQ created this revision.Aug 16 2018, 2:36 PM
lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
537

I personally would prefer being less fancy, and avoiding the comma operator, but I suppose it's a matter of style.

567

There's also DefArgVal

NoQ added inline comments.Aug 20 2018, 4:05 PM
lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
537

This isn't comma operator, just initializer list.

Alternatives are:

  • CallDescription CFRetain = {"CFRetain", 1} (longer but looks the same)
  • CallDescription CFRetain = CallDescription("CFRetain", 1), ... (longer and duplicates type)
  • CallDescription CFRetain; CFRetainReleaseChecker(): CFRetain("CFRetain", 1) (longer and duplicates variable name)
567

?

george.karpenkov accepted this revision.Aug 20 2018, 8:09 PM
This revision is now accepted and ready to land.Aug 20 2018, 8:09 PM
This revision was automatically updated to reflect the committed changes.