Skip to content

Commit 35fdec1

Browse files
haoNoQhaoNoQ
committed
[analyzer] CStringChecker: Modernize to use CallDescriptions.
This patch uses the new CDF_MaybeBuiltin flag to handle C library functions. It's mostly an NFC/refactoring pass, but it does fix a bug in handling memset() when it expands to __builtin___memset_chk() because the latter has one more argument and memset() handling code was trying to match the exact number of arguments. Now the code is deduplicated and there's less room for mistakes. Differential Revision: https://reviews.llvm.org/D62557 llvm-svn: 364868
1 parent f301096 commit 35fdec1

File tree

2 files changed

+64
-147
lines changed

2 files changed

+64
-147
lines changed

‎clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

+58-147
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,38 @@ class CStringChecker : public Checker< eval::Call,
7373

7474
typedef void (CStringChecker::*FnCheck)(CheckerContext &,
7575
const CallExpr *) const;
76+
CallDescriptionMap<FnCheck> Callbacks = {
77+
{{CDF_MaybeBuiltin, "memcpy", 3}, &CStringChecker::evalMemcpy},
78+
{{CDF_MaybeBuiltin, "mempcpy", 3}, &CStringChecker::evalMempcpy},
79+
{{CDF_MaybeBuiltin, "memcmp", 3}, &CStringChecker::evalMemcmp},
80+
{{CDF_MaybeBuiltin, "memmove", 3}, &CStringChecker::evalMemmove},
81+
{{CDF_MaybeBuiltin, "memset", 3}, &CStringChecker::evalMemset},
82+
{{CDF_MaybeBuiltin, "explicit_memset", 3}, &CStringChecker::evalMemset},
83+
{{CDF_MaybeBuiltin, "strcpy", 2}, &CStringChecker::evalStrcpy},
84+
{{CDF_MaybeBuiltin, "strncpy", 3}, &CStringChecker::evalStrncpy},
85+
{{CDF_MaybeBuiltin, "stpcpy", 2}, &CStringChecker::evalStpcpy},
86+
{{CDF_MaybeBuiltin, "strlcpy", 3}, &CStringChecker::evalStrlcpy},
87+
{{CDF_MaybeBuiltin, "strcat", 2}, &CStringChecker::evalStrcat},
88+
{{CDF_MaybeBuiltin, "strncat", 3}, &CStringChecker::evalStrncat},
89+
{{CDF_MaybeBuiltin, "strlcat", 3}, &CStringChecker::evalStrlcat},
90+
{{CDF_MaybeBuiltin, "strlen", 1}, &CStringChecker::evalstrLength},
91+
{{CDF_MaybeBuiltin, "strnlen", 2}, &CStringChecker::evalstrnLength},
92+
{{CDF_MaybeBuiltin, "strcmp", 2}, &CStringChecker::evalStrcmp},
93+
{{CDF_MaybeBuiltin, "strncmp", 3}, &CStringChecker::evalStrncmp},
94+
{{CDF_MaybeBuiltin, "strcasecmp", 2}, &CStringChecker::evalStrcasecmp},
95+
{{CDF_MaybeBuiltin, "strncasecmp", 3}, &CStringChecker::evalStrncasecmp},
96+
{{CDF_MaybeBuiltin, "strsep", 2}, &CStringChecker::evalStrsep},
97+
{{CDF_MaybeBuiltin, "bcopy", 3}, &CStringChecker::evalBcopy},
98+
{{CDF_MaybeBuiltin, "bcmp", 3}, &CStringChecker::evalMemcmp},
99+
{{CDF_MaybeBuiltin, "bzero", 2}, &CStringChecker::evalBzero},
100+
{{CDF_MaybeBuiltin, "explicit_bzero", 2}, &CStringChecker::evalBzero},
101+
};
102+
103+
// These require a bit of special handling.
104+
CallDescription StdCopy{{"std", "copy"}, 3},
105+
StdCopyBackward{{"std", "copy_backward"}, 3};
76106

107+
FnCheck identifyCall(const CallEvent &Call, CheckerContext &C) const;
77108
void evalMemcpy(CheckerContext &C, const CallExpr *CE) const;
78109
void evalMempcpy(CheckerContext &C, const CallExpr *CE) const;
79110
void evalMemmove(CheckerContext &C, const CallExpr *CE) const;
@@ -1201,9 +1232,6 @@ void CStringChecker::evalCopyCommon(CheckerContext &C,
12011232

12021233

12031234
void CStringChecker::evalMemcpy(CheckerContext &C, const CallExpr *CE) const {
1204-
if (CE->getNumArgs() < 3)
1205-
return;
1206-
12071235
// void *memcpy(void *restrict dst, const void *restrict src, size_t n);
12081236
// The return value is the address of the destination buffer.
12091237
const Expr *Dest = CE->getArg(0);
@@ -1213,9 +1241,6 @@ void CStringChecker::evalMemcpy(CheckerContext &C, const CallExpr *CE) const {
12131241
}
12141242

12151243
void CStringChecker::evalMempcpy(CheckerContext &C, const CallExpr *CE) const {
1216-
if (CE->getNumArgs() < 3)
1217-
return;
1218-
12191244
// void *mempcpy(void *restrict dst, const void *restrict src, size_t n);
12201245
// The return value is a pointer to the byte following the last written byte.
12211246
const Expr *Dest = CE->getArg(0);
@@ -1225,9 +1250,6 @@ void CStringChecker::evalMempcpy(CheckerContext &C, const CallExpr *CE) const {
12251250
}
12261251

12271252
void CStringChecker::evalMemmove(CheckerContext &C, const CallExpr *CE) const {
1228-
if (CE->getNumArgs() < 3)
1229-
return;
1230-
12311253
// void *memmove(void *dst, const void *src, size_t n);
12321254
// The return value is the address of the destination buffer.
12331255
const Expr *Dest = CE->getArg(0);
@@ -1237,18 +1259,12 @@ void CStringChecker::evalMemmove(CheckerContext &C, const CallExpr *CE) const {
12371259
}
12381260

12391261
void CStringChecker::evalBcopy(CheckerContext &C, const CallExpr *CE) const {
1240-
if (CE->getNumArgs() < 3)
1241-
return;
1242-
12431262
// void bcopy(const void *src, void *dst, size_t n);
12441263
evalCopyCommon(C, CE, C.getState(),
12451264
CE->getArg(2), CE->getArg(1), CE->getArg(0));
12461265
}
12471266

12481267
void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE) const {
1249-
if (CE->getNumArgs() < 3)
1250-
return;
1251-
12521268
// int memcmp(const void *s1, const void *s2, size_t n);
12531269
CurrentFunctionDescription = "memory comparison function";
12541270

@@ -1323,18 +1339,12 @@ void CStringChecker::evalMemcmp(CheckerContext &C, const CallExpr *CE) const {
13231339

13241340
void CStringChecker::evalstrLength(CheckerContext &C,
13251341
const CallExpr *CE) const {
1326-
if (CE->getNumArgs() < 1)
1327-
return;
1328-
13291342
// size_t strlen(const char *s);
13301343
evalstrLengthCommon(C, CE, /* IsStrnlen = */ false);
13311344
}
13321345

13331346
void CStringChecker::evalstrnLength(CheckerContext &C,
13341347
const CallExpr *CE) const {
1335-
if (CE->getNumArgs() < 2)
1336-
return;
1337-
13381348
// size_t strnlen(const char *s, size_t maxlen);
13391349
evalstrLengthCommon(C, CE, /* IsStrnlen = */ true);
13401350
}
@@ -1459,9 +1469,6 @@ void CStringChecker::evalstrLengthCommon(CheckerContext &C, const CallExpr *CE,
14591469
}
14601470

14611471
void CStringChecker::evalStrcpy(CheckerContext &C, const CallExpr *CE) const {
1462-
if (CE->getNumArgs() < 2)
1463-
return;
1464-
14651472
// char *strcpy(char *restrict dst, const char *restrict src);
14661473
evalStrcpyCommon(C, CE,
14671474
/* returnEnd = */ false,
@@ -1470,9 +1477,6 @@ void CStringChecker::evalStrcpy(CheckerContext &C, const CallExpr *CE) const {
14701477
}
14711478

14721479
void CStringChecker::evalStrncpy(CheckerContext &C, const CallExpr *CE) const {
1473-
if (CE->getNumArgs() < 3)
1474-
return;
1475-
14761480
// char *strncpy(char *restrict dst, const char *restrict src, size_t n);
14771481
evalStrcpyCommon(C, CE,
14781482
/* returnEnd = */ false,
@@ -1481,9 +1485,6 @@ void CStringChecker::evalStrncpy(CheckerContext &C, const CallExpr *CE) const {
14811485
}
14821486

14831487
void CStringChecker::evalStpcpy(CheckerContext &C, const CallExpr *CE) const {
1484-
if (CE->getNumArgs() < 2)
1485-
return;
1486-
14871488
// char *stpcpy(char *restrict dst, const char *restrict src);
14881489
evalStrcpyCommon(C, CE,
14891490
/* returnEnd = */ true,
@@ -1492,9 +1493,6 @@ void CStringChecker::evalStpcpy(CheckerContext &C, const CallExpr *CE) const {
14921493
}
14931494

14941495
void CStringChecker::evalStrlcpy(CheckerContext &C, const CallExpr *CE) const {
1495-
if (CE->getNumArgs() < 3)
1496-
return;
1497-
14981496
// char *strlcpy(char *dst, const char *src, size_t n);
14991497
evalStrcpyCommon(C, CE,
15001498
/* returnEnd = */ true,
@@ -1504,9 +1502,6 @@ void CStringChecker::evalStrlcpy(CheckerContext &C, const CallExpr *CE) const {
15041502
}
15051503

15061504
void CStringChecker::evalStrcat(CheckerContext &C, const CallExpr *CE) const {
1507-
if (CE->getNumArgs() < 2)
1508-
return;
1509-
15101505
//char *strcat(char *restrict s1, const char *restrict s2);
15111506
evalStrcpyCommon(C, CE,
15121507
/* returnEnd = */ false,
@@ -1515,9 +1510,6 @@ void CStringChecker::evalStrcat(CheckerContext &C, const CallExpr *CE) const {
15151510
}
15161511

15171512
void CStringChecker::evalStrncat(CheckerContext &C, const CallExpr *CE) const {
1518-
if (CE->getNumArgs() < 3)
1519-
return;
1520-
15211513
//char *strncat(char *restrict s1, const char *restrict s2, size_t n);
15221514
evalStrcpyCommon(C, CE,
15231515
/* returnEnd = */ false,
@@ -1526,9 +1518,6 @@ void CStringChecker::evalStrncat(CheckerContext &C, const CallExpr *CE) const {
15261518
}
15271519

15281520
void CStringChecker::evalStrlcat(CheckerContext &C, const CallExpr *CE) const {
1529-
if (CE->getNumArgs() < 3)
1530-
return;
1531-
15321521
// FIXME: strlcat() uses a different rule for bound checking, i.e. 'n' means
15331522
// a different thing as compared to strncat(). This currently causes
15341523
// false positives in the alpha string bound checker.
@@ -1885,35 +1874,23 @@ void CStringChecker::evalStrcpyCommon(CheckerContext &C, const CallExpr *CE,
18851874
}
18861875

18871876
void CStringChecker::evalStrcmp(CheckerContext &C, const CallExpr *CE) const {
1888-
if (CE->getNumArgs() < 2)
1889-
return;
1890-
18911877
//int strcmp(const char *s1, const char *s2);
18921878
evalStrcmpCommon(C, CE, /* isBounded = */ false, /* ignoreCase = */ false);
18931879
}
18941880

18951881
void CStringChecker::evalStrncmp(CheckerContext &C, const CallExpr *CE) const {
1896-
if (CE->getNumArgs() < 3)
1897-
return;
1898-
18991882
//int strncmp(const char *s1, const char *s2, size_t n);
19001883
evalStrcmpCommon(C, CE, /* isBounded = */ true, /* ignoreCase = */ false);
19011884
}
19021885

19031886
void CStringChecker::evalStrcasecmp(CheckerContext &C,
19041887
const CallExpr *CE) const {
1905-
if (CE->getNumArgs() < 2)
1906-
return;
1907-
19081888
//int strcasecmp(const char *s1, const char *s2);
19091889
evalStrcmpCommon(C, CE, /* isBounded = */ false, /* ignoreCase = */ true);
19101890
}
19111891

19121892
void CStringChecker::evalStrncasecmp(CheckerContext &C,
19131893
const CallExpr *CE) const {
1914-
if (CE->getNumArgs() < 3)
1915-
return;
1916-
19171894
//int strncasecmp(const char *s1, const char *s2, size_t n);
19181895
evalStrcmpCommon(C, CE, /* isBounded = */ true, /* ignoreCase = */ true);
19191896
}
@@ -2047,9 +2024,6 @@ void CStringChecker::evalStrcmpCommon(CheckerContext &C, const CallExpr *CE,
20472024

20482025
void CStringChecker::evalStrsep(CheckerContext &C, const CallExpr *CE) const {
20492026
//char *strsep(char **stringp, const char *delim);
2050-
if (CE->getNumArgs() < 2)
2051-
return;
2052-
20532027
// Sanity: does the search string parameter match the return type?
20542028
const Expr *SearchStrPtr = CE->getArg(0);
20552029
QualType CharPtrTy = SearchStrPtr->getType()->getPointeeType();
@@ -2118,7 +2092,7 @@ void CStringChecker::evalStdCopyBackward(CheckerContext &C,
21182092

21192093
void CStringChecker::evalStdCopyCommon(CheckerContext &C,
21202094
const CallExpr *CE) const {
2121-
if (CE->getNumArgs() < 3)
2095+
if (!CE->getArg(2)->getType()->isPointerType())
21222096
return;
21232097

21242098
ProgramStateRef State = C.getState();
@@ -2145,9 +2119,6 @@ void CStringChecker::evalStdCopyCommon(CheckerContext &C,
21452119
}
21462120

21472121
void CStringChecker::evalMemset(CheckerContext &C, const CallExpr *CE) const {
2148-
if (CE->getNumArgs() != 3)
2149-
return;
2150-
21512122
CurrentFunctionDescription = "memory set function";
21522123

21532124
const Expr *Mem = CE->getArg(0);
@@ -2196,9 +2167,6 @@ void CStringChecker::evalMemset(CheckerContext &C, const CallExpr *CE) const {
21962167
}
21972168

21982169
void CStringChecker::evalBzero(CheckerContext &C, const CallExpr *CE) const {
2199-
if (CE->getNumArgs() != 2)
2200-
return;
2201-
22022170
CurrentFunctionDescription = "memory clearance function";
22032171

22042172
const Expr *Mem = CE->getArg(0);
@@ -2241,110 +2209,53 @@ void CStringChecker::evalBzero(CheckerContext &C, const CallExpr *CE) const {
22412209
C.addTransition(State);
22422210
}
22432211

2244-
static bool isCPPStdLibraryFunction(const FunctionDecl *FD, StringRef Name) {
2245-
IdentifierInfo *II = FD->getIdentifier();
2246-
if (!II)
2247-
return false;
2248-
2249-
if (!AnalysisDeclContext::isInStdNamespace(FD))
2250-
return false;
2251-
2252-
if (II->getName().equals(Name))
2253-
return true;
2254-
2255-
return false;
2256-
}
22572212
//===----------------------------------------------------------------------===//
22582213
// The driver method, and other Checker callbacks.
22592214
//===----------------------------------------------------------------------===//
22602215

2261-
static CStringChecker::FnCheck identifyCall(const CallExpr *CE,
2262-
CheckerContext &C) {
2263-
const FunctionDecl *FDecl = C.getCalleeDecl(CE);
2264-
if (!FDecl)
2216+
CStringChecker::FnCheck CStringChecker::identifyCall(const CallEvent &Call,
2217+
CheckerContext &C) const {
2218+
const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
2219+
if (!CE)
2220+
return nullptr;
2221+
2222+
const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
2223+
if (!FD)
22652224
return nullptr;
22662225

2226+
if (Call.isCalled(StdCopy)) {
2227+
return &CStringChecker::evalStdCopy;
2228+
} else if (Call.isCalled(StdCopyBackward)) {
2229+
return &CStringChecker::evalStdCopyBackward;
2230+
}
2231+
22672232
// Pro-actively check that argument types are safe to do arithmetic upon.
22682233
// We do not want to crash if someone accidentally passes a structure
2269-
// into, say, a C++ overload of any of these functions.
2270-
if (isCPPStdLibraryFunction(FDecl, "copy")) {
2271-
if (CE->getNumArgs() < 3 || !CE->getArg(2)->getType()->isPointerType())
2272-
return nullptr;
2273-
return &CStringChecker::evalStdCopy;
2274-
} else if (isCPPStdLibraryFunction(FDecl, "copy_backward")) {
2275-
if (CE->getNumArgs() < 3 || !CE->getArg(2)->getType()->isPointerType())
2234+
// into, say, a C++ overload of any of these functions. We could not check
2235+
// that for std::copy because they may have arguments of other types.
2236+
for (auto I : CE->arguments()) {
2237+
QualType T = I->getType();
2238+
if (!T->isIntegralOrEnumerationType() && !T->isPointerType())
22762239
return nullptr;
2277-
return &CStringChecker::evalStdCopyBackward;
2278-
} else {
2279-
// An umbrella check for all C library functions.
2280-
for (auto I: CE->arguments()) {
2281-
QualType T = I->getType();
2282-
if (!T->isIntegralOrEnumerationType() && !T->isPointerType())
2283-
return nullptr;
2284-
}
22852240
}
22862241

2287-
// FIXME: Poorly-factored string switches are slow.
2288-
if (C.isCLibraryFunction(FDecl, "memcpy"))
2289-
return &CStringChecker::evalMemcpy;
2290-
else if (C.isCLibraryFunction(FDecl, "mempcpy"))
2291-
return &CStringChecker::evalMempcpy;
2292-
else if (C.isCLibraryFunction(FDecl, "memcmp"))
2293-
return &CStringChecker::evalMemcmp;
2294-
else if (C.isCLibraryFunction(FDecl, "memmove"))
2295-
return &CStringChecker::evalMemmove;
2296-
else if (C.isCLibraryFunction(FDecl, "memset") ||
2297-
C.isCLibraryFunction(FDecl, "explicit_memset"))
2298-
return &CStringChecker::evalMemset;
2299-
else if (C.isCLibraryFunction(FDecl, "strcpy"))
2300-
return &CStringChecker::evalStrcpy;
2301-
else if (C.isCLibraryFunction(FDecl, "strncpy"))
2302-
return &CStringChecker::evalStrncpy;
2303-
else if (C.isCLibraryFunction(FDecl, "stpcpy"))
2304-
return &CStringChecker::evalStpcpy;
2305-
else if (C.isCLibraryFunction(FDecl, "strlcpy"))
2306-
return &CStringChecker::evalStrlcpy;
2307-
else if (C.isCLibraryFunction(FDecl, "strcat"))
2308-
return &CStringChecker::evalStrcat;
2309-
else if (C.isCLibraryFunction(FDecl, "strncat"))
2310-
return &CStringChecker::evalStrncat;
2311-
else if (C.isCLibraryFunction(FDecl, "strlcat"))
2312-
return &CStringChecker::evalStrlcat;
2313-
else if (C.isCLibraryFunction(FDecl, "strlen"))
2314-
return &CStringChecker::evalstrLength;
2315-
else if (C.isCLibraryFunction(FDecl, "strnlen"))
2316-
return &CStringChecker::evalstrnLength;
2317-
else if (C.isCLibraryFunction(FDecl, "strcmp"))
2318-
return &CStringChecker::evalStrcmp;
2319-
else if (C.isCLibraryFunction(FDecl, "strncmp"))
2320-
return &CStringChecker::evalStrncmp;
2321-
else if (C.isCLibraryFunction(FDecl, "strcasecmp"))
2322-
return &CStringChecker::evalStrcasecmp;
2323-
else if (C.isCLibraryFunction(FDecl, "strncasecmp"))
2324-
return &CStringChecker::evalStrncasecmp;
2325-
else if (C.isCLibraryFunction(FDecl, "strsep"))
2326-
return &CStringChecker::evalStrsep;
2327-
else if (C.isCLibraryFunction(FDecl, "bcopy"))
2328-
return &CStringChecker::evalBcopy;
2329-
else if (C.isCLibraryFunction(FDecl, "bcmp"))
2330-
return &CStringChecker::evalMemcmp;
2331-
else if (C.isCLibraryFunction(FDecl, "bzero") ||
2332-
C.isCLibraryFunction(FDecl, "explicit_bzero"))
2333-
return &CStringChecker::evalBzero;
2242+
const FnCheck *Callback = Callbacks.lookup(Call);
2243+
if (Callback)
2244+
return *Callback;
23342245

23352246
return nullptr;
23362247
}
23372248

23382249
bool CStringChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
2339-
const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
2340-
FnCheck evalFunction = identifyCall(CE, C);
2250+
FnCheck Callback = identifyCall(Call, C);
23412251

23422252
// If the callee isn't a string function, let another checker handle it.
2343-
if (!evalFunction)
2253+
if (!Callback)
23442254
return false;
23452255

23462256
// Check and evaluate the call.
2347-
(this->*evalFunction)(C, CE);
2257+
const auto *CE = cast<CallExpr>(Call.getOriginExpr());
2258+
(this->*Callback)(C, CE);
23482259

23492260
// If the evaluate call resulted in no change, chain to the next eval call
23502261
// handler.

‎clang/test/Analysis/string.c

+6
Original file line numberDiff line numberDiff line change
@@ -1598,3 +1598,9 @@ void memset29_plain_int_zero() {
15981598
memset(&x, 0, sizeof(short));
15991599
clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}
16001600
}
1601+
1602+
void test_memset_chk() {
1603+
int x;
1604+
__builtin___memset_chk(&x, 0, sizeof(x), __builtin_object_size(&x, 0));
1605+
clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}
1606+
}

0 commit comments

Comments
 (0)