Index: cfe/trunk/lib/CodeGen/CGDecl.cpp =================================================================== --- cfe/trunk/lib/CodeGen/CGDecl.cpp +++ cfe/trunk/lib/CodeGen/CGDecl.cpp @@ -1010,12 +1010,18 @@ bool IsMSCatchParam = D.isExceptionVariable() && getTarget().getCXXABI().isMicrosoft(); - // Emit a lifetime intrinsic if meaningful. There's no point - // in doing this if we don't have a valid insertion point (?). + // 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) { - uint64_t size = CGM.getDataLayout().getTypeAllocSize(allocaTy); - emission.SizeForLifetimeMarkers = - EmitLifetimeStart(size, address.getPointer()); + // 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)) { + uint64_t size = CGM.getDataLayout().getTypeAllocSize(allocaTy); + emission.SizeForLifetimeMarkers = + EmitLifetimeStart(size, address.getPointer()); + } } else { assert(!emission.useLifetimeMarkers()); } Index: cfe/trunk/lib/CodeGen/CMakeLists.txt =================================================================== --- cfe/trunk/lib/CodeGen/CMakeLists.txt +++ cfe/trunk/lib/CodeGen/CMakeLists.txt @@ -82,6 +82,7 @@ SanitizerMetadata.cpp SwiftCallingConv.cpp TargetInfo.cpp + VarBypassDetector.cpp DEPENDS ${codegen_deps} Index: cfe/trunk/lib/CodeGen/CodeGenFunction.h =================================================================== --- cfe/trunk/lib/CodeGen/CodeGenFunction.h +++ cfe/trunk/lib/CodeGen/CodeGenFunction.h @@ -21,6 +21,7 @@ #include "CodeGenModule.h" #include "CodeGenPGO.h" #include "EHScopeStack.h" +#include "VarBypassDetector.h" #include "clang/AST/CharUnits.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/ExprObjC.h" @@ -141,6 +142,10 @@ LoopInfoStack LoopStack; CGBuilderTy Builder; + // Stores variables for which we can't generate correct lifetime markers + // because of jumps. + VarBypassDetector Bypasses; + /// \brief CGBuilder insert helper. This function is called after an /// instruction is created using Builder. void InsertHelper(llvm::Instruction *I, const llvm::Twine &Name, Index: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp =================================================================== --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp @@ -1066,6 +1066,13 @@ if (SpecDecl->hasBody(SpecDecl)) Loc = SpecDecl->getLocation(); + Stmt *Body = FD->getBody(); + + // Initialize helper which will detect jumps which can cause invalid lifetime + // markers. + if (Body && ShouldEmitLifetimeMarkers) + Bypasses.Init(Body); + // Emit the standard function prologue. StartFunction(GD, ResTy, Fn, FnInfo, Args, Loc, BodyRange.getBegin()); @@ -1095,7 +1102,7 @@ // Implicit copy-assignment gets the same special treatment as implicit // copy-constructors. emitImplicitAssignmentOperatorBody(Args); - } else if (Stmt *Body = FD->getBody()) { + } else if (Body) { EmitFunctionBody(Args, Body); } else llvm_unreachable("no definition for emitted function"); Index: cfe/trunk/lib/CodeGen/VarBypassDetector.h =================================================================== --- cfe/trunk/lib/CodeGen/VarBypassDetector.h +++ cfe/trunk/lib/CodeGen/VarBypassDetector.h @@ -0,0 +1,70 @@ +//===--- VarBypassDetector.cpp - Bypass jumps detector ------------*- C++ -*-=// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This file contains VarBypassDetector class, which is used to detect +// local variable declarations which can be bypassed by jumps. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_LIB_CODEGEN_VARBYPASSDETECTOR_H +#define LLVM_CLANG_LIB_CODEGEN_VARBYPASSDETECTOR_H + +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/SmallVector.h" + +namespace clang { + +class Decl; +class Stmt; +class VarDecl; + +namespace CodeGen { + +/// The class detects jumps which bypass local variables declaration: +/// goto L; +/// int a; +/// L: +/// +/// This is simplified version of JumpScopeChecker. Primary differences: +/// * Detects only jumps into the scope local variables. +/// * Does not detect jumps out of the scope of local variables. +/// * Not limited to variables with initializers, JumpScopeChecker is limited. +class VarBypassDetector { + // Scope information. Contains a parent scope and related variable + // declaration. + llvm::SmallVector, 48> Scopes; + // List of jumps with scopes. + llvm::SmallVector, 16> FromScopes; + // Lookup map to find scope for destinations. + llvm::DenseMap ToScopes; + // Set of variables which were bypassed by some jump. + llvm::DenseSet Bypasses; + // If true assume that all variables are being bypassed. + bool AlwaysBypassed = false; + +public: + void Init(const Stmt *Body); + + /// Returns true if the variable declaration was by bypassed by any goto or + /// switch statement. + bool IsBypassed(const VarDecl *D) const { + return AlwaysBypassed || Bypasses.find(D) != Bypasses.end(); + } + +private: + bool BuildScopeInformation(const Decl *D, unsigned &ParentScope); + bool BuildScopeInformation(const Stmt *S, unsigned &origParentScope); + void Detect(); + void Detect(unsigned From, unsigned To); +}; +} +} + +#endif Index: cfe/trunk/lib/CodeGen/VarBypassDetector.cpp =================================================================== --- cfe/trunk/lib/CodeGen/VarBypassDetector.cpp +++ cfe/trunk/lib/CodeGen/VarBypassDetector.cpp @@ -0,0 +1,168 @@ +//===--- VarBypassDetector.h - Bypass jumps detector --------------*- C++ -*-=// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "VarBypassDetector.h" + +#include "clang/AST/Decl.h" +#include "clang/AST/Expr.h" +#include "clang/AST/Stmt.h" + +using namespace clang; +using namespace CodeGen; + +/// Clear the object and pre-process for the given statement, usually function +/// body statement. +void VarBypassDetector::Init(const Stmt *Body) { + FromScopes.clear(); + ToScopes.clear(); + Bypasses.clear(); + Scopes = {{~0U, nullptr}}; + unsigned ParentScope = 0; + AlwaysBypassed = !BuildScopeInformation(Body, ParentScope); + if (!AlwaysBypassed) + Detect(); +} + +/// Build scope information for a declaration that is part of a DeclStmt. +/// Returns false if we failed to build scope information and can't tell for +/// which vars are being bypassed. +bool VarBypassDetector::BuildScopeInformation(const Decl *D, + unsigned &ParentScope) { + const VarDecl *VD = dyn_cast(D); + if (VD && VD->hasLocalStorage()) { + Scopes.push_back({ParentScope, VD}); + ParentScope = Scopes.size() - 1; + } + + if (const VarDecl *VD = dyn_cast(D)) + if (const Expr *Init = VD->getInit()) + return BuildScopeInformation(Init, ParentScope); + + return true; +} + +/// Walk through the statements, adding any labels or gotos to +/// LabelAndGotoScopes and recursively walking the AST as needed. +/// Returns false if we failed to build scope information and can't tell for +/// which vars are being bypassed. +bool VarBypassDetector::BuildScopeInformation(const Stmt *S, + unsigned &origParentScope) { + // If this is a statement, rather than an expression, scopes within it don't + // propagate out into the enclosing scope. Otherwise we have to worry about + // block literals, which have the lifetime of their enclosing statement. + unsigned independentParentScope = origParentScope; + unsigned &ParentScope = + ((isa(S) && !isa(S)) ? origParentScope + : independentParentScope); + + unsigned StmtsToSkip = 0u; + + switch (S->getStmtClass()) { + case Stmt::IndirectGotoStmtClass: + return false; + + case Stmt::SwitchStmtClass: + if (const Stmt *Init = cast(S)->getInit()) { + if (!BuildScopeInformation(Init, ParentScope)) + return false; + ++StmtsToSkip; + } + if (const VarDecl *Var = cast(S)->getConditionVariable()) { + if (!BuildScopeInformation(Var, ParentScope)) + return false; + ++StmtsToSkip; + } + // Fall through + + case Stmt::GotoStmtClass: + FromScopes.push_back({S, ParentScope}); + break; + + case Stmt::DeclStmtClass: { + const DeclStmt *DS = cast(S); + for (auto *I : DS->decls()) + if (!BuildScopeInformation(I, origParentScope)) + return false; + return true; + } + + case Stmt::CaseStmtClass: + case Stmt::DefaultStmtClass: + case Stmt::LabelStmtClass: + llvm_unreachable("the loop bellow handles labels and cases"); + break; + + default: + break; + } + + for (const Stmt *SubStmt : S->children()) { + if (!SubStmt) + continue; + if (StmtsToSkip) { + --StmtsToSkip; + continue; + } + + // Cases, labels, and defaults aren't "scope parents". It's also + // important to handle these iteratively instead of recursively in + // order to avoid blowing out the stack. + while (true) { + const Stmt *Next; + if (const SwitchCase *SC = dyn_cast(SubStmt)) + Next = SC->getSubStmt(); + else if (const LabelStmt *LS = dyn_cast(SubStmt)) + Next = LS->getSubStmt(); + else + break; + + ToScopes[SubStmt] = ParentScope; + SubStmt = Next; + } + + // Recursively walk the AST. + if (!BuildScopeInformation(SubStmt, ParentScope)) + return false; + } + return true; +} + +/// Checks each jump and stores each variable declaration they bypass. +void VarBypassDetector::Detect() { + for (const auto &S : FromScopes) { + const Stmt *St = S.first; + unsigned from = S.second; + if (const GotoStmt *GS = dyn_cast(St)) { + if (const LabelStmt *LS = GS->getLabel()->getStmt()) + Detect(from, ToScopes[LS]); + } else if (const SwitchStmt *SS = dyn_cast(St)) { + for (const SwitchCase *SC = SS->getSwitchCaseList(); SC; + SC = SC->getNextSwitchCase()) { + Detect(from, ToScopes[SC]); + } + } else { + llvm_unreachable("goto or switch was expected"); + } + } +} + +/// Checks the jump and stores each variable declaration it bypasses. +void VarBypassDetector::Detect(unsigned From, unsigned To) { + while (From != To) { + if (From < To) { + assert(Scopes[To].first < To); + const auto &ScopeTo = Scopes[To]; + To = ScopeTo.first; + Bypasses.insert(ScopeTo.second); + } else { + assert(Scopes[From].first < From); + From = Scopes[From].first; + } + } +} Index: cfe/trunk/test/CodeGen/lifetime2.c =================================================================== --- cfe/trunk/test/CodeGen/lifetime2.c +++ cfe/trunk/test/CodeGen/lifetime2.c @@ -1,8 +1,9 @@ -// RUN: %clang -S -emit-llvm -o - -O2 %s | FileCheck %s -check-prefix=O2 -// RUN: %clang -S -emit-llvm -o - -O0 %s | FileCheck %s -check-prefix=O0 +// RUN: %clang -S -emit-llvm -o - -O2 %s | FileCheck %s -check-prefixes=CHECK,O2 +// RUN: %clang -S -emit-llvm -o - -O0 %s | FileCheck %s -check-prefixes=CHECK,O0 extern int bar(char *A, int n); +// CHECK-LABEL: @foo // O0-NOT: @llvm.lifetime.start int foo (int n) { if (n) { @@ -15,3 +16,76 @@ return bar(A, 2); } } + +// CHECK-LABEL: @no_goto_bypass +void no_goto_bypass() { + // O2: @llvm.lifetime.start(i64 1 + 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 +} + +// CHECK-LABEL: @goto_bypass +void goto_bypass() { + { + // O2-NOT: @llvm.lifetime.start(i64 1 + // O2-NOT: @llvm.lifetime.end(i64 1 + char x; + l1: + bar(&x, 1); + } + goto l1; +} + +// CHECK-LABEL: @no_switch_bypass +void no_switch_bypass(int n) { + switch (n) { + case 1: { + // O2: @llvm.lifetime.start(i64 1 + // O2: @llvm.lifetime.end(i64 1 + char x; + bar(&x, 1); + break; + } + case 2: + n = n; + // O2: @llvm.lifetime.start(i64 5 + // O2: @llvm.lifetime.end(i64 5 + char y[5]; + bar(y, 5); + break; + } +} + +// CHECK-LABEL: @switch_bypass +void switch_bypass(int n) { + switch (n) { + case 1: + n = n; + // O2-NOT: @llvm.lifetime.start(i64 1 + // O2-NOT: @llvm.lifetime.end(i64 1 + char x; + bar(&x, 1); + break; + case 2: + bar(&x, 1); + break; + } +} + +// CHECK-LABEL: @indirect_jump +void indirect_jump(int n) { + char x; + // O2-NOT: @llvm.lifetime + void *T[] = {&&L}; + goto *T[n]; +L: + bar(&x, 1); +}