Skip to content

Commit 64b365a

Browse files
JustinStittnhukc
andcommitted
[Clang] Consider reachability for file-scope warnings on initializers
Suppress runtime warnings for unreachable code in global variable initializers while maintaining warnings for the same code in functions. For context, some global variable declarations using ternaries can emit warnings during initialization even when a particular expression is never used. This behavior differs from GCC since they properly emit warnings based on reachability -- even at file scope. The simplest example is: ```c $ cat test.c unsigned long long b1 = 1 ? 0 : 1ULL << 64; $ clang-21 -fsyntax-only test.c test.c:1:39: warning: shift count >= width of type [-Wshift-count-overflow] 1 | unsigned long long foo = 1 ? 0 : 1ULL << 64; | ^ ~~ $ gcc-13 -fsyntax-only test.c <empty> ``` Clang previously emitted a ``-Wshift-count-overflow`` warning regardless of branch taken. This PR constructs a CFG and defers runtime diagnostic emission until we can analyze the CFG for reachability. To be clear, the intention is only to modify initializers in global scope and only remove warnings if unreachable, no new warnings should pop up anywhere. Link: https://reviews.llvm.org/D63889 (original PR by Nathan Huckleberry) Link: ClangBuiltLinux/linux#92 Co-Authored-By: Nathan Huckleberry <[email protected]> Signed-off-by: Justin Stitt <[email protected]>
1 parent 76e71e0 commit 64b365a

File tree

12 files changed

+210
-57
lines changed

12 files changed

+210
-57
lines changed

clang/include/clang/Parse/Parser.h

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5223,10 +5223,31 @@ class Parser : public CodeCompletionHandler {
52235223
/// assignment-expression
52245224
/// '{' ...
52255225
/// \endverbatim
5226-
ExprResult ParseInitializer() {
5227-
if (Tok.isNot(tok::l_brace))
5228-
return ParseAssignmentExpression();
5229-
return ParseBraceInitializer();
5226+
ExprResult ParseInitializer(Decl *DeclForInitializer = nullptr) {
5227+
// Set DeclForInitializer for file-scope variables.
5228+
// For constexpr references, set it to suppress runtime warnings.
5229+
// For non-constexpr references, don't set it to avoid evaluation issues
5230+
// with self-referencing initializers. Local variables (including local
5231+
// constexpr) should emit runtime warnings.
5232+
if (DeclForInitializer && !Actions.ExprEvalContexts.empty()) {
5233+
if (auto *VD = dyn_cast<VarDecl>(DeclForInitializer)) {
5234+
if (VD->isFileVarDecl()) {
5235+
if (!VD->getType()->isReferenceType() || VD->isConstexpr()) {
5236+
Actions.ExprEvalContexts.back().DeclForInitializer =
5237+
DeclForInitializer;
5238+
}
5239+
}
5240+
}
5241+
}
5242+
5243+
ExprResult init;
5244+
if (Tok.isNot(tok::l_brace)) {
5245+
init = ParseAssignmentExpression();
5246+
} else {
5247+
init = ParseBraceInitializer();
5248+
}
5249+
5250+
return init;
52305251
}
52315252

52325253
/// MayBeDesignationStart - Return true if the current token might be the

clang/include/clang/Sema/AnalysisBasedWarnings.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,19 @@
1414
#define LLVM_CLANG_SEMA_ANALYSISBASEDWARNINGS_H
1515

1616
#include "clang/AST/Decl.h"
17+
#include "clang/Sema/ScopeInfo.h"
1718
#include "llvm/ADT/DenseMap.h"
19+
#include "llvm/ADT/MapVector.h"
1820
#include <memory>
1921

2022
namespace clang {
2123

24+
class AnalysisDeclContext;
2225
class Decl;
2326
class FunctionDecl;
2427
class QualType;
2528
class Sema;
29+
class VarDecl;
2630
namespace sema {
2731
class FunctionScopeInfo;
2832
class SemaPPCallbacks;
@@ -57,10 +61,14 @@ class AnalysisBasedWarnings {
5761

5862
enum VisitFlag { NotVisited = 0, Visited = 1, Pending = 2 };
5963
llvm::DenseMap<const FunctionDecl*, VisitFlag> VisitedFD;
64+
llvm::MapVector<VarDecl *, SmallVector<PossiblyUnreachableDiag, 4>>
65+
VarDeclPossiblyUnreachableDiags;
6066

6167
Policy PolicyOverrides;
6268
void clearOverrides();
6369

70+
void FlushDiagnostics(SmallVector<clang::sema::PossiblyUnreachableDiag, 4>);
71+
6472
/// \name Statistics
6573
/// @{
6674

@@ -107,6 +115,10 @@ class AnalysisBasedWarnings {
107115
// Issue warnings that require whole-translation-unit analysis.
108116
void IssueWarnings(TranslationUnitDecl *D);
109117

118+
void RegisterVarDeclWarning(VarDecl *VD, PossiblyUnreachableDiag PUD);
119+
120+
void IssueWarningsForRegisteredVarDecl(VarDecl *VD);
121+
110122
// Gets the default policy which is in effect at the given source location.
111123
Policy getPolicyInEffectAt(SourceLocation Loc);
112124

@@ -116,6 +128,10 @@ class AnalysisBasedWarnings {
116128
Policy &getPolicyOverrides() { return PolicyOverrides; }
117129

118130
void PrintStats() const;
131+
132+
void
133+
EmitPossiblyUnreachableDiags(AnalysisDeclContext &AC,
134+
SmallVector<PossiblyUnreachableDiag, 4> PUDs);
119135
};
120136

121137
} // namespace sema

clang/include/clang/Sema/Sema.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6750,6 +6750,11 @@ class Sema final : public SemaBase {
67506750
/// suffice, e.g., in a default function argument.
67516751
Decl *ManglingContextDecl;
67526752

6753+
/// Declaration for initializer if one is currently being
6754+
/// parsed. Used when an expression has a possibly unreachable
6755+
/// diagnostic to reference the declaration as a whole.
6756+
Decl *DeclForInitializer = nullptr;
6757+
67536758
/// If we are processing a decltype type, a set of call expressions
67546759
/// for which we have deferred checking the completeness of the return type.
67556760
SmallVector<CallExpr *, 8> DelayedDecltypeCalls;

clang/lib/Analysis/AnalysisDeclContext.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,11 @@ Stmt *AnalysisDeclContext::getBody(bool &IsAutosynthesized) const {
117117
return BD->getBody();
118118
else if (const auto *FunTmpl = dyn_cast_or_null<FunctionTemplateDecl>(D))
119119
return FunTmpl->getTemplatedDecl()->getBody();
120+
else if (const auto *VD = dyn_cast_or_null<VarDecl>(D)) {
121+
if (VD->hasGlobalStorage()) {
122+
return const_cast<Stmt *>(dyn_cast_or_null<Stmt>(VD->getInit()));
123+
}
124+
}
120125

121126
llvm_unreachable("unknown code decl");
122127
}

clang/lib/Parse/ParseDecl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2613,7 +2613,7 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
26132613
}
26142614

26152615
PreferredType.enterVariableInit(Tok.getLocation(), ThisDecl);
2616-
ExprResult Init = ParseInitializer();
2616+
ExprResult Init = ParseInitializer(ThisDecl);
26172617

26182618
// If this is the only decl in (possibly) range based for statement,
26192619
// our best guess is that the user meant ':' instead of '='.

clang/lib/Parse/ParseDeclCXX.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3359,7 +3359,7 @@ ExprResult Parser::ParseCXXMemberInitializer(Decl *D, bool IsFunction,
33593359
Diag(Tok, diag::err_ms_property_initializer) << PD;
33603360
return ExprError();
33613361
}
3362-
return ParseInitializer();
3362+
return ParseInitializer(D);
33633363
}
33643364

33653365
void Parser::SkipCXXMemberSpecification(SourceLocation RecordLoc,

clang/lib/Parse/ParseOpenMP.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,7 @@ void Parser::ParseOpenMPReductionInitializerForDecl(VarDecl *OmpPrivParm) {
339339
}
340340

341341
PreferredType.enterVariableInit(Tok.getLocation(), OmpPrivParm);
342-
ExprResult Init = ParseInitializer();
342+
ExprResult Init = ParseInitializer(OmpPrivParm);
343343

344344
if (Init.isInvalid()) {
345345
SkipUntil(tok::r_paren, tok::annot_pragma_openmp_end, StopBeforeMatch);

clang/lib/Sema/AnalysisBasedWarnings.cpp

Lines changed: 75 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2724,6 +2724,80 @@ static void flushDiagnostics(Sema &S, const sema::FunctionScopeInfo *fscope) {
27242724
S.Diag(D.Loc, D.PD);
27252725
}
27262726

2727+
void sema::AnalysisBasedWarnings::FlushDiagnostics(
2728+
const SmallVector<clang::sema::PossiblyUnreachableDiag, 4> PUDs) {
2729+
for (const auto &D : PUDs)
2730+
S.Diag(D.Loc, D.PD);
2731+
}
2732+
2733+
void sema::AnalysisBasedWarnings::EmitPossiblyUnreachableDiags(
2734+
AnalysisDeclContext &AC,
2735+
SmallVector<clang::sema::PossiblyUnreachableDiag, 4> PUDs) {
2736+
2737+
if (PUDs.empty()) {
2738+
return;
2739+
}
2740+
2741+
bool Analyzed = false;
2742+
2743+
for (const auto &D : PUDs) {
2744+
for (const Stmt *S : D.Stmts)
2745+
AC.registerForcedBlockExpression(S);
2746+
}
2747+
2748+
if (AC.getCFG()) {
2749+
Analyzed = true;
2750+
for (const auto &D : PUDs) {
2751+
bool AllReachable = true;
2752+
for (const Stmt *St : D.Stmts) {
2753+
const CFGBlock *block = AC.getBlockForRegisteredExpression(St);
2754+
CFGReverseBlockReachabilityAnalysis *cra =
2755+
AC.getCFGReachablityAnalysis();
2756+
if (block && cra) {
2757+
// Can this block be reached from the entrance?
2758+
if (!cra->isReachable(&AC.getCFG()->getEntry(), block)) {
2759+
AllReachable = false;
2760+
break;
2761+
}
2762+
}
2763+
// If we cannot map to a basic block, assume the statement is
2764+
// reachable.
2765+
}
2766+
2767+
if (AllReachable)
2768+
S.Diag(D.Loc, D.PD);
2769+
}
2770+
}
2771+
2772+
if (!Analyzed)
2773+
FlushDiagnostics(PUDs);
2774+
}
2775+
2776+
void sema::AnalysisBasedWarnings::RegisterVarDeclWarning(
2777+
VarDecl *VD, clang::sema::PossiblyUnreachableDiag PUD) {
2778+
VarDeclPossiblyUnreachableDiags[VD].emplace_back(PUD);
2779+
}
2780+
2781+
void sema::AnalysisBasedWarnings::IssueWarningsForRegisteredVarDecl(
2782+
VarDecl *VD) {
2783+
if (VarDeclPossiblyUnreachableDiags.find(VD) ==
2784+
VarDeclPossiblyUnreachableDiags.end()) {
2785+
return;
2786+
}
2787+
2788+
AnalysisDeclContext AC(nullptr, VD);
2789+
2790+
AC.getCFGBuildOptions().PruneTriviallyFalseEdges = true;
2791+
AC.getCFGBuildOptions().AddEHEdges = false;
2792+
AC.getCFGBuildOptions().AddInitializers = true;
2793+
AC.getCFGBuildOptions().AddImplicitDtors = true;
2794+
AC.getCFGBuildOptions().AddTemporaryDtors = true;
2795+
AC.getCFGBuildOptions().AddCXXNewAllocator = false;
2796+
AC.getCFGBuildOptions().AddCXXDefaultInitExprInCtors = true;
2797+
2798+
EmitPossiblyUnreachableDiags(AC, VarDeclPossiblyUnreachableDiags[VD]);
2799+
}
2800+
27272801
// An AST Visitor that calls a callback function on each callable DEFINITION
27282802
// that is NOT in a dependent context:
27292803
class CallableVisitor : public DynamicRecursiveASTVisitor {
@@ -2935,45 +3009,7 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
29353009
}
29363010

29373011
// Emit delayed diagnostics.
2938-
if (!fscope->PossiblyUnreachableDiags.empty()) {
2939-
bool analyzed = false;
2940-
2941-
// Register the expressions with the CFGBuilder.
2942-
for (const auto &D : fscope->PossiblyUnreachableDiags) {
2943-
for (const Stmt *S : D.Stmts)
2944-
AC.registerForcedBlockExpression(S);
2945-
}
2946-
2947-
if (AC.getCFG()) {
2948-
analyzed = true;
2949-
for (const auto &D : fscope->PossiblyUnreachableDiags) {
2950-
bool AllReachable = true;
2951-
for (const Stmt *S : D.Stmts) {
2952-
const CFGBlock *block = AC.getBlockForRegisteredExpression(S);
2953-
CFGReverseBlockReachabilityAnalysis *cra =
2954-
AC.getCFGReachablityAnalysis();
2955-
// FIXME: We should be able to assert that block is non-null, but
2956-
// the CFG analysis can skip potentially-evaluated expressions in
2957-
// edge cases; see test/Sema/vla-2.c.
2958-
if (block && cra) {
2959-
// Can this block be reached from the entrance?
2960-
if (!cra->isReachable(&AC.getCFG()->getEntry(), block)) {
2961-
AllReachable = false;
2962-
break;
2963-
}
2964-
}
2965-
// If we cannot map to a basic block, assume the statement is
2966-
// reachable.
2967-
}
2968-
2969-
if (AllReachable)
2970-
S.Diag(D.Loc, D.PD);
2971-
}
2972-
}
2973-
2974-
if (!analyzed)
2975-
flushDiagnostics(S, fscope);
2976-
}
3012+
EmitPossiblyUnreachableDiags(AC, fscope->PossiblyUnreachableDiags);
29773013

29783014
// Warning: check missing 'return'
29793015
if (P.enableCheckFallThrough) {

clang/lib/Sema/SemaDecl.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13100,6 +13100,13 @@ namespace {
1310013100
if (isa<ParmVarDecl>(OrigDecl))
1310113101
return;
1310213102

13103+
// Skip checking for file-scope constexpr variables - constant evaluation
13104+
// will produce appropriate errors without needing runtime diagnostics.
13105+
// Local constexpr should still emit runtime warnings.
13106+
if (auto *VD = dyn_cast<VarDecl>(OrigDecl))
13107+
if (VD->isConstexpr() && VD->isFileVarDecl())
13108+
return;
13109+
1310313110
E = E->IgnoreParens();
1310413111

1310513112
// Skip checking T a = a where T is not a record or reference type.
@@ -13727,6 +13734,16 @@ void Sema::DiagnoseUniqueObjectDuplication(const VarDecl *VD) {
1372713734
}
1372813735

1372913736
void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
13737+
// RAII helper to ensure DeclForInitializer is cleared on all exit paths
13738+
struct ClearDeclForInitializer {
13739+
Sema &S;
13740+
ClearDeclForInitializer(Sema &S) : S(S) {}
13741+
~ClearDeclForInitializer() {
13742+
if (!S.ExprEvalContexts.empty())
13743+
S.ExprEvalContexts.back().DeclForInitializer = nullptr;
13744+
}
13745+
} Clearer(*this);
13746+
1373013747
// If there is no declaration, there was an error parsing it. Just ignore
1373113748
// the initializer.
1373213749
if (!RealDecl) {
@@ -15045,6 +15062,10 @@ void Sema::FinalizeDeclaration(Decl *ThisDecl) {
1504515062
if (!VD)
1504615063
return;
1504715064

15065+
// Emit any deferred warnings for the variable's initializer, even if the
15066+
// variable is invalid
15067+
AnalysisWarnings.IssueWarningsForRegisteredVarDecl(VD);
15068+
1504815069
// Apply an implicit SectionAttr if '#pragma clang section bss|data|rodata' is active
1504915070
if (VD->hasGlobalStorage() && VD->isThisDeclarationADefinition() &&
1505015071
!inTemplateInstantiation() && !VD->hasAttr<SectionAttr>()) {

clang/lib/Sema/SemaExpr.cpp

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20550,25 +20550,38 @@ void Sema::MarkDeclarationsReferencedInExpr(Expr *E,
2055020550
/// namespace { auto *p = new double[3][false ? (1, 2) : 3]; }
2055120551
bool Sema::DiagIfReachable(SourceLocation Loc, ArrayRef<const Stmt *> Stmts,
2055220552
const PartialDiagnostic &PD) {
20553-
if (!Stmts.empty() && getCurFunctionOrMethodDecl()) {
20554-
if (!FunctionScopes.empty())
20555-
FunctionScopes.back()->PossiblyUnreachableDiags.push_back(
20556-
sema::PossiblyUnreachableDiag(PD, Loc, Stmts));
20557-
return true;
20558-
}
20559-
2056020553
// The initializer of a constexpr variable or of the first declaration of a
2056120554
// static data member is not syntactically a constant evaluated constant,
2056220555
// but nonetheless is always required to be a constant expression, so we
2056320556
// can skip diagnosing.
20564-
// FIXME: Using the mangling context here is a hack.
2056520557
if (auto *VD = dyn_cast_or_null<VarDecl>(
20566-
ExprEvalContexts.back().ManglingContextDecl)) {
20558+
ExprEvalContexts.back().DeclForInitializer)) {
2056720559
if (VD->isConstexpr() ||
2056820560
(VD->isStaticDataMember() && VD->isFirstDecl() && !VD->isInline()))
2056920561
return false;
20570-
// FIXME: For any other kind of variable, we should build a CFG for its
20571-
// initializer and check whether the context in question is reachable.
20562+
}
20563+
20564+
if (Stmts.empty()) {
20565+
Diag(Loc, PD);
20566+
return true;
20567+
}
20568+
20569+
if (getCurFunction()) {
20570+
FunctionScopes.back()->PossiblyUnreachableDiags.push_back(
20571+
sema::PossiblyUnreachableDiag(PD, Loc, Stmts));
20572+
return true;
20573+
}
20574+
20575+
// For non-constexpr file-scope variables with reachability context (non-empty
20576+
// Stmts), build a CFG for the initializer and check whether the context in
20577+
// question is reachable.
20578+
if (auto *VD = dyn_cast_or_null<VarDecl>(
20579+
ExprEvalContexts.back().DeclForInitializer)) {
20580+
if (VD->isFileVarDecl()) {
20581+
AnalysisWarnings.RegisterVarDeclWarning(
20582+
VD, sema::PossiblyUnreachableDiag(PD, Loc, Stmts));
20583+
return true;
20584+
}
2057220585
}
2057320586

2057420587
Diag(Loc, PD);

0 commit comments

Comments
 (0)