From ec6da3fb9d8c7859da22d9eb7e814faf2e5a524a Mon Sep 17 00:00:00 2001 From: Arseniy Zaostrovnykh Date: Wed, 12 Oct 2022 14:46:32 +0200 Subject: [PATCH] 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 --- .../Checkers/NoReturnFunctionChecker.cpp | 8 +++-- clang/test/Analysis/no-return.c | 36 ++++++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 clang/test/Analysis/no-return.c diff --git a/clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp index af208e8..17c3cb4 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(CE.getDecl())) BuildSinks = FD->hasAttr() || FD->isNoReturn(); - const Expr *Callee = CE.getOriginExpr(); - if (!BuildSinks && Callee) - BuildSinks = getFunctionExtInfo(Callee->getType()).getNoReturn(); + if (const CallExpr *CExpr = dyn_cast_or_null(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 0000000..872604e --- /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}} +} -- 2.7.4