[clang][JumpDiagnostics] ignore non-asm goto target scopes

[nickdesaulniers]

Summary

The current behavior of JumpScopeChecker::VerifyIndirectOrAsmJumps was
to cross validate the scope of every jumping statement (goto, asm goto)
against the scope of every label (even if the label was not even a
possible target of the asm goto).

When we have multiple asm goto's with unique targets, we could trigger
false positive build errors complaining that labels that weren't even in
the asm goto's label list could not be jumped to. Example:

error: cannot jump from this asm goto statement to one of its possible targets
asm goto(""::::foo);
note: possible target of asm goto statement
bar:
^

Fixes: https://github.com/ClangBuiltLinux/linux/issues/1886

[nickdesaulniers]

• add link to CBL

[nickdesaulniers]

• rebase

[nickdesaulniers]

• reword description

[nickdesaulniers]

• reword commit msg one more time

[jyu2]

LGTM.

[jyu2]

clang/lib/Sema/JumpDiagnostics.cpp

794

Just wonder why not just use TargetLabel instead?

[cor3ntin]

Thanks a lot for working on this.
This probably needs a release note entry

clang/lib/Sema/JumpDiagnostics.cpp

790

We don't need to make 3 copies!

791–800

I don't think the pointer adds that much, you can init capture by copy which simplifies a bit

clang/test/Sema/asm-goto.cpp

71

can you add more tests? Maybe something like that

cpp
void f() {
  try{
    __label__ label;
    asm goto("" : : : : label);
    label:;
  }
  catch(...){
    __label__ label;
    asm goto("" : : : : label);
    label:;
  };
  if constexpr(false) {
    __label__ label;
    asm goto("" : : : : label);
    label:;
  }
}

[cor3ntin]

clang/lib/Sema/JumpDiagnostics.cpp

791–800

I don't think the pointer adds that much, you can init capture by copy which simplifies a bit

s/pointer/comment

[rjmccall]

Wait, the whole point of this algorithm is that we have to do this whole complicated linear check against every label whose address is taken because we don't know where it's going. If we have a list of all the possible labels that the asm goto might jump to, why are we building a map of all the labels and then filtering out the ones that aren't listed? We should just be checking the listed destinations with the stricter triviality rule that indirect-goto uses.

[nickdesaulniers]

In D155342#4504812, @rjmccall wrote:

Wait, the whole point of this algorithm is that we have to do this whole complicated linear check against every label whose address is taken because we don't know where it's going. If we have a list of all the possible labels that the asm goto might jump to, why are we building a map of all the labels and then filtering out the ones that aren't listed? We should just be checking the listed destinations with the stricter triviality rule that indirect-goto uses.

Great point. Looks like I should probably be calling CheckJump from JumpScopeChecker::VerifyJumps instead. Let me work on that refactoring instead.

[nickdesaulniers]

• test just asm goto targets, as per @rjmccall (this lead to many cleanups)
• add release note and test case as per @cor3ntin
• remove changes to JumpScopeChecker::BuildScopeInformation

[nickdesaulniers]

clang/test/Sema/asm-goto.cpp

71

Done, but I have to put it somewhere else because:

1. exceptions are disabled in clang/test/Sema/*. This needs to go in clang/test/SemaCXX/
2. if constexpr is a C++17 addition.

[nickdesaulniers]

clang/lib/Sema/JumpDiagnostics.cpp

75

AsmJumps can now be removed, too.

[nickdesaulniers]

• remove AsmJumps

[nickdesaulniers]

clang/lib/Sema/JumpDiagnostics.cpp

728–729

should remove this reference to asm goto as well.

[nickdesaulniers]

• remove stale asm goto comment

[rjmccall]

Thanks, that's way better. Just a couple minor requests.

clang/docs/ReleaseNotes.rst

659

clang/lib/Sema/JumpDiagnostics.cpp

363

You can pull the GCCAsmStmtClass case right above this, make the cast unconditional (if (!cast<GCCAsmStmt>(S)->isAsmGoto()) break;), and then fall through into the GotoStmt case.

[nickdesaulniers]

• fix typo in release doc

clang/lib/Sema/JumpDiagnostics.cpp

363

I could hoist + use [[fallthrough]] but note that the case above Stmt::SwitchStmtClass also currently uses [[fallthrough]] here as well; so I would still need the dyn_cast.

With that in mind, do you still prefer the hoisting? I don't have a preference, but wanted to triple check that with you.

[rjmccall]

clang/lib/Sema/JumpDiagnostics.cpp

363

Ah, right. Personally, I would probably put this common path in a helper function, but you could also just duplicate it since it's so small. This also seems like an acceptable use-case for goto if you can stomach that — with a good label, it should still be very readable.

[nickdesaulniers]

clang/lib/Sema/JumpDiagnostics.cpp

363

Is https://reviews.llvm.org/D155522 what you had in mind? If so, then I'll squash it into this, otherwise can you state differently what you had in mind?

[nickdesaulniers]

clang/lib/Sema/JumpDiagnostics.cpp

363

oh, rereading what you wrote, I think I understand what you mean with gotos. Let me post a diff of that, for clarification.

[rjmccall]

I do like the look of https://reviews.llvm.org/D155522, yeah, since we have so many of these that aren't top-level in a case anyway.

[nickdesaulniers]

clang/lib/Sema/JumpDiagnostics.cpp

363

https://reviews.llvm.org/D155525 perhaps?

[nickdesaulniers]

• squash D155525 as per @rjmccall

[nickdesaulniers]

Thanks for the reviews and advice. Any parting thoughts @rjmccall ?

[rjmccall]

clang/lib/Sema/JumpDiagnostics.cpp

658

I think it would be good to leave a comment here like this:

We know the possible destinations of an asm goto, but we don't have the
ability to add code along those control flow edges, so we have to diagnose
jumps both in and out of scopes, just like we do with an indirect goto.

[nickdesaulniers]

• add comment as per @rjmccall

clang/lib/Sema/JumpDiagnostics.cpp

658

Depending on your definition of "we" (clang vs. llvm), llvm does have the ability to add code along those edges.

See llvm/lib/CodeGen/CallBrPrepare.cpp. I'll clarify in your comment that "clang does not split critical edges during code generation of llvm ... "

[rjmccall]

clang/lib/Sema/JumpDiagnostics.cpp

658

Okay, so, I don't really know much about this feature. I was thinking that the branch might go directly into some other assembly block, which would not be splittable. If the branch just goes to an arbitrary basic block in IR, then it would be straightforward for IRGen to just resolve the destination blocks for asm goto labels to some new block that does a normal goto to that label. If we did that, we wouldn't need extra restrictions here at all and could just check this like any other direct branch.

We don't need to do that work right away, but the comment should probably reflect the full state of affairs — "but clang's IR generation does not currently know how to add code along these control flow edges, so we have to diagnose jumps both in and out of scopes, like we do with indirect goto. If we ever add that ability to IRGen, this code could check these jumps just like ordinary gotos."

[nickdesaulniers]

clang/lib/Sema/JumpDiagnostics.cpp

658

Okay, so, I don't really know much about this feature.

"Run this block of asm, then continue to either the next statement or one of the explicit labels in the label list."

---

That comment still doesn't seem quite right to me.

asm goto is more like a direct goto or a switch in the sense that the cases or possible destination are known at compile time; that's not like indirect goto where you're jumping to literally anywhere.

We need to check the scopes like we would for direct goto, because we don't want to bypass non-trivial destructors.

---

Interestingly, it looks like some of the cases inclang/test/Sema/asm-goto.cpp, g++ permits, if you use the -fpermissive flag. Clang doesn't error that it doesn't recognize that flag, but it doesn't seem to do anything in clang, FWICT playing with it in godbolt.

---

That said, I would have thought

void test4cleanup(int*);
// No errors expected.
void test4(void) {
l0:;
    int x __attribute__((cleanup(test4cleanup)));
    asm goto("# %l0"::::l0);
}

To work with this change, but we still produce:

x.c:6:5: error: cannot jump from this asm goto statement to one of its possible targets
    6 |     asm goto("# %l0"::::l0);
      |     ^
x.c:4:1: note: possible target of asm goto statement
    4 | l0:;
      | ^
x.c:5:9: note: jump exits scope of variable with __attribute__((cleanup))
    5 |     int x __attribute__((cleanup(test4cleanup)));
      |         ^

Aren't those in the same scope though? I would have expected that to work just as if we had a direct goto l0 rather than the asm goto.

[rjmccall]

We need to check the scopes like we would for direct goto, because we don't want to bypass non-trivial destructors.

I think this is the basic misunderstanding here. Direct goto is allowed to jump out of scopes; it just runs destructors on the way. It is only a direct branch to the target block if there are no destructors along the path.

Indirect goto is not allowed to jump out of scopes because, in general, the labels could be in different scopes with different sets of destructors required, and so the only way we could run the right set of destructors along the way would be to dynamically compare the label value with specific labels, which would defeat the point of the feature. (This is unnecessary if all the labels are in the same scope, which is typical, but nobody has felt like extending the feature to recognize that pattern and be more general.)

[efriedma]

clang/lib/Sema/JumpDiagnostics.cpp

658

(There's some history here that the original implementation of asm goto treated it semantically more like an indirect goto, including the use of address-of-labels; for a variety of reasons, we changed it so it's more like a switch statement.)

Suppose we have:

void test4cleanup(int*);
void test4(void) {
    asm goto("# %l0"::::l0);
l0:;
    int x __attribute__((cleanup(test4cleanup)));
    asm goto("# %l0"::::l0);
}

To make this work correctly, the first goto needs to branch directly to the destination, but the second needs to branch to a call to test4cleanup(). It's probably not that hard to implement: instead of branching directly to the destination bb, each edge out of the callbr needs to branch to a newly created block, and that block needs to EmitBranchThroughCleanup() to the final destination. (We create such blocks anyway to handle output values, but the newly created blocks branch directly to the destination BasicBlock instead of using EmitBranchThroughCleanup().)

But until we implement that, we need the error message so we don't miscompile.

[nickdesaulniers]

• rewrite comment based on feedback from @rjmccall and @efriedma

[nickdesaulniers]

In D155342#4511879, @rjmccall wrote:

We need to check the scopes like we would for direct goto, because we don't want to bypass non-trivial destructors.

I think this is the basic misunderstanding here. Direct goto is allowed to jump out of scopes; it just runs destructors on the way. It is only a direct branch to the target block if there are no destructors along the path.

Indirect goto is not allowed to jump out of scopes because, in general, the labels could be in different scopes with different sets of destructors required

Ah, got it.

clang/lib/Sema/JumpDiagnostics.cpp

658

but the second needs to branch to a call to test4cleanup().

GCC does not behave that way (i.e. if the branch is taken from the asm goto to l0, test4cleanup is not run). In fact, if I remove the call to DiagnoseIndirectOrAsmJump below, we generate the same control flow that GCC 12 does. https://godbolt.org/z/Y6en3YsY1

Perhaps one could argue "that's surprising" or "not correct" but if we were to have such a difference then that would probably preclude the use of the unholy combination of asm goto and __attribute__((cleanup())) (famous last words).

Let me try again with the comment based on feedback thus far.

[efriedma]

clang/lib/Sema/JumpDiagnostics.cpp

658

I guess that's an argument for keeping around this diagnostic, at least for now.

Can you file a bug against gcc?

[jyknight]

clang/lib/Sema/JumpDiagnostics.cpp

658

That looks like a bug in GCC to me. Can you open a bug report against GCC with your test-case, and refer to the bug here?

I think we should not modify Clang to implement any non-error behavior until the GCC bug is addressed.

[rjmccall]

Yes, this diagnostic implements a core semantic rule and must be emitted. There are some situations where Clang will actually generate malformed IR (violating dominance rules) if you fail to diagnose jump violations correctly. The IR you get when there's a jump over a destructor is well-formed, but it's still arguably* a miscompile and must be diagnosed.

(*) It's a miscompile unless there's a rule saying it's U.B. to jump out of a scope. I don't see such a rule in any of the docs about this feature.

[nickdesaulniers]

• add link to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110728

[rjmccall]

LGTM

[nathanchance]

Thanks, I can confirm that the __attribute__((__cleanup__(...))) errors that we observed with Peter's work-in-progress guards series are resolved and that all the errors I observed when building the kernel after https://reviews.llvm.org/D154696 was merged are no longer present when that change is re-applied on top of this one.

[shafik]

clang/lib/Sema/JumpDiagnostics.cpp

669

Do we have a test that checks this diagnostic?

[nickdesaulniers]

clang/lib/Sema/JumpDiagnostics.cpp

669

Yes, all of the existing tests in clang/test/Sema/asm-goto.cpp are a result of this statement.

clang/docs/ReleaseNotes.rst

- Fix the contains-errors bit not being set for DeclRefExpr that refers to a
   (`#50243 <https://github.com/llvm/llvm-project/issues/50243>`_),
   (`#48636 <https://github.com/llvm/llvm-project/issues/48636>`_),
   (`#50320 <https://github.com/llvm/llvm-project/issues/50320>`_).
 - Fix an assertion when using ``\u0024`` (``$``) as an identifier, by disallowing
   that construct (`#62133 <https://github.com/llvm/llvm-project/issues/38717>_`).
 - Fix crash caused by PseudoObjectExprBitfields: NumSubExprs overflow.
   (`#63169 <https://github.com/llvm/llvm-project/issues/63169>_`)
 - Fix crash when casting an object to an array type.
   (`#63758 <https://github.com/llvm/llvm-project/issues/63758>_`)
+- Fixed false positive error diagnostic observed from mixing ``asm goto`` with
+  ``__attribute__((cleanup()))`` variables falsely warning that jumps to
+  non-targets would skip cleanup.
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Bug Fixes to Attribute Support
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 - Fixed a bug where attribute annotations on type specifiers (enums, classes,
   structs, unions, and scoped enums) were not properly ignored, resulting in

clang/lib/Sema/JumpDiagnostics.cpp

GotoScope(unsigned parentScope, unsigned InDiag, unsigned OutDiag,
       : ParentScope(parentScope), InDiag(InDiag), OutDiag(OutDiag), Loc(L) {}
   };
 
   SmallVector<GotoScope, 48> Scopes;
   llvm::DenseMap<Stmt*, unsigned> LabelAndGotoScopes;
   SmallVector<Stmt*, 16> Jumps;
 
   SmallVector<Stmt*, 4> IndirectJumps;
-  SmallVector<Stmt*, 4> AsmJumps;
-  SmallVector<AttributedStmt *, 4> MustTailStmts;
-  SmallVector<LabelDecl*, 4> IndirectJumpTargets;
-  SmallVector<LabelDecl*, 4> AsmJumpTargets;
+  SmallVector<LabelDecl *, 4> IndirectJumpTargets;
+  SmallVector<AttributedStmt *, 4> MustTailStmts;
+
 public:
   JumpScopeChecker(Stmt *Body, Sema &S);
 private:
   void BuildScopeInformation(Decl *D, unsigned &ParentScope);
   void BuildScopeInformation(VarDecl *D, const BlockDecl *BDecl,
                              unsigned &ParentScope);
   void BuildScopeInformation(CompoundLiteralExpr *CLE, unsigned &ParentScope);
   void BuildScopeInformation(Stmt *S, unsigned &origParentScope);
 
   void VerifyJumps();
-  void VerifyIndirectOrAsmJumps(bool IsAsmGoto);
+  void VerifyIndirectJumps();
   void VerifyMustTailStmts();
   void NoteJumpIntoScopes(ArrayRef<unsigned> ToScopes);
   void DiagnoseIndirectOrAsmJump(Stmt *IG, unsigned IGScope, LabelDecl *Target,
                                  unsigned TargetScope);
   void CheckJump(Stmt *From, Stmt *To, SourceLocation DiagLoc,
                  unsigned JumpDiag, unsigned JumpDiagWarning,
                  unsigned JumpDiagCXX98Compat);
   void CheckGotoStmt(GotoStmt *GS);
JumpScopeChecker::JumpScopeChecker(Stmt *Body, Sema &s)
 
   // Build information for the top level compound statement, so that we have a
   // defined scope record for every "goto" and label.
   unsigned BodyParentScope = 0;
   BuildScopeInformation(Body, BodyParentScope);
 
   // Check that all jumps we saw are kosher.
   VerifyJumps();
-  VerifyIndirectOrAsmJumps(false);
-  VerifyIndirectOrAsmJumps(true);
+  VerifyIndirectJumps();
   VerifyMustTailStmts();
 }
 
 /// GetDeepestCommonScope - Finds the innermost scope enclosing the
 /// two scopes.
 unsigned JumpScopeChecker::GetDeepestCommonScope(unsigned A, unsigned B) {
   while (A != B) {
     // Inner scopes are created after outer scopes and therefore have
void JumpScopeChecker::BuildScopeInformation(Stmt *S,
   }
 
   case Stmt::IndirectGotoStmtClass:
     // "goto *&&lbl;" is a special case which we treat as equivalent
     // to a normal goto.  In addition, we don't calculate scope in the
     // operand (to avoid recording the address-of-label use), which
     // works only because of the restricted set of expressions which
     // we detect as constant targets.
-    if (cast<IndirectGotoStmt>(S)->getConstantTarget()) {
-      LabelAndGotoScopes[S] = ParentScope;
-      Jumps.push_back(S);
-      return;
-    }
+    if (cast<IndirectGotoStmt>(S)->getConstantTarget())
+      goto RecordJumpScope;
 
     LabelAndGotoScopes[S] = ParentScope;
     IndirectJumps.push_back(S);
     break;
 
   case Stmt::SwitchStmtClass:
     // Evaluate the C++17 init stmt and condition variable
     // before entering the scope of the switch statement.
     if (Stmt *Init = cast<SwitchStmt>(S)->getInit()) {
       BuildScopeInformation(Init, ParentScope);
       ++StmtsToSkip;
     }
     if (VarDecl *Var = cast<SwitchStmt>(S)->getConditionVariable()) {
       BuildScopeInformation(Var, ParentScope);
       ++StmtsToSkip;
     }
+    goto RecordJumpScope;
+
+  case Stmt::GCCAsmStmtClass:
+    if (!cast<GCCAsmStmt>(S)->isAsmGoto())
+      break;
     [[fallthrough]];
 
   case Stmt::GotoStmtClass:
+  RecordJumpScope:
     // Remember both what scope a goto is in as well as the fact that we have
     // it.  This makes the second scan not have to walk the AST again.
     LabelAndGotoScopes[S] = ParentScope;
     Jumps.push_back(S);
     break;
 
-  case Stmt::GCCAsmStmtClass:
-    if (auto *GS = dyn_cast<GCCAsmStmt>(S))
-      if (GS->isAsmGoto()) {
-        // Remember both what scope a goto is in as well as the fact that we
-        // have it.  This makes the second scan not have to walk the AST again.
-        LabelAndGotoScopes[S] = ParentScope;
-        AsmJumps.push_back(GS);
-        for (auto *E : GS->labels())
-          AsmJumpTargets.push_back(E->getLabel());
-      }
-    break;
-
   case Stmt::IfStmtClass: {
     IfStmt *IS = cast<IfStmt>(S);
     if (!(IS->isConstexpr() || IS->isConsteval() ||
           IS->isObjCAvailabilityCheck()))
       break;
 
     unsigned Diag = diag::note_protected_by_if_available;
     if (IS->isConstexpr())
if (GotoStmt *GS = dyn_cast<GotoStmt>(Jump)) {
                   diag::err_goto_into_protected_scope,
                   diag::ext_goto_into_protected_scope,
                   diag::warn_cxx98_compat_goto_into_protected_scope);
       }
       CheckGotoStmt(GS);
       continue;
     }
 
+    // If an asm goto jumps to a different scope, things like destructors or
+    // initializers might not be run which may be suprising to users. Perhaps
+    // this behavior can be changed in the future, but today Clang will not
+    // generate such code. Produce a diagnostic instead. See also the
+    // discussion here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110728.
+    if (auto *G = dyn_cast<GCCAsmStmt>(Jump)) {
+      for (AddrLabelExpr *L : G->labels()) {
+        LabelDecl *LD = L->getLabel();
+        unsigned JumpScope = LabelAndGotoScopes[G];
+        unsigned TargetScope = LabelAndGotoScopes[LD->getStmt()];
+        if (JumpScope != TargetScope)
+          DiagnoseIndirectOrAsmJump(G, JumpScope, LD, TargetScope);
+      }
+      continue;
+    }
+
     // We only get indirect gotos here when they have a constant target.
     if (IndirectGotoStmt *IGS = dyn_cast<IndirectGotoStmt>(Jump)) {
       LabelDecl *Target = IGS->getConstantTarget();
       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);
       continue;
for (SwitchCase *SC = SS->getSwitchCaseList(); SC;
       else
         Loc = SC->getBeginLoc();
       CheckJump(SS, SC, Loc, diag::err_switch_into_protected_scope, 0,
                 diag::warn_cxx98_compat_switch_into_protected_scope);
     }
   }
 }
 
-/// VerifyIndirectOrAsmJumps - Verify whether any possible indirect goto or
-/// asm goto jump might cross a protection boundary.  Unlike direct jumps,
-/// indirect or asm goto jumps count cleanups as protection boundaries:
-/// since there's no way to know where the jump is going, we can't implicitly
-/// run the right cleanups the way we can with direct jumps.
-/// Thus, an indirect/asm jump is "trivial" if it bypasses no
-/// initializations and no teardowns.  More formally, an indirect/asm jump
-/// from A to B is trivial if the path out from A to DCA(A,B) is
-/// trivial and the path in from DCA(A,B) to B is trivial, where
-/// DCA(A,B) is the deepest common ancestor of A and B.
-/// Jump-triviality is transitive but asymmetric.
+/// VerifyIndirectJumps - Verify whether any possible indirect goto jump might
+/// cross a protection boundary.  Unlike direct jumps, indirect goto jumps
+/// count cleanups as protection boundaries: since there's no way to know where
+/// the jump is going, we can't implicitly run the right cleanups the way we
+/// can with direct jumps.  Thus, an indirect/asm jump is "trivial" if it
+/// bypasses no initializations and no teardowns.  More formally, an
+/// indirect/asm jump from A to B is trivial if the path out from A to DCA(A,B)
+/// is trivial and the path in from DCA(A,B) to B is trivial, where DCA(A,B) is
+/// the deepest common ancestor of A and B.  Jump-triviality is transitive but
+/// asymmetric.
 ///
 /// A path in is trivial if none of the entered scopes have an InDiag.
 /// A path out is trivial is none of the exited scopes have an OutDiag.
 ///
 /// Under these definitions, this function checks that the indirect
 /// jump between A and B is trivial for every indirect goto statement A
 /// and every label B whose address was taken in the function.
-void JumpScopeChecker::VerifyIndirectOrAsmJumps(bool IsAsmGoto) {
-  SmallVector<Stmt*, 4> GotoJumps = IsAsmGoto ? AsmJumps : IndirectJumps;
-  if (GotoJumps.empty())
+void JumpScopeChecker::VerifyIndirectJumps() {
+  if (IndirectJumps.empty())
     return;
-  SmallVector<LabelDecl *, 4> JumpTargets =
-      IsAsmGoto ? AsmJumpTargets : IndirectJumpTargets;
   // If there aren't any address-of-label expressions in this function,
   // complain about the first indirect goto.
-  if (JumpTargets.empty()) {
-    assert(!IsAsmGoto && "only indirect goto can get here");
-    S.Diag(GotoJumps[0]->getBeginLoc(),
+  if (IndirectJumpTargets.empty()) {
+    S.Diag(IndirectJumps[0]->getBeginLoc(),
            diag::err_indirect_goto_without_addrlabel);
     return;
   }
-  // Collect a single representative of every scope containing an
-  // indirect or asm goto.  For most code bases, this substantially cuts
-  // down on the number of jump sites we'll have to consider later.
+  // Collect a single representative of every scope containing an indirect
+  // goto.  For most code bases, this substantially cuts down on the number of
+  // jump sites we'll have to consider later.
   using JumpScope = std::pair<unsigned, Stmt *>;
   SmallVector<JumpScope, 32> JumpScopes;
   {
     llvm::DenseMap<unsigned, Stmt*> JumpScopesMap;
-    for (Stmt *IG : GotoJumps) {
+    for (Stmt *IG : IndirectJumps) {
       if (CHECK_PERMISSIVE(!LabelAndGotoScopes.count(IG)))
         continue;
       unsigned IGScope = LabelAndGotoScopes[IG];
       if (!JumpScopesMap.contains(IGScope))
         JumpScopesMap[IGScope] = IG;
     }
     JumpScopes.reserve(JumpScopesMap.size());
     for (auto &Pair : JumpScopesMap)
       JumpScopes.emplace_back(Pair);
   }
 
   // Collect a single representative of every scope containing a
   // label whose address was taken somewhere in the function.
   // For most code bases, there will be only one such scope.
   llvm::DenseMap<unsigned, LabelDecl*> TargetScopes;
-  for (LabelDecl *TheLabel : JumpTargets) {
+  for (LabelDecl *TheLabel : IndirectJumpTargets) {
     if (CHECK_PERMISSIVE(!LabelAndGotoScopes.count(TheLabel->getStmt())))
       continue;
     unsigned LabelScope = LabelAndGotoScopes[TheLabel->getStmt()];
     if (!TargetScopes.contains(LabelScope))
       TargetScopes[LabelScope] = TheLabel;
   }
 
   // For each target scope, make sure it's trivially reachable from
while (true) {
       // Stop if we can't trivially enter the current scope.
       if (Scopes[Min].InDiag) break;
 
       Min = Scopes[Min].ParentScope;
     }
 
     // Walk through all the jump sites, checking that they can trivially
     // reach this label scope.
     for (auto [JumpScope, JumpStmt] : JumpScopes) {
       unsigned Scope = JumpScope;
       // Walk out the "scope chain" for this scope, looking for a scope
       // we've marked reachable.  For well-formed code this amortizes
       // to O(JumpScopes.size() / Scopes.size()):  we only iterate
       // when we see something unmarked, and in well-formed code we
       // mark everything we iterate past.
       bool IsReachable = false;
       while (true) {
         if (Reachable.test(Scope)) {
           // If we find something reachable, mark all the scopes we just
           // walked through as reachable.
           for (unsigned S = JumpScope; S != Scope; S = Scopes[S].ParentScope)
             Reachable.set(S);
           IsReachable = true;
           break;
         }
 
         // Don't walk out if we've reached the top-level scope or we've

clang/test/Sema/asm-goto.cpp

int test3(int n)
   // expected-note@+1 {{possible target of asm goto statement}}
   return ({int a[n];label_true: 2;});
   // expected-note@+1 {{jump bypasses initialization of variable length array}}
   int b[n];
 // expected-note@+1 {{possible target of asm goto statement}}
 loop:
   return 0;
 }
+
+void test4cleanup(int*);
+// No errors expected.
+void test4(void) {
+  asm goto(""::::l0);
+l0:;
+  int x __attribute__((cleanup(test4cleanup)));
+  asm goto(""::::l1);
+l1:;
+}

clang/test/SemaCXX/goto2.cpp

-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -fcxx-exceptions %s
 // expected-no-diagnostics
 
 //PR9463
 int subfun(const char *text) {
   const char *tmp = text;
   return 0;
 }
 

 
 int main() {
   const char *ptr = text;
 
   fun(ptr);
 
   return 0;
 }
+
+void asm_goto_try_catch() {
+  try {
+    __label__ label;
+    asm goto("" : : : : label);
+    label:;
+  } catch(...) {
+    __label__ label;
+    asm goto("" : : : : label);
+    label:;
+  };
+  if constexpr(false) {
+    __label__ label;
+    asm goto("" : : : : label);
+    label:;
+  }
+}