Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions bazel/emscripten_toolchain/crosstool.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,11 @@ def _impl(ctx):
# https://emscripten.org/docs/debugging/Sanitizers.html
feature(name = "wasm_asan"),
feature(name = "wasm_ubsan"),

feature(
name = "output_format_js",
enabled = True,
),
]

crosstool_default_flag_sets = [
Expand Down Expand Up @@ -547,6 +552,11 @@ def _impl(ctx):
flags = ["-s", "PRINTF_LONG_DOUBLE=1"],
features = ["precise_long_double_printf"],
),
flag_set(
actions = all_link_actions,
flags = ["--oformat=js"],
features = ["output_format_js"],
),

# Opt
flag_set(
Expand Down
73 changes: 48 additions & 25 deletions bazel/emscripten_toolchain/link_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,20 @@

This wrapper currently serves the following purposes.

1. Ensures we always link to file with .js extension. The upstream default
it to link to an llvm bitcode file which is never (AFAICT) want to do that.

2. When building with --config=wasm the final output is multiple files, usually
1. When building with --config=wasm the final output is multiple files, usually
at least one .js and one .wasm file. Since the cc_binary link step only
allows a single output, we must tar up the outputs into a single file.

3. Add quotes around arguments that need them in the response file to work
2. Add quotes around arguments that need them in the response file to work
around a bazel quirk.

3. Ensure the external_debug_info section of the wasm points at the correct
bazel path.
"""

from __future__ import print_function

import argparse
import os
import subprocess
import sys
Expand All @@ -25,20 +26,8 @@
param_filename = sys.argv[1][1:]
param_file_args = [l.strip() for l in open(param_filename, 'r').readlines()]

output_index = param_file_args.index('-o') + 1
orig_output = js_output = param_file_args[output_index]
outdir = os.path.dirname(orig_output)

# google3-only(TODO(b/139440956): Default to False once the bug is fixed)
replace_response_file = any(' ' in a for a in param_file_args)

if not os.path.splitext(orig_output)[1]:
js_output = orig_output + '.js'
param_file_args[output_index] = js_output
replace_response_file = True

# Re-write response file if needed.
if replace_response_file:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(unrelated) I wonder if we can completely remove this at some point... how does bazel + clang deal with this issue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an internal P3 bug filed with the bazel folks that we're tracking.

if any(' ' in a for a in param_file_args):
new_param_filename = param_filename + '.modified'
with open(new_param_filename, 'w') as f:
for param in param_file_args:
Expand All @@ -54,8 +43,41 @@
if rtn != 0:
sys.exit(1)

js_name = os.path.basename(js_output)
base_name = os.path.splitext(js_name)[0]
# Parse the arguments that we gave to the linker to determine what the output
# file is named and what the output format is.
parser = argparse.ArgumentParser(add_help=False)
parser.add_argument('-o')
parser.add_argument('--oformat')
options = parser.parse_known_args(param_file_args)[0]
output_file = options.o
oformat = options.oformat
outdir = os.path.dirname(output_file)
base_name = os.path.basename(output_file)

# The output file name is the name of the build rule that was built.
# Add an appropriate file extension based on --oformat.
if oformat is not None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If oformat is None don't we want to default to "js" since that is what emcc will do?

How about parser.add_argument('--oformat', default='js') above and then you can remove this condition?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is a bit overly cautious, but I prefer to keep it in there. If oformat is None, I'd rather emcc handle what to do by default, just in case emcc ever changes. Also, in the crosstool we're setting --oformat=js as a default, so realistically this won't ever be None.

base_name_split = os.path.splitext(base_name)

# If the output name has no extension, give it the appropriate extension.
if not base_name_split[1]:
os.rename(output_file, output_file + '.' + oformat)

# If the output name does have an extension and it matches the output format,
# change the base_name so it doesn't have an extension.
elif base_name_split[1] == '.' + oformat:
base_name = base_name_split[0]

# If the output name does have an extension and it does not match the output
# format, change the base_name so it doesn't have an extension and rename
# the output_file so it has the proper extension.
# Note that if you do something like name your build rule "foo.js" and pass
# "--oformat=html", emscripten will write to the same file for both the js and
# html output, overwriting the js output entirely with the html.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds like there is still a bug in emscripten?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly. Emscripten could ensure it's only writing to files it just created and throw an error. I can file a bug for later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# Please don't do that.
else:
base_name = base_name_split[0]
os.rename(output_file, os.path.join(outdir, base_name + '.' + oformat))

files = []
extensions = [
Expand All @@ -67,7 +89,8 @@
'.worker.js',
'.data',
'.js.symbols',
'.wasm.debug.wasm'
'.wasm.debug.wasm',
'.html'
]

for ext in extensions:
Expand Down Expand Up @@ -112,7 +135,7 @@
binary_part = '1' + binary_part
final_bytes.append(int(binary_part, 2))
# Finally, add the actual filename.
final_bytes.extend(base_name + '.wasm.debug.wasm')
final_bytes.extend((base_name + '.wasm.debug.wasm').encode())

# Write our length + filename bytes to a temp file.
with open('debugsection.tmp', 'wb+') as f:
Expand All @@ -134,11 +157,11 @@
if len(files) > 1:
cmd = ['tar', 'cf', 'tmp.tar'] + files
subprocess.check_call(cmd, cwd=outdir)
os.rename(os.path.join(outdir, 'tmp.tar'), orig_output)
os.rename(os.path.join(outdir, 'tmp.tar'), output_file)
elif len(files) == 1:
# Otherwise, if only have a single output than move it to the expected name
if files[0] != os.path.basename(orig_output):
os.rename(os.path.join(outdir, files[0]), orig_output)
if files[0] != os.path.basename(output_file):
os.rename(os.path.join(outdir, files[0]), output_file)
else:
print('emcc.py did not appear to output any known files!')
sys.exit(1)
Expand Down
1 change: 1 addition & 0 deletions bazel/emscripten_toolchain/wasm_binary.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ def main():
ensure(os.path.join(args.output_path, stem + '.fetch.js'))
ensure(os.path.join(args.output_path, stem + '.js.symbols'))
ensure(os.path.join(args.output_path, stem + '.wasm.debug.wasm'))
ensure(os.path.join(args.output_path, stem + '.html'))


if __name__ == '__main__':
Expand Down
2 changes: 2 additions & 0 deletions bazel/emscripten_toolchain/wasm_cc_binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ def _wasm_binary_impl(ctx):
ctx.outputs.data,
ctx.outputs.symbols,
ctx.outputs.dwarf,
ctx.outputs.html,
]

ctx.actions.run(
Expand Down Expand Up @@ -103,6 +104,7 @@ def _wasm_binary_outputs(name, cc_target):
"data": "{}/{}.data".format(name, basename),
"symbols": "{}/{}.js.symbols".format(name, basename),
"dwarf": "{}/{}.wasm.debug.wasm".format(name, basename),
"html": "{}/{}.html".format(name, basename),
}

return outputs
Expand Down