<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div>On Mar 26, 2013, at 7:55 AM, Alexey Samsonov <<a href="mailto:samsonov@google.com">samsonov@google.com</a>> wrote:</div><blockquote type="cite"><div dir="ltr"><div><div class="gmail_extra"><div class="gmail_quote">On Mon, Mar 25, 2013 at 11:23 PM, Alexey Samsonov <span dir="ltr"><<a href="mailto:samsonov@google.com" target="_blank">samsonov@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto; "><div dir="ltr">Hi Nadav!<div><br></div><div>Cool you guys are working on it! I have a raw local patch that does the same (emits llvm.lifetime start/end intrinsics), and</div>
<div>I hope to provide some feedback tomorrow, when I look at this commit in more details. Some of my concerns:</div>
<div>1) Rafael mentioned a thread where I asked about llvm.lifetime intrinsics semantics and behavior - you may be</div><div>interested in it. I think we may resurrect it and clarify their semantics.</div><div>
2) I'm afraid that immediate enabling of stack coloring may break things - I've seen that similar gcc option</div><div>had to be disabled on some codebases because of the bugs in the code where a local variable was used after</div>
<div>it went out of scope.</div><div>3) I actually want to emit lifetime.start/end intrinsics in one of AddressSanitizer modes (to implement -fsanitize=use-after-scope</div><div>option) to reliably detect bugs in (2).</div>
<div><br></div></div><div class="gmail_extra"><div class="h5"><br><br><div class="gmail_quote">On Sat, Mar 23, 2013 at 10:43 AM, Nadav Rotem <span dir="ltr"><<a href="mailto:nrotem@apple.com" target="_blank">nrotem@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: nadav<br>
Date: Sat Mar 23 01:43:35 2013<br>
New Revision: 177819<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=177819&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=177819&view=rev</a><br>
Log:<br>
Make clang to mark static stack allocations with lifetime markers to enable a more aggressive stack coloring.<br>
Patch by John McCall with help by Shuxin Yang.<br>
<a href="rdar://13115369">rdar://13115369</a><br>
<br>
<br>
Added:<br>
cfe/trunk/test/CodeGen/lifetime2.c<br>
Modified:<br>
cfe/trunk/lib/CodeGen/CGDecl.cpp<br>
cfe/trunk/lib/CodeGen/CGStmt.cpp<br>
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp<br>
cfe/trunk/lib/CodeGen/CodeGenFunction.h<br>
cfe/trunk/lib/CodeGen/CodeGenModule.cpp<br>
cfe/trunk/lib/CodeGen/CodeGenModule.h<br>
cfe/trunk/test/CodeGenObjC/arc-blocks.m<br>
cfe/trunk/test/CodeGenObjC/arc.m<br>
<br>
Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=177819&r1=177818&r2=177819&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=177819&r1=177818&r2=177819&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)<br>
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Sat Mar 23 01:43:35 2013<br>
@@ -452,6 +452,22 @@ namespace {<br>
CGF.EmitCall(FnInfo, CleanupFn, ReturnValueSlot(), Args);<br>
}<br>
};<br>
+<br>
+ /// A cleanup to call @llvm.lifetime.end.<br>
+ class CallLifetimeEnd : public EHScopeStack::Cleanup {<br>
+ llvm::Value *Addr;<br>
+ llvm::Value *Size;<br>
+ public:<br>
+ CallLifetimeEnd(llvm::Value *addr, llvm::Value *size)<br>
+ : Addr(addr), Size(size) {}<br>
+<br>
+ void Emit(CodeGenFunction &CGF, Flags flags) {<br>
+ llvm::Value *castAddr = CGF.Builder.CreateBitCast(Addr, CGF.Int8PtrTy);<br>
+ CGF.Builder.CreateCall2(CGF.CGM.getLLVMLifetimeEndFn(),<br>
+ Size, castAddr)<br></blockquote></div></div></div></blockquote><div><br></div><div style="">I don't think you need getLLVMLifetimeEndFn() methods, lazy declarations of llvm.lifetime intrinsics etc.</div></div></div></div></div></blockquote><div><br></div><div>It's not strictly necessary, no, but that doesn't mean it's not better to cache them. Some code can end up fetching these declarations a *lot*.</div><br><blockquote type="cite"><div dir="ltr"><div><div class="gmail_extra"><div class="gmail_quote">
<div style="">You can use CGF.Builder.CreateLifetimeEnd(llvm::AllocaInst *AI, llvm::ConstantInt *AllocaSize)</div><div style="">and same for CGF.Builder.CreateLifetimeStart(...).</div><div style="">It will automatically do a sanity check: your Addr should in fact come from AllocaInst, and AllocaSize should be constant.</div></div></div></div></div></blockquote><div><br></div>The verifier should be checking these, so we're not ultimately losing safety.</div><div><br></div><div><blockquote type="cite"><div dir="ltr"><div><div class="gmail_extra"><div class="gmail_quote"><div style="">I used more generic function for determining alloca size that is passed to llvm.lifetime intrinsic:</div></div></div></div></div></blockquote><div><br></div><div>Clang never emits the array form of alloca except for VLAs, where the lifetime is explicitly managed anyway.</div><div><br></div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="gmail_extra"><div class="h5"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto; ">
+llvm::Constant *CodeGenModule::getLLVMLifetimeStartFn() {<br>
+ if (LifetimeStartFn) return LifetimeStartFn;<br>
+ LifetimeStartFn = llvm::Intrinsic::getDeclaration(&getModule(),<br>
+ llvm::Intrinsic::lifetime_start);<br>
+ return LifetimeStartFn;<br>
+}<br>
+<br>
+/// Lazily declare the @llvm.lifetime.end intrinsic.<br>
+llvm::Constant *CodeGenModule::getLLVMLifetimeEndFn() {<br>
+ if (LifetimeEndFn) return LifetimeEndFn;<br>
+ LifetimeEndFn = llvm::Intrinsic::getDeclaration(&getModule(),<br>
+ llvm::Intrinsic::lifetime_end);<br>
+ return LifetimeEndFn;<br>
+}<br>
+<br>
namespace {<br>
/// A cleanup to perform a release of an object at the end of a<br>
/// function. This is used to balance out the incoming +1 of a<br>
<br>
Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=177819&r1=177818&r2=177819&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=177819&r1=177818&r2=177819&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/CodeGen/CGStmt.cpp (original)<br>
+++ cfe/trunk/lib/CodeGen/CGStmt.cpp Sat Mar 23 01:43:35 2013<br>
@@ -319,6 +319,12 @@ CodeGenFunction::getJumpDestForLabel(con<br>
}<br>
<br>
void CodeGenFunction::EmitLabel(const LabelDecl *D) {<br>
+ // Add this label to the current lexical scope if we're within any<br>
+ // normal cleanups. Jumps "in" to this label --- when permitted by<br>
+ // the language --- may need to be routed around such cleanups.<br>
+ if (EHStack.hasNormalCleanups() && CurLexicalScope)<br>
+ CurLexicalScope->addLabel(D);<br></blockquote></div></div></div></blockquote><div><br></div><div style="">Why do you need this?</div></div></div></div></blockquote><div><br></div>See the comment. We're using the cleanups mechanism, but that creates a</div><div>situation where it's valid to goto into a cleanup, and so we need to adjust</div><div>label depths when we leave that scope.</div><div><br></div><div>Note that we'll likely lose any useful ability to optimize lifetimes in this case</div><div>because post-dominance will be broken.</div><div><br></div><div>Thanks for the review!</div><div><br></div><div>John.</div></body></html>