Skip to content

Commit

Permalink
[analyzer] StdLibraryFunctionsChecker: Add support to lookup types
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Gabor Marton committed May 29, 2020
1 parent 9e0b52e commit 634258b
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 16 deletions.
68 changes: 56 additions & 12 deletions clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
Expand Up @@ -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())
Expand All @@ -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();
Expand Down Expand Up @@ -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}})});
};
Expand All @@ -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)),
})
Expand Down Expand Up @@ -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)});
Expand Down
Expand Up @@ -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}} \
Expand Down
19 changes: 19 additions & 0 deletions 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();
}
23 changes: 23 additions & 0 deletions 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();
}
6 changes: 3 additions & 3 deletions clang/test/Analysis/std-c-library-functions.c
Expand Up @@ -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);
Expand Down Expand Up @@ -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) {

Expand Down

0 comments on commit 634258b

Please sign in to comment.