[PATCH] PrettyStackTrace should use ManagedStatic for its ThreadLocal

[Dan Gohman]

On Thu, Sep 12, 2013 at 8:56 PM, Filip Pizlo <fpizlo at apple.com> wrote:
>
> On Sep 12, 2013, at 8:48 PM, Dan Gohman <sunfish at google.com> wrote:
>
> On Thu, Sep 12, 2013 at 6:33 PM, Filip Pizlo <fpizlo at apple.com> wrote:
>
>
> On Sep 12, 2013, at 5:49 PM, Dan Gohman <sunfish at google.com> wrote:
>
> On Thu, Sep 12, 2013 at 5:00 PM, Filip Pizlo <fpizlo at apple.com> wrote:
>
> llvm::outs() has a static local that closes stdout in its destructor.
> Obviously, this *really* has to run atexit, and probably can't run at
> llvm_shutdown() time.  Alternatively, it shouldn't close stdout.
>
>
> Is the issue just that you want it to flush its output?
>
>
> The issue is that it needs to detect pending I/O errors.
>
>
> What would you do with pending I/O errors in an atexit() handler?
>
>
> The code is out there :-). It calls report_fatal_error.
>
>
> Ah, got it - I thought you meant that it had some recovery.  Is it clear
> that this behavior makes sense for llvm::outs, though?  To be clear, I'm not
> asking about changing raw_ostream's behavior in general, but changing the
> behavior of llvm:outs in particular.  We could get the best of both worlds:
>
> - raw_ostream clients that aren't llvm:outs retains all of the current
> behavior and performance.
>
> - llvm::outs has slightly different behavior, that doesn't require atexit().

In the current design, clients can do this:

int main() {
  outs() << "hello world\n";
}

and unlike the common hello world in standard C++, this program exits
with a non-zero exit status if it encounters an output error. This
happens automatically, and by default, which is nice, because it's
easy to forget error checking if it has to be done manually. Try it
with a redirect to /dev/full to see it in action, for example. atexit
is the mechanism that makes this possible. Maybe better designs are
possible, but this is what the current design is.

>
>
>
>
> In that case why not do the equivalent of _setvbuf?
>
>
> _setvbuf is for libc buffering. raw_ostream's implementation was
> designed to avoid libc buffering because it wasn't as fast.
>
>
> I understand that _setvbuf is for libc.  But why can't raw_ostream do the
> equivalent for the purpose of llvm::outs()?  Is the concern that a program
> might use outs() (i.e. outputting to stdout) so heavily that buffering
> becomes critical?
>
>
> I don't understand what you're asking here.
>
>
> I'm asking because I don't know, and would like to learn.  I find your
> explanations useful, so that if I try to do more ManagedStatic-like
> refactorings, I'll know what I'm doing.  There's also the possibility that
> I'm on to something but just haven't described it clearly enough, yet.
>
> It's preferable (and sometimes obligatory) for non-compiler-tool clients of
> LLVM to not end up with atexit() handlers installed by LLVM.  The llvm::outs
> one appears to be less evil and maybe it's even necessary, but I'm not sure
> I understand why, yet.

Clients which don't use llvm::outs won't get an atexit call registered.

>
> raw_ostream is a simple
> utility for simple compiler-like tools. It's useful for these tools to
> exit with a non-zero exit status if they encounter any output error.
> raw_ostream's current design achieves this automatically, and by
> default (though it can be overridden).
>
>
> Right, but is llvm::outs() used in a manner where it makes sense for output
> errors to result in a hard failure? I.e. is it common for tools to be used
> in a mode where they output data to fd=1, and is it critical for that
> use-case to report_fatal_error if the I/O fails (and that error didn't
> already shoot you down with SIGPIPE or similar)?

Yes. For example, it's used for compiler-like tools running on systems
which sometimes run out of disk space, where it's useful to have
compilations fail instead of silently "succeeding" and letting the
build haplessly carry on only to fail later in strange ways due to
having truncated data. And for a compiler-like tool, fd=1 can be an
output file just like any other.

Dan