<div dir="ltr"><div><br></div><div style>Done. New patch attached.</div><div style><br></div><div style>(I'll update the patch in the bug later)</div><div style><br></div><div style>Was there a recent style normalization effort in the codebase? (I try to emulate the style of the context)</div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Jan 10, 2013 at 3:53 PM, Jim Grosbach <span dir="ltr"><<a href="mailto:grosbach@apple.com" target="_blank">grosbach@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div>Missed one.</div><div><br></div>s/CreateMemoryManager/createMemoryManager/<div><br>
</div><div>OK to commit with that change.<div><div class="h5"><br><div><br><div><div>On Jan 10, 2013, at 11:50 AM, Michael Muller <<a href="mailto:mmuller@google.com" target="_blank">mmuller@google.com</a>> wrote:</div>
<br><blockquote type="cite"><div dir="ltr"><div class="gmail_default">Ping?</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jan 8, 2013 at 6:03 PM, Michael Muller <span dir="ltr"><<a href="mailto:mmuller@google.com" target="_blank">mmuller@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_default"><br></div><div class="gmail_extra"><br><br><div class="gmail_quote"><div>
On Tue, Jan 8, 2013 at 5:52 PM, Jim Grosbach <span dir="ltr"><<a href="mailto:grosbach@apple.com" target="_blank">grosbach@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">Looks good to me, with a couple nits:<div><br></div><div><div>+ virtual RecordingJITMemoryManager* CreateMemoryManager() {</div>
<div><br></div><div>The '*' should bind to the identifier, not the type. The function name should start with lower case. There's a few other instances in the patch of the former. Please double-check.</div></div>
</div></blockquote><div><br></div></div><div>All fixed. </div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">
<div><div><br>
</div><div>Thanks!</div><div><br></div><div>-Jim</div><div><br></div><div><br><div><div><div>On Jan 8, 2013, at 2:43 PM, Michael Muller <<a href="mailto:mmuller@google.com" target="_blank">mmuller@google.com</a>> wrote:</div>
<br></div><blockquote type="cite"><div><div dir="ltr"><div><br></div><div>New patch attached, incorporates Misha's comments and current trunk.</div><div class="gmail_extra"><br><br><div class="gmail_quote">
On Fri, Jan 4, 2013 at 7:44 PM, Misha Brukman <span dir="ltr"><<a href="mailto:brukman@gmail.com" target="_blank">brukman@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>On Wed, Dec 19, 2012 at 1:55 PM, Michael Muller <span dir="ltr"><<a href="mailto:mmuller@enduden.com" target="_blank">mmuller@enduden.com</a>></span> wrote:</div><div class="gmail_quote"><div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
See <a href="http://llvm.org/bugs/show_bug.cgi?id=13678" target="_blank">http://llvm.org/bugs/show_bug.cgi?id=13678</a> for more history.<br>
<br>
See also:<br>
<a href="http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-August/052766.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-August/052766.html</a><br>
<a href="http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120910/150422.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120910/150422.html</a><br>
<a href="http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120917/150801.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120917/150801.html</a><br>
<a href="http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-December/056829.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-December/056829.html</a><br>
<br>
When you run out of space writing to a buffer, the buffer management code<br>
simply stops writing at the end of the buffer. It is the responsibility of<br>
the caller to verify that it has stayed in bounds and perform a retry with<br>
a larger memory estimate if not. The function writing code does this, but<br>
the exception table code following it does not. The end result is that<br>
exception table pointers can get registered pointing to invalid data, causing<br>
seg-faults when an exception is thrown.<br></blockquote><div><br></div></div><div>Minor drive-by style comments (I wish there were a web UI for entering these in context):</div><div><ul><li>s/;;/;/</li><li>ASSERT_TRUE(actual == expected) --> ASSERT_EQ(expected, actual)</li>
<li>add space around binary operators (-, *, etc.), e.g.:</li></ul></div><div><pre style="word-wrap:break-word;white-space:pre-wrap">+ ActualSize = (CurBufferPtr-BufferBegin)*2;</pre></div></div></blockquote><div><br>
</div><div>I'm sure I copied that from somewhere, but looking at the file now I see nothing resembling such formatting. Fixed.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_quote"><div><pre style="word-wrap:break-word;white-space:pre-wrap"></pre></div><div><ul><li>fix arg alignment of "ActualSize" here:</li></ul></div><div><pre style="word-wrap:break-word;white-space:pre-wrap">
+ while (true) {
+ BufferBegin = CurBufferPtr = MemMgr->startExceptionTable(F.getFunction(),
+ ActualSize);</pre></div></div></blockquote><div><br></div><div>Done.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_quote">Misha</div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br>Michael Muller<br>Ym9yZWQ/
</div></div>
</div><span><ExceptionTableOverflowFix-v5.patch></span></blockquote></div><br></div></div></div></blockquote></div></div></div><span><font color="#888888"><br><br clear="all"><div><br></div>-- <br>
Michael Muller<br>Ym9yZWQ/
</font></span></div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br>Michael Muller<br>Ym9yZWQ/
</div>
</blockquote></div><br></div></div></div></div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br>Michael Muller<br>Ym9yZWQ/
</div>