[llvm-objcopy] Remove no-op flush of errs

[abrachet]

@jhenderson said that I should put this into a micro patch and that didn't need review if all tests pass. They did so I went ahead and committed this before review. https://github.com/llvm/llvm-project/blob/master/llvm/lib/Support/raw_ostream.cpp#L854 shows that errs is unbuferred.

[lebedev.ri]

In D63313#1543002, @abrachet wrote:

https://github.com/llvm/llvm-project/blob/master/llvm/lib/Support/raw_ostream.cpp#L854 shows that errs is unbuferred.

That's actually kind-of a problem. Unless *something* ensures that all the output
that was written into that is flushed, it may end up being dropped, rL360124.

[jhenderson]

In D63313#1543159, @lebedev.ri wrote:

In D63313#1543002, @abrachet wrote:

https://github.com/llvm/llvm-project/blob/master/llvm/lib/Support/raw_ostream.cpp#L854 shows that errs is unbuferred.

That's actually kind-of a problem. Unless *something* ensures that all the output
that was written into that is flushed, it may end up being dropped, rL360124.

It depends on the stream being used errs() shouldn't need flushing because of its unbuffered state. Other streams (as fixed by rL360124) will do.

[lebedev.ri]

In D63313#1543284, @jhenderson wrote:

In D63313#1543159, @lebedev.ri wrote:

In D63313#1543002, @abrachet wrote:

https://github.com/llvm/llvm-project/blob/master/llvm/lib/Support/raw_ostream.cpp#L854 shows that errs is unbuferred.

That's actually kind-of a problem. Unless *something* ensures that all the output
that was written into that is flushed, it may end up being dropped, rL360124.

It depends on the stream being used errs() shouldn't need flushing because of its unbuffered state.

What about llvm::outs()?

Other streams (as fixed by rL360124) will do.

[jhenderson]

In D63313#1543286, @lebedev.ri wrote:

What about llvm::outs()?

llvm::outs() is buffered, so would need flushing, although I vaguely remember seeing something in the code somewhere that automatically did it on process termination.

To be clear, is there an issue with this specific change, which is dealing with llvm::errs() only?

[lebedev.ri]

In D63313#1543330, @jhenderson wrote:

In D63313#1543286, @lebedev.ri wrote:

What about llvm::outs()?

llvm::outs() is buffered, so would need flushing,

Ah yes, i see it now, in llvm::errs() (in raw_ostream.cpp) the stream is created explicitly unbuffered, which is not the case for llvm::outs()

although I vaguely remember seeing something in the code somewhere that automatically did it on process termination.

No, there is no such thing i'm afraid, else i would not have had that bug..

To be clear, is there an issue with this specific change, which is dealing with llvm::errs() only?

No, looks like all good, sorry about the noise.

[abrachet]

although I vaguely remember seeing something in the code somewhere that automatically did it on process termination.

No, there is no such thing i'm afraid, else i would not have had that bug..

raw_ostream does not flush in its destructor (although it does assert that the buffer is empty) but all sub classes in raw_ostream.h flush their buffers in their destructors. outs() would have its destructor called by __cxa_atexit, @jhenderson is right.

raw_fd_ostream's destructor can be found here: https://github.com/llvm/llvm-project/blob/master/llvm/lib/Support/raw_ostream.cpp#L592

llvm/tools/llvm-objcopy/llvm-objcopy.cpp

 namespace llvm {
 namespace objcopy {
 
 // The name this program was invoked as.
 StringRef ToolName;
 
 LLVM_ATTRIBUTE_NORETURN void error(Twine Message) {
   WithColor::error(errs(), ToolName) << Message << "\n";
-  errs().flush();
   exit(1);
 }
 
 LLVM_ATTRIBUTE_NORETURN void error(Error E) {
   assert(E);
   std::string Buf;
   raw_string_ostream OS(Buf);
   logAllUnhandledErrors(std::move(E), OS);