Skip to content

Commit ec6da3f

Browse files
nectosteakhal
authored andcommitted
Fix false positive related to handling of [[noreturn]] function pointers
Before this change, the `NoReturnFunctionChecker` was missing function pointers with a `[[noreturn]]` attribute, while `CFG` was constructed taking that into account, which leads CSA to take impossible paths. The reason was that the `NoReturnFunctionChecker` was looking for the attribute in the type of the entire call expression rather than the type of the function being called. This change makes the `[[noreturn]]` attribute of a function pointer visible to `NoReturnFunctionChecker`. This leads to a more coherent behavior of the CSA on the AST involving. Reviewed By: xazax.hun Differential Revision: https://reviews.llvm.org/D135682
1 parent 78c66cf commit ec6da3f

File tree

2 files changed

+41
-3
lines changed

2 files changed

+41
-3
lines changed

clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,11 @@ void NoReturnFunctionChecker::checkPostCall(const CallEvent &CE,
4444
if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(CE.getDecl()))
4545
BuildSinks = FD->hasAttr<AnalyzerNoReturnAttr>() || FD->isNoReturn();
4646

47-
const Expr *Callee = CE.getOriginExpr();
48-
if (!BuildSinks && Callee)
49-
BuildSinks = getFunctionExtInfo(Callee->getType()).getNoReturn();
47+
if (const CallExpr *CExpr = dyn_cast_or_null<CallExpr>(CE.getOriginExpr());
48+
CExpr && !BuildSinks) {
49+
if (const Expr *C = CExpr->getCallee())
50+
BuildSinks = getFunctionExtInfo(C->getType()).getNoReturn();
51+
}
5052

5153
if (!BuildSinks && CE.isGlobalCFunction()) {
5254
if (const IdentifierInfo *II = CE.getCalleeIdentifier()) {

clang/test/Analysis/no-return.c

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
2+
3+
typedef void(fatal_fun)() __attribute__((__noreturn__));
4+
fatal_fun* fatal_fptr;
5+
void fatal_decl() __attribute__((__noreturn__));
6+
7+
int rng();
8+
9+
/// This code calls a [[noreturn]] function pointer, which used to be handled
10+
/// inconsistently between AST builder and CSA.
11+
/// In the result, CSA produces a path where this function returns non-0.
12+
int return_zero_or_abort_by_fnptr() {
13+
if (rng()) fatal_fptr();
14+
return 0;
15+
}
16+
17+
/// This function calls a [[noreturn]] function.
18+
/// If it does return, it always returns 0.
19+
int return_zero_or_abort_by_direct_fun() {
20+
if (rng()) fatal_decl();
21+
return 0;
22+
}
23+
24+
/// Trigger a division by zero issue depending on the return value
25+
/// of the called functions.
26+
int caller() {
27+
int x = 0;
28+
// The following if branches must never be taken.
29+
if (return_zero_or_abort_by_fnptr())
30+
return 1 / x; // no-warning: Dead code.
31+
if (return_zero_or_abort_by_direct_fun())
32+
return 1 / x; // no-warning: Dead code.
33+
34+
// Make sure the warning is still reported when viable.
35+
return 1 / x; // expected-warning {{Division by zero}}
36+
}

0 commit comments

Comments
 (0)