Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows Native mangles bytecode output using cout #1159

Closed
llvmbot opened this issue May 23, 2006 · 12 comments
Closed

Windows Native mangles bytecode output using cout #1159

llvmbot opened this issue May 23, 2006 · 12 comments
Labels
bugzilla Issues migrated from bugzilla portability tools:llvm-as

Comments

@llvmbot
Copy link
Member

llvmbot commented May 23, 2006

Bugzilla Link 787
Resolution FIXED
Resolved on Feb 22, 2010 12:45
Version 1.0
OS Windows XP
Reporter LLVM Bugzilla Contributor
CC @lattner

Extended Description

When using the Windows native build of version 1.6 (I've seen no sign this was
fixed in 1.7, although testing that is tricky since the win32 build in 1.7 is
pretty unstable) llvm-as output using cout uses text output, replacing newlines
with CR+LF characters, mangling the output.

Examples of this can be found in test case
Regression/Linker/2002-07-17-GlobalFail.ll where the following line has an error:
llvm-as < 2002-07-17-GlobalFail.ll > %s.bc
because the output is being written to stdout and redirected to %s.bc.
This also occurs when getting input from stdin (cin) in llc, lli, llvm-dis, and opt.

A solution to this has been discussed on the mailing list under the name "Binary
output to cout on Windows", and is reposted below, essentially changing the mode
on stdin and stdout to binary.

The code that needs to be run before input/output is done on stdin/stdout is:
int result = _setmode( _fileno(stdin), _O_BINARY );
if( result == -1 )
{ std::cerr<<"Cannot set input mode to binary."<<std::endl; return 1; }
result = _setmode( _fileno(stdout), _O_BINARY );
if( result == -1 )
{ std::cerr<<"Cannot set output mode to binary."<<std::endl; return 1; }
Error checking included. The files that need to be included for this code are
, <io.h>, and <fcntl.h>.

@llvmbot
Copy link
Member Author

llvmbot commented May 23, 2006

Thanks Michael,

I won't be able to get to this for a few days (probably this weekend). If you'd
like to submit some patches, here's basically what you need to do:

  1. Add a function to include/llvm/System/Program.h to set binary mode on a stream
  2. Implement that function in lib/System/Win32/Program.inc and
    lib/System/Unix/Program.inc
  3. Call that function in lib/Bytecode/Reader/Reader.cpp and
    lib/Bytecode/Writer/Writer.cpp
  4. Alternatively, call the function in the various tools//.cpp as appropriate.
    This should go in the main program.

I'm not sure if step 3 will work because of the lateness of the time at which
the reader or writer is invoked. Step 3 is the right place to put this in
because it will automatically affect all programs that read or write bytecode
files. However, it might not be technically feasible depending on the
requirements of the operating system (setting it before any I/O is done).

Thanks for filing this well written PR!

@llvmbot
Copy link
Member Author

llvmbot commented May 23, 2006

Adding the call in Bytecode/Writer/Writer.cpp WriteBytecodeToFile() and
Bytecode/Reader/ReaderWrappers.cpp BytecodeBufferReader::BytecodeBufferReader()
seems to work correctly.

@llvmbot
Copy link
Member Author

llvmbot commented Jun 5, 2006

Michael,

Since you have it working, could you provide the patch so we can commit the
change and resolve this bug?

Thanks,

Reid.

@llvmbot
Copy link
Member Author

llvmbot commented Jun 6, 2006

I meant to say BytecodeStdinReader::BytecodeStdinReader() rather than
Bytecode/Reader/ReaderWrappers.cpp BytecodeBufferReader::BytecodeBufferReader()

I downloaded the compressed source rather than getting it with CVS. I'm
retesting the change with a clean version of the source, then I'll submit a patch.

@llvmbot
Copy link
Member Author

llvmbot commented Jun 6, 2006

Sounds great. I look forward to closing this bug and solving this problem for
all win32 users.

Reid.

@llvmbot
Copy link
Member Author

llvmbot commented Jun 6, 2006

Patch to fix this bug.

@llvmbot
Copy link
Member Author

llvmbot commented Jun 6, 2006

Thanks for the patch. The output for the BytecodeWriter isn't quite correct. The
call will change std::cout but the bytecode writer might not be writing to
std::cout, so it needs to check. In any event, I'll use the patch and fix this
myself.

Thanks!

@llvmbot
Copy link
Member Author

llvmbot commented Jun 6, 2006

That shouldn't be an issue, since whenever you're not writing to std::cout,
you're writing to a stream you've created, and therefore have constructed with
the binary mode flag.

@llvmbot
Copy link
Member Author

llvmbot commented Jun 6, 2006

Nevermind, I misinterpreted what you were saying.

@llvmbot
Copy link
Member Author

llvmbot commented Jun 8, 2006

Michael,

Your patch has been applied, tested, and found to work on Unix. This bug is now
resolved. Could you please verify that the patch fixes the issue on your Windows
platform. If so, please mark this PR as "VERIFIED".

Thanks,

Reid.

Patches Applied:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060605/035455.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060605/035456.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060605/035457.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060605/035458.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20060605/035459.html

@llvmbot
Copy link
Member Author

llvmbot commented Jun 8, 2006

I have the patch working as you've entered it using the base 1.7 release.
However there seem to be errors in the Visual Studio build process in the latest
cvs version that prevent it from building.

@llvmbot
Copy link
Member Author

llvmbot commented Jun 8, 2006

Yeah, Jeff Cohen usually updates the win32 stuff but I haven't seen him around
lately. I'd do it but I don't have a win32 build environment. If you can update
the build stuff yourself for Visual Studio, we'll accept the patch. Or perhaps
you could get in contact with Jeff to update it.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla portability tools:llvm-as
Projects
None yet
Development

No branches or pull requests

1 participant