Author: ahatanak Date: Wed Jan 25 16:55:13 2017 New Revision: 293106 URL: http://llvm.org/viewvc/llvm-project?rev=293106&view=rev Log: [CodeGen] Suppress emission of lifetime markers if a label has been seen in the current lexical scope.
clang currently emits the lifetime.start marker of a variable when the variable comes into scope even though a variable's lifetime starts at the entry of the block with which it is associated, according to the C standard. This normally doesn't cause any problems, but in the rare case where a goto jumps backwards past the variable declaration to an earlier point in the block (see the test case added to lifetime2.c), it can cause mis-compilation. To prevent such mis-compiles, this commit conservatively disables emitting lifetime variables when a label has been seen in the current block. This problem was discussed on cfe-dev here: http://lists.llvm.org/pipermail/cfe-dev/2016-July/050066.html rdar://problem/30153946 Differential Revision: https://reviews.llvm.org/D27680 Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp cfe/trunk/lib/CodeGen/CodeGenFunction.h cfe/trunk/test/CodeGen/lifetime2.c Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=293106&r1=293105&r2=293106&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CGDecl.cpp (original) +++ cfe/trunk/lib/CodeGen/CGDecl.cpp Wed Jan 25 16:55:13 2017 @@ -1022,11 +1022,21 @@ CodeGenFunction::EmitAutoVarAlloca(const // Emit a lifetime intrinsic if meaningful. There's no point in doing this // if we don't have a valid insertion point (?). if (HaveInsertPoint() && !IsMSCatchParam) { - // goto or switch-case statements can break lifetime into several - // regions which need more efforts to handle them correctly. PR28267 - // This is rare case, but it's better just omit intrinsics than have - // them incorrectly placed. - if (!Bypasses.IsBypassed(&D)) { + // If there's a jump into the lifetime of this variable, its lifetime + // gets broken up into several regions in IR, which requires more work + // to handle correctly. For now, just omit the intrinsics; this is a + // rare case, and it's better to just be conservatively correct. + // PR28267. + // + // We have to do this in all language modes if there's a jump past the + // declaration. We also have to do it in C if there's a jump to an + // earlier point in the current block because non-VLA lifetimes begin as + // soon as the containing block is entered, not when its variables + // actually come into scope; suppressing the lifetime annotations + // completely in this case is unnecessarily pessimistic, but again, this + // is rare. + if (!Bypasses.IsBypassed(&D) && + !(!getLangOpts().CPlusPlus && hasLabelBeenSeenInCurrentScope())) { uint64_t size = CGM.getDataLayout().getTypeAllocSize(allocaTy); emission.SizeForLifetimeMarkers = EmitLifetimeStart(size, address.getPointer()); Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=293106&r1=293105&r2=293106&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original) +++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Wed Jan 25 16:55:13 2017 @@ -212,6 +212,13 @@ public: /// value. This is invalid iff the function has no return value. Address ReturnValue; + /// Return true if a label was seen in the current scope. + bool hasLabelBeenSeenInCurrentScope() const { + if (CurLexicalScope) + return CurLexicalScope->hasLabels(); + return !LabelMap.empty(); + } + /// AllocaInsertPoint - This is an instruction in the entry block before which /// we prefer to insert allocas. llvm::AssertingVH<llvm::Instruction> AllocaInsertPt; @@ -620,6 +627,10 @@ public: rescopeLabels(); } + bool hasLabels() const { + return !Labels.empty(); + } + void rescopeLabels(); }; Modified: cfe/trunk/test/CodeGen/lifetime2.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/lifetime2.c?rev=293106&r1=293105&r2=293106&view=diff ============================================================================== --- cfe/trunk/test/CodeGen/lifetime2.c (original) +++ cfe/trunk/test/CodeGen/lifetime2.c Wed Jan 25 16:55:13 2017 @@ -1,7 +1,7 @@ -// RUN: %clang -S -emit-llvm -o - -O2 %s | FileCheck %s -check-prefixes=CHECK,O2 -// RUN: %clang -S -emit-llvm -o - -O2 -Xclang -disable-lifetime-markers %s \ +// RUN: %clang_cc1 -S -emit-llvm -o - -O2 -disable-llvm-passes %s | FileCheck %s -check-prefixes=CHECK,O2 +// RUN: %clang_cc1 -S -emit-llvm -o - -O2 -disable-lifetime-markers %s \ // RUN: | FileCheck %s -check-prefixes=CHECK,O0 -// RUN: %clang -S -emit-llvm -o - -O0 %s | FileCheck %s -check-prefixes=CHECK,O0 +// RUN: %clang_cc1 -S -emit-llvm -o - -O0 %s | FileCheck %s -check-prefixes=CHECK,O0 extern int bar(char *A, int n); @@ -25,13 +25,11 @@ void no_goto_bypass() { char x; l1: bar(&x, 1); - // O2: @llvm.lifetime.start(i64 5 - // O2: @llvm.lifetime.end(i64 5 char y[5]; bar(y, 5); goto l1; // Infinite loop - // O2-NOT: @llvm.lifetime.end(i64 1 + // O2-NOT: @llvm.lifetime.end( } // CHECK-LABEL: @goto_bypass @@ -91,3 +89,24 @@ void indirect_jump(int n) { L: bar(&x, 1); } + +// O2-LABEL: define i32 @jump_backward_over_declaration( +// O2-NOT: call void @llvm.lifetime.{{.*}}(i64 4, + +extern void foo2(int p); + +int jump_backward_over_declaration(int a) { + int *p = 0; +label1: + if (p) { + foo2(*p); + return 0; + } + + int i = 999; + if (a != 2) { + p = &i; + goto label1; + } + return -1; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
