[CodeGen][ObjC]

[ahatanak]

Summary

This fixes a bug in EmitObjCForCollectionStmt which is causing clang to generate malformed IR.

When the following code (which I think is legal, at least when it is not compiled with ARC) is compiled:

$ cat test1.m

@interface Obj
@end
void bar(void);
void foo(Obj *o) {
  Obj *i;
  for (i in o) {
  thing:
    bar();
  }
  goto thing;
}

the compilation terminates with the messages "Instruction does not dominate all uses!".

This patch fixes the bug by using temporary local variables instead of PHI instructions and moving instructions to locations that dominate their uses.

The example I showed still fails to compile (with an assertion) when compiled with -fobjc-arc, but I think that is a separate bug.

rdar://problem/31670637

[rjmccall]

I wouldn't mind unconditionally banning this in JumpDiagnostics, actually.

[ahatanak]

Unconditionally ban jumping into body of ObjC fast enumeration loop.

[rjmccall]

Thanks, this looks great. A few tweaks about the diagnostic and LGTM.

include/clang/Basic/DiagnosticSemaKinds.td

4985 ↗

(On Diff #95667)

Capitalization: "Objective-C"

It's probably better to just say "fast enumeration loop" and leave off "body".

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

 def note_protected_by_vla_type_alias : Note<
   "jump bypasses initialization of VLA type alias">;
 def note_protected_by_constexpr_if : Note<
   "jump enters controlled statement of constexpr if">;
 def note_protected_by_if_available : Note<
   "jump enters controlled statement of if available">;
 def note_protected_by_vla : Note<
   "jump bypasses initialization of variable length array">;
+def note_protected_by_objc_fast_enumeration : Note<
+  "jump enters Objective-C fast enumeration loop">;
 def note_protected_by_objc_try : Note<
   "jump bypasses initialization of @try block">;
 def note_protected_by_objc_catch : Note<
   "jump bypasses initialization of @catch block">;
 def note_protected_by_objc_finally : Note<
   "jump bypasses initialization of @finally block">;
 def note_protected_by_objc_synchronized : Note<
   "jump bypasses initialization of @synchronized block">;

cfe/trunk/lib/Sema/JumpDiagnostics.cpp

void JumpScopeChecker::BuildScopeInformation(Stmt *S,
   unsigned StmtsToSkip = 0u;
 
   // If we found a label, remember that it is in ParentScope scope.
   switch (S->getStmtClass()) {
   case Stmt::AddrLabelExprClass:
     IndirectJumpTargets.push_back(cast<AddrLabelExpr>(S)->getLabel());
     break;
 
+  case Stmt::ObjCForCollectionStmtClass: {
+    auto *CS = cast<ObjCForCollectionStmt>(S);
+    unsigned Diag = diag::note_protected_by_objc_fast_enumeration;
+    unsigned NewParentScope = Scopes.size();
+    Scopes.push_back(GotoScope(ParentScope, Diag, 0, S->getLocStart()));
+    BuildScopeInformation(CS->getBody(), NewParentScope);
+    return;
+  }
+
   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;

cfe/trunk/lib/Sema/SemaStmt.cpp

Sema::CheckObjCForCollectionOperand(SourceLocation forLoc, Expr *collection) {
   // Wrap up any cleanups in the expression.
   return collection;
 }
 
 StmtResult
 Sema::ActOnObjCForCollectionStmt(SourceLocation ForLoc,
                                  Stmt *First, Expr *collection,
                                  SourceLocation RParenLoc) {
+  getCurFunction()->setHasBranchProtectedScope();
 
   ExprResult CollectionExprResult =
     CheckObjCForCollectionOperand(ForLoc, collection);
 
   if (First) {
     QualType FirstType;
     if (DeclStmt *DS = dyn_cast<DeclStmt>(First)) {
       if (!DS->isSingleDecl())

cfe/trunk/test/SemaObjC/foreach.m

 @interface Test2
 @property (assign) id prop;
 @end
 void test2(NSObject<NSFastEnumeration> *collection) {
   Test2 *obj;
   for (obj.prop in collection) { /* expected-error {{selector element is not a valid lvalue}} */
   }
 }
+
+int cond();
+
+void test3(NSObject<NSFastEnumeration> *a0, NSObject<NSFastEnumeration> *a1) {
+  for (id i in a0) { /* expected-note 2 {{jump enters Objective-C fast enumeration loop}} */
+    for (id j in a1) { /* expected-note 2 {{jump enters Objective-C fast enumeration loop}} */
+      (void)i, (void)j;
+label0:
+      if (cond())
+        goto label1;
+    }
+label1:
+    if (cond())
+      goto label0; /* expected-error {{cannot jump from this goto statement to its label}} */
+    if (cond())
+      goto label2;
+  }
+
+label2:
+  if (cond())
+    goto label0; /* expected-error {{cannot jump from this goto statement to its label}} */
+  if (cond())
+    goto label1; /* expected-error{{cannot jump from this goto statement to its label}} */
+}