[clang] 543f112 - [C] Add -Wjump-bypasses-init (#138009)
via cfe-commits
cfe-commits at lists.llvm.org
Fri May 2 06:06:35 PDT 2025
Author: Aaron Ballman
Date: 2025-05-02T09:06:31-04:00
New Revision: 543f112e148a81de290d099f10784dc3ff698aa4
URL: https://github.com/llvm/llvm-project/commit/543f112e148a81de290d099f10784dc3ff698aa4
DIFF: https://github.com/llvm/llvm-project/commit/543f112e148a81de290d099f10784dc3ff698aa4.diff
LOG: [C] Add -Wjump-bypasses-init (#138009)
We already diagnose when a jump bypasses initialization in C++ because
such code is ill-formed there. However, we were not using this to
diagnose those same jumps in C.
-Wjump-bypasses-init is grouped under -Wc++-compat and diagnoses this
situation as a compatibility issue with C++. This diagnostic is off by
default.
The diagnostic could perhaps be enabled by default for C, but due to the
design of jump diagnostic handling, it not a trivial task. For now,
we'll add the diagnostic as off-by-default so we get incremental
improvement, but a follow-up could try to refactor jump diagnostics so
we can enable this by default in C and have it as a C++ compatibility
diagnostic as well.
Added:
clang/test/Sema/warn-jump-bypasses-init.c
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/JumpDiagnostics.cpp
clang/lib/Sema/SemaDecl.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2c65fc4667562..6652ee9b8ed4b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -203,6 +203,10 @@ C Language Changes
``-Wunterminated-string-initialization``. However, this diagnostic is not
silenced by the ``nonstring`` attribute as these initializations are always
incompatible with C++.
+- Added ``-Wjump-bypasses-init``, which is off by default and grouped under
+ ``-Wc++-compat``. It diagnoses when a jump (``goto`` to its label, ``switch``
+ to its ``case``) will bypass the initialization of a local variable, which is
+ invalid in C++.
- Added the existing ``-Wduplicate-decl-specifier`` diagnostic, which is on by
default, to ``-Wc++-compat`` because duplicated declaration specifiers are
not valid in C++.
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 58439553d41c9..3c24387630d0c 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -177,11 +177,12 @@ def DefaultConstInit : DiagGroup<"default-const-init",
def ImplicitVoidPtrCast : DiagGroup<"implicit-void-ptr-cast">;
def ImplicitIntToEnumCast : DiagGroup<"implicit-int-enum-cast",
[ImplicitEnumEnumCast]>;
+def JumpBypassesInit : DiagGroup<"jump-bypasses-init">;
def TentativeDefnCompat : DiagGroup<"tentative-definition-compat">;
def CXXCompat: DiagGroup<"c++-compat", [ImplicitVoidPtrCast, DefaultConstInit,
- ImplicitIntToEnumCast, CppKeywordInC,
- HiddenCppDecl, TentativeDefnCompat,
- InitStringTooLongForCpp,
+ ImplicitIntToEnumCast, HiddenCppDecl,
+ InitStringTooLongForCpp, CppKeywordInC,
+ TentativeDefnCompat, JumpBypassesInit,
DuplicateDeclSpecifier]>;
def ExternCCompat : DiagGroup<"extern-c-compat">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 604bb56d28252..c630f7851c055 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6561,11 +6561,17 @@ def ext_goto_into_protected_scope : ExtWarn<
def warn_cxx98_compat_goto_into_protected_scope : Warning<
"jump from this goto statement to its label is incompatible with C++98">,
InGroup<CXX98Compat>, DefaultIgnore;
+def warn_cpp_compat_goto_into_protected_scope : Warning<
+ "jump from this goto statement to its label is incompatible with C++">,
+ InGroup<JumpBypassesInit>, DefaultIgnore;
def err_switch_into_protected_scope : Error<
"cannot jump from switch statement to this case label">;
def warn_cxx98_compat_switch_into_protected_scope : Warning<
"jump from switch statement to this case label is incompatible with C++98">,
InGroup<CXX98Compat>, DefaultIgnore;
+def warn_cpp_compat_switch_into_protected_scope : Warning<
+ "jump from switch statement to this case label is incompatible with C++">,
+ InGroup<JumpBypassesInit>, DefaultIgnore;
def err_indirect_goto_without_addrlabel : Error<
"indirect goto in function with no address-of-label expressions">;
def err_indirect_goto_in_protected_scope : Error<
@@ -6573,6 +6579,10 @@ def err_indirect_goto_in_protected_scope : Error<
def warn_cxx98_compat_indirect_goto_in_protected_scope : Warning<
"jump from this %select{indirect|asm}0 goto statement to one of its possible targets "
"is incompatible with C++98">, InGroup<CXX98Compat>, DefaultIgnore;
+def warn_cpp_compat_indirect_goto_in_protected_scope : Warning<
+ "jump from this %select{indirect|asm}0 goto statement to one of its possible "
+ "targets is incompatible with C++">,
+ InGroup<JumpBypassesInit>, DefaultIgnore;
def note_indirect_goto_target : Note<
"possible target of %select{indirect|asm}0 goto statement">;
def note_protected_by_variable_init : Note<
diff --git a/clang/lib/Sema/JumpDiagnostics.cpp b/clang/lib/Sema/JumpDiagnostics.cpp
index edcfffa2b3894..6d71b26801107 100644
--- a/clang/lib/Sema/JumpDiagnostics.cpp
+++ b/clang/lib/Sema/JumpDiagnostics.cpp
@@ -93,7 +93,7 @@ class JumpScopeChecker {
unsigned TargetScope);
void CheckJump(Stmt *From, Stmt *To, SourceLocation DiagLoc,
unsigned JumpDiag, unsigned JumpDiagWarning,
- unsigned JumpDiagCXX98Compat);
+ unsigned JumpDiagCompat);
void CheckGotoStmt(GotoStmt *GS);
const Attr *GetMustTailAttr(AttributedStmt *AS);
@@ -179,9 +179,8 @@ static ScopePair GetDiagForGotoScopeDecl(Sema &S, const Decl *D) {
}
}
- if (const Expr *Init = VD->getInit(); S.Context.getLangOpts().CPlusPlus &&
- VD->hasLocalStorage() && Init &&
- !Init->containsErrors()) {
+ if (const Expr *Init = VD->getInit();
+ VD->hasLocalStorage() && Init && !Init->containsErrors()) {
// C++11 [stmt.dcl]p3:
// A program that jumps from a point where a variable with automatic
// storage duration is not in scope to a point where it is in scope
@@ -680,7 +679,9 @@ void JumpScopeChecker::VerifyJumps() {
CheckJump(GS, GS->getLabel()->getStmt(), GS->getGotoLoc(),
diag::err_goto_into_protected_scope,
diag::ext_goto_into_protected_scope,
- diag::warn_cxx98_compat_goto_into_protected_scope);
+ S.getLangOpts().CPlusPlus
+ ? diag::warn_cxx98_compat_goto_into_protected_scope
+ : diag::warn_cpp_compat_goto_into_protected_scope);
}
CheckGotoStmt(GS);
continue;
@@ -708,7 +709,9 @@ void JumpScopeChecker::VerifyJumps() {
CheckJump(IGS, Target->getStmt(), IGS->getGotoLoc(),
diag::err_goto_into_protected_scope,
diag::ext_goto_into_protected_scope,
- diag::warn_cxx98_compat_goto_into_protected_scope);
+ S.getLangOpts().CPlusPlus
+ ? diag::warn_cxx98_compat_goto_into_protected_scope
+ : diag::warn_cpp_compat_goto_into_protected_scope);
continue;
}
@@ -725,7 +728,9 @@ void JumpScopeChecker::VerifyJumps() {
else
Loc = SC->getBeginLoc();
CheckJump(SS, SC, Loc, diag::err_switch_into_protected_scope, 0,
- diag::warn_cxx98_compat_switch_into_protected_scope);
+ S.getLangOpts().CPlusPlus
+ ? diag::warn_cxx98_compat_switch_into_protected_scope
+ : diag::warn_cpp_compat_switch_into_protected_scope);
}
}
}
@@ -867,6 +872,13 @@ static bool IsCXX98CompatWarning(Sema &S, unsigned InDiagNote) {
InDiagNote == diag::note_protected_by_variable_non_pod;
}
+/// Returns true if a particular note should be a C++ compatibility warning in
+/// C mode with -Wc++-compat.
+static bool IsCppCompatWarning(Sema &S, unsigned InDiagNote) {
+ return !S.getLangOpts().CPlusPlus &&
+ InDiagNote == diag::note_protected_by_variable_init;
+}
+
/// Produce primary diagnostic for an indirect jump statement.
static void DiagnoseIndirectOrAsmJumpStmt(Sema &S, Stmt *Jump,
LabelDecl *Target, bool &Diagnosed) {
@@ -906,34 +918,43 @@ void JumpScopeChecker::DiagnoseIndirectOrAsmJump(Stmt *Jump, unsigned JumpScope,
S.Diag(Scopes[I].Loc, Scopes[I].OutDiag);
}
- SmallVector<unsigned, 10> ToScopesCXX98Compat;
+ SmallVector<unsigned, 10> ToScopesCXX98Compat, ToScopesCppCompat;
// Now walk into the scopes containing the label whose address was taken.
for (unsigned I = TargetScope; I != Common; I = Scopes[I].ParentScope)
if (IsCXX98CompatWarning(S, Scopes[I].InDiag))
ToScopesCXX98Compat.push_back(I);
+ else if (IsCppCompatWarning(S, Scopes[I].InDiag))
+ ToScopesCppCompat.push_back(I);
else if (Scopes[I].InDiag) {
DiagnoseIndirectOrAsmJumpStmt(S, Jump, Target, Diagnosed);
S.Diag(Scopes[I].Loc, Scopes[I].InDiag);
}
- // Diagnose this jump if it would be ill-formed in C++98.
- if (!Diagnosed && !ToScopesCXX98Compat.empty()) {
+ // Diagnose this jump if it would be ill-formed in C++[98].
+ if (!Diagnosed) {
bool IsAsmGoto = isa<GCCAsmStmt>(Jump);
- S.Diag(Jump->getBeginLoc(),
- diag::warn_cxx98_compat_indirect_goto_in_protected_scope)
- << IsAsmGoto;
- S.Diag(Target->getStmt()->getIdentLoc(), diag::note_indirect_goto_target)
- << IsAsmGoto;
- NoteJumpIntoScopes(ToScopesCXX98Compat);
+ auto Diag = [&](unsigned DiagId, const SmallVectorImpl<unsigned> &Notes) {
+ S.Diag(Jump->getBeginLoc(), DiagId) << IsAsmGoto;
+ S.Diag(Target->getStmt()->getIdentLoc(), diag::note_indirect_goto_target)
+ << IsAsmGoto;
+ NoteJumpIntoScopes(Notes);
+ };
+ if (!ToScopesCXX98Compat.empty())
+ Diag(diag::warn_cxx98_compat_indirect_goto_in_protected_scope,
+ ToScopesCXX98Compat);
+ else if (!ToScopesCppCompat.empty())
+ Diag(diag::warn_cpp_compat_indirect_goto_in_protected_scope,
+ ToScopesCppCompat);
}
}
/// CheckJump - Validate that the specified jump statement is valid: that it is
/// jumping within or out of its current scope, not into a deeper one.
void JumpScopeChecker::CheckJump(Stmt *From, Stmt *To, SourceLocation DiagLoc,
- unsigned JumpDiagError, unsigned JumpDiagWarning,
- unsigned JumpDiagCXX98Compat) {
+ unsigned JumpDiagError,
+ unsigned JumpDiagWarning,
+ unsigned JumpDiagCompat) {
if (CHECK_PERMISSIVE(!LabelAndGotoScopes.count(From)))
return;
if (CHECK_PERMISSIVE(!LabelAndGotoScopes.count(To)))
@@ -973,15 +994,16 @@ void JumpScopeChecker::CheckJump(Stmt *From, Stmt *To, SourceLocation DiagLoc,
if (CommonScope == ToScope) return;
// Pull out (and reverse) any scopes we might need to diagnose skipping.
- SmallVector<unsigned, 10> ToScopesCXX98Compat;
+ SmallVector<unsigned, 10> ToScopesCompat;
SmallVector<unsigned, 10> ToScopesError;
SmallVector<unsigned, 10> ToScopesWarning;
for (unsigned I = ToScope; I != CommonScope; I = Scopes[I].ParentScope) {
if (S.getLangOpts().MSVCCompat && JumpDiagWarning != 0 &&
IsMicrosoftJumpWarning(JumpDiagError, Scopes[I].InDiag))
ToScopesWarning.push_back(I);
- else if (IsCXX98CompatWarning(S, Scopes[I].InDiag))
- ToScopesCXX98Compat.push_back(I);
+ else if (IsCXX98CompatWarning(S, Scopes[I].InDiag) ||
+ IsCppCompatWarning(S, Scopes[I].InDiag))
+ ToScopesCompat.push_back(I);
else if (Scopes[I].InDiag)
ToScopesError.push_back(I);
}
@@ -1001,10 +1023,10 @@ void JumpScopeChecker::CheckJump(Stmt *From, Stmt *To, SourceLocation DiagLoc,
NoteJumpIntoScopes(ToScopesError);
}
- // Handle -Wc++98-compat warnings if the jump is well-formed.
- if (ToScopesError.empty() && !ToScopesCXX98Compat.empty()) {
- S.Diag(DiagLoc, JumpDiagCXX98Compat);
- NoteJumpIntoScopes(ToScopesCXX98Compat);
+ // Handle -Wc++98-compat or -Wc++-compat warnings if the jump is well-formed.
+ if (ToScopesError.empty() && !ToScopesCompat.empty()) {
+ S.Diag(DiagLoc, JumpDiagCompat);
+ NoteJumpIntoScopes(ToScopesCompat);
}
}
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index d7421934032cf..8cabebfe81c73 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13724,15 +13724,17 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
return;
}
- if (VDecl->hasLocalStorage())
- setFunctionHasBranchProtectedScope();
-
if (DiagnoseUnexpandedParameterPack(Init, UPPC_Initializer)) {
VDecl->setInvalidDecl();
return;
}
}
+ // If the variable has an initializer and local storage, check whether
+ // anything jumps over the initialization.
+ if (VDecl->hasLocalStorage())
+ setFunctionHasBranchProtectedScope();
+
// OpenCL 1.1 6.5.2: "Variables allocated in the __local address space inside
// a kernel function cannot be initialized."
if (VDecl->getType().getAddressSpace() == LangAS::opencl_local) {
diff --git a/clang/test/Sema/warn-jump-bypasses-init.c b/clang/test/Sema/warn-jump-bypasses-init.c
new file mode 100644
index 0000000000000..a9604742bf50c
--- /dev/null
+++ b/clang/test/Sema/warn-jump-bypasses-init.c
@@ -0,0 +1,60 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=c,both -Wjump-bypasses-init %s
+// RUN: %clang_cc1 -fsyntax-only -verify=c,both -Wc++-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify=good %s
+// RUN: %clang_cc1 -fsyntax-only -verify=cxx,both -x c++ %s
+// good-no-diagnostics
+
+void goto_func_1(void) {
+ goto ouch; // c-warning {{jump from this goto statement to its label is incompatible with C++}} \
+ cxx-error {{cannot jump from this goto statement to its label}}
+ int i = 12; // both-note {{jump bypasses variable initialization}}
+
+ouch:
+ ;
+}
+
+void goto_func_2(void) {
+ goto ouch;
+ static int i = 12; // This initialization is not jumped over, so no warning.
+
+ouch:
+ ;
+}
+
+void switch_func(int i) {
+ switch (i) {
+ int x = 12; // both-note {{jump bypasses variable initialization}}
+ case 0: // c-warning {{jump from switch statement to this case label is incompatible with C++}} \
+ cxx-error {{cannot jump from switch statement to this case label}}
+ break;
+ }
+}
+
+// Statement expressions are a bit strange in that they seem to allow for
+// jumping past initialization without being diagnosed, even in C++. Perhaps
+// this should change?
+void f(void) {
+ ({
+ goto ouch;
+ int i = 12;
+ });
+
+ for (int i = ({ goto ouch; int x = 10; x;}); i < 0; ++i) {
+ }
+
+ouch:
+ ;
+}
+
+void indirect(int n) {
+DirectJump:
+ ;
+
+ void *Table[] = {&&DirectJump, &&Later};
+ goto *Table[n]; // c-warning {{jump from this indirect goto statement to one of its possible targets is incompatible with C++}} \
+ cxx-error {{cannot jump from this indirect goto statement to one of its possible targets}}
+
+ int x = 12; // both-note {{jump bypasses variable initialization}}
+Later: // both-note {{possible target of indirect goto statement}}
+ ;
+}
More information about the cfe-commits
mailing list