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

detect_windows_bitness_inconsistency #1958

Closed

Conversation

juj
Copy link
Collaborator

@juj juj commented Dec 30, 2013

Improve error reporting when emscripten link step fails. In particular on Windows detect wrong bitness combinations of the toolchain that can cause python -> llvm-link invocation to fail on Windows with a 'Error 6: handle is invalid' error message.

…r on Windows detect wrong bitness combinations of the toolchain that can cause python -> llvm-link invocation to fail on Windows with a 'Error 6: handle is invalid' error message.
@kripken
Copy link
Member

kripken commented Dec 30, 2013

I'm curious, what's going on here? Seems like if we link proper bitcode files, it shouldn't matter what bitness the llvm executables are? And why does it matter what bitness python is, if it runs llvm in a different process?

@juj
Copy link
Collaborator Author

juj commented Dec 30, 2013

I don't know what happens here. If I install latest 32bit python and 64-bit clang, that line always throws an exception. Falling back to 32-bit clang or using 64bit python fixes the issue, so I wanted to document and decode that error message to a more friendly one.

@kripken
Copy link
Member

kripken commented Dec 30, 2013

I am worried about pushing this without us understanding what's going on
first though - seems like a bandaid. Unless you feel there is value in
pushing it meanwhile?

On Mon, Dec 30, 2013 at 10:38 AM, juj notifications@github.com wrote:

I don't know what happens here. If I install latest 32bit python and
64-bit clang, that line always throws an exception. Falling back to 32-bit
clang or using 64bit python fixes the issue, so I wanted to document and
decode that error message to a more friendly one.


Reply to this email directly or view it on GitHubhttps://github.com//pull/1958#issuecomment-31360613
.

@juj
Copy link
Collaborator Author

juj commented Dec 31, 2013

Even though this commit doesn't fix anything, I think this is extremely helpful, since now anyone getting this error will immediately get acknowledgement that the 'team' has seen this error as well, as well as give a suggestion of how to proceed to resolve it, and a diagnostic if they're running a mixed-bitness toolchain. The commit is careful to not hide the original error message, the exception is still printed.

That said, if you still feel that it's too sketchy, then we can leave this out.

@kripken
Copy link
Member

kripken commented Dec 31, 2013

Let's see what @vvuk has to say on this, he has experience with this kind of thing.

@vvuk
Copy link
Contributor

vvuk commented Dec 31, 2013

Odd. 32-bit processes can launch 64-bit ones and vice versa; they can't link the other-bitness libraries and the like, as you would expect. But I wonder if python is trying to launch llvm-link in such a way that causes problems? @juj can you see if you can make a standalone .py script that launches llvm-link in the same way, that causes the problem with a 32-bit python/64-bit llvm? Would be easier to diagnose that way.

@juj
Copy link
Collaborator Author

juj commented Jan 1, 2014

This issue seems to only arise when Visual Studio spawns emcc. Calling it manually from command line does not reproduce the error. The 'handle is invalid' makes it feel like it is somehow related to the processes sharing piped stdout and stderr handles to each other, but it does not affect whether I use the EM_POPEN_WORKAROUND or adjust the popen pipes. Creating a script llvm.py that simply contains

import subprocess
subprocess.Popen(['C:/path/to/llvm-link']).communicate()

and routing emcc.bat to

@echo off
python "%~dp0\llvm.py" %*

reproduces the error. Spawned via Visual Studio, it gives

1>------ Build started: Project: MathGeoLib, Configuration: Debug Emscripten ------
1>Build started 1/1/2014 10:05:22 PM.
1>InitializeBuildStatus:
1>  Touching "MathGeoLib.dir\Debug\MathGeoLib.unsuccessfulbuild".
1>CustomBuild:
1>  All outputs are up-to-date.
1>ClCompile:
1>  All outputs are up-to-date.
1>Lib:
1>  
1>  C:\Projects\Samples_builds\clearscreen_emcc_vs2010\tmp\MathGeoLib>python "C:\Projects\emsdk\emscripten\incoming\\llvm.py" @C:\Users\clb\AppData\Local\Temp\85270735e7c4452d845c3667397ef036.rsp 
1>  Traceback (most recent call last):
1>    File "C:\Projects\emsdk\emscripten\incoming\\llvm.py", line 3, in <module>
1>      subprocess.Popen(['C:/Projects/emsdk/clang/3.2_64bit/bin/llvm-link']).communicate()
1>    File "C:\Projects\emsdk\python\2.7.5.3_32bit\lib\subprocess.py", line 711, in __init__
1>      errread, errwrite)
1>    File "C:\Projects\emsdk\python\2.7.5.3_32bit\lib\subprocess.py", line 948, in _execute_child
1>      startupinfo)
1>  WindowsError: [Error 6] The handle is invalid
1>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\Platforms\Emscripten\Microsoft.Cpp.Emscripten.Targets(69,5): error MSB6006: "C:\Projects\emsdk\emscripten\incoming\emcc.bat" exited with code 1.
1>
1>Build FAILED.
1>
1>Time Elapsed 00:00:00.24
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

but spawned from command line is ok:

C:\Projects\emsdk\emscripten\incoming>emcc.bat

C:\Projects\emsdk\emscripten\incoming>python "C:\Projects\emsdk\emscripten\incoming\\llvm.py"
llvm-link: Not enough positional command line arguments specified!
Must specify at least 1 positional arguments: See: C:/Projects/emsdk/clang/3.2_64bit/bin/llvm-link -help

Using python 2.7.5.3 64-bit instead of 2.7.5.3 32-bit succeeds, or using llvm 3.2 32-bit instead of llvm 64-bit succeeds.

@vvuk
Copy link
Contributor

vvuk commented Jan 1, 2014

I would modify subprocess.py to dump the creationflags and startupinfo struct, at the point where CreateProcess is called since that's what causing the error. Then compare the two (VS vs. from command line). The CreateProcess docs have this bit of info:

The caller is responsible for ensuring that the standard handle fields in STARTUPINFO contain valid handle values. These fields are copied unchanged to the child process without validation, even when the dwFlags member specifies STARTF_USESTDHANDLES. Incorrect values can cause the child process to misbehave or crash

Though the error is implying some validation; but it's some arg that's being passed in to that call that's causing the problem. I'd also inspect hStdInput/Output/Error and see if they're valid. There might be security issues at play too, somehow.

@juj
Copy link
Collaborator Author

juj commented Jan 1, 2014

Thanks, let me close this pr in interim.

@juj juj closed this Jan 1, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants