[clang] fe0972d - [analyzer][StdLibraryFunctionsChecker] Do not match based on the restrict qualifier in C++

[Gabor Marton via cfe-commits]

Author: Gabor Marton
Date: 2020-09-04T11:48:38+02:00
New Revision: fe0972d3e4a65b4c5f5fa602b17ad30e463050b3

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

LOG: [analyzer][StdLibraryFunctionsChecker] Do not match based on the restrict qualifier in C++

The "restrict" keyword is illegal in C++, however, many libc
implementations use the "__restrict" compiler intrinsic in functions
prototypes. The "__restrict" keyword qualifies a type as a restricted type
even in C++.
In case of any non-C99 languages, we don't want to match based on the
restrict qualifier because we cannot know if the given libc implementation
qualifies the paramter type or not.

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

Added: 
    clang/test/Analysis/std-c-library-functions-restrict.c
    clang/test/Analysis/std-c-library-functions-restrict.cpp

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

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index c65d58e49d78..2c20422a9cc4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -744,21 +744,38 @@ bool StdLibraryFunctionsChecker::evalCall(const CallEvent &Call,
 bool StdLibraryFunctionsChecker::Signature::matches(
     const FunctionDecl *FD) const {
   assert(!isInvalid());
-  // Check number of arguments:
+  // Check the number of arguments.
   if (FD->param_size() != ArgTys.size())
     return false;
 
-  // Check return type.
-  if (!isIrrelevant(RetTy))
-    if (RetTy != FD->getReturnType().getCanonicalType())
+  // The "restrict" keyword is illegal in C++, however, many libc
+  // implementations use the "__restrict" compiler intrinsic in functions
+  // prototypes. The "__restrict" keyword qualifies a type as a restricted type
+  // even in C++.
+  // In case of any non-C99 languages, we don't want to match based on the
+  // restrict qualifier because we cannot know if the given libc implementation
+  // qualifies the paramter type or not.
+  auto RemoveRestrict = [&FD](QualType T) {
+    if (!FD->getASTContext().getLangOpts().C99)
+      T.removeLocalRestrict();
+    return T;
+  };
+
+  // Check the return type.
+  if (!isIrrelevant(RetTy)) {
+    QualType FDRetTy = RemoveRestrict(FD->getReturnType().getCanonicalType());
+    if (RetTy != FDRetTy)
       return false;
+  }
 
-  // Check argument types.
+  // Check the argument types.
   for (size_t I = 0, E = ArgTys.size(); I != E; ++I) {
     QualType ArgTy = ArgTys[I];
     if (isIrrelevant(ArgTy))
       continue;
-    if (ArgTy != FD->getParamDecl(I)->getType().getCanonicalType())
+    QualType FDArgTy =
+        RemoveRestrict(FD->getParamDecl(I)->getType().getCanonicalType());
+    if (ArgTy != FDArgTy)
       return false;
   }
 
@@ -989,6 +1006,12 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
       for (const Summary &S : Summaries)
         operator()(Name, S);
     }
+    // Add the same summary for 
diff erent names with the Signature explicitly
+    // given.
+    void operator()(std::vector<StringRef> Names, Signature Sign, Summary Sum) {
+      for (StringRef Name : Names)
+        operator()(Name, Sign, Sum);
+    }
   } addToFunctionSummaryMap(ACtx, FunctionSummaryMap, DisplayLoadedSummaries);
 
   // Below are helpers functions to create the summaries.
@@ -2048,6 +2071,11 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
                 EvalCallAsPure)
             .ArgConstraint(BufferSize(/*Buffer=*/ArgNo(0), /*BufSize=*/ArgNo(1),
                                       /*BufSizeMultiplier=*/ArgNo(2))));
+    addToFunctionSummaryMap(
+        {"__test_restrict_param_0", "__test_restrict_param_1",
+         "__test_restrict_param_2"},
+        Signature(ArgTypes{VoidPtrRestrictTy}, RetType{VoidTy}),
+        Summary(EvalCallAsPure));
   }
 }
 

diff  --git a/clang/test/Analysis/std-c-library-functions-restrict.c b/clang/test/Analysis/std-c-library-functions-restrict.c
new file mode 100644
index 000000000000..7cf5f2bc630a
--- /dev/null
+++ b/clang/test/Analysis/std-c-library-functions-restrict.c
@@ -0,0 +1,24 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-checker=debug.StdCLibraryFunctionsTester \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -triple i686-unknown-linux 2>&1 | FileCheck %s
+
+// The signatures for these functions are the same and they specify their
+// parameter with the restrict qualifier. In C, the signature should match only
+// if the restrict qualifier is there on the parameter. Thus, the summary
+// should be loaded for the last two declarations only.
+void __test_restrict_param_0(void *p);
+void __test_restrict_param_1(void *__restrict p);
+void __test_restrict_param_2(void *restrict p);
+
+// CHECK-NOT: Loaded summary for: void __test_restrict_param_0
+//     CHECK: Loaded summary for: void __test_restrict_param_1(void *restrict p)
+//     CHECK: Loaded summary for: void __test_restrict_param_2(void *restrict p)
+
+// Must have at least one call expression to initialize the summary map.
+int bar(void);
+void foo() {
+  bar();
+}

diff  --git a/clang/test/Analysis/std-c-library-functions-restrict.cpp b/clang/test/Analysis/std-c-library-functions-restrict.cpp
new file mode 100644
index 000000000000..d1cd090f5ef8
--- /dev/null
+++ b/clang/test/Analysis/std-c-library-functions-restrict.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-checker=debug.StdCLibraryFunctionsTester \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -triple i686-unknown-linux 2>&1 | FileCheck %s
+
+// The signatures for these functions are the same and they specify their
+// parameter with the restrict qualifier. In C++, however, we are more
+// indulgent and we do not match based on this qualifier. Thus, the given
+// signature should match for both of the declarations below, i.e the summary
+// should be loaded for both of them.
+void __test_restrict_param_0(void *p);
+void __test_restrict_param_1(void *__restrict p);
+// The below declaration is illegal, "restrict" is not a keyword in C++.
+// void __test_restrict_param_2(void *restrict p);
+
+// CHECK: Loaded summary for: void __test_restrict_param_0(void *p)
+// CHECK: Loaded summary for: void __test_restrict_param_1(void *__restrict p)
+
+// Must have at least one call expression to initialize the summary map.
+int bar(void);
+void foo() {
+  bar();
+}