Skip to content

Commit d2793c2

Browse files
vbvictorvbvictor
committed
[clang][analyzer] fix crash when modelling 'getline'
1 parent 7468718 commit d2793c2

File tree

4 files changed

+237
-3
lines changed

4 files changed

+237
-3
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1060,6 +1060,9 @@ impact the linker behaviour like the other `-static-*` flags.
10601060
Crash and bug fixes
10611061
^^^^^^^^^^^^^^^^^^^
10621062

1063+
- Fixed a crash in ``UnixAPIMisuseChecker`` and ``MallocChecker`` when analyzing
1064+
code with non-standard ``getline`` or ``getdelim`` function signatures.
1065+
10631066
Improvements
10641067
^^^^^^^^^^^^
10651068

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

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1482,11 +1482,63 @@ static bool isFromStdNamespace(const CallEvent &Call) {
14821482
return FD->isInStdNamespace();
14831483
}
14841484

1485+
static bool IsGetDelim(const CallEvent &Call, CheckerContext &C) {
1486+
const FunctionDecl *FD = dyn_cast_if_present<FunctionDecl>(Call.getDecl());
1487+
if (!FD || FD->getKind() != Decl::Function)
1488+
return false;
1489+
1490+
const StringRef FName = C.getCalleeName(FD);
1491+
const bool IsDelim = FName == "getdelim";
1492+
const bool IsLine = FName == "getline";
1493+
1494+
unsigned ExpectedNumArgs = 0;
1495+
if (IsDelim)
1496+
ExpectedNumArgs = 4;
1497+
else if (IsLine)
1498+
ExpectedNumArgs = 3;
1499+
else
1500+
return false;
1501+
1502+
if (FD->getNumParams() != ExpectedNumArgs)
1503+
return false;
1504+
1505+
// Should be char**
1506+
const QualType CharPtrType = FD->getParamDecl(0)->getType()->getPointeeType();
1507+
if (CharPtrType.isNull())
1508+
return false;
1509+
const QualType CharType = CharPtrType->getPointeeType();
1510+
if (CharType.isNull() || !CharType->isCharType())
1511+
return false;
1512+
1513+
// Should be size_t*
1514+
const QualType SizePtrType = FD->getParamDecl(1)->getType();
1515+
if (!SizePtrType->isPointerType())
1516+
return false;
1517+
const QualType SizeArgType = SizePtrType->getPointeeType();
1518+
const ASTContext &Ctx = C.getASTContext();
1519+
if (SizeArgType.isNull() ||
1520+
!Ctx.hasSameUnqualifiedType(SizeArgType, Ctx.getSizeType()))
1521+
return false;
1522+
1523+
// For getdelim, should be int
1524+
if (IsDelim)
1525+
if (!FD->getParamDecl(2)->getType()->isIntegerType())
1526+
return false;
1527+
1528+
// Last parameter should be FILE*
1529+
const QualType Pointee =
1530+
FD->getParamDecl(IsDelim ? 3 : 2)->getType()->getPointeeType();
1531+
if (Pointee.isNull() || Pointee.getAsString() != "FILE")
1532+
return false;
1533+
1534+
return true;
1535+
}
1536+
14851537
void MallocChecker::preGetdelim(ProgramStateRef State, const CallEvent &Call,
14861538
CheckerContext &C) const {
14871539
// Discard calls to the C++ standard library function std::getline(), which
14881540
// is completely unrelated to the POSIX getline() that we're checking.
1489-
if (isFromStdNamespace(Call))
1541+
if (isFromStdNamespace(Call) || !IsGetDelim(Call, C))
14901542
return;
14911543

14921544
const auto LinePtr = getPointeeVal(Call.getArgSVal(0), State);
@@ -1509,7 +1561,7 @@ void MallocChecker::checkGetdelim(ProgramStateRef State, const CallEvent &Call,
15091561
CheckerContext &C) const {
15101562
// Discard calls to the C++ standard library function std::getline(), which
15111563
// is completely unrelated to the POSIX getline() that we're checking.
1512-
if (isFromStdNamespace(Call))
1564+
if (isFromStdNamespace(Call) || !IsGetDelim(Call, C))
15131565
return;
15141566

15151567
// Handle the post-conditions of getline and getdelim:

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

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,55 @@ ProgramStateRef UnixAPIMisuseChecker::EnsurePtrNotNull(
147147
return PtrNotNull;
148148
}
149149

150+
static bool IsGetDelim(const CallEvent &Call, const FunctionDecl *FD,
151+
CheckerContext &C) {
152+
const StringRef FName = C.getCalleeName(FD);
153+
const bool IsDelim = FName == "getdelim";
154+
const bool IsLine = FName == "getline";
155+
156+
unsigned ExpectedNumArgs = 0;
157+
if (IsDelim)
158+
ExpectedNumArgs = 4;
159+
else if (IsLine)
160+
ExpectedNumArgs = 3;
161+
else
162+
return false;
163+
164+
if (FD->getNumParams() != ExpectedNumArgs)
165+
return false;
166+
167+
// Should be char**
168+
const QualType CharPtrType = FD->getParamDecl(0)->getType()->getPointeeType();
169+
if (CharPtrType.isNull())
170+
return false;
171+
const QualType CharType = CharPtrType->getPointeeType();
172+
if (CharType.isNull() || !CharType->isCharType())
173+
return false;
174+
175+
// Should be size_t*
176+
const QualType SizePtrType = FD->getParamDecl(1)->getType();
177+
if (!SizePtrType->isPointerType())
178+
return false;
179+
const QualType SizeArgType = SizePtrType->getPointeeType();
180+
const ASTContext &Ctx = C.getASTContext();
181+
if (SizeArgType.isNull() ||
182+
!Ctx.hasSameUnqualifiedType(SizeArgType, Ctx.getSizeType()))
183+
return false;
184+
185+
// For getdelim, should be int
186+
if (IsDelim)
187+
if (!FD->getParamDecl(2)->getType()->isIntegerType())
188+
return false;
189+
190+
// Last parameter should be FILE*
191+
const QualType Pointee =
192+
FD->getParamDecl(IsDelim ? 3 : 2)->getType()->getPointeeType();
193+
if (Pointee.isNull() || Pointee.getAsString() != "FILE")
194+
return false;
195+
196+
return true;
197+
}
198+
150199
//===----------------------------------------------------------------------===//
151200
// "open" (man 2 open)
152201
//===----------------------------------------------------------------------===/
@@ -176,7 +225,7 @@ void UnixAPIMisuseChecker::checkPreCall(const CallEvent &Call,
176225
else if (FName == "pthread_once")
177226
CheckPthreadOnce(C, Call);
178227

179-
else if (is_contained({"getdelim", "getline"}, FName))
228+
else if (IsGetDelim(Call, FD, C))
180229
CheckGetDelim(C, Call);
181230
}
182231
void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext &C,

‎clang/test/Analysis/getline-unixapi-invalid-signatures.c

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_CORRECT
2+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_GETLINE_1
3+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_GETLINE_2
4+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_GETLINE_3
5+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_GETLINE_4
6+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_GETLINE_5
7+
// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -verify %s -DTEST_GETLINE_GH144884
8+
9+
// emulator of "system-header-simulator.h" because of redefinition of 'getline' function
10+
typedef struct _FILE FILE;
11+
typedef __typeof(sizeof(int)) size_t;
12+
typedef long ssize_t;
13+
#define NULL 0
14+
15+
int fclose(FILE *fp);
16+
FILE *tmpfile(void);
17+
18+
#ifdef TEST_CORRECT
19+
ssize_t getline(char **lineptr, size_t *n, FILE *stream);
20+
ssize_t getdelim(char **lineptr, size_t *n, int delimiter, FILE *stream);
21+
22+
void test_correct() {
23+
FILE *F1 = tmpfile();
24+
if (!F1)
25+
return;
26+
char *buffer = NULL;
27+
getline(&buffer, NULL, F1); // expected-warning {{Size pointer might be NULL}}
28+
fclose(F1);
29+
}
30+
31+
void test_delim_correct() {
32+
FILE *F1 = tmpfile();
33+
if (!F1)
34+
return;
35+
char *buffer = NULL;
36+
getdelim(&buffer, NULL, ',', F1); // expected-warning {{Size pointer might be NULL}}
37+
fclose(F1);
38+
}
39+
#endif
40+
41+
#ifdef TEST_GETLINE_1
42+
// expected-no-diagnostics
43+
ssize_t getline(int lineptr);
44+
45+
void test() {
46+
FILE *F1 = tmpfile();
47+
if (!F1)
48+
return;
49+
int buffer = 0;
50+
getline(buffer);
51+
fclose(F1);
52+
}
53+
#endif
54+
55+
#ifdef TEST_GETLINE_2
56+
// expected-no-diagnostics
57+
ssize_t getline(char **lineptr, size_t *n);
58+
59+
void test() {
60+
FILE *F1 = tmpfile();
61+
if (!F1)
62+
return;
63+
char *buffer = NULL;
64+
getline(&buffer, NULL);
65+
fclose(F1);
66+
}
67+
#endif
68+
69+
#ifdef TEST_GETLINE_3
70+
// expected-no-diagnostics
71+
ssize_t getline(char **lineptr, size_t n, FILE *stream);
72+
73+
void test() {
74+
FILE *F1 = tmpfile();
75+
if (!F1)
76+
return;
77+
char *buffer = NULL;
78+
getline(&buffer, 0, F1);
79+
fclose(F1);
80+
}
81+
#endif
82+
83+
#ifdef TEST_GETLINE_4
84+
// expected-no-diagnostics
85+
ssize_t getline(char **lineptr, size_t *n, int stream);
86+
ssize_t getdelim(char **lineptr, size_t *n, int delimiter, int stream);
87+
88+
void test() {
89+
FILE *F1 = tmpfile();
90+
if (!F1)
91+
return;
92+
char *buffer = NULL;
93+
getline(&buffer, NULL, 1);
94+
fclose(F1);
95+
}
96+
97+
void test_delim() {
98+
FILE *F1 = tmpfile();
99+
if (!F1)
100+
return;
101+
char *buffer = NULL;
102+
getdelim(&buffer, NULL, ',', 1);
103+
fclose(F1);
104+
}
105+
#endif
106+
107+
#ifdef TEST_GETLINE_5
108+
// expected-no-diagnostics
109+
ssize_t getdelim(char **lineptr, size_t *n, const char* delimiter, FILE *stream);
110+
111+
void test_delim() {
112+
FILE *F1 = tmpfile();
113+
if (!F1)
114+
return;
115+
char *buffer = NULL;
116+
getdelim(&buffer, NULL, ",", F1);
117+
fclose(F1);
118+
}
119+
#endif
120+
121+
#ifdef TEST_GETLINE_GH144884
122+
// expected-no-diagnostics
123+
struct AW_string {};
124+
void getline(int *, struct AW_string);
125+
void top() {
126+
struct AW_string line;
127+
int getline_file_info;
128+
getline(&getline_file_info, line);
129+
}
130+
#endif

0 commit comments

Comments
 (0)