From fdcd18b4c90ba59dcb8bd7b8267e61f6e0f016eb Mon Sep 17 00:00:00 2001 From: Akira Hatanaka Date: Wed, 25 Jan 2017 22:55:13 +0000 Subject: [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 llvm-svn: 293106 --- clang/lib/CodeGen/CGDecl.cpp | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) (limited to 'clang/lib/CodeGen/CGDecl.cpp') diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp index 0a88b2310be..b7c1743f62e 100644 --- a/clang/lib/CodeGen/CGDecl.cpp +++ b/clang/lib/CodeGen/CGDecl.cpp @@ -1022,11 +1022,21 @@ CodeGenFunction::EmitAutoVarAlloca(const VarDecl &D) { // 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()); -- cgit v1.2.3