[PATCH] PrettyStackTrace should use ManagedStatic for its ThreadLocal

[Reid Kleckner]

I don't think this is a good way to go.  Does LLVM support reinitializing
and calling llvm_shutdown() multiple times?  If so, this won't work.

How about adding a variant of ManagedStatic that registers the destructor
with atexit(), just like C++ dynamic initializers do, instead of the LLVM
shutdown machinery?

Then we could convert llvm::outs() to use it, thereby potential
initialization races on Windows.


On Thu, Sep 12, 2013 at 12:07 PM, Filip Pizlo <fpizlo at apple.com> wrote:

> This patch converts PrettyStackTrace to use ManagedStatic.
>  PrettyStackTrace is both important to convert to ManagedStatic, and
> surprisingly tricky.
>
> It's important because LLVM uses it in a lot of places.  This means that
> if you have a program that calls into LLVM on one thread while calling
> exit() on another without first doing a clean shutdown, you'll see
> assertions or crashes inside of PrettyStackTrace and/or inside ThreadLocal.
>
> This patch was surprisingly tricky because we often use
> PrettyStackTraceProgram in a way that causes its destruction to occur after
> shutdown.  I added a function called llvm_shutdown_has_been_called() to
> allow ~PrettyStackTrace to skip removing itself from the stack if we're
> past shutdown.  This is as safe as it gets: if we're done with shutdown
> then ManagedStatics are all destroyed and poisoned and so the only
> alternative to skipping removal from the stack is to trip over the assert
> that comes just after.  I went with this rather conservative approach
> because after seeing that llc did shutdown before ~PrettyStackTraceProgram,
> I sort of made the assumption that probably a bunch of llvm clients do it,
> and I didn't want them to start asserting or crashing on exit.
>
> I'm curious if there are better ways to do this, that don't involve
> revealing llvm_shutdown_has_been_called().  For example I considered making
> PrettyStackTraceProgram skip removal of the stack trace entry from the
> stack, but that felt even more yucky than my currently solution.  I also
> considered putting llvm_shutdown_obj before PrettyStackTraceProgram in all
> tools, but this felt too risky - I'm conservatively assuming outside
> clients might already be relying on this idiom being fine.
>
> -Filip
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130912/3de0f007/attachment.html>