[llvm-commits] [ASan] The first version of the RTL for Windows (issue 5647052)

[Kostya Serebryany]

On Wed, Feb 8, 2012 at 10:19 AM, <Aaron.Ballman at gmail.com> wrote:

> Added previous comments as well as one new one about DbgHelp being
> thread unsafe.
>
>
>
> http://codereview.appspot.com/**5647052/diff/6001/lib/asan/**asan_win.cc<http://codereview.appspot.com/5647052/diff/6001/lib/asan/asan_win.cc>
> File lib/asan/asan_win.cc (right):
>
> http://codereview.appspot.com/**5647052/diff/6001/lib/asan/**
> asan_win.cc#newcode88<http://codereview.appspot.com/5647052/diff/6001/lib/asan/asan_win.cc#newcode88>
> lib/asan/asan_win.cc:88: CHECK(VirtualQuery(&mbi /* on stack */,
>
> The stack will always be a single block, but I think what you're getting
> there is including the stack's guard page.  Are you sure you want that?
> Also, I think that's only getting you the reserved size and not the
> committed size.
>
> http://codereview.appspot.com/**5647052/diff/6001/lib/asan/**
> asan_win.cc#newcode98<http://codereview.appspot.com/5647052/diff/6001/lib/asan/asan_win.cc#newcode98>
> lib/asan/asan_win.cc:98: ScopedLock lock(&dbghelp_lock);
> On 2012/02/08 18:03:10, kcc wrote:
>
>> OMG. Ok for now, but in the long run this is not going to work.
>> We will need our own unwinder.
>>
>
> Even with our own unwinder, the symbolicator is single-threaded only on
> Windows.  So we'll need some form of locking in WinSymbolize.
>

[replying using the kosher e-mail]
I don't care about the performance of symbolicator too much -- it is
invoked only once.
But the performance of the unwinder is very important since it is invoked
on every malloc/free.
Even our super fast unwinder on linux is responsible for up-to 20% of asan
run-time cost.

--kcc


>
> http://codereview.appspot.com/**5647052/diff/6001/lib/asan/**
> asan_win.cc#newcode103<http://codereview.appspot.com/5647052/diff/6001/lib/asan/asan_win.cc#newcode103>
> lib/asan/asan_win.cc:103: size_t cs_ret = CaptureStackBackTrace(1,
> max_size, tmp, NULL),
>
> Since you're already using DbgHelp APIs, would StackWalk64 make a bit
> more sense?  IIRC, it's a bit more accurate than
> CaptureStackBackTrace.  Certainly it gives you more control.
>
> You may want to look at LLVMUnhandledExceptionFilter in Signals.inc
> because a lot of the stack walking fun is already done there.  Perhaps
> we could refactor it to use common code?
>
> http://codereview.appspot.com/**5647052/<http://codereview.appspot.com/5647052/>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120208/c22788b2/attachment.html>