<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Feb 20, 2015 at 8:23 AM, Dmitry Vyukov <span dir="ltr"><<a href="mailto:dvyukov@google.com" target="_blank">dvyukov@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"><span class="">On Fri, Feb 20, 2015 at 7:01 PM, Timur Iskhodzhanov <<a href="mailto:timurrrr@google.com">timurrrr@google.com</a>> wrote:<br>
> [The discussion is a little bit lengthy to understand from a first read, so<br>
> don't blame me too much if I haven't understood the question]<br>
><br>
> Unfortunately, I don't know how to reliably do linker-initialized<br>
> CriticalSections on Windows.  [+Dmitry, David or Reid might help]<br>
<br>
</span>Windows does not have statically initialized mutexes.<br>
There is only Once (INIT_ONCE_STATIC_INIT), if you are OK with working<br>
only on Vista+. Having static Once you can init a mutex lazily.<br>
<div class=""><div class="h5"><br></div></div></blockquote><div><br></div><div>Actually, Windows does have such a mutex.  You can use a SRWLOCK and statically initialize it with SRWLOCK_INIT.</div><div><br></div><div>If you can't upgrade to SRWLOCK (introduced in Vista), you can resort to this trick with CRITICAL_SECTION: <a href="https://chromium.googlesource.com/webm/libvpx/+/1a23086bc667054a1da6942750975f53a504c1de/vp8/common/rtcd.c#32">https://chromium.googlesource.com/webm/libvpx/+/1a23086bc667054a1da6942750975f53a504c1de/vp8/common/rtcd.c#32</a></div><div> </div><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=""><div class="h5">
<br>
> Currently there is a hack in BlockingMutex::Lock @ sanitizer_win.cc that<br>
> makes global mutexes work on Windows, but I think even this implementation<br>
> fails to start sometimes.<br>
><br>
> Also please note we DON'T yet have a public buildbot that runs ASan tests on<br>
> Windows.  I've been working on one this week and it's green, but it only<br>
> compiles:<br>
> <a href="http://lab.llvm.org:8011/builders/sanitizer-windows" target="_blank">http://lab.llvm.org:8011/builders/sanitizer-windows</a><br>
> Hopefully it will start running tests next week.<br>
><br>
><br>
> On Fri Feb 20 2015 at 8:28:36 AM Yuri Gribov <<a href="mailto:tetra2005@gmail.com">tetra2005@gmail.com</a>> wrote:<br>
>><br>
>> On Fri, Feb 20, 2015 at 5:19 AM, Alexey Samsonov <<a href="mailto:vonosmas@gmail.com">vonosmas@gmail.com</a>><br>
>> wrote:<br>
>> ><br>
>> > On Wed, Feb 18, 2015 at 5:29 AM, Yury Gribov <<a href="mailto:y.gribov@samsung.com">y.gribov@samsung.com</a>><br>
>> > wrote:<br>
>> >><br>
>> >> On 02/06/2015 09:10 AM, Yury Gribov wrote:<br>
>> >>><br>
>> >>> On 02/04/2015 07:23 PM, David Blaikie wrote:<br>
>> >>>><br>
>> >>>> On Wed, Feb 4, 2015 at 3:56 AM, Yury Gribov <<a href="mailto:tetra2005@gmail.com">tetra2005@gmail.com</a>><br>
>> >>>> wrote:<br>
>> >>>><br>
>> >>>>>> Yes, I think enabling -Wglobal-constructors for ASan (and for all<br>
>> >>>>>> the<br>
>> >>>>><br>
>> >>>>> rest sanitizers) will be great.<br>
>> >>>>><br>
>> >>>>><br>
>> >>>>> FYI it was relatively easy to get this working on Linux (with ~500<br>
>> >>>>> lines<br>
>> >>>>> of changes). Unfortunately Windows compiler lacks too many necessary<br>
>> >>>>> features: explicit initialization of array members, constexpr,<br>
>> >>>>> unrestricted<br>
>> >>>>> unions (all still missing in VS2013 and we still use VS2012). Having<br>
>> >>>>> #if<br>
>> >>>>> WINDOWS all over the place isn't an option as well so I'm afraid we<br>
>> >>>>> are out<br>
>> >>>>> of luck.<br>
>> >>>>><br>
>> >>>><br>
>> >>>> Perhaps there's a relatively small number of structures (probably<br>
>> >>>> templates) we could create to keep all of this logic (and Windows<br>
>> >>>> workarounds/suppressions) in one place so it could scale to all the<br>
>> >>>> use<br>
>> >>>> cases we have?<br>
>> >>><br>
>> >>><br>
>> >>> Thanks for the idea, I'll check this next week.<br>
>> >><br>
>> >><br>
>> >> Folks,<br>
>> >><br>
>> >> Here is a draft version (not full-blown patch but rather a PoC). The<br>
>> >> main<br>
>> >> part is in sanitizer_internal_defs.h (classes LinkerInitializedArray<br>
>> >> and<br>
>> >> LinkerInitializedStructArray). I also slightly changed atomic types in<br>
>> >> sanitizer_atomic.h (volatile members can't be linker-initialized). Does<br>
>> >> this<br>
>> >> look sane in general?<br>
>> ><br>
>> > +Timur for Windows parts.<br>
>> > Overall, yes, although missing constexpr is such a pain (sigh). Why is<br>
>> > it<br>
>> > necessary to initialize all fields to 0 in LinkerInitialized<br>
>> > constructor?<br>
>><br>
>> You mean some classes may want non-zero values there? That's true, I<br>
>> planned to address this (the patch is a draft anyway).<br>
>><br>
>> >> I've did full rebuild and regtests on Linux (compiler-rt built by trunk<br>
>> >> Clang, GCC 4.8 and Clang 3.1). I haven't done full testing on Windows<br>
>> >> and<br>
>> >> Mac but at least relevant parts of sanitizer_internal_defs.h compiled<br>
>> >> fine.<br>
>> >><br>
>> >> If all this makes sense, how should I proceed with a refactoring of<br>
>> >> this<br>
>> >> scale? Is it mandatory to verify _all_ platforms or I could to some<br>
>> >> extent<br>
>> >> rely on respected maintainers to fix errors? The former would be quite<br>
>> >> a<br>
>> >> burden.<br>
>> ><br>
>> > Well, at least for LLVM, we have buildbots (for Linux/Mac/Windows), so<br>
>> > it's<br>
>> > fine to test your change on available platforms, and then watch the<br>
>> > buildbots closely.<br>
>><br>
>> Ok.<br>
>><br>
>> > It would be awesome to rollout this out gradually, first by introducing<br>
>> > LinkerInitializedArray, then converting global objects to use constexpr<br>
>> > constructor one by one, and enabling -Wglobal-constructor as a final<br>
>> > change.<br>
>><br>
>> Right.<br>
>><br>
>> -Y<br>
</div></div></blockquote></div><br></div></div>