vitalybuka updated this revision to Diff 72907.
vitalybuka added a comment.
Herald added subscribers: mgorny, beanz.
rebase
https://reviews.llvm.org/D24695
Files:
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CMakeLists.txt
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/VarBypassDetector.cpp
lib/CodeGen/VarBypassDetector.h
test/CodeGen/lifetime2.c
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;
+ }
+}
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<std::pair<unsigned, const VarDecl *>, 48> Scopes;
+ // Lookup map to file scope for jumps and its destinations.
+ llvm::DenseMap<const Stmt *, unsigned> LabelAndGotoScopes;
+ // Set of variables which were bypassed by some jump.
+ llvm::DenseSet<const VarDecl *> 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<VarDecl>(D);
+ if (VD && VD->hasLocalStorage()) {
+ Scopes.push_back({ParentScope, VD});
+ ParentScope = Scopes.size() - 1;
+ }
+
+ if (const VarDecl *VD = dyn_cast<VarDecl>(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<Expr>(S) && !isa<StmtExpr>(S)) ? origParentScope
+ : independentParentScope);
+
+ unsigned StmtsToSkip = 0u;
+
+ switch (S->getStmtClass()) {
+ case Stmt::SwitchStmtClass:
+ if (const Stmt *Init = cast<SwitchStmt>(S)->getInit()) {
+ BuildScopeInformation(Init, ParentScope);
+ ++StmtsToSkip;
+ }
+ if (const VarDecl *Var = cast<SwitchStmt>(S)->getConditionVariable()) {
+ BuildScopeInformation(Var, ParentScope);
+ ++StmtsToSkip;
+ }
+ // Fall through
+
+ case Stmt::GotoStmtClass:
+ LabelAndGotoScopes[S] = ParentScope;
+ break;
+
+ case Stmt::DeclStmtClass: {
+ const DeclStmt *DS = cast<DeclStmt>(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<CaseStmt>(SubStmt))
+ Next = CS->getSubStmt();
+ else if (const DefaultStmt *DS = dyn_cast<DefaultStmt>(SubStmt))
+ Next = DS->getSubStmt();
+ else if (const LabelStmt *LS = dyn_cast<LabelStmt>(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<GotoStmt>(St)) {
+ if (const LabelStmt *LS = GS->getLabel()->getStmt())
+ Detect(GS, LS);
+ } else if (const SwitchStmt *SS = dyn_cast<SwitchStmt>(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: 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,
@@ -1175,6 +1180,9 @@
llvm::BasicBlock *TerminateHandler;
llvm::BasicBlock *TrapBB;
+ /// True if we need emit the life-time markers.
+ const bool ShouldEmitLifetimeMarkers;
+
/// Add a kernel metadata node to the named metadata node 'opencl.kernels'.
/// In the kernel metadata node, reference the kernel function and metadata
/// nodes for its optional attribute qualifiers (OpenCL 1.1 6.7.2):
Index: lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -38,29 +38,46 @@
using namespace clang;
using namespace CodeGen;
+/// shouldEmitLifetimeMarkers - Decide whether we need emit the life-time
+/// markers.
+static bool shouldEmitLifetimeMarkers(const CodeGenOptions &CGOpts,
+ const LangOptions &LangOpts) {
+ // Asan uses markers for use-after-scope checks.
+ if (CGOpts.SanitizeAddressUseAfterScope)
+ return true;
+
+ // Disable lifetime markers in msan builds.
+ // FIXME: Remove this when msan works with lifetime markers.
+ if (LangOpts.Sanitize.has(SanitizerKind::Memory))
+ return false;
+
+ // For now, only in optimized builds.
+ return CGOpts.OptimizationLevel != 0;
+}
+
CodeGenFunction::CodeGenFunction(CodeGenModule &cgm, bool suppressNewContext)
: CodeGenTypeCache(cgm), CGM(cgm), Target(cgm.getTarget()),
Builder(cgm, cgm.getModule().getContext(), llvm::ConstantFolder(),
CGBuilderInserterTy(this)),
CurFn(nullptr), ReturnValue(Address::invalid()),
- CapturedStmtInfo(nullptr),
- SanOpts(CGM.getLangOpts().Sanitize), IsSanitizerScope(false),
- CurFuncIsThunk(false), AutoreleaseResult(false), SawAsmBlock(false),
- IsOutlinedSEHHelper(false),
- BlockInfo(nullptr), BlockPointer(nullptr),
- LambdaThisCaptureField(nullptr), NormalCleanupDest(nullptr),
- NextCleanupDestIndex(1), FirstBlockInfo(nullptr), EHResumeBlock(nullptr),
- ExceptionSlot(nullptr), EHSelectorSlot(nullptr),
- DebugInfo(CGM.getModuleDebugInfo()),
+ CapturedStmtInfo(nullptr), SanOpts(CGM.getLangOpts().Sanitize),
+ IsSanitizerScope(false), CurFuncIsThunk(false), AutoreleaseResult(false),
+ SawAsmBlock(false), IsOutlinedSEHHelper(false), BlockInfo(nullptr),
+ BlockPointer(nullptr), LambdaThisCaptureField(nullptr),
+ NormalCleanupDest(nullptr), NextCleanupDestIndex(1),
+ FirstBlockInfo(nullptr), EHResumeBlock(nullptr), ExceptionSlot(nullptr),
+ EHSelectorSlot(nullptr), DebugInfo(CGM.getModuleDebugInfo()),
DisableDebugInfo(false), DidCallStackSave(false), IndirectBranch(nullptr),
PGO(cgm), SwitchInsn(nullptr), SwitchWeights(nullptr),
CaseRangeBlock(nullptr), UnreachableBlock(nullptr), NumReturnExprs(0),
NumSimpleReturnExprs(0), CXXABIThisDecl(nullptr),
CXXABIThisValue(nullptr), CXXThisValue(nullptr),
CXXStructorImplicitParamDecl(nullptr),
CXXStructorImplicitParamValue(nullptr), OutermostConditional(nullptr),
CurLexicalScope(nullptr), TerminateLandingPad(nullptr),
- TerminateHandler(nullptr), TrapBB(nullptr) {
+ TerminateHandler(nullptr), TrapBB(nullptr),
+ ShouldEmitLifetimeMarkers(
+ shouldEmitLifetimeMarkers(CGM.getCodeGenOpts(), CGM.getLangOpts())) {
if (!suppressNewContext)
CGM.getCXXABI().getMangleContext().startNewFunction();
@@ -1049,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());
@@ -1078,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: 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/CGDecl.cpp
===================================================================
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -916,29 +916,12 @@
EmitAutoVarCleanups(emission);
}
-/// shouldEmitLifetimeMarkers - Decide whether we need emit the life-time
-/// markers.
-static bool shouldEmitLifetimeMarkers(const CodeGenOptions &CGOpts,
- const LangOptions &LangOpts) {
- // Asan uses markers for use-after-scope checks.
- if (CGOpts.SanitizeAddressUseAfterScope)
- return true;
-
- // Disable lifetime markers in msan builds.
- // FIXME: Remove this when msan works with lifetime markers.
- if (LangOpts.Sanitize.has(SanitizerKind::Memory))
- return false;
-
- // For now, only in optimized builds.
- return CGOpts.OptimizationLevel != 0;
-}
-
/// Emit a lifetime.begin marker if some criteria are satisfied.
/// \return a pointer to the temporary size Value if a marker was emitted, null
/// otherwise
llvm::Value *CodeGenFunction::EmitLifetimeStart(uint64_t Size,
llvm::Value *Addr) {
- if (!shouldEmitLifetimeMarkers(CGM.getCodeGenOpts(), getLangOpts()))
+ if (!ShouldEmitLifetimeMarkers)
return nullptr;
llvm::Value *SizeV = llvm::ConstantInt::get(Int64Ty, Size);
@@ -1058,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());
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
