Index: lib/CodeGen/CGDecl.cpp =================================================================== --- lib/CodeGen/CGDecl.cpp +++ lib/CodeGen/CGDecl.cpp @@ -1041,12 +1041,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: lib/CodeGen/CMakeLists.txt =================================================================== --- lib/CodeGen/CMakeLists.txt +++ lib/CodeGen/CMakeLists.txt @@ -80,6 +80,7 @@ SanitizerMetadata.cpp SwiftCallingConv.cpp TargetInfo.cpp + VarBypassDetector.cpp DEPENDS ${codegen_deps} Index: lib/CodeGen/CodeGenFunction.h =================================================================== --- lib/CodeGen/CodeGenFunction.h +++ 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" @@ -140,6 +141,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: lib/CodeGen/CodeGenFunction.cpp =================================================================== --- lib/CodeGen/CodeGenFunction.cpp +++ lib/CodeGen/CodeGenFunction.cpp @@ -1065,6 +1065,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()); @@ -1094,7 +1101,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: lib/CodeGen/VarBypassDetector.h =================================================================== --- /dev/null +++ lib/CodeGen/VarBypassDetector.h @@ -0,0 +1,67 @@ +//===--- 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 { + +/// VarBypassDetector - This 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. +/// * FIXME: Does not support indirect jumps. +class VarBypassDetector { + // Scope information. Contains a parent scope and related variable + // declaration. + llvm::SmallVector, 48> Scopes; + // Lookup map to file scope for jumps and its destinations. + llvm::DenseMap LabelAndGotoScopes; + // Set of variables which were bypassed by some jump. + llvm::DenseSet Bypasses; + +public: + void Init(const Stmt *Body); + + /// IsBypassed - Returns true if the variable declaration was by bypassed by + /// any goto or switch statement. + bool IsBypassed(const VarDecl *D) const { + return Bypasses.find(D) != Bypasses.end(); + } + +private: + void BuildScopeInformation(const Decl *D, unsigned &ParentScope); + void BuildScopeInformation(const Stmt *S, unsigned &origParentScope); + void Detect(); + void Detect(const Stmt *FromStmt, const Stmt *ToStmt); +}; +} +} + +#endif Index: lib/CodeGen/VarBypassDetector.cpp =================================================================== --- /dev/null +++ lib/CodeGen/VarBypassDetector.cpp @@ -0,0 +1,154 @@ +//===--- 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; + +/// Init - Clear the object and pre-process for the given statement, usually +/// function body statement. +void VarBypassDetector::Init(const Stmt *Body) { + LabelAndGotoScopes.clear(); + Bypasses.clear(); + Scopes = {{~0U, nullptr}}; + unsigned ParentScope = 0; + BuildScopeInformation(Body, ParentScope); + Detect(); +} + +/// BuildScopeInformation - Build scope information for a declaration that is +/// part of a DeclStmt. +void 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()) + BuildScopeInformation(Init, ParentScope); +} + +/// BuildScopeInformation - Walk through the statements, adding any labels or +/// gotos to LabelAndGotoScopes and recursively walking the AST as needed. +void 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::SwitchStmtClass: + if (const Stmt *Init = cast(S)->getInit()) { + BuildScopeInformation(Init, ParentScope); + ++StmtsToSkip; + } + if (const VarDecl *Var = cast(S)->getConditionVariable()) { + BuildScopeInformation(Var, ParentScope); + ++StmtsToSkip; + } + // Fall through + + case Stmt::GotoStmtClass: + LabelAndGotoScopes[S] = ParentScope; + break; + + case Stmt::DeclStmtClass: { + const DeclStmt *DS = cast(S); + for (auto *I : DS->decls()) + BuildScopeInformation(I, origParentScope); + return; + } + + case Stmt::CaseStmtClass: + case Stmt::DefaultStmtClass: + case Stmt::LabelStmtClass: + LabelAndGotoScopes[S] = ParentScope; + 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 CaseStmt *CS = dyn_cast(SubStmt)) + Next = CS->getSubStmt(); + else if (const DefaultStmt *DS = dyn_cast(SubStmt)) + Next = DS->getSubStmt(); + else if (const LabelStmt *LS = dyn_cast(SubStmt)) + Next = LS->getSubStmt(); + else + break; + + LabelAndGotoScopes[SubStmt] = ParentScope; + SubStmt = Next; + } + + // Recursively walk the AST. + BuildScopeInformation(SubStmt, ParentScope); + } +} + +/// Detect - Checks each jump and stores each variable declaration they bypass. +void VarBypassDetector::Detect() { + for (const auto &S : LabelAndGotoScopes) { + const Stmt *St = S.first; + if (const GotoStmt *GS = dyn_cast(St)) { + if (const LabelStmt *LS = GS->getLabel()->getStmt()) + Detect(GS, LS); + } else if (const SwitchStmt *SS = dyn_cast(St)) { + for (const SwitchCase *SC = SS->getSwitchCaseList(); SC; + SC = SC->getNextSwitchCase()) { + Detect(SS, SC); + } + } + } +} + +/// Detect - Checks the jump and stores each variable declaration it bypasses. +void VarBypassDetector::Detect(const Stmt *FromStmt, const Stmt *ToStmt) { + unsigned From = LabelAndGotoScopes[FromStmt]; + unsigned To = LabelAndGotoScopes[ToStmt]; + 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: test/CodeGen/lifetime2.c =================================================================== --- test/CodeGen/lifetime2.c +++ 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,66 @@ 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; + } +}