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
intel · Oct 12, 2022 · ec6da3f · ec6da3f
1 parent 78c66cf
commit ec6da3f
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 3 deletions.
diff --git a/clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
@@ -44,9 +44,11 @@ void NoReturnFunctionChecker::checkPostCall(const CallEvent &CE,
   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()) {

diff --git a/clang/test/Analysis/no-return.c b/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}}
+}