[C] Add -Wjump-bypasses-init #138009
Open
[C] Add -Wjump-bypasses-init #138009
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
AaronBallman
added
clang
AaronBallman
requested review from
jyknight,
erichkeane,
Fznamznon,
efriedma-quic and
Sirraide
|
@llvm/pr-subscribers-clang Author: Aaron Ballman (AaronBallman) ChangesWe 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. Full diff: https://github.com/llvm/llvm-project/pull/138009.diff 7 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5cc1a36fac1e8..91c526c4cbd42 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -188,6 +188,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++.
C2y Feature Support
^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 75e8fc541305b..ec324cd5a8d62 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -166,9 +166,11 @@ def DefaultConstInit : DiagGroup<"default-const-init", [DefaultConstInitUnsafe]>
def ImplicitVoidPtrCast : DiagGroup<"implicit-void-ptr-cast">;
def ImplicitIntToEnumCast : DiagGroup<"implicit-int-enum-cast",
[ImplicitEnumEnumCast]>;
+def JumpBypassesInit : DiagGroup<"jump-bypasses-init">;
def CXXCompat: DiagGroup<"c++-compat", [ImplicitVoidPtrCast, DefaultConstInit,
ImplicitIntToEnumCast, HiddenCppDecl,
- InitStringTooLongForCpp]>;
+ InitStringTooLongForCpp,
+ JumpBypassesInit]>;
def ExternCCompat : DiagGroup<"extern-c-compat">;
def KeywordCompat : DiagGroup<"keyword-compat">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 823d4bb687497..1c9a98f565fdc 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<
diff --git a/clang/lib/Sema/JumpDiagnostics.cpp b/clang/lib/Sema/JumpDiagnostics.cpp
index edcfffa2b3894..8b124fa76e046 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) {
@@ -932,8 +944,9 @@ void JumpScopeChecker::DiagnoseIndirectOrAsmJump(Stmt *Jump, unsigned JumpScope,
/// 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 +986,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 +1015,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 dfc718eedc1d9..2260159b68293 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13705,15 +13705,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..9ed7d0463f85a
--- /dev/null
+++ b/clang/test/Sema/warn-jump-bypasses-init.c
@@ -0,0 +1,31 @@
+// 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;
+ }
+}
diff --git a/clang/test/SemaOpenACC/no-branch-in-out.c b/clang/test/SemaOpenACC/no-branch-in-out.c
index 37126d8f2200e..b537799c38acc 100644
--- a/clang/test/SemaOpenACC/no-branch-in-out.c
+++ b/clang/test/SemaOpenACC/no-branch-in-out.c
@@ -560,13 +560,14 @@ LABEL3:{} // #GOTOLBL3
void IndirectGoto3_Loop() {
void* ptr;
#pragma acc parallel loop// #GOTOPAR_LOOP3
- for (unsigned i = 0; i < 5; ++i) {
+ for (unsigned i = 0; i < 5; ++i) { // #INIT
LABEL3:{} // #GOTOLBL3_2
ptr = &&LABEL3;
}
-// expected-error@+3{{cannot jump from this indirect goto statement to one of its possible targets}}
+// expected-error@+4{{cannot jump from this indirect goto statement to one of its possible targets}}
// expected-note@#GOTOLBL3_2{{possible target of indirect goto statement}}
// expected-note@#GOTOPAR_LOOP3{{invalid branch into OpenACC Compute/Combined Construct}}
+// expected-note@#INIT {{jump bypasses variable initialization}}
goto *ptr;
}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
erichkeane
approved these changes
Apr 30, 2025
We don't enable |
efriedma-quic
approved these changes
May 1, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.