Skip to content

Commit 3ce78f5

Browse files
Yu Shanzeroomega
authored andcommitted
[analyzer] Ignore annotations if func is inlined.
When we annotating a function header so that it could be used by other TU, we also need to make sure the function is parsed correctly within the same TU. So if we can find the function's implementation, ignore the annotations, otherwise, false positive would occur. Move the escape by value case to post call and do not escape the handle if the function is inlined and we have analyzed the handle. Differential Revision: https://reviews.llvm.org/D91902
1 parent 875b4fd commit 3ce78f5

File tree

2 files changed

+51
-5
lines changed

2 files changed

+51
-5
lines changed

clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -331,11 +331,6 @@ void FuchsiaHandleChecker::checkPreCall(const CallEvent &Call,
331331
return;
332332
}
333333
}
334-
if (!hasFuchsiaAttr<UseHandleAttr>(PVD) &&
335-
PVD->getType()->isIntegerType()) {
336-
// Working around integer by-value escapes.
337-
State = State->set<HStateMap>(Handle, HandleState::getEscaped());
338-
}
339334
}
340335
}
341336
C.addTransition(State);
@@ -347,6 +342,10 @@ void FuchsiaHandleChecker::checkPostCall(const CallEvent &Call,
347342
if (!FuncDecl)
348343
return;
349344

345+
// If we analyzed the function body, then ignore the annotations.
346+
if (C.wasInlined)
347+
return;
348+
350349
ProgramStateRef State = C.getState();
351350

352351
std::vector<std::function<std::string(BugReport & BR)>> Notes;
@@ -417,6 +416,14 @@ void FuchsiaHandleChecker::checkPostCall(const CallEvent &Call,
417416
});
418417
State = State->set<HStateMap>(
419418
Handle, HandleState::getMaybeAllocated(ResultSymbol));
419+
} else if (!hasFuchsiaAttr<UseHandleAttr>(PVD) &&
420+
PVD->getType()->isIntegerType()) {
421+
// Working around integer by-value escapes.
422+
// The by-value escape would not be captured in checkPointerEscape.
423+
// If the function was not analyzed (otherwise wasInlined should be
424+
// true) and there is no annotation on the handle, we assume the handle
425+
// is escaped.
426+
State = State->set<HStateMap>(Handle, HandleState::getEscaped());
420427
}
421428
}
422429
}

clang/test/Analysis/fuchsia_handle.cpp

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,45 @@ void checkHandlePtrInStructureLeak() {
315315
// expected-note@-1 {{Potential leak of handle}}
316316
}
317317

318+
// Assume this function's declaration that has the release annotation is in one
319+
// header file while its implementation is in another file. We have to annotate
320+
// the declaration because it might be used outside the TU.
321+
// We also want to make sure it is okay to call the function within the same TU.
322+
zx_status_t test_release_handle(zx_handle_t handle ZX_HANDLE_RELEASE) {
323+
return zx_handle_close(handle);
324+
}
325+
326+
void checkReleaseImplementedFunc() {
327+
zx_handle_t a, b;
328+
zx_channel_create(0, &a, &b);
329+
zx_handle_close(a);
330+
test_release_handle(b);
331+
}
332+
333+
void use_handle(zx_handle_t handle) {
334+
// Do nothing.
335+
}
336+
337+
void test_call_by_value() {
338+
zx_handle_t a, b;
339+
zx_channel_create(0, &a, &b);
340+
zx_handle_close(a);
341+
use_handle(b);
342+
zx_handle_close(b);
343+
}
344+
345+
void test_call_by_value_leak() {
346+
zx_handle_t a, b;
347+
zx_channel_create(0, &a, &b); // expected-note {{Handle allocated through 3rd parameter}}
348+
zx_handle_close(a);
349+
// Here we are passing handle b as integer value to a function that could be
350+
// analyzed by the analyzer, thus the handle should not be considered escaped.
351+
// After the function 'use_handle', handle b is still tracked and should be
352+
// reported leaked.
353+
use_handle(b);
354+
} // expected-warning {{Potential leak of handle}}
355+
// expected-note@-1 {{Potential leak of handle}}
356+
318357
// RAII
319358

320359
template <typename T>

0 commit comments

Comments
 (0)