Wasm polyfill prototype #3766
Wasm polyfill prototype #3766
Conversation
Conflicts: src/deps_info.json tests/test_core.py
Conflicts: tests/test_browser.py
| @@ -931,6 +931,15 @@ Functions | |||
| :returns: A pointer to the preloaded image or NULL. | |||
| :rtype: char* | |||
|
|
|||
| .. c:function:: char *emscripten_print_double(double x, char *to) | |||
There was a problem hiding this comment.
I think we should always favor safe forms, and require here that as a third parameter, the number of bytes that this function can write (including null terminator) is passed to emscripten_print_double. That way accidental buffer overflows are reduced.
|
Looks fairly ok, I don't have much to review about the wasm packer, except that it seems to use assert()s for input validation(?), I don't see much error handling. The wasm packer will be a part of public facing tools, so I think it needs good error reporting to diagnose issues, but those can be improved as we go. Currently looks like the tests won't run on Windows because of the Makefile? |
|
All comments should be addressed or replied to. |
| @@ -216,8 +216,8 @@ def build(self, src, dirname, filename, output_processor=None, main_file=None, a | |||
| ['-I', dirname, '-I', os.path.join(dirname, 'include')] + \ | |||
| map(lambda include: '-I' + include, includes) + \ | |||
| ['-c', f, '-o', f + '.o'] | |||
| output = Popen(args, stdout=PIPE, stderr=self.stderr_redirect if not DEBUG else None).communicate()[0] | |||
| assert os.path.exists(f + '.o'), 'Source compilation error: ' + output | |||
| output = subprocess.check_call(args, stdout=PIPE, stderr=self.stderr_redirect if not DEBUG else None) | |||
There was a problem hiding this comment.
Reading a second pass, I notice that here and below, check_call is used with PIPE. That is not allowed, since it is subject to a deadlock: https://docs.python.org/2/library/subprocess.html . (I think this is dumb from python and python should throw an exception instead if check_call is used with PIPE. They are practically documenting their bugs as misfeatures.) Popen should be used here instead.
There was a problem hiding this comment.
I removed the PIPE - how is that? Seems cleaner to me. The only downside is we might get some clang frontend warnings, but if those are annoying, we can fix those on a per-test basis, tests shouldn't have warnings probably anyhow.
|
Thanks for bearing with my comments! lgtm with that one python |
|
I had some fun with the merge, but looks like all is good locally, pushed. |
Integrate @lukewagner's wasm polyfill prototype. This lets
emcc -s WASM=1emit the asm.js code compressed by the polyfill, and at runtime it decompresses it. This can reduce code size by around 30%.Aside from code compression, this may also allow us to more easily experiment with emitting proper WebAssembly, when that spec is finalized and a polyfill is available. This will depend on developments in WebAssembly which is still very much in progress, so it's hard to tell how this will work out, but this pull request should not be too invasive or risky to merge meanwhile, I think.
This adds some browser tests, of a non-trivial program using iostream. There is also an option for a full test mode in core (
asm2w) which passes, however I don't think it's worth the bot slowness, so it is off. If we start to work on the polyfill code though, we might want to turn it on.