<br><br><div class="gmail_quote">On Wed, Feb 8, 2012 at 10:19 AM,  <span dir="ltr"><<a href="mailto:Aaron.Ballman@gmail.com">Aaron.Ballman@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Added previous comments as well as one new one about DbgHelp being<br>
thread unsafe.<div class="im"><br>
<br>
<br>
<a href="http://codereview.appspot.com/5647052/diff/6001/lib/asan/asan_win.cc" target="_blank">http://codereview.appspot.com/<u></u>5647052/diff/6001/lib/asan/<u></u>asan_win.cc</a><br>
File lib/asan/asan_win.cc (right):<br>
<br>
</div><a href="http://codereview.appspot.com/5647052/diff/6001/lib/asan/asan_win.cc#newcode88" target="_blank">http://codereview.appspot.com/<u></u>5647052/diff/6001/lib/asan/<u></u>asan_win.cc#newcode88</a><br>
lib/asan/asan_win.cc:88: CHECK(VirtualQuery(&mbi /* on stack */,<div class="im"><br>
The stack will always be a single block, but I think what you're getting<br>
there is including the stack's guard page.  Are you sure you want that?<br>
Also, I think that's only getting you the reserved size and not the<br>
committed size.<br>
<br>
</div><div class="im"><a href="http://codereview.appspot.com/5647052/diff/6001/lib/asan/asan_win.cc#newcode98" target="_blank">http://codereview.appspot.com/<u></u>5647052/diff/6001/lib/asan/<u></u>asan_win.cc#newcode98</a><br>

lib/asan/asan_win.cc:98: ScopedLock lock(&dbghelp_lock);<br></div><div class="im">
On 2012/02/08 18:03:10, kcc wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
OMG. Ok for now, but in the long run this is not going to work.<br>
We will need our own unwinder.<br>
</blockquote>
<br></div>
Even with our own unwinder, the symbolicator is single-threaded only on<br>
Windows.  So we'll need some form of locking in WinSymbolize.<br></blockquote><div><br></div><div>[replying using the kosher e-mail]</div><div><div style>I don't care about the performance of symbolicator too much -- it is invoked only once. </div>
<div style>But the performance of the unwinder is very important since it is invoked on every malloc/free. </div><div style>Even our super fast unwinder on linux is responsible for up-to 20% of asan run-time cost. </div></div>
<div style><br></div><div style>--kcc </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<a href="http://codereview.appspot.com/5647052/diff/6001/lib/asan/asan_win.cc#newcode103" target="_blank">http://codereview.appspot.com/<u></u>5647052/diff/6001/lib/asan/<u></u>asan_win.cc#newcode103</a><br>
lib/asan/asan_win.cc:103: size_t cs_ret = CaptureStackBackTrace(1,<br>
max_size, tmp, NULL),<div class="im"><br>
Since you're already using DbgHelp APIs, would StackWalk64 make a bit<br>
more sense?  IIRC, it's a bit more accurate than<br>
CaptureStackBackTrace.  Certainly it gives you more control.<br>
<br>
You may want to look at LLVMUnhandledExceptionFilter in Signals.inc<br>
because a lot of the stack walking fun is already done there.  Perhaps<br>
we could refactor it to use common code?<br>
<br>
</div><a href="http://codereview.appspot.com/5647052/" target="_blank">http://codereview.appspot.com/<u></u>5647052/</a><br>
</blockquote></div><br>