[clang] 634258b - [analyzer] StdLibraryFunctionsChecker: Add support to lookup types

Gabor Marton via cfe-commits cfe-commits at lists.llvm.org
Fri May 29 08:58:51 PDT 2020


Author: Gabor Marton
Date: 2020-05-29T17:42:05+02:00
New Revision: 634258b80606c4bb8192077239a089ae5842781a

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

LOG: [analyzer] StdLibraryFunctionsChecker: Add support to lookup types

Summary:
In this patch I am trying to get rid of the `Irrelevant` types from the
signatures of the functions from the standard C library. For that I've
introduced `lookupType()` to be able to lookup arbitrary types in the global
scope. This makes it possible to define the signatures precisely.

Note 1) `fread`'s signature is now fixed to have the proper `FILE *restrict`
type when C99 is the language.
Note 2) There are still existing `Irrelevant` types, but they are all from
POSIX. I am planning to address those together with the missing POSIX functions
(in D79433).

Reviewers: xazax.hun, NoQ, Szelethus, balazske

Subscribers: whisperity, baloghadamsoftware, szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, gamesh411, Charusso, steakhal, ASDenysPetrov, cfe-commits

Tags: #clang

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

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

Modified: 
    clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
    clang/test/Analysis/std-c-library-functions-arg-constraints.c
    clang/test/Analysis/std-c-library-functions.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
index 578f6ad46b84..6feae56502f1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -725,6 +725,26 @@ StdLibraryFunctionsChecker::findFunctionSummary(const CallEvent &Call,
   return findFunctionSummary(FD, C);
 }
 
+llvm::Optional<QualType> lookupType(StringRef Name, const ASTContext &ACtx) {
+  IdentifierInfo &II = ACtx.Idents.get(Name);
+  auto LookupRes = ACtx.getTranslationUnitDecl()->lookup(&II);
+  if (LookupRes.size() == 0)
+    return None;
+
+  // Prioritze typedef declarations.
+  // This is needed in case of C struct typedefs. E.g.:
+  //   typedef struct FILE FILE;
+  // In this case, we have a RecordDecl 'struct FILE' with the name 'FILE' and
+  // we have a TypedefDecl with the name 'FILE'.
+  for (Decl *D : LookupRes) {
+    if (auto *TD = dyn_cast<TypedefNameDecl>(D))
+      return ACtx.getTypeDeclType(TD).getCanonicalType();
+  }
+  assert(LookupRes.size() == 1 && "Type identifier should be unique");
+  auto *D = cast<TypeDecl>(LookupRes.front());
+  return ACtx.getTypeDeclType(D).getCanonicalType();
+}
+
 void StdLibraryFunctionsChecker::initFunctionSummaries(
     CheckerContext &C) const {
   if (!FunctionSummaryMap.empty())
@@ -747,13 +767,16 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
   const QualType SizeTy = ACtx.getSizeType();
   const QualType VoidPtrTy = ACtx.VoidPtrTy; // void *
   const QualType VoidPtrRestrictTy =
-      ACtx.getRestrictType(VoidPtrTy); // void *restrict
+      ACtx.getLangOpts().C99 ? ACtx.getRestrictType(VoidPtrTy) // void *restrict
+                             : VoidPtrTy;
   const QualType ConstVoidPtrTy =
       ACtx.getPointerType(ACtx.VoidTy.withConst()); // const void *
   const QualType ConstCharPtrTy =
       ACtx.getPointerType(ACtx.CharTy.withConst()); // const char *
   const QualType ConstVoidPtrRestrictTy =
-      ACtx.getRestrictType(ConstVoidPtrTy); // const void *restrict
+      ACtx.getLangOpts().C99
+          ? ACtx.getRestrictType(ConstVoidPtrTy) // const void *restrict
+          : ConstVoidPtrTy;
 
   const RangeInt IntMax = BVF.getMaxValue(IntTy).getLimitedValue();
   const RangeInt LongMax = BVF.getMaxValue(LongTy).getLimitedValue();
@@ -871,10 +894,20 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
     return std::make_shared<NotNullConstraint>(ArgN);
   };
 
+  Optional<QualType> FileTy = lookupType("FILE", ACtx);
+  Optional<QualType> FilePtrTy, FilePtrRestrictTy;
+  if (FileTy) {
+    // FILE *
+    FilePtrTy = ACtx.getPointerType(*FileTy);
+    // FILE *restrict
+    FilePtrRestrictTy =
+        ACtx.getLangOpts().C99 ? ACtx.getRestrictType(*FilePtrTy) : *FilePtrTy;
+  }
+
   using RetType = QualType;
   // Templates for summaries that are reused by many functions.
   auto Getc = [&]() {
-    return Summary(ArgTypes{Irrelevant}, RetType{IntTy}, NoEvalCall)
+    return Summary(ArgTypes{*FilePtrTy}, RetType{IntTy}, NoEvalCall)
         .Case({ReturnValueCondition(WithinRange,
                                     {{EOFv, EOFv}, {0, UCharRangeMax}})});
   };
@@ -885,17 +918,18 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
                ReturnValueCondition(WithinRange, Range(-1, Max))});
   };
   auto Fread = [&]() {
-    return Summary(ArgTypes{VoidPtrRestrictTy, Irrelevant, SizeTy, Irrelevant},
-                   RetType{SizeTy}, NoEvalCall)
+    return Summary(
+               ArgTypes{VoidPtrRestrictTy, SizeTy, SizeTy, *FilePtrRestrictTy},
+               RetType{SizeTy}, NoEvalCall)
         .Case({
             ReturnValueCondition(LessThanOrEq, ArgNo(2)),
         })
         .ArgConstraint(NotNull(ArgNo(0)));
   };
   auto Fwrite = [&]() {
-    return Summary(
-               ArgTypes{ConstVoidPtrRestrictTy, Irrelevant, SizeTy, Irrelevant},
-               RetType{SizeTy}, NoEvalCall)
+    return Summary(ArgTypes{ConstVoidPtrRestrictTy, SizeTy, SizeTy,
+                            *FilePtrRestrictTy},
+                   RetType{SizeTy}, NoEvalCall)
         .Case({
             ReturnValueCondition(LessThanOrEq, ArgNo(2)),
         })
@@ -1042,23 +1076,33 @@ void StdLibraryFunctionsChecker::initFunctionSummaries(
                  ReturnValueCondition(WithinRange, SingleValue(0))}));
 
   // The getc() family of functions that returns either a char or an EOF.
-  addToFunctionSummaryMap("getc", Getc());
-  addToFunctionSummaryMap("fgetc", Getc());
+  if (FilePtrTy) {
+    addToFunctionSummaryMap("getc", Getc());
+    addToFunctionSummaryMap("fgetc", Getc());
+  }
   addToFunctionSummaryMap(
       "getchar", Summary(ArgTypes{}, RetType{IntTy}, NoEvalCall)
                      .Case({ReturnValueCondition(
                          WithinRange, {{EOFv, EOFv}, {0, UCharRangeMax}})}));
 
   // read()-like functions that never return more than buffer size.
+  if (FilePtrRestrictTy) {
+    addToFunctionSummaryMap("fread", Fread());
+    addToFunctionSummaryMap("fwrite", Fwrite());
+  }
+
   // We are not sure how ssize_t is defined on every platform, so we
   // provide three variants that should cover common cases.
+  // FIXME these are actually defined by POSIX and not by the C standard, we
+  // should handle them together with the rest of the POSIX functions.
   addToFunctionSummaryMap("read", {Read(IntTy, IntMax), Read(LongTy, LongMax),
                                    Read(LongLongTy, LongLongMax)});
   addToFunctionSummaryMap("write", {Read(IntTy, IntMax), Read(LongTy, LongMax),
                                     Read(LongLongTy, LongLongMax)});
-  addToFunctionSummaryMap("fread", Fread());
-  addToFunctionSummaryMap("fwrite", Fwrite());
+
   // getline()-like functions either fail or read at least the delimiter.
+  // FIXME these are actually defined by POSIX and not by the C standard, we
+  // should handle them together with the rest of the POSIX functions.
   addToFunctionSummaryMap("getline",
                           {Getline(IntTy, IntMax), Getline(LongTy, LongMax),
                            Getline(LongLongTy, LongLongMax)});

diff  --git a/clang/test/Analysis/std-c-library-functions-arg-constraints.c b/clang/test/Analysis/std-c-library-functions-arg-constraints.c
index 60338128ec89..b99248d337b3 100644
--- a/clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ b/clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -64,7 +64,7 @@ void test_alnum_symbolic2(int x) {
 
 typedef struct FILE FILE;
 typedef typeof(sizeof(int)) size_t;
-size_t fread(void *restrict, size_t, size_t, FILE *);
+size_t fread(void *restrict, size_t, size_t, FILE *restrict);
 void test_notnull_concrete(FILE *fp) {
   fread(0, sizeof(int), 10, fp); // \
   // report-warning{{Function argument constraint is not satisfied}} \

diff  --git a/clang/test/Analysis/std-c-library-functions-lookup.c b/clang/test/Analysis/std-c-library-functions-lookup.c
new file mode 100644
index 000000000000..495562a2a5a4
--- /dev/null
+++ b/clang/test/Analysis/std-c-library-functions-lookup.c
@@ -0,0 +1,19 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -triple i686-unknown-linux 2>&1 | FileCheck %s
+
+// CHECK: Loaded summary for: unsigned int fread(void *restrict, size_t, size_t, FILE *restrict)
+
+typedef typeof(sizeof(int)) size_t;
+typedef struct FILE FILE;
+size_t fread(void *restrict, size_t, size_t, FILE *restrict);
+
+// 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-lookup.cpp b/clang/test/Analysis/std-c-library-functions-lookup.cpp
new file mode 100644
index 000000000000..888ab27d501f
--- /dev/null
+++ b/clang/test/Analysis/std-c-library-functions-lookup.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -triple i686-unknown-linux 2>&1 | FileCheck %s
+
+//      CHECK: Loaded summary for: size_t fread(void *, size_t, size_t, FILE *)
+//  CHECK-NOT: Loaded summary for: size_t fread(void *, size_t, size_t, MyFile *)
+
+typedef unsigned int size_t;
+typedef struct FILE FILE;
+size_t fread(void *, size_t, size_t, FILE *);
+
+struct MyFile;
+size_t fread(void *, size_t, size_t, MyFile *);
+
+// 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.c b/clang/test/Analysis/std-c-library-functions.c
index c4762d74ea63..dbe7d102bb62 100644
--- a/clang/test/Analysis/std-c-library-functions.c
+++ b/clang/test/Analysis/std-c-library-functions.c
@@ -53,10 +53,10 @@
 // CHECK-NEXT: Loaded summary for: int getc(FILE *)
 // CHECK-NEXT: Loaded summary for: int fgetc(FILE *)
 // CHECK-NEXT: Loaded summary for: int getchar()
+// CHECK-NEXT: Loaded summary for: unsigned int fread(void *restrict, size_t, size_t, FILE *restrict)
+// CHECK-NEXT: Loaded summary for: unsigned int fwrite(const void *restrict, size_t, size_t, FILE *restrict)
 // CHECK-NEXT: Loaded summary for: ssize_t read(int, void *, size_t)
 // CHECK-NEXT: Loaded summary for: ssize_t write(int, const void *, size_t)
-// CHECK-NEXT: Loaded summary for: unsigned int fread(void *restrict, size_t, size_t, FILE *)
-// CHECK-NEXT: Loaded summary for: unsigned int fwrite(const void *restrict, size_t, size_t, FILE *restrict)
 // CHECK-NEXT: Loaded summary for: ssize_t getline(char **, size_t *, FILE *)
 
 void clang_analyzer_eval(int);
@@ -104,7 +104,7 @@ void test_read_write(int fd, char *buf) {
   }
 }
 
-size_t fread(void *restrict, size_t, size_t, FILE *);
+size_t fread(void *restrict, size_t, size_t, FILE *restrict);
 size_t fwrite(const void *restrict, size_t, size_t, FILE *restrict);
 void test_fread_fwrite(FILE *fp, int *buf) {
 


        


More information about the cfe-commits mailing list