StephenFan created this revision.
StephenFan added reviewers: MaskRay, vitalybuka, rsmith.
Herald added a project: All.
StephenFan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Fix https://github.com/llvm/llvm-project/issues/56356
For following test case:
extern int bar(char *A, int n);
void goto_bypass(void) {
{
char x;
l1:
bar(&x, 1);
}
goto l1;
}
And using `clang -cc1 -O2 -S -emit-llvm` to compile it.
In the past, due to the existence of bypassed label, the lifetime
intrinsic would not be generated. This was also the cause of pr56356.
In this patch, if the variable is bypassed, we do variable
allocation, emit lifetime-start intrinsic and record the lifetime-start
intrinsic in LexicalScope. Then When emitting the bypass label, we emit
the lifetime instrinsic again to make sure the lifetime of the bypassed
variable is start again. Address sanitizer will capture these lifetime
intrinsics and instrument poison and unpoison code. Finally pr56356 can
be resolved.
Here is the new llvm-ir of the test case above.
define dso_local void @goto_bypass() local_unnamed_addr #0 {
entry:
%x = alloca i8, align 1
call void @llvm.lifetime.start.p0(i64 1, ptr nonnull %x) #3
br label %l1
l1: ; preds = %l1, %entry
call void @llvm.lifetime.start.p0(i64 1, ptr nonnull %x) #3
%call = call i32 @bar(ptr noundef nonnull %x, i32 noundef 1) #3
call void @llvm.lifetime.end.p0(i64 1, ptr nonnull %x) #3
br label %l1
}
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D129448
Files:
clang/lib/CodeGen/CGDecl.cpp
clang/lib/CodeGen/CGStmt.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/test/CodeGen/lifetime2.c
Index: clang/test/CodeGen/lifetime2.c
===================================================================
--- clang/test/CodeGen/lifetime2.c
+++ clang/test/CodeGen/lifetime2.c
@@ -35,11 +35,12 @@
// CHECK-LABEL: @goto_bypass
void goto_bypass(void) {
{
- // O2-NOT: @llvm.lifetime.start.p0i8(i64 1
- // O2-NOT: @llvm.lifetime.end.p0i8(i64 1
+ // O2: @llvm.lifetime.start.p0i8(i64 1
char x;
l1:
+ // O2: @llvm.lifetime.start.p0i8(i64 1
bar(&x, 1);
+ // O2: @llvm.lifetime.end.p0i8(i64 1
}
goto l1;
}
@@ -69,8 +70,8 @@
switch (n) {
case 1:
n = n;
- // O2-NOT: @llvm.lifetime.start.p0i8(i64 1
- // O2-NOT: @llvm.lifetime.end.p0i8(i64 1
+ // O2: @llvm.lifetime.start.p0i8(i64 1
+ // O2: @llvm.lifetime.end.p0i8(i64 1
char x;
bar(&x, 1);
break;
@@ -83,7 +84,7 @@
// CHECK-LABEL: @indirect_jump
void indirect_jump(int n) {
char x;
- // O2-NOT: @llvm.lifetime
+ // O2: @llvm.lifetime
void *T[] = {&&L};
goto *T[n];
L:
Index: clang/lib/CodeGen/CodeGenFunction.h
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -38,6 +38,7 @@
#include "llvm/ADT/MapVector.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
+#include "llvm/IR/IntrinsicInst.h"
#include "llvm/IR/ValueHandle.h"
#include "llvm/Support/Debug.h"
#include "llvm/Transforms/Utils/SanitizerStats.h"
@@ -931,6 +932,7 @@
class LexicalScope : public RunCleanupsScope {
SourceRange Range;
SmallVector<const LabelDecl*, 4> Labels;
+ SmallVector<const llvm::CallInst *, 4> LifetimeStartMarkers;
LexicalScope *ParentScope;
LexicalScope(const LexicalScope &) = delete;
@@ -950,6 +952,19 @@
Labels.push_back(label);
}
+ void addLifetimeStartMarker(const llvm::CallInst *LifetimeStartMarker) {
+ assert(isa<llvm::IntrinsicInst>(LifetimeStartMarker) &&
+ cast<llvm::IntrinsicInst>(LifetimeStartMarker)->getIntrinsicID() ==
+ llvm::Intrinsic::lifetime_start &&
+ "LifetimeStartMarker Is not a lifetime start intrinsic");
+ LifetimeStartMarkers.push_back(LifetimeStartMarker);
+ }
+
+ const SmallVector<const llvm::CallInst *, 4> &
+ getLifetimeStartMarkers() const {
+ return LifetimeStartMarkers;
+ }
+
/// Exit this cleanup scope, emitting any accumulated
/// cleanups.
~LexicalScope() {
@@ -2922,7 +2937,8 @@
void EmitSehTryScopeBegin();
void EmitSehTryScopeEnd();
- llvm::Value *EmitLifetimeStart(llvm::TypeSize Size, llvm::Value *Addr);
+ llvm::Value *EmitLifetimeStart(llvm::TypeSize Size, llvm::Value *Addr,
+ bool isBypassed = false);
void EmitLifetimeEnd(llvm::Value *Size, llvm::Value *Addr);
llvm::Value *EmitCXXNewExpr(const CXXNewExpr *E);
Index: clang/lib/CodeGen/CGStmt.cpp
===================================================================
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -647,6 +647,12 @@
EmitBlock(Dest.getBlock());
+ if (CurLexicalScope)
+ llvm::for_each(CurLexicalScope->getLifetimeStartMarkers(),
+ [this](const llvm::CallInst *LifetimeStartMarker) {
+ Builder.Insert(LifetimeStartMarker->clone());
+ });
+
// Emit debug info for labels.
if (CGDebugInfo *DI = getDebugInfo()) {
if (CGM.getCodeGenOpts().hasReducedDebugInfo()) {
Index: clang/lib/CodeGen/CGDecl.cpp
===================================================================
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -1330,7 +1330,8 @@
/// \return a pointer to the temporary size Value if a marker was emitted, null
/// otherwise
llvm::Value *CodeGenFunction::EmitLifetimeStart(llvm::TypeSize Size,
- llvm::Value *Addr) {
+ llvm::Value *Addr,
+ bool isBypassed) {
if (!ShouldEmitLifetimeMarkers)
return nullptr;
@@ -1342,6 +1343,8 @@
Addr = Builder.CreateBitCast(Addr, AllocaInt8PtrTy);
llvm::CallInst *C =
Builder.CreateCall(CGM.getLLVMLifetimeStartFn(), {SizeV, Addr});
+ if (isBypassed && CurLexicalScope)
+ CurLexicalScope->addLifetimeStartMarker(C);
C->setDoesNotThrow();
return SizeV;
}
@@ -1551,24 +1554,10 @@
// 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) {
- // 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())) {
+ if (!(!getLangOpts().CPlusPlus && hasLabelBeenSeenInCurrentScope())) {
llvm::TypeSize Size = CGM.getDataLayout().getTypeAllocSize(allocaTy);
- emission.SizeForLifetimeMarkers =
- EmitLifetimeStart(Size, AllocaAddr.getPointer());
+ emission.SizeForLifetimeMarkers = EmitLifetimeStart(
+ Size, AllocaAddr.getPointer(), Bypasses.IsBypassed(&D));
}
} else {
assert(!emission.useLifetimeMarkers());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
