[PATCH] [analyzer][Review request] Partial fix for PR19102.

Jordan Rose jordan_rose at apple.com
Fri Aug 1 19:07:06 PDT 2014


IIRC, constructors are never inlined for 'new' right now because we never got the right region, and I was always hoping we'd implement proper allocator support first (http://llvm.org/bugs/show_bug.cgi?id=12014). Karthik Bhat was working on this but I don't remember how far he got. We also didn't have destructor support for 'delete' until last fall (also thanks to Karthik).

Other than that this is looking pretty good. More tests would be nice, and possibly a test on a real code base that would otherwise have false positives. Unfortunately the only one I can think of is LLVM itself, but even that could tell us how many false positives we've gone down by.

================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:768
@@ +767,3 @@
+
+  const CXXConstructExpr* ConstructE = NE->getConstructExpr();
+  if (!ConstructE)
----------------
Style nitpick: * next to the variable name.

================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:772-773
@@ +771,4 @@
+
+  QualType ConstructedTy = NE->getAllocatedType().getCanonicalType();
+  if (!ConstructedTy->getAsCXXRecordDecl())
+    return false;
----------------
getAsCXXRecordDecl will call getCanonicalType for you, so you can skip that step.

================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:776
@@ +775,3 @@
+
+  CXXConstructorDecl* CtorD = ConstructE->getConstructor();
+
----------------
Please make this const (and move the *).

================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:785-786
@@ +784,4 @@
+
+    CtorParamPointeeT = getDeepPointeeType(CtorParamPointeeT).
+                        getCanonicalType().getUnqualifiedType();
+
----------------
I don't think you need to worry about either canonicalizing the type or stripping qualifiers.

http://reviews.llvm.org/D4025






More information about the cfe-commits mailing list