kubamracek created this revision.
kubamracek added a project: Sanitizers.

CodeGenFunction::EmitObjCForCollectionStmt currently emits lifetime markers for 
the loop variable in an inconsistent way:  `lifetime.start` is emitted before 
the loop is entered, but `lifetime.end` is emitted inside the loop:

  ; entry block
    %u = alloca %1*, align 8
    %1 = bitcast %1** %u to i8*
    call void @llvm.lifetime.start(i64 8, i8* %1) #5
    ...
  
  ; loop body
    ...
    %14 = bitcast %1** %u to i8*
    call void @llvm.lifetime.end(i64 8, i8* %14) #5
    ...
    br i1 %21, ... ; loop
  
  ; loop ended
    ret void

AddressSanitizer uses these markers to track out-of-scope accesses to local 
variables, and we get false positives in Obj-C foreach loops (in the 2nd 
iteration of the loop).  The markers of the loop variable need to be either 
both inside the loop (so that we poison and unpoison the variable in each 
iteration), or both outside.  This patch implements the "both inside" approach 
and makes EmitObjCForCollectionStmt emit:

  ; entry block
    %u = alloca %1*, align 8
    ...
  
  ; loop body
    %12 = bitcast %1** %u to i8*
    call void @llvm.lifetime.start(i64 8, i8* %12) #5
    ...
    %14 = bitcast %1** %u to i8*
    call void @llvm.lifetime.end(i64 8, i8* %14) #5
    br label %15
  
  ; loop ended
    ret void

The test fixups are only changing the order of allocas.   There's some related 
discussion at https://reviews.llvm.org/D18618.


Repository:
  rL LLVM

https://reviews.llvm.org/D32029

Files:
  lib/CodeGen/CGObjC.cpp
  test/CodeGenObjC/arc-foreach.m
  test/CodeGenObjC/arc-ternary-op.m


Index: test/CodeGenObjC/arc-ternary-op.m
===================================================================
--- test/CodeGenObjC/arc-ternary-op.m
+++ test/CodeGenObjC/arc-ternary-op.m
@@ -120,9 +120,9 @@
 
   // CHECK-LABEL:    define void @test2(
   // CHECK:      [[COND:%.*]] = alloca i32,
-  // CHECK:      alloca i8*
   // CHECK:      [[CLEANUP_SAVE:%.*]] = alloca i8*
   // CHECK:      [[RUN_CLEANUP:%.*]] = alloca i1
+  // CHECK:      alloca i8*
   //   Evaluate condition; cleanup disabled by default.
   // CHECK:      [[T0:%.*]] = load i32, i32* [[COND]],
   // CHECK-NEXT: icmp ne i32 [[T0]], 0
Index: test/CodeGenObjC/arc-foreach.m
===================================================================
--- test/CodeGenObjC/arc-foreach.m
+++ test/CodeGenObjC/arc-foreach.m
@@ -24,9 +24,9 @@
 
 // CHECK-LP64-LABEL:    define void @test0(
 // CHECK-LP64:      [[ARRAY:%.*]] = alloca [[ARRAY_T:%.*]]*,
-// CHECK-LP64-NEXT: [[X:%.*]] = alloca i8*,
 // CHECK-LP64-NEXT: [[STATE:%.*]] = alloca [[STATE_T:%.*]],
 // CHECK-LP64-NEXT: [[BUFFER:%.*]] = alloca [16 x i8*], align 8
+// CHECK-LP64-NEXT: [[X:%.*]] = alloca i8*,
 // CHECK-LP64-NEXT: [[BLOCK:%.*]] = alloca [[BLOCK_T:<{.*}>]],
 
 // Initialize 'array'.
@@ -97,9 +97,9 @@
 
 // CHECK-LP64-LABEL:    define void @test1(
 // CHECK-LP64:      alloca [[ARRAY_T:%.*]]*,
-// CHECK-LP64-NEXT: [[X:%.*]] = alloca i8*,
 // CHECK-LP64-NEXT: [[STATE:%.*]] = alloca [[STATE_T:%.*]],
 // CHECK-LP64-NEXT: alloca [16 x i8*], align 8
+// CHECK-LP64-NEXT: [[X:%.*]] = alloca i8*,
 // CHECK-LP64-NEXT: [[BLOCK:%.*]] = alloca [[BLOCK_T:<{.*}>]],
 
 // CHECK-LP64:      [[T0:%.*]] = getelementptr inbounds [[STATE_T]], 
[[STATE_T]]* [[STATE]], i32 0, i32 1
@@ -160,7 +160,7 @@
 
   // CHECK-LP64-LABEL:    define void @test3(
   // CHECK-LP64:      [[ARRAY:%.*]] = alloca [[ARRAY_T]]*, align 8
-  // CHECK-LP64-NEXT: [[X:%.*]] = alloca i8*, align 8
+  // CHECK-LP64:      [[X:%.*]] = alloca i8*, align 8
   // CHECK-LP64:      [[T0:%.*]] = load i8*, i8** [[X]], align 8
   // CHECK-LP64-NEXT: [[T1:%.*]] = icmp ne i8* [[T0]], null
   // CHECK-LP64-NEXT: br i1 [[T1]],
Index: lib/CodeGen/CGObjC.cpp
===================================================================
--- lib/CodeGen/CGObjC.cpp
+++ lib/CodeGen/CGObjC.cpp
@@ -1469,11 +1469,6 @@
   if (DI)
     DI->EmitLexicalBlockStart(Builder, S.getSourceRange().getBegin());
 
-  // The local variable comes into scope immediately.
-  AutoVarEmission variable = AutoVarEmission::invalid();
-  if (const DeclStmt *SD = dyn_cast<DeclStmt>(S.getElement()))
-    variable = EmitAutoVarAlloca(*cast<VarDecl>(SD->getSingleDecl()));
-
   JumpDest LoopEnd = getJumpDestInCurrentScope("forcoll.end");
 
   // Fast enumeration state.
@@ -1625,8 +1620,10 @@
   bool elementIsVariable;
   LValue elementLValue;
   QualType elementType;
+  AutoVarEmission variable = AutoVarEmission::invalid();
   if (const DeclStmt *SD = dyn_cast<DeclStmt>(S.getElement())) {
     // Initialize the variable, in case it's a __block variable or something.
+    variable = EmitAutoVarAlloca(*cast<VarDecl>(SD->getSingleDecl()));
     EmitAutoVarInit(variable);
 
     const VarDecl* D = cast<VarDecl>(SD->getSingleDecl());


Index: test/CodeGenObjC/arc-ternary-op.m
===================================================================
--- test/CodeGenObjC/arc-ternary-op.m
+++ test/CodeGenObjC/arc-ternary-op.m
@@ -120,9 +120,9 @@
 
   // CHECK-LABEL:    define void @test2(
   // CHECK:      [[COND:%.*]] = alloca i32,
-  // CHECK:      alloca i8*
   // CHECK:      [[CLEANUP_SAVE:%.*]] = alloca i8*
   // CHECK:      [[RUN_CLEANUP:%.*]] = alloca i1
+  // CHECK:      alloca i8*
   //   Evaluate condition; cleanup disabled by default.
   // CHECK:      [[T0:%.*]] = load i32, i32* [[COND]],
   // CHECK-NEXT: icmp ne i32 [[T0]], 0
Index: test/CodeGenObjC/arc-foreach.m
===================================================================
--- test/CodeGenObjC/arc-foreach.m
+++ test/CodeGenObjC/arc-foreach.m
@@ -24,9 +24,9 @@
 
 // CHECK-LP64-LABEL:    define void @test0(
 // CHECK-LP64:      [[ARRAY:%.*]] = alloca [[ARRAY_T:%.*]]*,
-// CHECK-LP64-NEXT: [[X:%.*]] = alloca i8*,
 // CHECK-LP64-NEXT: [[STATE:%.*]] = alloca [[STATE_T:%.*]],
 // CHECK-LP64-NEXT: [[BUFFER:%.*]] = alloca [16 x i8*], align 8
+// CHECK-LP64-NEXT: [[X:%.*]] = alloca i8*,
 // CHECK-LP64-NEXT: [[BLOCK:%.*]] = alloca [[BLOCK_T:<{.*}>]],
 
 // Initialize 'array'.
@@ -97,9 +97,9 @@
 
 // CHECK-LP64-LABEL:    define void @test1(
 // CHECK-LP64:      alloca [[ARRAY_T:%.*]]*,
-// CHECK-LP64-NEXT: [[X:%.*]] = alloca i8*,
 // CHECK-LP64-NEXT: [[STATE:%.*]] = alloca [[STATE_T:%.*]],
 // CHECK-LP64-NEXT: alloca [16 x i8*], align 8
+// CHECK-LP64-NEXT: [[X:%.*]] = alloca i8*,
 // CHECK-LP64-NEXT: [[BLOCK:%.*]] = alloca [[BLOCK_T:<{.*}>]],
 
 // CHECK-LP64:      [[T0:%.*]] = getelementptr inbounds [[STATE_T]], [[STATE_T]]* [[STATE]], i32 0, i32 1
@@ -160,7 +160,7 @@
 
   // CHECK-LP64-LABEL:    define void @test3(
   // CHECK-LP64:      [[ARRAY:%.*]] = alloca [[ARRAY_T]]*, align 8
-  // CHECK-LP64-NEXT: [[X:%.*]] = alloca i8*, align 8
+  // CHECK-LP64:      [[X:%.*]] = alloca i8*, align 8
   // CHECK-LP64:      [[T0:%.*]] = load i8*, i8** [[X]], align 8
   // CHECK-LP64-NEXT: [[T1:%.*]] = icmp ne i8* [[T0]], null
   // CHECK-LP64-NEXT: br i1 [[T1]],
Index: lib/CodeGen/CGObjC.cpp
===================================================================
--- lib/CodeGen/CGObjC.cpp
+++ lib/CodeGen/CGObjC.cpp
@@ -1469,11 +1469,6 @@
   if (DI)
     DI->EmitLexicalBlockStart(Builder, S.getSourceRange().getBegin());
 
-  // The local variable comes into scope immediately.
-  AutoVarEmission variable = AutoVarEmission::invalid();
-  if (const DeclStmt *SD = dyn_cast<DeclStmt>(S.getElement()))
-    variable = EmitAutoVarAlloca(*cast<VarDecl>(SD->getSingleDecl()));
-
   JumpDest LoopEnd = getJumpDestInCurrentScope("forcoll.end");
 
   // Fast enumeration state.
@@ -1625,8 +1620,10 @@
   bool elementIsVariable;
   LValue elementLValue;
   QualType elementType;
+  AutoVarEmission variable = AutoVarEmission::invalid();
   if (const DeclStmt *SD = dyn_cast<DeclStmt>(S.getElement())) {
     // Initialize the variable, in case it's a __block variable or something.
+    variable = EmitAutoVarAlloca(*cast<VarDecl>(SD->getSingleDecl()));
     EmitAutoVarInit(variable);
 
     const VarDecl* D = cast<VarDecl>(SD->getSingleDecl());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to