chrisdangelo updated this revision to Diff 394017.
chrisdangelo added a comment.
These changes allow the analyzer to silence an issue discovered by 
MallocChecker if the SymRef or Statement in question is of a struct that has 
been declared with the compiler attribute annotation "reference_counted".

In the previous iteration of this diff, SymRef alone was used to determine the 
declared type, and if it was "reference_counted". Previously, if the SymRef was 
found to be "reference_counted" an analyzer warning would not be issued.

In the current changes, the static analyzer is more lenient and robust. Now, 
when a MallocChecker issue is discovered, each node in the bug path is visited, 
and any use of the pointer in question is checked for its type. If the type is 
found to be "reference_counted" the bug is marked invalid and ultimately not 
delivered to the developer. In the current changes, a pointer in question is 
checked for its type both by using the SymRef and using the type information in 
the AST.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113622/new/

https://reviews.llvm.org/D113622

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/test/Analysis/malloc-annotations.c

Index: clang/test/Analysis/malloc-annotations.c
===================================================================
--- clang/test/Analysis/malloc-annotations.c
+++ clang/test/Analysis/malloc-annotations.c
@@ -30,6 +30,44 @@
 };
 struct stuff myglobalstuff;
 
+struct BMockStruct {
+  int number;
+};
+
+struct AMockStruct {
+  struct BMockStruct bMockStruct;
+};
+
+struct __attribute__((reference_counted)) AnnotatedRefCountedStruct {
+  int mockRefCount;
+  struct AnnotatedRefCountedStruct *mockNext;
+  struct StructContainingAnnotatedRefCountedStruct *unannotatedStructPtr;
+  struct AMockStruct aMockStruct;
+};
+
+struct StructContainingAnnotatedRefCountedStruct {
+  struct AnnotatedRefCountedStruct *refCountedStructPtr;
+  void *opaquePtr;
+  struct AMockStruct aMockStruct;
+};
+
+void my_use_after_free_internal(void *p);
+
+void my_use_after_free_external(void *p) {
+  my_use_after_free_internal(p);
+}
+
+typedef struct AnnotatedRefCountedStruct TypeDefAnnotatedRefCountedStruct;
+struct AnnotatedRefCountedStruct *CreateAnnotatedRefCountedStruct(void);
+TypeDefAnnotatedRefCountedStruct *CreateTypeDefAnnotatedRefCountedStruct(void);
+struct StructContainingAnnotatedRefCountedStruct *CreateStructContainingAnnotatedRefCountedStruct(void);
+typedef struct __attribute__((reference_counted)) UnknownStruct TypeDefUnknownStruct;
+TypeDefUnknownStruct *CreateTypeDefUnknownStruct(void);
+typedef struct __attribute__((reference_counted)) UnknownStruct *TypeDefUnknownStructRef;
+TypeDefUnknownStructRef CreateTypeDefUnknownStructRef(void);
+void *CreateUntypedPointer(void);
+void __attribute__((ownership_takes(malloc, 1))) my_typed_free(struct AnnotatedRefCountedStruct *);
+
 void f1() {
   int *p = malloc(12);
   return; // expected-warning{{Potential leak of memory pointed to by}}
@@ -273,3 +311,179 @@
   my_freeBoth(p, q);
 }
 
+struct AnnotatedRefCountedStruct *testAnnotatedRefCountedStructIgnoresUseAfterFree() {
+  struct AnnotatedRefCountedStruct *p = CreateAnnotatedRefCountedStruct();
+  my_free(p);
+
+  return p;
+}
+
+TypeDefAnnotatedRefCountedStruct *testTypeDefAnnotatedRefCountedStructIgnoresUseAfterFree() {
+  TypeDefAnnotatedRefCountedStruct *p = CreateTypeDefAnnotatedRefCountedStruct();
+  my_free(p);
+
+  return p;
+}
+
+void testTypeDefAnnotatedRefCountedStructIgnoresDoubleFree() {
+  TypeDefAnnotatedRefCountedStruct *p = CreateTypeDefAnnotatedRefCountedStruct();
+
+  my_free(p);
+  my_free(p);
+}
+
+void testCompileTimeAnnotatedTypeIsSufficientToIgnoreDoubleFree() {
+  TypeDefAnnotatedRefCountedStruct *p = CreateUntypedPointer();
+
+  my_free(p);
+  my_free(p);
+}
+
+void testCompileTimeAnnotatedTypeAtUseSiteIsSufficientToIgnoreDoubleFree() {
+  void *p = CreateUntypedPointer();
+
+  my_free(p);
+  my_typed_free(p);
+}
+
+void testCompileTimeDoubleCastTypeCheckingIgnoresUseAfterFree() {
+  TypeDefAnnotatedRefCountedStruct *p = CreateUntypedPointer();
+
+  my_free(p);
+  my_use_after_free_external(p);
+}
+
+TypeDefUnknownStruct *testTypeDefUnkownStructIgnoresUseAfterFree() {
+  TypeDefUnknownStruct *p = CreateTypeDefUnknownStruct();
+
+  my_free(p);
+  return p;
+}
+
+TypeDefUnknownStructRef testTypeDefUnkownStructRefIgnoresUseAfterFree() {
+  TypeDefUnknownStructRef ref = CreateTypeDefUnknownStructRef();
+
+  my_free(ref);
+  return ref;
+}
+
+TypeDefUnknownStruct *testTypeDefUnkownStructFromArrayIgnoresUseAfterFree() {
+  TypeDefUnknownStruct *p[1] = {};
+  p[0] = CreateTypeDefUnknownStruct();
+
+  my_free(p[0]);
+  return p[0];
+}
+
+struct AnnotatedRefCountedStruct *testCreateUntypedPointerWhereCallsiteKnowsIgnoreUseAfterFree() {
+  struct AnnotatedRefCountedStruct *p = CreateUntypedPointer();
+  my_free(p);
+
+  return p;
+}
+
+struct AnnotatedRefCountedStruct *testCreateUntypedPointerWhereReturnTypeKnowsIgnoreUseAfterFree() {
+  void *p = CreateUntypedPointer();
+  my_typed_free(p);
+
+  return p;
+}
+
+TypeDefUnknownStructRef testCreateUntypedPointerWhereReturnTypeIsRefAndKnowsIgnoreUseAfterFree() {
+  void *p = CreateUntypedPointer();
+  my_free(p);
+
+  return p;
+}
+
+int testMemberExpressionTypedBaseIsUsedAsTypeToIgnoreUseAfterFree() {
+  struct AnnotatedRefCountedStruct *p = CreateAnnotatedRefCountedStruct();
+  my_free(p);
+
+  return p->mockRefCount;
+}
+
+struct AnnotatedRefCountedStruct *testUseAfterFreeIsIgnoredWhenEmbedded() {
+  struct StructContainingAnnotatedRefCountedStruct *p = CreateUntypedPointer();
+  p->refCountedStructPtr = CreateUntypedPointer();
+  my_free(p->refCountedStructPtr);
+  my_free(p);
+
+  return p->refCountedStructPtr; // expected-warning{{Use of memory after it is freed}}
+}
+
+int testMemberExpressionUntypedBaseIsUsedAsTypeToIgnoreUseAfterFree() {
+  struct AnnotatedRefCountedStruct *p = CreateUntypedPointer();
+  my_free(p);
+
+  return p->mockRefCount;
+}
+
+int testMemberExpressionTypedInitiatedMemberIsUsedAsTypeToIgnoreUseAfterFree() {
+  struct AnnotatedRefCountedStruct *p = CreateAnnotatedRefCountedStruct();
+  p->mockNext = CreateAnnotatedRefCountedStruct();
+  my_free(p->mockNext);
+
+  return p->mockNext->mockRefCount;
+}
+
+int testMemberExpressionUnTypedInitiedMemberIsUsedAsTypeToIgnoreUseAfterFree() {
+  struct AnnotatedRefCountedStruct *p = CreateUntypedPointer();
+  p->mockNext = CreateUntypedPointer();
+  my_free(p->mockNext);
+
+  return p->mockNext->mockRefCount;
+}
+
+struct StructContainingAnnotatedRefCountedStruct *testMemberExpressionIsUsedAsTypeToShowUseAfterFree() {
+  struct StructContainingAnnotatedRefCountedStruct *s = CreateStructContainingAnnotatedRefCountedStruct();
+  my_free(s);
+
+  return s; // expected-warning{{Use of memory after it is freed}}
+}
+
+void *testMemberExpressionIndicatesUseAfterFree() {
+  struct StructContainingAnnotatedRefCountedStruct *s = CreateStructContainingAnnotatedRefCountedStruct();
+  my_free(s);
+
+  return s->opaquePtr; // expected-warning{{Use of memory after it is freed}}
+}
+
+void *testUseAfterFreeIsIgnoredFromRootMemberExprBase() {
+  struct AnnotatedRefCountedStruct *p = CreateAnnotatedRefCountedStruct();
+  my_free(p);
+
+  return p->unannotatedStructPtr->opaquePtr;
+}
+
+int testUseAfterFreeIsDiscoveredFromRootMemberExprBaseWhenChainContainsRefCountedDereference() {
+  struct StructContainingAnnotatedRefCountedStruct *p = CreateUntypedPointer();
+  p->refCountedStructPtr = CreateUntypedPointer();
+  my_free(p->refCountedStructPtr);
+  my_free(p);
+
+  return p->refCountedStructPtr->mockRefCount; // expected-warning{{Use of memory after it is freed}}
+}
+
+void *testEmbeddedUseAfterFreeIsDiscovered() {
+  struct AnnotatedRefCountedStruct *p = CreateUntypedPointer();
+  p->mockNext = CreateUntypedPointer();
+  p->unannotatedStructPtr = CreateStructContainingAnnotatedRefCountedStruct();
+  my_free(p->mockNext->unannotatedStructPtr);
+
+  return p->mockNext->unannotatedStructPtr->opaquePtr; // expected-warning{{Use of memory after it is freed}}
+}
+
+int testChainedMemberExpressionsWithDotsDiscoversUseAfterFree() {
+  struct StructContainingAnnotatedRefCountedStruct *p = CreateStructContainingAnnotatedRefCountedStruct();
+  my_free(p);
+
+  return p->aMockStruct.bMockStruct.number; // expected-warning{{Use of memory after it is freed}}
+}
+
+int testChainedMemberExpressionsToReferenceCountedUntypedWithDotsIgnoresUseAfterFree() {
+  struct AnnotatedRefCountedStruct *p = CreateUntypedPointer();
+  my_free(p);
+
+  return p->aMockStruct.bMockStruct.number;
+}
\ No newline at end of file
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -216,6 +216,21 @@
 /// Check if the memory associated with this symbol was released.
 static bool isReleased(SymbolRef Sym, CheckerContext &C);
 
+/// Check if the if the qualified type is declared to be using reference
+/// counting
+static bool isReferenceCountedAttributedQualType(QualType QT);
+
+/// Check if the if the value or the declaration for this value is declared to
+/// be using referencing counting
+static bool isReferenceCountedAttributed(SymbolRef Sym, const Stmt *S);
+
+/// Check if the memory associated with this symbol is using reference counting
+static bool isReferenceCountedAttributedSymRef(SymbolRef Sym);
+
+/// Check if this expression type has been declared to using referencing
+/// counting
+static bool isReferenceCountedAttributedExpr(const Stmt *S);
+
 /// Update the RefState to reflect the new memory allocation.
 /// The optional \p RetVal parameter specifies the newly allocated pointer
 /// value; if unspecified, the value of expression \p E is used.
@@ -2956,6 +2971,45 @@
   return (RS && RS->isReleased());
 }
 
+static bool isReferenceCountedAttributedQualType(QualType QT) {
+  if (QT.isNull())
+    return false;
+
+  QualType PT = QT->getPointeeType();
+  if (PT.isNull())
+    return false;
+
+  Decl *RD = PT->getAsRecordDecl();
+  if (!RD)
+    return false;
+
+  return RD->hasAttr<ReferenceCountedAttr>();
+}
+
+static bool isReferenceCountedAttributedSymRef(SymbolRef Sym) {
+  QualType QT = Sym->getType();
+  if (QT.isNull())
+    return false;
+
+  return isReferenceCountedAttributedQualType(QT);
+}
+
+static bool isReferenceCountedAttributedExpr(const Stmt *S) {
+  const Expr *E = dyn_cast_or_null<Expr>(S);
+  if (!E)
+    return false;
+
+  QualType QT = E->getType();
+  return isReferenceCountedAttributedQualType(QT);
+}
+
+static bool isReferenceCountedAttributed(SymbolRef Sym, const Stmt *S) {
+  bool result = isReferenceCountedAttributedSymRef(Sym) ||
+                isReferenceCountedAttributedExpr(S);
+
+  return result;
+}
+
 bool MallocChecker::suppressDeallocationsInSuspiciousContexts(
     const CallEvent &Call, CheckerContext &C) const {
   if (Call.getNumArgs() == 0)
@@ -3335,6 +3389,16 @@
   const RefState *RSPrev = statePrev->get<RegionState>(Sym);
 
   const Stmt *S = N->getStmtForDiagnostics();
+
+  SymbolRef SR = nullptr;
+  const Expr *E = dyn_cast_or_null<Expr>(S);
+  if (E && E->isPRValue())
+    SR = N->getSVal(E).getAsSymbol();
+
+  if (SR == Sym)
+    if (isReferenceCountedAttributed(SR, S))
+      BR.markInvalid(getTag(), S);
+
   // When dealing with containers, we sometimes want to give a note
   // even if the statement is missing.
   if (!S && (!RSCurr || RSCurr->getAllocationFamily() != AF_InnerBuffer))
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1694,6 +1694,13 @@
   let SimpleHandler = 1;
 }
 
+def ReferenceCounted : InheritableAttr {
+  let Spellings = [Clang<"reference_counted">];
+  let Subjects = SubjectList<[Record]>;
+  let Documentation = [Undocumented];
+  let SimpleHandler = 1;
+}
+
 def ReturnsTwice : InheritableAttr {
   let Spellings = [GCC<"returns_twice">];
   let Subjects = SubjectList<[Function]>;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to