I was just fixing a bug that was caused by `stdout` being closed before the runtime has done `fflush(stdout)` [or however this is implemented in the runtime]. The cause of this seems to be that `outs()` returns a static object created from `raw_fd_stream(STDOUT_FILENO, true)` - the `true` being the `shouldClose` parameter. Surely LLVM is not supposed to close `stdout` as part of its operations? -- Mats
> On 2015-Jan-23, at 09:52, mats petersson <mats at planetcatfish.com> wrote: > > I was just fixing a bug that was caused by `stdout` being closed > before the runtime has done `fflush(stdout)` [or however this is > implemented in the runtime]. > > The cause of this seems to be that `outs()` returns a static object > created from `raw_fd_stream(STDOUT_FILENO, true)` - the `true` being > the `shouldClose` parameter. > > Surely LLVM is not supposed to close `stdout` as part of its operations?Looks like this was added in r111643: commit 5d56d9df928c48571980efe8d4205de8ab557b7c Author: Dan Gohman <gohman at apple.com> Date: Fri Aug 20 16:44:56 2010 +0000 Make outs() close its file when its stream is destructed, so that pending output errors are detected. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk at 111643 91177308-0d34-0410-b5e6-96231b3b80d8 Certainly looks intentional. Comment in the code is: // Delete the file descriptor when the program exits, forcing error // detection. If you don't want this behavior, don't use outs(). I don't really have an opinion on whether this is a good idea, but it does suggest that any use of `outs()` in `lib/` is a bug. $ git grep -l 'outs()' -- lib/ lib/ExecutionEngine/Interpreter/ExternalFunctions.cpp lib/IR/GCOV.cpp lib/Support/CommandLine.cpp lib/Support/FormattedStream.cpp lib/Support/TargetRegistry.cpp lib/Support/raw_ostream.cpp lib/Target/CppBackend/CPPBackend.cpp All of these cases seem to be for outputting help text of some sort. But probably they should be passed a reference to a `raw_ostream`, so that the caller gets to choose whether or not to use `outs()`. Were you hitting one of these?
We've had this argument before. IMO LLVM should not be in the business of closing stdout, and no code in lib/ should print to stdout because users may expect output there (-o -). On Fri, Jan 23, 2015 at 10:20 AM, Duncan P. N. Exon Smith < dexonsmith at apple.com> wrote:> > > On 2015-Jan-23, at 09:52, mats petersson <mats at planetcatfish.com> wrote: > > > > I was just fixing a bug that was caused by `stdout` being closed > > before the runtime has done `fflush(stdout)` [or however this is > > implemented in the runtime]. > > > > The cause of this seems to be that `outs()` returns a static object > > created from `raw_fd_stream(STDOUT_FILENO, true)` - the `true` being > > the `shouldClose` parameter. > > > > Surely LLVM is not supposed to close `stdout` as part of its operations? > > Looks like this was added in r111643: > > commit 5d56d9df928c48571980efe8d4205de8ab557b7c > Author: Dan Gohman <gohman at apple.com> > Date: Fri Aug 20 16:44:56 2010 +0000 > > Make outs() close its file when its stream is destructed, so that > pending output errors are detected. > > > git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk at 111643 > 91177308-0d34-0410-b5e6-96231b3b80d8 > > Certainly looks intentional. Comment in the code is: > > // Delete the file descriptor when the program exits, forcing error > // detection. If you don't want this behavior, don't use outs(). > > I don't really have an opinion on whether this is a good idea, but it > does suggest that any use of `outs()` in `lib/` is a bug. > > $ git grep -l 'outs()' -- lib/ > lib/ExecutionEngine/Interpreter/ExternalFunctions.cpp > lib/IR/GCOV.cpp > lib/Support/CommandLine.cpp > lib/Support/FormattedStream.cpp > lib/Support/TargetRegistry.cpp > lib/Support/raw_ostream.cpp > lib/Target/CppBackend/CPPBackend.cpp > > All of these cases seem to be for outputting help text of some sort. But > probably they should be passed a reference to a `raw_ostream`, so that the > caller gets to choose whether or not to use `outs()`. > > Were you hitting one of these? > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150123/f918db20/attachment.html>
So, I'm only "fixing the compiler code" (by setting `shouldClose
false`), but in essence the code we have is something like this (much
simplified):
int main(...)
{
printf("Some stuff\n");
...
use_clang_to_compile_opencl();
run_opencl();
..
printf("Some more stuff\n");
}
and then `myprog > file.txt`
In this case, clang and llvm do not output anything (as far as I'm
aware), but the "Some stuff" and "Some more stuff" does not
appear in
file.txt. Using strace, my colleague found the problem as `close(1)`
followed by `write(1, "...") = EBADF`.
[The ACTUAL code is a benchmark that is part of our test-code, and the
test is failing]
I'm fairly sure that whether llvm actually uses `outs` or not, it will
be constructed and destroyed based on it's existance. I will have a
look at this in more detail, but the general question still remains:
Should LLVM close `STDOUT_FILENO` in a destructor? I don't think so. If so,
why?
--
Mats
On 23 January 2015 at 18:20, Duncan P. N. Exon Smith
<dexonsmith at apple.com> wrote:>
>> On 2015-Jan-23, at 09:52, mats petersson <mats at
planetcatfish.com> wrote:
>>
>> I was just fixing a bug that was caused by `stdout` being closed
>> before the runtime has done `fflush(stdout)` [or however this is
>> implemented in the runtime].
>>
>> The cause of this seems to be that `outs()` returns a static object
>> created from `raw_fd_stream(STDOUT_FILENO, true)` - the `true` being
>> the `shouldClose` parameter.
>>
>> Surely LLVM is not supposed to close `stdout` as part of its
operations?
>
> Looks like this was added in r111643:
>
> commit 5d56d9df928c48571980efe8d4205de8ab557b7c
> Author: Dan Gohman <gohman at apple.com>
> Date: Fri Aug 20 16:44:56 2010 +0000
>
> Make outs() close its file when its stream is destructed, so that
> pending output errors are detected.
>
>
> git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk at 111643
91177308-0d34-0410-b5e6-96231b3b80d8
>
> Certainly looks intentional. Comment in the code is:
>
> // Delete the file descriptor when the program exits, forcing error
> // detection. If you don't want this behavior, don't use
outs().
>
> I don't really have an opinion on whether this is a good idea, but it
> does suggest that any use of `outs()` in `lib/` is a bug.
>
> $ git grep -l 'outs()' -- lib/
> lib/ExecutionEngine/Interpreter/ExternalFunctions.cpp
> lib/IR/GCOV.cpp
> lib/Support/CommandLine.cpp
> lib/Support/FormattedStream.cpp
> lib/Support/TargetRegistry.cpp
> lib/Support/raw_ostream.cpp
> lib/Target/CppBackend/CPPBackend.cpp
>
> All of these cases seem to be for outputting help text of some sort. But
> probably they should be passed a reference to a `raw_ostream`, so that the
> caller gets to choose whether or not to use `outs()`.
>
> Were you hitting one of these?