[clang] ec6da3f - Fix false positive related to handling of [[noreturn]] function pointers

[Balazs Benics via cfe-commits]

Author: Arseniy Zaostrovnykh
Date: 2022-10-12T14:46:32+02:00
New Revision: ec6da3fb9d8c7859da22d9eb7e814faf2e5a524a

URL: https://github.com/llvm/llvm-project/commit/ec6da3fb9d8c7859da22d9eb7e814faf2e5a524a
DIFF: https://github.com/llvm/llvm-project/commit/ec6da3fb9d8c7859da22d9eb7e814faf2e5a524a.diff

LOG: 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

Added: 
    clang/test/Analysis/no-return.c

Modified: 
    clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
index af208e8673187..17c3cb4e9e04c 100644
--- 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
new file mode 100644
index 0000000000000..872604ebd71d4
--- /dev/null
+++ 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}}
+}