Skip to content

Commit d05132d

Browse files
committed
[analyzer] Add annotation attribute to trust retain count implementation
Add support to the retain-count checker for an annotation indicating that a function's implementation should be trusted by the retain count checker. Functions with these attributes will not be inlined and the arguments will be treating as escaping. Adding this annotation avoids spurious diagnostics when the implementation of a reference counting operation is visible but the analyzer can't reason precisely about the ref count. Patch by Malhar Thakkar! Differential Revision: https://reviews.llvm.org/D34937 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@308416 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent 0bfdbc6 commit d05132d

File tree

2 files changed

+86
-5
lines changed

2 files changed

+86
-5
lines changed

lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp

+31-4
Original file line numberDiff line numberDiff line change
@@ -1304,6 +1304,21 @@ RetainSummaryManager::getCFSummaryGetRule(const FunctionDecl *FD) {
13041304
DoNothing, DoNothing);
13051305
}
13061306

1307+
/// Returns true if the declaration 'D' is annotated with 'rcAnnotation'.
1308+
static bool hasRCAnnotation(const Decl *D, StringRef rcAnnotation) {
1309+
for (const auto *Ann : D->specific_attrs<AnnotateAttr>()) {
1310+
if (Ann->getAnnotation() == rcAnnotation)
1311+
return true;
1312+
}
1313+
return false;
1314+
}
1315+
1316+
/// Returns true if the function declaration 'FD' contains
1317+
/// 'rc_ownership_trusted_implementation' annotate attribute.
1318+
static bool isTrustedReferenceCountImplementation(const FunctionDecl *FD) {
1319+
return hasRCAnnotation(FD, "rc_ownership_trusted_implementation");
1320+
}
1321+
13071322
//===----------------------------------------------------------------------===//
13081323
// Summary creation for Selectors.
13091324
//===----------------------------------------------------------------------===//
@@ -3380,6 +3395,9 @@ bool RetainCountChecker::evalCall(const CallExpr *CE, CheckerContext &C) const {
33803395

33813396
// See if it's one of the specific functions we know how to eval.
33823397
bool canEval = false;
3398+
// See if the function has 'rc_ownership_trusted_implementation'
3399+
// annotate attribute. If it does, we will not inline it.
3400+
bool hasTrustedImplementationAnnotation = false;
33833401

33843402
QualType ResultTy = CE->getCallReturnType(C.getASTContext());
33853403
if (ResultTy->isObjCIdType()) {
@@ -3395,6 +3413,11 @@ bool RetainCountChecker::evalCall(const CallExpr *CE, CheckerContext &C) const {
33953413
cocoa::isRefType(ResultTy, "CV", FName)) {
33963414
canEval = isRetain(FD, FName) || isAutorelease(FD, FName) ||
33973415
isMakeCollectable(FD, FName);
3416+
} else {
3417+
if (FD->getDefinition()) {
3418+
canEval = isTrustedReferenceCountImplementation(FD->getDefinition());
3419+
hasTrustedImplementationAnnotation = canEval;
3420+
}
33983421
}
33993422
}
34003423

@@ -3404,8 +3427,11 @@ bool RetainCountChecker::evalCall(const CallExpr *CE, CheckerContext &C) const {
34043427
// Bind the return value.
34053428
const LocationContext *LCtx = C.getLocationContext();
34063429
SVal RetVal = state->getSVal(CE->getArg(0), LCtx);
3407-
if (RetVal.isUnknown()) {
3408-
// If the receiver is unknown, conjure a return value.
3430+
if (RetVal.isUnknown() ||
3431+
(hasTrustedImplementationAnnotation && !ResultTy.isNull())) {
3432+
// If the receiver is unknown or the function has
3433+
// 'rc_ownership_trusted_implementation' annotate attribute, conjure a
3434+
// return value.
34093435
SValBuilder &SVB = C.getSValBuilder();
34103436
RetVal = SVB.conjureSymbolVal(nullptr, CE, LCtx, ResultTy, C.blockCount());
34113437
}
@@ -3421,8 +3447,9 @@ bool RetainCountChecker::evalCall(const CallExpr *CE, CheckerContext &C) const {
34213447
Binding = getRefBinding(state, Sym);
34223448

34233449
// Invalidate the argument region.
3424-
state = state->invalidateRegions(ArgRegion, CE, C.blockCount(), LCtx,
3425-
/*CausesPointerEscape*/ false);
3450+
state = state->invalidateRegions(
3451+
ArgRegion, CE, C.blockCount(), LCtx,
3452+
/*CausesPointerEscape*/ hasTrustedImplementationAnnotation);
34263453

34273454
// Restore the refcount status of the argument.
34283455
if (Binding)

test/Analysis/retain-release-inline.m

+55-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
//
1313
// It includes the basic definitions for the test cases below.
1414
//===----------------------------------------------------------------------===//
15-
15+
#define NULL 0
1616
typedef unsigned int __darwin_natural_t;
1717
typedef unsigned long uintptr_t;
1818
typedef unsigned int uint32_t;
@@ -267,6 +267,10 @@ + (id)array;
267267

268268
extern CFStringRef CFStringCreateWithCStringNoCopy(CFAllocatorRef alloc, const char *cStr, CFStringEncoding encoding, CFAllocatorRef contentsDeallocator);
269269

270+
typedef struct {
271+
int ref;
272+
} isl_basic_map;
273+
270274
//===----------------------------------------------------------------------===//
271275
// Test cases.
272276
//===----------------------------------------------------------------------===//
@@ -285,6 +289,7 @@ void test() {
285289
foo(s);
286290
bar(s);
287291
}
292+
288293
void test_neg() {
289294
NSString *s = [[NSString alloc] init]; // no-warning
290295
foo(s);
@@ -294,6 +299,55 @@ void test_neg() {
294299
bar(s);
295300
}
296301

302+
__attribute__((cf_returns_retained)) isl_basic_map *isl_basic_map_cow(__attribute__((cf_consumed)) isl_basic_map *bmap);
303+
void free(void *);
304+
305+
// As 'isl_basic_map_free' is annotated with 'rc_ownership_trusted_implementation', RetainCountChecker trusts its
306+
// implementation and doesn't analyze its body. If the annotation 'rc_ownership_trusted_implementation' is removed,
307+
// a leak warning is raised by RetainCountChecker as the analyzer is unable to detect a decrement in the reference
308+
// count of 'bmap' along the path in 'isl_basic_map_free' assuming the predicate of the second 'if' branch to be
309+
// true or assuming both the predicates in the function to be false.
310+
__attribute__((annotate("rc_ownership_trusted_implementation"))) isl_basic_map *isl_basic_map_free(__attribute__((cf_consumed)) isl_basic_map *bmap) {
311+
if (!bmap)
312+
return NULL;
313+
314+
if (--bmap->ref > 0)
315+
return NULL;
316+
317+
free(bmap);
318+
return NULL;
319+
}
320+
321+
// As 'isl_basic_map_copy' is annotated with 'rc_ownership_trusted_implementation', RetainCountChecker trusts its
322+
// implementation and doesn't analyze its body. If that annotation is removed, a 'use-after-release' warning might
323+
// be raised by RetainCountChecker as the pointer which is passed as an argument to this function and the pointer
324+
// which is returned from the function point to the same memory location.
325+
__attribute__((annotate("rc_ownership_trusted_implementation"))) __attribute__((cf_returns_retained)) isl_basic_map *isl_basic_map_copy(isl_basic_map *bmap) {
326+
if (!bmap)
327+
return NULL;
328+
329+
bmap->ref++;
330+
return bmap;
331+
}
332+
333+
void test_use_after_release_with_trusted_implementation_annotate_attribute(__attribute__((cf_consumed)) isl_basic_map *bmap) {
334+
// After this call, 'bmap' has a +1 reference count.
335+
bmap = isl_basic_map_cow(bmap);
336+
// After the call to 'isl_basic_map_copy', 'bmap' has a +1 reference count.
337+
isl_basic_map *temp = isl_basic_map_cow(isl_basic_map_copy(bmap));
338+
// After this call, 'bmap' has a +0 reference count.
339+
isl_basic_map *temp2 = isl_basic_map_cow(bmap); // no-warning
340+
isl_basic_map_free(temp2);
341+
isl_basic_map_free(temp);
342+
}
343+
344+
void test_leak_with_trusted_implementation_annotate_attribute(__attribute__((cf_consumed)) isl_basic_map *bmap) {
345+
// After this call, 'bmap' has a +1 reference count.
346+
bmap = isl_basic_map_cow(bmap); // no-warning
347+
// After this call, 'bmap' has a +0 reference count.
348+
isl_basic_map_free(bmap);
349+
}
350+
297351
//===----------------------------------------------------------------------===//
298352
// Test returning retained and not-retained values.
299353
//===----------------------------------------------------------------------===//

0 commit comments

Comments
 (0)