[llvm] r297624 - Fix crash when multiple raw_fd_ostreams to stdout are created.

[Rafael Avila de Espindola via llvm-commits]

I will revert.

Cheers,
Rafael

Bruno Cardoso Lopes <bruno.cardoso at gmail.com> writes:

> Hi Rafael,
>
> This is failing in
> http://green.lab.llvm.org/green/job/Verify-Machineinstrs_AArch64/5226,
> can you take a look?
>
> Thanks,
>
> On Mon, Mar 13, 2017 at 7:45 AM, Rafael Espindola via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>> Author: rafael
>> Date: Mon Mar 13 09:45:06 2017
>> New Revision: 297624
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=297624&view=rev
>> Log:
>> Fix crash when multiple raw_fd_ostreams to stdout are created.
>>
>> If raw_fd_ostream is constructed with the path of "-", it claims
>> ownership of the stdout file descriptor. This means that it closes
>> stdout when it is destroyed. If there are multiple users of
>> raw_fd_ostream wrapped around stdout, then a crash can occur because
>> of operations on a closed stream.
>>
>> An example of this would be running something like "clang -S -o - -MD
>> -MF - test.cpp". Alternatively, using outs() (which creates a local
>> version of raw_fd_stream to stdout) anywhere combined with such a
>> stream usage would cause the crash.
>>
>> The fix duplicates the stdout file descriptor when used within
>> raw_fd_ostream, so that only that particular descriptor is closed when
>> the stream is destroyed.
>>
>> Patch by James Henderson!
>>
>> Added:
>>     llvm/trunk/test/Other/writing-to-stdout.ll
>> Modified:
>>     llvm/trunk/lib/Support/raw_ostream.cpp
>>     llvm/trunk/unittests/Support/raw_ostream_test.cpp
>>
>> Modified: llvm/trunk/lib/Support/raw_ostream.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/raw_ostream.cpp?rev=297624&r1=297623&r2=297624&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Support/raw_ostream.cpp (original)
>> +++ llvm/trunk/lib/Support/raw_ostream.cpp Mon Mar 13 09:45:06 2017
>> @@ -473,7 +473,7 @@ static int getFD(StringRef Filename, std
>>      // possible.
>>      if (!(Flags & sys::fs::F_Text))
>>        sys::ChangeStdoutToBinary();
>> -    return STDOUT_FILENO;
>> +    return dup(STDOUT_FILENO);
>>    }
>>
>>    int FD;
>>
>> Added: llvm/trunk/test/Other/writing-to-stdout.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/writing-to-stdout.ll?rev=297624&view=auto
>> ==============================================================================
>> --- llvm/trunk/test/Other/writing-to-stdout.ll (added)
>> +++ llvm/trunk/test/Other/writing-to-stdout.ll Mon Mar 13 09:45:06 2017
>> @@ -0,0 +1,14 @@
>> +; Often LLVM tools use "-" to indicate that output should be written to stdout
>> +; instead of a file. This behaviour is implemented by the raw_fd_ostream class.
>> +; This test verifies that when doing so multiple times we don't try to access a
>> +; closed STDOUT_FILENO. The exact options used in this test are unimportant, as
>> +; long as they write to stdout using raw_fd_ostream.
>> +; RUN: llc %s -o=- -pass-remarks-output=- -filetype=asm | FileCheck %s
>> +; foobar should appear as a function somewhere in the assembly file.
>> +; CHECK: foobar
>> +; !Analysis appears at the start of pass-remarks-output.
>> +; CHECK: !Analysis
>> +
>> +define void @foobar() {
>> +  ret void
>> +}
>>
>> Modified: llvm/trunk/unittests/Support/raw_ostream_test.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/raw_ostream_test.cpp?rev=297624&r1=297623&r2=297624&view=diff
>> ==============================================================================
>> --- llvm/trunk/unittests/Support/raw_ostream_test.cpp (original)
>> +++ llvm/trunk/unittests/Support/raw_ostream_test.cpp Mon Mar 13 09:45:06 2017
>> @@ -9,6 +9,7 @@
>>
>>  #include "gtest/gtest.h"
>>  #include "llvm/ADT/SmallString.h"
>> +#include "llvm/Support/FileSystem.h"
>>  #include "llvm/Support/Format.h"
>>  #include "llvm/Support/raw_ostream.h"
>>
>> @@ -330,4 +331,11 @@ TEST(raw_ostreamTest, FormattedHexBytes)
>>              "0007: 68 69 6a 6b 6c        |hijkl|",
>>              format_bytes_with_ascii_str(B.take_front(12), 0, 7, 1));
>>  }
>> +
>> +TEST(raw_fd_ostreamTest, multiple_raw_fd_ostream_to_stdout) {
>> +  std::error_code EC;
>> +
>> +  { raw_fd_ostream("-", EC, sys::fs::OpenFlags::F_None); }
>> +  { raw_fd_ostream("-", EC, sys::fs::OpenFlags::F_None); }
>> +}
>>  }
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
>
> -- 
> Bruno Cardoso Lopes
> http://www.brunocardoso.cc