[PATCH] D135682: Fix false positive related to handling of [[noreturn]] function pointers
Arseniy Zaostrovnykh via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 11 08:44:50 PDT 2022
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()) {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D135682.466827.patch
Type: text/x-patch
Size: 2267 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221011/2919dba6/attachment-0001.bin>
More information about the cfe-commits
mailing list