Skip to content

Commit ce187ad

Browse files
author
einvbri
committed
Fix 2 crashes seen when analyzing memcached
Current changes were causing analysis of memcached to crash. This update addresses 2 of 3 crashes found so far, adds a covering LIT test and improves LIT test coverage. All clang/test/Analysis LITs now pass.
1 parent e987893 commit ce187ad

File tree

5 files changed

+189
-2
lines changed

5 files changed

+189
-2
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ class ThreadModeling : public Checker<check::PreCall> {
3232

3333

3434
void ThreadModeling::checkPreCall(const CallEvent &Call, CheckerContext &C) const {
35+
return;
36+
#if 0
3537
if (!ThreadCreateCalls.contains(Call)) {
3638
return;
3739
}
@@ -67,7 +69,7 @@ void ThreadModeling::checkPreCall(const CallEvent &Call, CheckerContext &C) cons
6769
// 6. Resolve AST to Call
6870
// 7. Inline Call
6971

70-
72+
#endif
7173
}
7274

7375
const FunctionDecl *ThreadModeling::GetFunctionDecl(SVal V, CheckerContext &C) const {
@@ -82,4 +84,4 @@ void clang::ento::registerThreadModeling(CheckerManager &Mgr) {
8284

8385
bool clang::ento::shouldRegisterThreadModeling(const CheckerManager &) {
8486
return true;
85-
}
87+
}

‎clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,12 @@ void ExprEngine::threadBifurcate(CallEvent const &Call, Decl const *D,
561561
if (auto const *FR = dyn_cast<FunctionCodeRegion>(SRR))
562562
StartRoutine = dyn_cast<FunctionDecl>(FR->getDecl());
563563

564+
// There may not be an actual function bound to the 3rd
565+
// argument of pthread_create because of analyzer limitations,
566+
// so detect that case and return at this point since it cannot be modeled
567+
if (!StartRoutine)
568+
return;
569+
564570
assert(StartRoutine && "start_routine should be a valid function pointer");
565571
assert(StartRoutine->hasBody() && "start_routine must be well defined");
566572

‎clang/test/Analysis/SD-tests/thread-modeling-funcptr.c‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
// RUN: %clang_analyze_cc1 -Wno-strict-prototypes -Wno-error=implicit-int -verify %s \
2+
// RUN: -analyzer-checker=core,unix -DPTHREAD_MODEL=1 \
3+
// RUN: -analyzer-checker=debug.ExprInspection -analyzer-config model-pthreads=true
4+
//
5+
// RUN: %clang_analyze_cc1 -Wno-strict-prototypes -Wno-error=implicit-int -verify %s \
6+
// RUN: -analyzer-checker=core,unix -DNO_PTHREAD_MODEL=1 \
7+
// RUN: -analyzer-checker=debug.ExprInspection -analyzer-config model-pthreads=false
8+
9+
#define NULL ((void*) 0)
10+
typedef __typeof(sizeof(int)) size_t;
11+
typedef unsigned long int pthread_t;
12+
typedef struct __pthread_attr pthread_attr_t;
13+
14+
int pthread_create(pthread_t *thread, const pthread_attr_t *attr, void *(*start_routine)(void *), void *arg);
15+
16+
void clang_analyzer_checkInlined(int);
17+
void clang_analyzer_dump(int);
18+
19+
typedef struct _mystruct {
20+
int a;
21+
int b;
22+
} mystruct, *pmystruct;
23+
24+
void *malloc(size_t sz);
25+
void free (void* ptr);
26+
27+
static void* thread_function(void* arg)
28+
{
29+
pmystruct ps = (pmystruct) arg;
30+
// should expect to fail the test at this line if you set the checkInlined to true
31+
int *ptr = (int *)arg;
32+
clang_analyzer_dump(*ptr); // expected-warning-re{{reg_${{[[:digit:]]+}}<int Element{SymRegion{reg_${{[[:digit:]]+}}<void * arg>},0 S64b,int}}}
33+
free(arg);
34+
return NULL;
35+
}
36+
37+
void create_worker( void *(*func)(void *), void * arg) {
38+
pthread_t p1;
39+
pthread_create(&p1, NULL, func, arg);
40+
}
41+
42+
int mem[256];
43+
44+
int test()
45+
{
46+
pmystruct ps = (pmystruct) malloc(sizeof(mystruct));
47+
ps->a = 1;
48+
ps->b = 2;
49+
50+
// The static analyzer gives up on analyzing a code path for
51+
// iterations that exceed a limit.
52+
// See https://discourse.llvm.org/t/loop-handling-improvement-plans/80417
53+
//
54+
// To prove this to yourself, change 256 to 1 and rerun. Change
55+
// to a few different numbers and explore where the LIT test
56+
// starts to produce unexpected values.
57+
for (int i=0; i<256; i++) {
58+
mem[i] = 0;
59+
}
60+
create_worker(thread_function, ps);
61+
62+
return 0;
63+
}
64+

‎clang/test/Analysis/SD-tests/thread-modeling-malloc.c‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// RUN: %clang_analyze_cc1 -Wno-strict-prototypes -Wno-error=implicit-int -verify %s \
2+
// RUN: -analyzer-checker=core,unix -DPTHREAD_MODEL=1 \
3+
// RUN: -analyzer-checker=debug.ExprInspection -analyzer-config model-pthreads=true
4+
//
5+
// RUN: %clang_analyze_cc1 -Wno-strict-prototypes -Wno-error=implicit-int -verify %s \
6+
// RUN: -analyzer-checker=core,unix -DNO_PTHREAD_MODEL=1 \
7+
// RUN: -analyzer-checker=debug.ExprInspection -analyzer-config model-pthreads=false
8+
9+
#define NULL ((void*) 0)
10+
typedef __typeof(sizeof(int)) size_t;
11+
typedef unsigned long int pthread_t;
12+
typedef struct __pthread_attr pthread_attr_t;
13+
14+
int pthread_create(pthread_t *thread, const pthread_attr_t *attr, void *(*start_routine)(void *), void *arg);
15+
16+
void clang_analyzer_checkInlined(int);
17+
void clang_analyzer_dump(int);
18+
19+
typedef struct _mystruct {
20+
int a;
21+
int b;
22+
} mystruct, *pmystruct;
23+
24+
void *malloc(size_t sz);
25+
void free (void* ptr);
26+
27+
void* thread_function(void* arg)
28+
{
29+
pmystruct ps = (pmystruct) arg;
30+
// should expect to fail the test at this line if you set the checkInlined to true
31+
#ifdef PTHREAD_MODEL
32+
clang_analyzer_checkInlined(1); // expected-warning{{TRUE}}
33+
#endif
34+
int *ptr = (int *)arg;
35+
#ifdef PTHREAD_MODEL
36+
clang_analyzer_dump(ps->a); // expected-warning{{1 S32b}}
37+
clang_analyzer_dump(ps->b); // expected-warning{{2 S32b}}
38+
#endif
39+
#ifdef NO_PTHREAD_MODEL
40+
clang_analyzer_dump(*ptr); // expected-warning-re{{reg_${{[[:digit:]]+}}<int Element{SymRegion{reg_${{[[:digit:]]+}}<void * arg>},0 S64b,int}}}
41+
#endif
42+
return NULL;
43+
}
44+
45+
int test()
46+
{
47+
pmystruct ps = (pmystruct) malloc(sizeof(mystruct));
48+
ps->a = 1;
49+
ps->b = 2;
50+
pthread_t p1;
51+
pthread_create(&p1, NULL, &thread_function, ps);
52+
53+
#ifdef PTHREAD_MODEL
54+
return 0; // expected-warning{{Potential leak of memory pointed to by 'ps'}}
55+
#endif
56+
#ifdef NO_PTHREAD_MODEL
57+
return 0;
58+
#endif
59+
}

‎clang/test/Analysis/SD-tests/thread-modeling-struct.c‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// RUN: %clang_analyze_cc1 -Wno-strict-prototypes -Wno-error=implicit-int -verify %s \
2+
// RUN: -analyzer-checker=core,unix -DPTHREAD_MODEL=1 \
3+
// RUN: -analyzer-checker=debug.ExprInspection -analyzer-config model-pthreads=true
4+
//
5+
// RUN: %clang_analyze_cc1 -Wno-strict-prototypes -Wno-error=implicit-int -verify %s \
6+
// RUN: -analyzer-checker=core,unix -DNO_PTHREAD_MODEL=1 \
7+
// RUN: -analyzer-checker=debug.ExprInspection -analyzer-config model-pthreads=false
8+
9+
#define NULL ((void*) 0)
10+
typedef __typeof(sizeof(int)) size_t;
11+
typedef unsigned long int pthread_t;
12+
typedef struct __pthread_attr pthread_attr_t;
13+
14+
int pthread_create(pthread_t *thread, const pthread_attr_t *attr, void *(*start_routine)(void *), void *arg);
15+
16+
void clang_analyzer_checkInlined(int);
17+
void clang_analyzer_dump(int);
18+
19+
typedef struct _mystruct {
20+
int a;
21+
int b;
22+
} mystruct, *pmystruct;
23+
24+
void *malloc(size_t sz);
25+
void free (void* ptr);
26+
27+
void* thread_function(void* arg)
28+
{
29+
pmystruct ps = (pmystruct) arg;
30+
// should expect to fail the test at this line if you set the checkInlined to true
31+
#ifdef PTHREAD_MODEL
32+
clang_analyzer_checkInlined(1); // expected-warning{{TRUE}}
33+
#endif
34+
int *ptr = (int *)arg;
35+
#ifdef PTHREAD_MODEL
36+
clang_analyzer_dump(ps->a); // expected-warning{{1 S32b}}
37+
clang_analyzer_dump(ps->b); // expected-warning{{2 S32b}}
38+
#endif
39+
#ifdef NO_PTHREAD_MODEL
40+
clang_analyzer_dump(*ptr); // expected-warning-re{{reg_${{[[:digit:]]+}}<int Element{SymRegion{reg_${{[[:digit:]]+}}<void * arg>},0 S64b,int}}}
41+
#endif
42+
free(arg);
43+
return NULL;
44+
}
45+
46+
int test()
47+
{
48+
pmystruct ps = (pmystruct) malloc(sizeof(mystruct));
49+
ps->a = 1;
50+
ps->b = 2;
51+
pthread_t p1;
52+
pthread_create(&p1, NULL, &thread_function, ps);
53+
54+
return 0;
55+
}
56+

0 commit comments

Comments
 (0)