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