Skip to content

Commit ad25da2

Browse files
vtjnashvtjnash
committed
[GCChecker] fix a few tests by looking through casts
1 parent dd6af44 commit ad25da2

File tree

3 files changed

+36
-23
lines changed

3 files changed

+36
-23
lines changed

‎src/clangsa/GCChecker.cpp

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ class GCChecker
200200
const MemRegion *Region);
201201

202202
static bool isGCTrackedType(QualType Type);
203+
static bool isGCTracked(const Expr *E);
203204
bool isGloballyRootedType(QualType Type) const;
204205
static void dumpState(const ProgramStateRef &State);
205206
static bool declHasAnnotation(const clang::Decl *D, const char *which);
@@ -703,12 +704,8 @@ void GCChecker::checkBeginFunction(CheckerContext &C) const {
703704
}
704705
}
705706

706-
#if LLVM_VERSION_MAJOR >= 7
707707
void GCChecker::checkEndFunction(const clang::ReturnStmt *RS,
708708
CheckerContext &C) const {
709-
#else
710-
void GCChecker::checkEndFunction(CheckerContext &C) const {
711-
#endif
712709
ProgramStateRef State = C.getState();
713710
bool Changed = false;
714711
if (State->get<GCDisabledAt>() == C.getStackFrame()->getIndex()) {
@@ -789,6 +786,19 @@ bool GCChecker::isGCTrackedType(QualType QT) {
789786
QT);
790787
}
791788

789+
bool GCChecker::isGCTracked(const Expr *E) {
790+
while (1) {
791+
if (isGCTrackedType(E->getType()))
792+
return true;
793+
if (auto ICE = dyn_cast<ImplicitCastExpr>(E))
794+
E = ICE->getSubExpr();
795+
else if (auto CE = dyn_cast<CastExpr>(E))
796+
E = CE->getSubExpr();
797+
else
798+
return false;
799+
}
800+
}
801+
792802
bool GCChecker::isGloballyRootedType(QualType QT) const {
793803
return isJuliaType(
794804
[](StringRef Name) { return Name.endswith_lower("jl_sym_t"); }, QT);
@@ -1053,7 +1063,7 @@ SymbolRef GCChecker::getSymbolForResult(const Expr *Result,
10531063
QualType QT = Result->getType();
10541064
if (!QT->isPointerType())
10551065
return nullptr;
1056-
if (OldValS || GCChecker::isGCTrackedType(QT)) {
1066+
if (OldValS || GCChecker::isGCTracked(Result)) {
10571067
Loaded = C.getSValBuilder().conjureSymbolVal(
10581068
nullptr, Result, C.getLocationContext(), Result->getType(),
10591069
C.blockCount());
@@ -1082,7 +1092,7 @@ void GCChecker::checkDerivingExpr(const Expr *Result, const Expr *Parent,
10821092
State->set<GCValueMap>(NewSym, ValueState::getRooted(nullptr, -1)));
10831093
return;
10841094
}
1085-
if (!isGCTrackedType(Result->getType())) {
1095+
if (!isGCTracked(Result)) {
10861096
// TODO: We may want to refine this. This is to track pointers through the
10871097
// array list in jl_module_t.
10881098
bool ParentIsModule = isJuliaType(
@@ -1091,8 +1101,7 @@ void GCChecker::checkDerivingExpr(const Expr *Result, const Expr *Parent,
10911101
bool ResultIsArrayList = isJuliaType(
10921102
[](StringRef Name) { return Name.endswith_lower("arraylist_t"); },
10931103
Result->getType());
1094-
if (!(ParentIsModule && ResultIsArrayList) &&
1095-
isGCTrackedType(Parent->getType())) {
1104+
if (!(ParentIsModule && ResultIsArrayList) && isGCTracked(Parent)) {
10961105
ResultTracked = false;
10971106
}
10981107
}
@@ -1144,7 +1153,7 @@ void GCChecker::checkDerivingExpr(const Expr *Result, const Expr *Parent,
11441153
}
11451154
if (!OldValS) {
11461155
// This way we'll get better diagnostics
1147-
if (isGCTrackedType(Result->getType())) {
1156+
if (isGCTracked(Result)) {
11481157
C.addTransition(
11491158
State->set<GCValueMap>(NewSym, ValueState::getUntracked()));
11501159
}
@@ -1167,8 +1176,7 @@ void GCChecker::checkPostStmt(const ArraySubscriptExpr *ASE,
11671176
const MemRegion *Region = C.getSVal(ASE->getLHS()).getAsRegion();
11681177
ProgramStateRef State = C.getState();
11691178
SValExplainer Ex(C.getASTContext());
1170-
if (Region && Region->getAs<ElementRegion>() &&
1171-
isGCTrackedType(ASE->getType())) {
1179+
if (Region && Region->getAs<ElementRegion>() && isGCTracked(ASE)) {
11721180
const RootState *RS =
11731181
State->get<GCRootMap>(Region->getAs<ElementRegion>()->getSuperRegion());
11741182
if (RS) {
@@ -1192,7 +1200,7 @@ void GCChecker::checkPostStmt(const MemberExpr *ME, CheckerContext &C) const {
11921200
// It is possible for the member itself to be gcrooted, so check that first
11931201
const MemRegion *Region = C.getSVal(ME).getAsRegion();
11941202
ProgramStateRef State = C.getState();
1195-
if (Region && isGCTrackedType(ME->getType())) {
1203+
if (Region && isGCTracked(ME)) {
11961204
if (const RootState *RS = State->get<GCRootMap>(Region)) {
11971205
ValueState ValS = ValueState::getRooted(Region, RS->RootedAtDepth);
11981206
SymbolRef NewSym = getSymbolForResult(ME, &ValS, State, C);
@@ -1274,7 +1282,7 @@ void GCChecker::checkPreCall(const CallEvent &Call, CheckerContext &C) const {
12741282
SourceRange range;
12751283
if (const Expr *E = Call.getArgExpr(idx)) {
12761284
range = E->getSourceRange();
1277-
if (!isGCTrackedType(E->getType()))
1285+
if (!isGCTracked(E))
12781286
continue;
12791287
}
12801288
if (ValState->isPotentiallyFreed()) {

‎test/clangsa/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ include $(JULIAHOME)/Make.inc
44

55
check: $(SRCDIR)
66

7-
TESTS = $(patsubst $(SRCDIR)/%,%,$(wildcard $(SRCDIR)/*.c)) $(wildcard $(SRCDIR)/*.cpp)
7+
TESTS = $(patsubst $(SRCDIR)/%,%,$(wildcard $(SRCDIR)/*.c) $(wildcard $(SRCDIR)/*.cpp))
88

99
$(SRCDIR) $(TESTS):
1010
PATH=$(build_bindir):$(build_depsbindir):$$PATH \

‎test/clangsa/MissingRoots.c

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ extern void process_unrooted(jl_value_t *maybe_unrooted JL_MAYBE_UNROOTED);
1010
extern void jl_gc_safepoint();
1111

1212
void unrooted_argument() {
13-
look_at_value((jl_value_t*)jl_svec1(NULL)); // expected-warning{{Passing non-rooted value as argument to function}}
13+
look_at_value((jl_value_t*)jl_svec1(NULL)); // expected-warning{{Passing non-rooted value as argument to function that may GC}}
1414
// expected-note@-1{{Passing non-rooted value as argument to function}}
1515
// expected-note@-2{{Started tracking value here}}
1616
};
@@ -22,9 +22,10 @@ void simple_svec() {
2222
}
2323

2424
jl_value_t *simple_missing_root() {
25-
jl_svec_t *val = jl_svec1(NULL);
26-
jl_gc_safepoint();
27-
return jl_svecref(val, 0); // XXX-expected-warning{{Passing non-rooted value as argument to function}}
25+
jl_svec_t *val = jl_svec1(NULL); // expected-note{{Started tracking value here}}
26+
jl_gc_safepoint(); // expected-note{{Value may have been GCed here}}
27+
return jl_svecref(val, 0); // expected-warning{{Argument value may have been GCed}}
28+
// expected-note@-1{{Argument value may have been GCed}}
2829
};
2930

3031
jl_value_t *root_value() {
@@ -130,14 +131,18 @@ jl_value_t *late_root2() {
130131

131132
jl_value_t *already_freed() {
132133
jl_svec_t *val = NULL;
133-
JL_GC_PUSH1(&val);
134-
val = jl_svec1(NULL);
135-
JL_GC_POP();
136-
jl_gc_safepoint();
137-
jl_value_t *ret = jl_svecref(val, 0);
134+
JL_GC_PUSH1(&val); // expected-note{{GC frame changed here}}
135+
val = jl_svec1(NULL); // expected-note{{Started tracking value here}}
136+
// expected-note@-1{{Value was rooted here}}
137+
JL_GC_POP(); // expected-note{{GC frame changed here}}
138+
// expected-note@-1{{Root was released here}}
139+
jl_gc_safepoint(); // expected-note{{Value may have been GCed here}}
140+
jl_value_t *ret = jl_svecref(val, 0); // expected-warning{{Argument value may have been GCed}}
141+
// expected-note@-1{{Argument value may have been GCed}}
138142
return ret;
139143
};
140144

145+
141146
int field_access() {
142147
jl_svec_t *val = jl_svec1(NULL); // expected-note {{Started tracking value here}}
143148
jl_gc_safepoint(); // expected-note{{Value may have been GCed here}}

0 commit comments

Comments
 (0)