[PATCH] [ASan] Make BlockingMutex really linker initialized.

[Timur Iskhodzhanov]

[The discussion is a little bit lengthy to understand from a first read, so
don't blame me too much if I haven't understood the question]

Unfortunately, I don't know how to reliably do linker-initialized
CriticalSections on Windows.  [+Dmitry, David or Reid might help]

Currently there is a hack in BlockingMutex::Lock @ sanitizer_win.cc that
makes global mutexes work on Windows, but I think even this implementation
fails to start sometimes.

Also please note we DON'T yet have a public buildbot that runs ASan tests
on Windows.  I've been working on one this week and it's green, but it only
compiles:
http://lab.llvm.org:8011/builders/sanitizer-windows
Hopefully it will start running tests next week.

On Fri Feb 20 2015 at 8:28:36 AM Yuri Gribov <tetra2005 at gmail.com> wrote:

> On Fri, Feb 20, 2015 at 5:19 AM, Alexey Samsonov <vonosmas at gmail.com>
> wrote:
> >
> > On Wed, Feb 18, 2015 at 5:29 AM, Yury Gribov <y.gribov at samsung.com>
> wrote:
> >>
> >> On 02/06/2015 09:10 AM, Yury Gribov wrote:
> >>>
> >>> On 02/04/2015 07:23 PM, David Blaikie wrote:
> >>>>
> >>>> On Wed, Feb 4, 2015 at 3:56 AM, Yury Gribov <tetra2005 at gmail.com>
> wrote:
> >>>>
> >>>>>> Yes, I think enabling -Wglobal-constructors for ASan (and for all
> the
> >>>>>
> >>>>> rest sanitizers) will be great.
> >>>>>
> >>>>>
> >>>>> FYI it was relatively easy to get this working on Linux (with ~500
> >>>>> lines
> >>>>> of changes). Unfortunately Windows compiler lacks too many necessary
> >>>>> features: explicit initialization of array members, constexpr,
> >>>>> unrestricted
> >>>>> unions (all still missing in VS2013 and we still use VS2012). Having
> >>>>> #if
> >>>>> WINDOWS all over the place isn't an option as well so I'm afraid we
> >>>>> are out
> >>>>> of luck.
> >>>>>
> >>>>
> >>>> Perhaps there's a relatively small number of structures (probably
> >>>> templates) we could create to keep all of this logic (and Windows
> >>>> workarounds/suppressions) in one place so it could scale to all the
> use
> >>>> cases we have?
> >>>
> >>>
> >>> Thanks for the idea, I'll check this next week.
> >>
> >>
> >> Folks,
> >>
> >> Here is a draft version (not full-blown patch but rather a PoC). The
> main
> >> part is in sanitizer_internal_defs.h (classes LinkerInitializedArray and
> >> LinkerInitializedStructArray). I also slightly changed atomic types in
> >> sanitizer_atomic.h (volatile members can't be linker-initialized). Does
> this
> >> look sane in general?
> >
> > +Timur for Windows parts.
> > Overall, yes, although missing constexpr is such a pain (sigh). Why is it
> > necessary to initialize all fields to 0 in LinkerInitialized constructor?
>
> You mean some classes may want non-zero values there? That's true, I
> planned to address this (the patch is a draft anyway).
>
> >> I've did full rebuild and regtests on Linux (compiler-rt built by trunk
> >> Clang, GCC 4.8 and Clang 3.1). I haven't done full testing on Windows
> and
> >> Mac but at least relevant parts of sanitizer_internal_defs.h compiled
> fine.
> >>
> >> If all this makes sense, how should I proceed with a refactoring of this
> >> scale? Is it mandatory to verify _all_ platforms or I could to some
> extent
> >> rely on respected maintainers to fix errors? The former would be quite a
> >> burden.
> >
> > Well, at least for LLVM, we have buildbots (for Linux/Mac/Windows), so
> it's
> > fine to test your change on available platforms, and then watch the
> > buildbots closely.
>
> Ok.
>
> > It would be awesome to rollout this out gradually, first by introducing
> > LinkerInitializedArray, then converting global objects to use constexpr
> > constructor one by one, and enabling -Wglobal-constructor as a final
> change.
>
> Right.
>
> -Y
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150220/4abcf607/attachment.html>