[PATCH] Fix initialization problems with PassRegistry

[Reid Kleckner]

To me the core problem is that things aren't being destroyed in reverse
order of construction.  This is the timeline:

start ctor of PassListener
start ctor of PassRegistry
finish ctor of PassRegistry
finish ctor of PassListener
... main ...
llvm_shutdown
start and finish dtor of PassRegistry
... return from main...
start dtor of PassListener -> references the "destroyed" PassRegistry

The fact that this "recovers" by reinitializing the PassRegistry is pretty
bogus.

As a workaround, I'm strongly in favor of not reinitializing the
PassRegistry when attempting to deregister the listener.  We'd need some
helper on ManagedStatic to let us check if the internal pointer is null.


On Wed, Jun 11, 2014 at 1:00 PM, Zachary Turner <zturner at google.com> wrote:

> Another alternative is writing to immutable static (not ManagedStatic)
> data structures during static initialization, and then copying them over to
> the ManagedStatics after main begins.
>
> The fundamental problem, at least in my mind, is this scenario:
>
> 1) During static construction, you access a ManagedStatic, causing it to
> be allocated.
> 2) Before main exits, you call llvm_shutdown to destroy ManagedStatics,
> including the one allocated in #1
> 3) During static destruction, you do the inverse operation of what you did
> in #1.  This also accesses a ManagedStatic, after llvm_shutdown has been
> called, and allocates another one.
>
> Plus, it only resolves ordering insofar as the ordering of the static
> constructors is resolved.  Which is to say, not very much.  You get
> deterministic destruction order, but you still get non-deterministic
> construction order, because you dont' know in which order the static
> constructors which use the ManagedStatics will be called in.
>
>
> On Wed, Jun 11, 2014 at 12:41 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
>
>> On Wed, Jun 11, 2014 at 11:54 AM, Zachary Turner <zturner at google.com>
>> wrote:
>> > I might be able to solve the lock problem by just deleting the lock
>> access from the destructor, as you mention, or making the lock an instance
>> variable of the PassRegistry class.  But I still think the way I've done is
>> the only real way to solve the problem with the PassRegistrationListener.
>>
>> It seems like the problem with PassRegistrationListener would be
>> solved by having a static device (other than
>> PassRegistrationListener's ctor itself) that simply doesn't remove the
>> listener in its dtor. The problem being then you change the behavior
>> of non-static PassRegistrationListeners (since you have to remove the
>> registration machinery from PassRegistrationListener's ctor/dtor).
>> Global registration devices are pretty common.
>>
>> All that being said, removing global ctors is a soft goal of the
>> project, so I'm OK doing it for those reasons - I'm just trying not to
>> conflate different goals/problems & understand which things we're
>> doing for what reasons.
>>
>> > Ultimately, the root of the problem is that ManagedStatics are being
>> accessed during static initialization and shutdown.
>>
>> I don't actually see those things as inherently wrong. Something like
>> a ManagedStatic is the only way to resolve ordering in global init
>> (yes, the alternative is to restrict the project to never have globals
>> that access other globals - but that's not the only correct answer).
>> During destruction is more subtle, certainly.
>>
>> > This needs to be prevented, and I think we need to try to get to a
>> point where we can actually enforce, by way of asserts, that you cannot
>> touch a ManagedStatic before main is entered or after it returns.
>> >
>> > http://reviews.llvm.org/D3996
>> >
>> >
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140611/432fc0c1/attachment.html>