arseniy-sonar created this revision.
arseniy-sonar added reviewers: xazax.hun, martong, NoQ, Szelethus, steakhal.
arseniy-sonar added a project: clang.
Herald added a subscriber: rnkovacs.
Herald added a project: All.
arseniy-sonar requested review of this revision.
Herald added a subscriber: cfe-commits.
Before this change, the NoReturnFunctionChecker was missing function pointers
with a [[noreturn]] attribute, while CFG was constructed taking that into
account, which lead 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
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D135682
Files:
clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
clang/test/Analysis/no-return.c
Index: clang/test/Analysis/no-return.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/no-return.c
@@ -0,0 +1,36 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+typedef void(fatal_fun)() __attribute__((__noreturn__));
+fatal_fun* fatal_fptr;
+void fatal_decl() __attribute__((__noreturn__));
+
+int rng();
+
+/// This code calls a [[noreturn]] function pointer, which used to be handled
+/// inconsistently between AST builder and CSA.
+/// In the result, CSA produces a path where this function returns non-0.
+int return_zero_or_abort_by_fnptr() {
+ if (rng()) fatal_fptr();
+ return 0;
+}
+
+/// This function calls a [[noreturn]] function.
+/// If it does return, it always returns 0.
+int return_zero_or_abort_by_direct_fun() {
+ if (rng()) fatal_decl();
+ return 0;
+}
+
+/// Trigger a division by zero issue depending on the return value
+/// of the called functions.
+int caller() {
+ int x = 0;
+ // The following if branches must never be taken.
+ if (return_zero_or_abort_by_fnptr())
+ return 1 / x; // no-warning: Dead code.
+ if (return_zero_or_abort_by_direct_fun())
+ return 1 / x; // no-warning: Dead code.
+
+ // Make sure the warning is still reported when viable.
+ return 1 / x; // expected-warning {{Division by zero}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
@@ -44,9 +44,11 @@
if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(CE.getDecl()))
BuildSinks = FD->hasAttr<AnalyzerNoReturnAttr>() || FD->isNoReturn();
- const Expr *Callee = CE.getOriginExpr();
- if (!BuildSinks && Callee)
- BuildSinks = getFunctionExtInfo(Callee->getType()).getNoReturn();
+ if (const CallExpr* CExpr = dyn_cast_or_null<CallExpr>(CE.getOriginExpr());
+ CExpr && !BuildSinks) {
+ if (const Expr *C = CExpr->getCallee())
+ BuildSinks = getFunctionExtInfo(C->getType()).getNoReturn();
+ }
if (!BuildSinks && CE.isGlobalCFunction()) {
if (const IdentifierInfo *II = CE.getCalleeIdentifier()) {
Index: clang/test/Analysis/no-return.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/no-return.c
@@ -0,0 +1,36 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+typedef void(fatal_fun)() __attribute__((__noreturn__));
+fatal_fun* fatal_fptr;
+void fatal_decl() __attribute__((__noreturn__));
+
+int rng();
+
+/// This code calls a [[noreturn]] function pointer, which used to be handled
+/// inconsistently between AST builder and CSA.
+/// In the result, CSA produces a path where this function returns non-0.
+int return_zero_or_abort_by_fnptr() {
+ if (rng()) fatal_fptr();
+ return 0;
+}
+
+/// This function calls a [[noreturn]] function.
+/// If it does return, it always returns 0.
+int return_zero_or_abort_by_direct_fun() {
+ if (rng()) fatal_decl();
+ return 0;
+}
+
+/// Trigger a division by zero issue depending on the return value
+/// of the called functions.
+int caller() {
+ int x = 0;
+ // The following if branches must never be taken.
+ if (return_zero_or_abort_by_fnptr())
+ return 1 / x; // no-warning: Dead code.
+ if (return_zero_or_abort_by_direct_fun())
+ return 1 / x; // no-warning: Dead code.
+
+ // Make sure the warning is still reported when viable.
+ return 1 / x; // expected-warning {{Division by zero}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
@@ -44,9 +44,11 @@
if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(CE.getDecl()))
BuildSinks = FD->hasAttr<AnalyzerNoReturnAttr>() || FD->isNoReturn();
- const Expr *Callee = CE.getOriginExpr();
- if (!BuildSinks && Callee)
- BuildSinks = getFunctionExtInfo(Callee->getType()).getNoReturn();
+ if (const CallExpr* CExpr = dyn_cast_or_null<CallExpr>(CE.getOriginExpr());
+ CExpr && !BuildSinks) {
+ if (const Expr *C = CExpr->getCallee())
+ BuildSinks = getFunctionExtInfo(C->getType()).getNoReturn();
+ }
if (!BuildSinks && CE.isGlobalCFunction()) {
if (const IdentifierInfo *II = CE.getCalleeIdentifier()) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
