[clang] 9b1e4a8 - [analyzer] Fix NSErrorChecker false positives on constructors.

[Artem Dergachev via cfe-commits]

Author: Artem Dergachev
Date: 2020-04-06T20:33:38+03:00
New Revision: 9b1e4a8218b7fcb8900d0382704fdb4e38b52eb0

URL: https://github.com/llvm/llvm-project/commit/9b1e4a8218b7fcb8900d0382704fdb4e38b52eb0
DIFF: https://github.com/llvm/llvm-project/commit/9b1e4a8218b7fcb8900d0382704fdb4e38b52eb0.diff

LOG: [analyzer] Fix NSErrorChecker false positives on constructors.

Constructors and delete operators cannot return a boolean value.
Therefore they cannot possibly follow the NS/CFError-related coding
conventions.

Patch by Valeriy Savchenko!

Differential Revision: https://reviews.llvm.org/D77551

Added: 
    clang/test/Analysis/SpecialFunctionsCFError.cpp

Modified: 
    clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
index 2dee1e459a06..5f68788fafcd 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
@@ -95,6 +95,15 @@ class CFErrorFunctionChecker
 };
 }
 
+static bool hasReservedReturnType(const FunctionDecl *D) {
+  if (isa<CXXConstructorDecl>(D))
+    return true;
+
+  // operators delete and delete[] are required to have 'void' return type
+  auto OperatorKind = D->getOverloadedOperator();
+  return OperatorKind == OO_Delete || OperatorKind == OO_Array_Delete;
+}
+
 void CFErrorFunctionChecker::checkASTDecl(const FunctionDecl *D,
                                         AnalysisManager &mgr,
                                         BugReporter &BR) const {
@@ -102,6 +111,8 @@ void CFErrorFunctionChecker::checkASTDecl(const FunctionDecl *D,
     return;
   if (!D->getReturnType()->isVoidType())
     return;
+  if (hasReservedReturnType(D))
+    return;
 
   if (!II)
     II = &D->getASTContext().Idents.get("CFErrorRef");

diff  --git a/clang/test/Analysis/SpecialFunctionsCFError.cpp b/clang/test/Analysis/SpecialFunctionsCFError.cpp
new file mode 100644
index 000000000000..3517933f6bcc
--- /dev/null
+++ b/clang/test/Analysis/SpecialFunctionsCFError.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,osx.coreFoundation.CFError \
+// RUN:   -verify %s
+
+typedef unsigned long size_t;
+struct __CFError {};
+typedef struct __CFError *CFErrorRef;
+void *malloc(size_t);
+
+class Foo {
+public:
+  Foo(CFErrorRef *error) {} // no-warning
+
+  void operator delete(void *pointer, CFErrorRef *error) { // no-warning
+    return;
+  }
+
+  void operator delete[](void *pointer, CFErrorRef *error) { // no-warning
+    return;
+  }
+
+  // Check that we report warnings for operators when it can be useful
+  void operator()(CFErrorRef *error) {} // expected-warning {{Function accepting CFErrorRef* should have a non-void return value to indicate whether or not an error occurred}}
+};
+
+// Check that global delete operator is not bothered as well
+void operator delete(void *pointer, CFErrorRef *error) { // no-warning
+  return;
+}