[PATCH] D81156: [Support] Add stream tie function and use it for errs()

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D81156#2088609 <https://reviews.llvm.org/D81156#2088609>, @sanjoy.google wrote:

> In D81156#2084324 <https://reviews.llvm.org/D81156#2084324>, @jhenderson wrote:
>
> > In D81156#2084273 <https://reviews.llvm.org/D81156#2084273>, @sammccall wrote:
> >
> > > In D81156#2084209 <https://reviews.llvm.org/D81156#2084209>, @jhenderson wrote:
> > >
> > > > In D81156#2084176 <https://reviews.llvm.org/D81156#2084176>, @sammccall wrote:
> > > >
> > > > > This is surprising and unsafe behaviour. Can we reconsider changing the default, at least until a more careful audit of the places errs() is used?
> > > > >  In particular, if errs is used from multiple threads (with a lock) and outs is used from a single thread (so no lock needed) then this introduces racy flushes to outs() concurrent with writes.
> > > >
> > > >
> > > > Thanks for highlighting this, it's been brought up on D81525 <https://reviews.llvm.org/D81525> as well, so rather than discuss it in two places, we'll look at it there, if that's okay?
> > >
> > >
> > > Happy to discuss there. It's a separate issue though, the discussion there is about initialization being racy,
> >
> >
> > Yeah, I realised that after posting that previous post, sorry! Still, probably best to keep the discussion in one place for now.
>
>
> Did we resolve this?  In an internal XLA build tsan complains about this being racy, and I'm not sure if the problem is with how we are using `llvm::dbgs()` or with this patch.  If the latter, please rollback while we figure out a non-racy way to achieve the same thing.


Does 030897523d43e3296f69d25a71a140d9e5793c6a <https://reviews.llvm.org/rG030897523d43e3296f69d25a71a140d9e5793c6a> fix the race?

(clangd used tie now. .debug_line parsing depends on it. Reverting is more disruptive than simply disabling tie-by-default behavior)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81156/new/

https://reviews.llvm.org/D81156