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}} + ; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
