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

emscripten linking with many dependencies broken on Windows #46999

Closed
Moxinilian opened this issue Dec 25, 2017 · 18 comments · Fixed by #47507
Closed

emscripten linking with many dependencies broken on Windows #46999

Moxinilian opened this issue Dec 25, 2017 · 18 comments · Fixed by #47507
Labels
C-bug Category: This is a bug. O-wasm Target: WASM (WebAssembly), http://webassembly.org/ O-windows Operating system: Windows

Comments

@Moxinilian
Copy link
Contributor

Hello.

I have been trying to compile a rather big project for wasm32-unknown-emscripten using cargo on Windows.
Unfortunately, the linking process fails when trying to run emcc.bat because the command line invoked is too long for Windows' limitations (about 8k characters).
error: linking with `emcc.bat` failed: exit code: 1
note: The command line is too long.

After manually counting, the command indeed is too big with in my case about 11k characters, as it includes the absolute path of every single .o and .rlib files. My project, while including piston as a dependency, does not have an incredibly large amount of them, which makes me believe this could be an important issue in the long run (this does not apply to Linux however as the maximum character limit usually is much higher).
After a little bit of digging, emscripten seems to provide a workaround for this problem by accepting parameters from files. However it has to be handled by the user.

Thanks in advance for your help.

@dataf3l
Copy link

dataf3l commented Dec 25, 2017

why not use a folder like c:\a so that it works?

@Moxinilian
Copy link
Contributor Author

Moxinilian commented Dec 25, 2017

Well yes, that would work in my specific case.
But do you really want that in a final product?

@ritiek
Copy link
Member

ritiek commented Dec 25, 2017

Maybe #46113 is related.

@alexcrichton
Copy link
Member

@Moxinilian can you provide the full error message? It looks like this may be a bug in emscripten, not in rustc.

@Moxinilian
Copy link
Contributor Author

Moxinilian commented Dec 26, 2017

@alexcrichton I uploaded the project I tried to compile to this repo
https://github.com/Moxinilian/emscripten_bug
log.txt contains the entire output

edit: i'm adding the verbose output too
edit: done

@kennytm kennytm added C-bug Category: This is a bug. O-wasm Target: WASM (WebAssembly), http://webassembly.org/ O-windows Operating system: Windows labels Dec 26, 2017
@alexcrichton
Copy link
Member

Ah ok thanks for the extra info @Moxinilian!

So for a bit of background, rustc does indeed have safeguards in place for reinvoking the linker if necessary when the command line is deemed too large. What's happening here, though, is that the error is happening at the wrong time.

The current detection relies on the spawning of the process itself to fail (aka the OS tells us about the failure) but it looks like cmd at least is succeeding. Some spawned process actually prints out The command line is too long but it's not clear to me whether that's cmd or emcc.bat.

@Moxinilian
Copy link
Contributor Author

@alexcrichton Is there any additional test you'd want me to run to try to figure this out?

@alexcrichton
Copy link
Member

If you'd like to debug further that'd be great. We'd first need to understand what's actually failing here (cmd or something in emscripten). Once that's known we could figure out if it's reasonable for us to catch the error and do something about it.

@Moxinilian
Copy link
Contributor Author

Moxinilian commented Dec 26, 2017

@alexcrichton So I edited emcc.bat to print a dummy message when it gets called.
As the message did not show up, I came to the conclusion that cmd itself refuses to run the command.
I tried to run it with a shorter command and the message showed up.

This is surprising because that means rust would have failed to survive cmd's refusal.
I'm not an OS expert so I'm not sure what raw_os_error actually returns, but here cmd's return code is 1.

@alexcrichton
Copy link
Member

Ok so sounds like we successfully spawn a cmd process and then cmd fails to spawn its own subprocess. That's going to be much trickier to handle an catch, but it at least sounds like what's happening here at least.

@Moxinilian
Copy link
Contributor Author

A dirty way to catch it would be to simply check the error description. At least it is self explanatory so there won't be any false positive.

@Moxinilian
Copy link
Contributor Author

Another way I just thought of would be to try to summon emcc.bat without cmd, maybe trying to run it using something similar to C's system("emcc.bat args");. Maybe this would allow better handling.

@VictorKoenders
Copy link

VictorKoenders commented Jan 2, 2018

Assuming emcc.bat and em++ are working in a similar matter, this issue seems to provide a solution. We can simply write all the command parameters to an .rsp file, and then pass that file to the emcc.bat

@Moxinilian
Copy link
Contributor Author

Moxinilian commented Jan 2, 2018

Honestly, I believe doing this approach by default would be the best solution.
The performance overhead is negligible when it's only about linking, and this would cover all future problems.
It's simply a matter of deleting this match.
I can not try it right now but if anyone is interested to remove it and see if it works.

@projektir
Copy link
Contributor

Writing the arguments to file looks like a good solution here, yeah.

@forbjok
Copy link

forbjok commented Jan 8, 2018

I'm running into this issue as well when trying to use the cargo-web crate. Any ETA on this being fixed in nightly?

@Moxinilian
Copy link
Contributor Author

If nobody does it before I do, I'll make file parameters default behavior, check if nothing breaks and make a pull request by Sunday.

@alexcrichton
Copy link
Member

I've posted a PR for this at #47507

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jan 20, 2018
When spawning a linker rustc has historically been known to blow OS limits for
the command line being too large, notably on Windows. This is especially true of
incremental compilation where there can be dozens of object files per
compilation. The compiler currently has logic for detecting a failure to spawn
and instead passing arguments via a file instead, but this failure detection
only triggers if a process actually fails to spawn.

Unfortunately on Windows we've got something else to worry about which is
`cmd.exe`. The compiler may be running a linker through `cmd.exe` where
`cmd.exe` has a limit of 8192 on the command line vs 32k on `CreateProcess`.
Moreso rustc actually succeeds in spawning `cmd.exe` today, it's just that after
it's running `cmd.exe` fails to spawn its child, which rustc doesn't currently
detect.

Consequently this commit updates the logic for the spawning the linker on
Windows to instead have a heuristic to see if we need to pass arguments via a
file. This heuristic is an overly pessimistic and "inaccurate" calculation which
just calls `len` on a bunch of `OsString` instances (where `len` is not
precisely the length in u16 elements). This number, when exceeding the 6k
threshold, will force rustc to always pass arguments through a file.

This strategy should avoid us trying to parse the output on Windows of the
linker to see if it successfully spawned yet failed to actually sub-spawn the
linker. We may just be passing arguments through files a little more commonly
now...

The motivation for this commit was a recent bug in Gecko [1] when beta testing,
notably when incremental compilation was enabled it blew out the limit on
`cmd.exe`. This commit will also fix rust-lang#46999 as well though as emscripten uses a
bat script as well (and we're blowing the limit there).

[1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1430886

Closes rust-lang#46999
bors added a commit that referenced this issue Jan 21, 2018
…ster

rustc: Lower link args to `@`-files on Windows more

When spawning a linker rustc has historically been known to blow OS limits for
the command line being too large, notably on Windows. This is especially true of
incremental compilation where there can be dozens of object files per
compilation. The compiler currently has logic for detecting a failure to spawn
and instead passing arguments via a file instead, but this failure detection
only triggers if a process actually fails to spawn.

Unfortunately on Windows we've got something else to worry about which is
`cmd.exe`. The compiler may be running a linker through `cmd.exe` where
`cmd.exe` has a limit of 8192 on the command line vs 32k on `CreateProcess`.
Moreso rustc actually succeeds in spawning `cmd.exe` today, it's just that after
it's running `cmd.exe` fails to spawn its child, which rustc doesn't currently
detect.

Consequently this commit updates the logic for the spawning the linker on
Windows to instead have a heuristic to see if we need to pass arguments via a
file. This heuristic is an overly pessimistic and "inaccurate" calculation which
just calls `len` on a bunch of `OsString` instances (where `len` is not
precisely the length in u16 elements). This number, when exceeding the 6k
threshold, will force rustc to always pass arguments through a file.

This strategy should avoid us trying to parse the output on Windows of the
linker to see if it successfully spawned yet failed to actually sub-spawn the
linker. We may just be passing arguments through files a little more commonly
now...

The motivation for this commit was a recent bug in Gecko [1] when beta testing,
notably when incremental compilation was enabled it blew out the limit on
`cmd.exe`. This commit will also fix #46999 as well though as emscripten uses a
bat script as well (and we're blowing the limit there).

[1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1430886

Closes #46999
alexcrichton added a commit to alexcrichton/rust that referenced this issue Jan 22, 2018
When spawning a linker rustc has historically been known to blow OS limits for
the command line being too large, notably on Windows. This is especially true of
incremental compilation where there can be dozens of object files per
compilation. The compiler currently has logic for detecting a failure to spawn
and instead passing arguments via a file instead, but this failure detection
only triggers if a process actually fails to spawn.

Unfortunately on Windows we've got something else to worry about which is
`cmd.exe`. The compiler may be running a linker through `cmd.exe` where
`cmd.exe` has a limit of 8192 on the command line vs 32k on `CreateProcess`.
Moreso rustc actually succeeds in spawning `cmd.exe` today, it's just that after
it's running `cmd.exe` fails to spawn its child, which rustc doesn't currently
detect.

Consequently this commit updates the logic for the spawning the linker on
Windows to instead have a heuristic to see if we need to pass arguments via a
file. This heuristic is an overly pessimistic and "inaccurate" calculation which
just calls `len` on a bunch of `OsString` instances (where `len` is not
precisely the length in u16 elements). This number, when exceeding the 6k
threshold, will force rustc to always pass arguments through a file.

This strategy should avoid us trying to parse the output on Windows of the
linker to see if it successfully spawned yet failed to actually sub-spawn the
linker. We may just be passing arguments through files a little more commonly
now...

The motivation for this commit was a recent bug in Gecko [1] when beta testing,
notably when incremental compilation was enabled it blew out the limit on
`cmd.exe`. This commit will also fix rust-lang#46999 as well though as emscripten uses a
bat script as well (and we're blowing the limit there).

[1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1430886

Closes rust-lang#46999
bors added a commit that referenced this issue Jan 22, 2018
…ster

rustc: Lower link args to `@`-files on Windows more

When spawning a linker rustc has historically been known to blow OS limits for
the command line being too large, notably on Windows. This is especially true of
incremental compilation where there can be dozens of object files per
compilation. The compiler currently has logic for detecting a failure to spawn
and instead passing arguments via a file instead, but this failure detection
only triggers if a process actually fails to spawn.

Unfortunately on Windows we've got something else to worry about which is
`cmd.exe`. The compiler may be running a linker through `cmd.exe` where
`cmd.exe` has a limit of 8192 on the command line vs 32k on `CreateProcess`.
Moreso rustc actually succeeds in spawning `cmd.exe` today, it's just that after
it's running `cmd.exe` fails to spawn its child, which rustc doesn't currently
detect.

Consequently this commit updates the logic for the spawning the linker on
Windows to instead have a heuristic to see if we need to pass arguments via a
file. This heuristic is an overly pessimistic and "inaccurate" calculation which
just calls `len` on a bunch of `OsString` instances (where `len` is not
precisely the length in u16 elements). This number, when exceeding the 6k
threshold, will force rustc to always pass arguments through a file.

This strategy should avoid us trying to parse the output on Windows of the
linker to see if it successfully spawned yet failed to actually sub-spawn the
linker. We may just be passing arguments through files a little more commonly
now...

The motivation for this commit was a recent bug in Gecko [1] when beta testing,
notably when incremental compilation was enabled it blew out the limit on
`cmd.exe`. This commit will also fix #46999 as well though as emscripten uses a
bat script as well (and we're blowing the limit there).

[1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1430886

Closes #46999
MaloJaffre pushed a commit to MaloJaffre/rust that referenced this issue Jan 23, 2018
When spawning a linker rustc has historically been known to blow OS limits for
the command line being too large, notably on Windows. This is especially true of
incremental compilation where there can be dozens of object files per
compilation. The compiler currently has logic for detecting a failure to spawn
and instead passing arguments via a file instead, but this failure detection
only triggers if a process actually fails to spawn.

Unfortunately on Windows we've got something else to worry about which is
`cmd.exe`. The compiler may be running a linker through `cmd.exe` where
`cmd.exe` has a limit of 8192 on the command line vs 32k on `CreateProcess`.
Moreso rustc actually succeeds in spawning `cmd.exe` today, it's just that after
it's running `cmd.exe` fails to spawn its child, which rustc doesn't currently
detect.

Consequently this commit updates the logic for the spawning the linker on
Windows to instead have a heuristic to see if we need to pass arguments via a
file. This heuristic is an overly pessimistic and "inaccurate" calculation which
just calls `len` on a bunch of `OsString` instances (where `len` is not
precisely the length in u16 elements). This number, when exceeding the 6k
threshold, will force rustc to always pass arguments through a file.

This strategy should avoid us trying to parse the output on Windows of the
linker to see if it successfully spawned yet failed to actually sub-spawn the
linker. We may just be passing arguments through files a little more commonly
now...

The motivation for this commit was a recent bug in Gecko [1] when beta testing,
notably when incremental compilation was enabled it blew out the limit on
`cmd.exe`. This commit will also fix rust-lang#46999 as well though as emscripten uses a
bat script as well (and we're blowing the limit there).

[1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1430886

Closes rust-lang#46999

(cherry picked from commit 66366f9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-wasm Target: WASM (WebAssembly), http://webassembly.org/ O-windows Operating system: Windows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants