diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -1776,6 +1776,8 @@ return const_cast(this)->getParent(); } + Decl *getLastDecl() const { return LastDecl; } + /// getLexicalParent - Returns the containing lexical DeclContext. May be /// different from getParent, e.g.: /// diff --git a/clang/include/clang/Sema/AnalysisBasedWarnings.h b/clang/include/clang/Sema/AnalysisBasedWarnings.h --- a/clang/include/clang/Sema/AnalysisBasedWarnings.h +++ b/clang/include/clang/Sema/AnalysisBasedWarnings.h @@ -13,7 +13,9 @@ #ifndef LLVM_CLANG_SEMA_ANALYSISBASEDWARNINGS_H #define LLVM_CLANG_SEMA_ANALYSISBASEDWARNINGS_H +#include "clang/AST/Decl.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/MapVector.h" namespace clang { @@ -23,8 +25,10 @@ class ObjCMethodDecl; class QualType; class Sema; +class AnalysisDeclContext; namespace sema { class FunctionScopeInfo; + class PossiblyUnreachableDiag; } namespace sema { @@ -49,6 +53,8 @@ enum VisitFlag { NotVisited = 0, Visited = 1, Pending = 2 }; llvm::DenseMap VisitedFD; + llvm::MapVector> + VarDeclPossiblyUnreachableDiags; /// \name Statistics /// @{ @@ -86,15 +92,25 @@ /// @} + void FlushDiagnostics(SmallVector); + public: AnalysisBasedWarnings(Sema &s); void IssueWarnings(Policy P, FunctionScopeInfo *fscope, const Decl *D, QualType BlockType); + void RegisterVarDeclWarning(VarDecl *VD, PossiblyUnreachableDiag PUD); + + void IssueWarningsForRegisteredVarDecl(VarDecl *VD); + Policy getDefaultPolicy() { return DefaultPolicy; } void PrintStats() const; + + void + EmitPossiblyUnreachableDiags(AnalysisDeclContext &AC, + SmallVector PUDs); }; }} // end namespace clang::sema diff --git a/clang/lib/Analysis/AnalysisDeclContext.cpp b/clang/lib/Analysis/AnalysisDeclContext.cpp --- a/clang/lib/Analysis/AnalysisDeclContext.cpp +++ b/clang/lib/Analysis/AnalysisDeclContext.cpp @@ -119,6 +119,11 @@ return BD->getBody(); else if (const auto *FunTmpl = dyn_cast_or_null(D)) return FunTmpl->getTemplatedDecl()->getBody(); + else if (const auto *VD = dyn_cast_or_null(D)) { + if (VD->hasGlobalStorage()) { + return const_cast(dyn_cast(VD->getInit())); + } + } llvm_unreachable("unknown code decl"); } diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -1964,7 +1964,7 @@ // warnings on a function, method, or block. //===----------------------------------------------------------------------===// -clang::sema::AnalysisBasedWarnings::Policy::Policy() { +sema::AnalysisBasedWarnings::Policy::Policy() { enableCheckFallThrough = 1; enableCheckUnreachable = 0; enableThreadSafetyAnalysis = 0; @@ -1975,17 +1975,12 @@ return (unsigned)!D.isIgnored(diag, SourceLocation()); } -clang::sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s) - : S(s), - NumFunctionsAnalyzed(0), - NumFunctionsWithBadCFGs(0), - NumCFGBlocks(0), - MaxCFGBlocksPerFunction(0), - NumUninitAnalysisFunctions(0), - NumUninitAnalysisVariables(0), - MaxUninitAnalysisVariablesPerFunction(0), - NumUninitAnalysisBlockVisits(0), - MaxUninitAnalysisBlockVisitsPerFunction(0) { +sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s) + : S(s), NumFunctionsAnalyzed(0), NumFunctionsWithBadCFGs(0), + NumCFGBlocks(0), MaxCFGBlocksPerFunction(0), + NumUninitAnalysisFunctions(0), NumUninitAnalysisVariables(0), + MaxUninitAnalysisVariablesPerFunction(0), NumUninitAnalysisBlockVisits(0), + MaxUninitAnalysisBlockVisitsPerFunction(0) { using namespace diag; DiagnosticsEngine &D = S.getDiagnostics(); @@ -2003,15 +1998,62 @@ isEnabled(D, warn_use_in_invalid_state); } -static void flushDiagnostics(Sema &S, const sema::FunctionScopeInfo *fscope) { - for (const auto &D : fscope->PossiblyUnreachableDiags) +void sema::AnalysisBasedWarnings::FlushDiagnostics( + const SmallVector PUDs) { + for (const auto &D : PUDs) S.Diag(D.Loc, D.PD); } -void clang::sema:: -AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P, - sema::FunctionScopeInfo *fscope, - const Decl *D, QualType BlockType) { +void sema::AnalysisBasedWarnings::EmitPossiblyUnreachableDiags( + AnalysisDeclContext &AC, + SmallVector PUDs) { + + if (PUDs.empty()) { + return; + } + + bool analyzed = false; + + // Register the expressions with the CFGBuilder. + for (const auto &D : PUDs) { + for (const Stmt *S : D.Stmts) + AC.registerForcedBlockExpression(S); + } + + if (AC.getCFG()) { + analyzed = true; + for (const auto &D : PUDs) { + bool AllReachable = true; + for (const Stmt *S : D.Stmts) { + const CFGBlock *block = AC.getBlockForRegisteredExpression(S); + CFGReverseBlockReachabilityAnalysis *cra = + AC.getCFGReachablityAnalysis(); + // FIXME: We should be able to assert that block is non-null, but + // the CFG analysis can skip potentially-evaluated expressions in + // edge cases; see test/Sema/vla-2.c. + if (block && cra) { + // Can this block be reached from the entrance? + if (!cra->isReachable(&AC.getCFG()->getEntry(), block)) { + AllReachable = false; + break; + } + } + // If we cannot map to a basic block, assume the statement is + // reachable. + } + + if (AllReachable) + S.Diag(D.Loc, D.PD); + } + } + + if (!analyzed) + FlushDiagnostics(PUDs); +} + +void sema::AnalysisBasedWarnings::IssueWarnings( + sema::AnalysisBasedWarnings::Policy P, sema::FunctionScopeInfo *fscope, + const Decl *D, QualType BlockType) { // We avoid doing analysis-based warnings when there are errors for // two reasons: @@ -2033,7 +2075,7 @@ if (Diags.hasUncompilableErrorOccurred()) { // Flush out any possibly unreachable diagnostics. - flushDiagnostics(S, fscope); + FlushDiagnostics(fscope->PossiblyUnreachableDiags); return; } @@ -2085,45 +2127,7 @@ } // Emit delayed diagnostics. - if (!fscope->PossiblyUnreachableDiags.empty()) { - bool analyzed = false; - - // Register the expressions with the CFGBuilder. - for (const auto &D : fscope->PossiblyUnreachableDiags) { - for (const Stmt *S : D.Stmts) - AC.registerForcedBlockExpression(S); - } - - if (AC.getCFG()) { - analyzed = true; - for (const auto &D : fscope->PossiblyUnreachableDiags) { - bool AllReachable = true; - for (const Stmt *S : D.Stmts) { - const CFGBlock *block = AC.getBlockForRegisteredExpression(S); - CFGReverseBlockReachabilityAnalysis *cra = - AC.getCFGReachablityAnalysis(); - // FIXME: We should be able to assert that block is non-null, but - // the CFG analysis can skip potentially-evaluated expressions in - // edge cases; see test/Sema/vla-2.c. - if (block && cra) { - // Can this block be reached from the entrance? - if (!cra->isReachable(&AC.getCFG()->getEntry(), block)) { - AllReachable = false; - break; - } - } - // If we cannot map to a basic block, assume the statement is - // reachable. - } - - if (AllReachable) - S.Diag(D.Loc, D.PD); - } - } - - if (!analyzed) - flushDiagnostics(S, fscope); - } + EmitPossiblyUnreachableDiags(AC, fscope->PossiblyUnreachableDiags); // Warning: check missing 'return' if (P.enableCheckFallThrough) { @@ -2249,7 +2253,27 @@ } } -void clang::sema::AnalysisBasedWarnings::PrintStats() const { +void sema::AnalysisBasedWarnings::RegisterVarDeclWarning( + VarDecl *VD, clang::sema::PossiblyUnreachableDiag PUD) { + VarDeclPossiblyUnreachableDiags[VD].emplace_back(PUD); +} + +void sema::AnalysisBasedWarnings::IssueWarningsForRegisteredVarDecl( + VarDecl *VD) { + AnalysisDeclContext AC(nullptr, VD); + + AC.getCFGBuildOptions().PruneTriviallyFalseEdges = true; + AC.getCFGBuildOptions().AddEHEdges = false; + AC.getCFGBuildOptions().AddInitializers = true; + AC.getCFGBuildOptions().AddImplicitDtors = true; + AC.getCFGBuildOptions().AddTemporaryDtors = true; + AC.getCFGBuildOptions().AddCXXNewAllocator = false; + AC.getCFGBuildOptions().AddCXXDefaultInitExprInCtors = true; + + EmitPossiblyUnreachableDiags(AC, VarDeclPossiblyUnreachableDiags[VD]); +} + +void sema::AnalysisBasedWarnings::PrintStats() const { llvm::errs() << "\n*** Analysis Based Warnings Stats:\n"; unsigned NumCFGsBuilt = NumFunctionsAnalyzed - NumFunctionsWithBadCFGs; diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -31,6 +31,7 @@ #include "clang/Lex/Lexer.h" // TODO: Extract static functions to fix layering. #include "clang/Lex/ModuleLoader.h" // TODO: Sema shouldn't depend on Lex #include "clang/Lex/Preprocessor.h" // Included for isCodeCompletionEnabled() +#include "clang/Sema/AnalysisBasedWarnings.h" #include "clang/Sema/CXXFieldCollector.h" #include "clang/Sema/DeclSpec.h" #include "clang/Sema/DelayedDiagnostic.h" @@ -11849,6 +11850,8 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) { if (var->isInvalidDecl()) return; + AnalysisWarnings.IssueWarningsForRegisteredVarDecl(var); + if (getLangOpts().OpenCL) { // OpenCL v2.0 s6.12.5 - Every block variable declaration must have an // initialiser diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -288,6 +288,9 @@ UnparsedDefaultArgInstantiations.erase(InstPos); } + // Check for delayed warnings + AnalysisWarnings.IssueWarningsForRegisteredVarDecl(Param); + return false; } @@ -333,7 +336,21 @@ return; } + // Temporarily change the context and add param to it. + // Allows DiagRuntimeBehavior to register this param with + // any possibly warnings. + // Param gets added back to context when function is added + // to context. + // FIXME: Params should probably be diagnosed after being + // actually added to context. Either params get added to + // context before the function scope or diagnostics are + // delayed until function scope is added. + DeclContext *Cur = CurContext; + CurContext = Param->getDeclContext(); + CurContext->addDecl(Param); SetParamDefaultArgument(Param, DefaultArg, EqualLoc); + CurContext->removeDecl(Param); + CurContext = Cur; } /// ActOnParamUnparsedDefaultArgument - We've seen a default diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -4881,6 +4881,8 @@ "default argument expression has capturing blocks?"); } + AnalysisWarnings.IssueWarningsForRegisteredVarDecl(Param); + // We already type-checked the argument, so we know it works. // Just mark all of the declarations in this potentially-evaluated expression // as being "referenced". @@ -16666,8 +16668,8 @@ case ExpressionEvaluationContext::PotentiallyEvaluated: case ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed: if (!Stmts.empty() && getCurFunctionOrMethodDecl()) { - FunctionScopes.back()->PossiblyUnreachableDiags. - push_back(sema::PossiblyUnreachableDiag(PD, Loc, Stmts)); + FunctionScopes.back()->PossiblyUnreachableDiags.push_back( + PossiblyUnreachableDiag(PD, Loc, Stmts)); return true; } @@ -16676,13 +16678,29 @@ // but nonetheless is always required to be a constant expression, so we // can skip diagnosing. // FIXME: Using the mangling context here is a hack. + // + // Mangling context seems to only be defined on constexpr vardecl that + // displayed errors. + // This skips warnings that were already emitted as notes on errors. if (auto *VD = dyn_cast_or_null( ExprEvalContexts.back().ManglingContextDecl)) { if (VD->isConstexpr() || (VD->isStaticDataMember() && VD->isFirstDecl() && !VD->isInline())) break; - // FIXME: For any other kind of variable, we should build a CFG for its - // initializer and check whether the context in question is reachable. + } + + // For any other kind of variable, we should build a CFG for its + // initializer and check whether the context in question is reachable. + if (auto *VD = dyn_cast_or_null(CurContext->getLastDecl())) { + if (VD->getDefinition()) { + VD = VD->getDefinition(); + } + // FIXME: Some cases aren't picked up by path analysis currently + if (!Stmts.empty() && VD->isFileVarDecl()) { + AnalysisWarnings.RegisterVarDeclWarning( + VD, PossiblyUnreachableDiag(PD, Loc, Stmts)); + return true; + } } Diag(Loc, PD); diff --git a/clang/test/Sema/warn-unreachable-warning-var-decl.cpp b/clang/test/Sema/warn-unreachable-warning-var-decl.cpp new file mode 100644 --- /dev/null +++ b/clang/test/Sema/warn-unreachable-warning-var-decl.cpp @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -verify %s +int e = 1 ? 0 : 1 / 0; +int g = 1 ? 1 / 0 : 0; // expected-warning{{division by zero is undefined}} + +#define SHIFT(n) (((n) == 32) ? 0 : ((1 << (n)) - 1)) + +int x = SHIFT(32); +int y = SHIFT(0); + +// FIXME: Expressions in lambdas aren't ignored +int z = []() { + return 1 ? 0 : 1 / 0; // expected-warning{{division by zero is undefined}} +}(); + +int f(void) { + int x = 1 ? 0 : 1 / 0; + return x; +} + +template +struct X0 { + static T value; +}; + +template +struct X1 { + static T value; +}; + +template +T X0::value = 3.14; // expected-warning{{implicit conversion from 'double' to 'int' changes value from 3.14 to 3}} + +template +T X1::value = 1 ? 0 : 1 / 0; + +template struct X0; // expected-note{{in instantiation of static data member 'X0::value' requested here}} + +constexpr signed char c1 = 100 * 2; // expected-warning{{changes value}} +constexpr signed char c2 = '\x64' * '\2'; // expected-warning{{changes value}} +constexpr int shr_32 = 0 >> 32; // expected-error {{constant expression}} expected-note {{shift count 32 >= width of type}}