Skip to content

Commit f49f6b0

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 47ea854 commit f49f6b0

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
@@ -6765,6 +6765,11 @@ class Sema final : public SemaBase {
67656765
/// suffice, e.g., in a default function argument.
67666766
Decl *ManglingContextDecl;
67676767

6768+
/// Declaration for initializer if one is currently being
6769+
/// parsed. Used when an expression has a possibly unreachable
6770+
/// diagnostic to reference the declaration as a whole.
6771+
Decl *DeclForInitializer = nullptr;
6772+
67686773
/// If we are processing a decltype type, a set of call expressions
67696774
/// for which we have deferred checking the completeness of the return type.
67706775
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
@@ -2734,6 +2734,80 @@ static void flushDiagnostics(Sema &S, const sema::FunctionScopeInfo *fscope) {
27342734
S.Diag(D.Loc, D.PD);
27352735
}
27362736

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

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

29883024
// Warning: check missing 'return'
29893025
if (P.enableCheckFallThrough) {

clang/lib/Sema/SemaDecl.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13118,6 +13118,13 @@ namespace {
1311813118
if (isa<ParmVarDecl>(OrigDecl))
1311913119
return;
1312013120

13121+
// Skip checking for file-scope constexpr variables - constant evaluation
13122+
// will produce appropriate errors without needing runtime diagnostics.
13123+
// Local constexpr should still emit runtime warnings.
13124+
if (auto *VD = dyn_cast<VarDecl>(OrigDecl))
13125+
if (VD->isConstexpr() && VD->isFileVarDecl())
13126+
return;
13127+
1312113128
E = E->IgnoreParens();
1312213129

1312313130
// Skip checking T a = a where T is not a record or reference type.
@@ -13745,6 +13752,16 @@ void Sema::DiagnoseUniqueObjectDuplication(const VarDecl *VD) {
1374513752
}
1374613753

1374713754
void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
13755+
// RAII helper to ensure DeclForInitializer is cleared on all exit paths
13756+
struct ClearDeclForInitializer {
13757+
Sema &S;
13758+
ClearDeclForInitializer(Sema &S) : S(S) {}
13759+
~ClearDeclForInitializer() {
13760+
if (!S.ExprEvalContexts.empty())
13761+
S.ExprEvalContexts.back().DeclForInitializer = nullptr;
13762+
}
13763+
} Clearer(*this);
13764+
1374813765
// If there is no declaration, there was an error parsing it. Just ignore
1374913766
// the initializer.
1375013767
if (!RealDecl) {
@@ -15070,6 +15087,10 @@ void Sema::FinalizeDeclaration(Decl *ThisDecl) {
1507015087
if (!VD)
1507115088
return;
1507215089

15090+
// Emit any deferred warnings for the variable's initializer, even if the
15091+
// variable is invalid
15092+
AnalysisWarnings.IssueWarningsForRegisteredVarDecl(VD);
15093+
1507315094
// Apply an implicit SectionAttr if '#pragma clang section bss|data|rodata' is active
1507415095
if (VD->hasGlobalStorage() && VD->isThisDeclarationADefinition() &&
1507515096
!inTemplateInstantiation() && !VD->hasAttr<SectionAttr>()) {

clang/lib/Sema/SemaExpr.cpp

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20573,25 +20573,38 @@ void Sema::MarkDeclarationsReferencedInExpr(Expr *E,
2057320573
/// namespace { auto *p = new double[3][false ? (1, 2) : 3]; }
2057420574
bool Sema::DiagIfReachable(SourceLocation Loc, ArrayRef<const Stmt *> Stmts,
2057520575
const PartialDiagnostic &PD) {
20576-
if (!Stmts.empty() && getCurFunctionOrMethodDecl()) {
20577-
if (!FunctionScopes.empty())
20578-
FunctionScopes.back()->PossiblyUnreachableDiags.push_back(
20579-
sema::PossiblyUnreachableDiag(PD, Loc, Stmts));
20580-
return true;
20581-
}
20582-
2058320576
// The initializer of a constexpr variable or of the first declaration of a
2058420577
// static data member is not syntactically a constant evaluated constant,
2058520578
// but nonetheless is always required to be a constant expression, so we
2058620579
// can skip diagnosing.
20587-
// FIXME: Using the mangling context here is a hack.
2058820580
if (auto *VD = dyn_cast_or_null<VarDecl>(
20589-
ExprEvalContexts.back().ManglingContextDecl)) {
20581+
ExprEvalContexts.back().DeclForInitializer)) {
2059020582
if (VD->isConstexpr() ||
2059120583
(VD->isStaticDataMember() && VD->isFirstDecl() && !VD->isInline()))
2059220584
return false;
20593-
// FIXME: For any other kind of variable, we should build a CFG for its
20594-
// initializer and check whether the context in question is reachable.
20585+
}
20586+
20587+
if (Stmts.empty()) {
20588+
Diag(Loc, PD);
20589+
return true;
20590+
}
20591+
20592+
if (getCurFunction()) {
20593+
FunctionScopes.back()->PossiblyUnreachableDiags.push_back(
20594+
sema::PossiblyUnreachableDiag(PD, Loc, Stmts));
20595+
return true;
20596+
}
20597+
20598+
// For non-constexpr file-scope variables with reachability context (non-empty
20599+
// Stmts), build a CFG for the initializer and check whether the context in
20600+
// question is reachable.
20601+
if (auto *VD = dyn_cast_or_null<VarDecl>(
20602+
ExprEvalContexts.back().DeclForInitializer)) {
20603+
if (VD->isFileVarDecl()) {
20604+
AnalysisWarnings.RegisterVarDeclWarning(
20605+
VD, sema::PossiblyUnreachableDiag(PD, Loc, Stmts));
20606+
return true;
20607+
}
2059520608
}
2059620609

2059720610
Diag(Loc, PD);

0 commit comments

Comments
 (0)