This is an archive of the discontinued LLVM Phabricator instance.

llvm-config: Explicitly flush ostream so output can be piped into other programs.
Needs ReviewPublic

Authored by tstellarAMD on Nov 10 2015, 9:28 AM.

Details

Reviewers
jroelofs
beanz
Summary

This is required on my Fedora 21 system, but not on Ubuntu based systems I tested.
Maybe it has something to do with the default shell being used?

Diff Detail

Event Timeline

tstellarAMD retitled this revision from to llvm-config: Explicitly flush ostream so output can be piped into other programs..
tstellarAMD updated this object.
tstellarAMD added a reviewer: jroelofs.
tstellarAMD added a subscriber: llvm-commits.
jroelofs added inline comments.Nov 10 2015, 9:51 AM
tools/llvm-config/llvm-config.cpp
422

Maybe it's best to create a RAII object here that calls OS.flush() in its destructor?

522

otherwise you have to call flush here...

549

and here...

556

and here...

559

and here, which is fragile.

jroelofs added inline comments.Nov 10 2015, 9:52 AM
test/tools/llvm-config/flush-stdout.test
1

Please use FileCheck instead of grep... we're trying to eliminate grep from testcases (slowly).

MatzeB added a subscriber: MatzeB.Nov 10 2015, 10:08 AM

This is confusing, shouldn't the destructor of the object behind outs() take care of the flushing on program exit?

This is confusing, shouldn't the destructor of the object behind outs() take care of the flushing on program exit?

The destructor does normally flush, but I think the what is happening is that the file descriptor is closed before the destructor is called, so the flush doesn't do anything.

What's happening is that the destructor at least of the static variable in llvm::outs() doesn't run when LLVM_LINK_TOOLS_DYLIB is enabled. Probably this affects other global variables as well.

nhaehnle added a subscriber: beanz.

I ran into this again and decided to investigate a bit more. The root cause is r223805, which explicitly disables atexit for the dynamic library build.

This behaviour is controlled by LLVM_DISABLE_LLVM_DYLIB_ATEXIT, which tries to default to OFF when the tools (i.e. llvm-config) is linked against the dynamic library. As far as I can tell, the problem is basically that we did (as one is likely to do):

  1. Set up a dylib build. This sets LLVM_DISABLE_LLVM_DYLIB_ATEXIT to ON.
  2. Notice that all the tool executables become gigantic and turn on LLVM_LINK_LLVM_DYLIB.
  3. Oops, CMake remembers the old setting of LLVM_DISABLE_LLVM_DYLIB_ATEXIT and llvm-config is now unusable from build scripts.

So while explicitly setting LLVM_DISABLE_LLVM_DYLIB_ATEXIT to OFF is a (extremely hard to discover) workaround, this really needs to be fixed somehow.