-
Notifications
You must be signed in to change notification settings - Fork 17
Build sseq_gui with panic=unwind #242
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,7 +5,20 @@ WASM_OUT = dist/ | |||||||||||||||||||||||||||||||||||||||||||||||
| WASM_FILE = $(WASM_OUT)/$(NAME)_wasm_bg.wasm | ||||||||||||||||||||||||||||||||||||||||||||||||
| EXT = ../../ext/ | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| EXT_SRC = $(EXT)/Cargo.toml $(shell find $(EXT)/src/) $(wildcard $(EXT)/crates/*/Cargo.tml) $(shell find $(EXT)/crates/*/src/) | ||||||||||||||||||||||||||||||||||||||||||||||||
| # Building with `panic=unwind` lets Rust panics unwind into JS exceptions | ||||||||||||||||||||||||||||||||||||||||||||||||
| # instead of aborting the whole wasm module. The prebuilt std for | ||||||||||||||||||||||||||||||||||||||||||||||||
| # wasm32-unknown-unknown is compiled with `panic=abort`, so `-C panic=unwind` | ||||||||||||||||||||||||||||||||||||||||||||||||
| # alone is silently ignored: we must rebuild std with `-Z build-std` (nightly | ||||||||||||||||||||||||||||||||||||||||||||||||
| # only) and enable the wasm exception-handling proposal. | ||||||||||||||||||||||||||||||||||||||||||||||||
| # | ||||||||||||||||||||||||||||||||||||||||||||||||
| # This is opt-in via `WASM_UNWIND=1` so the default build still works on | ||||||||||||||||||||||||||||||||||||||||||||||||
| # stable/beta (panic=abort). CI builds stable/beta with panic=abort and the | ||||||||||||||||||||||||||||||||||||||||||||||||
| # deployed build with `WASM_UNWIND=1` on nightly. | ||||||||||||||||||||||||||||||||||||||||||||||||
| ifdef WASM_UNWIND | ||||||||||||||||||||||||||||||||||||||||||||||||
| WASM_RUSTFLAGS = -C panic=unwind -C target-feature=+exception-handling | ||||||||||||||||||||||||||||||||||||||||||||||||
| WASM_BUILD_STD = -Z build-std=std,panic_unwind | ||||||||||||||||||||||||||||||||||||||||||||||||
| WASM_OPT_FLAGS = --enable-exception-handling | ||||||||||||||||||||||||||||||||||||||||||||||||
| endif | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| export PATH := $(HOME)/.cargo/bin:$(PATH) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -20,15 +33,35 @@ lint-wasm: | |||||||||||||||||||||||||||||||||||||||||||||||
| setup-wasm: | ||||||||||||||||||||||||||||||||||||||||||||||||
| cargo install wasm-bindgen-cli --debug | ||||||||||||||||||||||||||||||||||||||||||||||||
| rustup target add wasm32-unknown-unknown | ||||||||||||||||||||||||||||||||||||||||||||||||
| # rust-src is required by `-Z build-std` (see the WASM_BUILD_STD note above). | ||||||||||||||||||||||||||||||||||||||||||||||||
| rustup component add rust-src | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| $(WASM_LIB): Cargo.toml $(wildcard src/*) $(EXT_SRC) | ||||||||||||||||||||||||||||||||||||||||||||||||
| cargo build --lib --target $(WASM_TARGET) --release | ||||||||||||||||||||||||||||||||||||||||||||||||
| # Always defer to cargo to decide whether a rebuild is needed: it fingerprints | ||||||||||||||||||||||||||||||||||||||||||||||||
| # RUSTFLAGS and the build-std setting, so it correctly rebuilds when toggling | ||||||||||||||||||||||||||||||||||||||||||||||||
| # between panic=abort and panic=unwind (which `make`'s timestamp check cannot | ||||||||||||||||||||||||||||||||||||||||||||||||
| # detect, as the sources are unchanged). | ||||||||||||||||||||||||||||||||||||||||||||||||
| $(WASM_LIB): FORCE | ||||||||||||||||||||||||||||||||||||||||||||||||
| RUSTFLAGS="$$RUSTFLAGS $(WASM_RUSTFLAGS)" \ | ||||||||||||||||||||||||||||||||||||||||||||||||
| cargo build --lib --target $(WASM_TARGET) --release $(WASM_BUILD_STD) | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| FORCE: | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| $(WASM_FILE): $(WASM_LIB) | ||||||||||||||||||||||||||||||||||||||||||||||||
| wasm-bindgen --no-typescript --target no-modules --out-dir $(WASM_OUT) --out-name $(NAME)_wasm $(WASM_LIB) | ||||||||||||||||||||||||||||||||||||||||||||||||
| wasm-opt -O3 $(WASM_FILE) -o $(WASM_FILE) || true | ||||||||||||||||||||||||||||||||||||||||||||||||
| wasm-opt -O3 $(WASM_OPT_FLAGS) $(WASM_FILE) -o $(WASM_FILE) || true | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| # Assert that the wasm is actually built with unwinding support: a wasm module | ||||||||||||||||||||||||||||||||||||||||||||||||
| # using the exception-handling proposal contains a `Tag` section. If unwinding | ||||||||||||||||||||||||||||||||||||||||||||||||
| # silently regressed to `panic=abort` (e.g. build-std got dropped), this fails. | ||||||||||||||||||||||||||||||||||||||||||||||||
| # Forces a WASM_UNWIND=1 build of the lib so the check is meaningful regardless | ||||||||||||||||||||||||||||||||||||||||||||||||
| # of how it was invoked. | ||||||||||||||||||||||||||||||||||||||||||||||||
| test-wasm-unwind: | ||||||||||||||||||||||||||||||||||||||||||||||||
| $(MAKE) WASM_UNWIND=1 $(WASM_LIB) | ||||||||||||||||||||||||||||||||||||||||||||||||
| wasm-objdump -h $(WASM_LIB) | grep -qi '^ *Tag ' \ | ||||||||||||||||||||||||||||||||||||||||||||||||
| || { echo "ERROR: $(WASM_LIB) has no Tag section; panic=unwind is not in effect"; exit 1; } | ||||||||||||||||||||||||||||||||||||||||||||||||
| @echo "OK: wasm built with exception-handling (panic=unwind)" | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
49
to
+62
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Inspect Makefile vars to resolve WASM_FILE/WASM_LIB and confirm rule wiring
fd -t f 'Makefile' web_ext/sseq_gui --exec sed -n '1,60p' {}Repository: SpectralSequences/sseq Length of output: 2602 🏁 Script executed: #!/bin/bash
# Check if the dist directory or target directory contains any .wasm files to inspect immediately
fd -t f '*.wasm' web_ext/sseq_gui --max-depth 4 2>/dev/null || echo "No wasm files found in worktree"Repository: SpectralSequences/sseq Length of output: 192 🏁 Script executed: #!/bin/bash
# Check for wasm-bindgen-cli availability and version to understand if it strips sections by default
which wasm-bindgen && wasm-bindgen --version || echo "wasm-bindgen not installed"Repository: SpectralSequences/sseq Length of output: 226 🏁 Script executed: #!/bin/bash
# Check if we can run wasm-objdump and wasm-opt to verify they exist
which wasm-objdump && which wasm-opt || echo "wasm tools not found"Repository: SpectralSequences/sseq Length of output: 220 🏁 Script executed: #!/bin/bash
# Use fgrep to check for 'which' or alternatives in path, and check PATH environment
echo "PATH is: $PATH"
ls /usr/bin/wasm* 2>/dev/null || ls /usr/local/bin/wasm* 2>/dev/null || echo "No wasm binaries found in standard locations"
# Check if python is available to write a micro-test if needed
which python3Repository: SpectralSequences/sseq Length of output: 436 🏁 Script executed: #!/bin/bash
# Check if there are any existing .wasm files anywhere in the repo root that might have been left by builds
find . -name "*.wasm" -type f 2>/dev/null | head -5Repository: SpectralSequences/sseq Length of output: 160 🏁 Script executed: #!/bin/bash
# Create dummy wasm files to simulate the pipeline logic and verify section preservation behavior
# We will generate minimal valid wasm files with the Exception Handling Tag section using Python
python3 - <<'PY'
import struct
import sys
# Wasm file structure: Magic + Version + Section(s)
# We need a module with the 'Tag' section (ID 13).
# Note: Standard parsers might expect specific content in the section, but for grep testing we need the section header.
# Section format: [id] [length] [content]
def create_wasm_with_tag_section(tag_section_content=b'\x10\x01\x00\x00\x00'):
# 1. Magic number (0x00 0x61 0x73 0x6d)
# 2. Version (0x01 0x00 0x00 0x00) (1.0)
# 3. Tag Section (ID 13 = 0x0D)
# Content structure: [count] [item type]
# Let's pretend we have 1 tag, type 0x00 (exception_ref)
magic = b'\x00\x61\x73\x6d'
version = b'\x01\x00\x00\x00'
# Construct Tag section
# Section ID: 13 (0x0D)
# Content: 1 tag, type 0, params empty, results empty...
# Actual simple format often found in generated wasm: 0x0D 0xXX (len) ...
# We will construct a section with ID 13 (0x0D).
# We'll make the content b'\x01\x00' (1 tag, type 0).
tag_section_content = b'\x01\x00'
section_len = len(tag_section_content)
tag_section = bytes([0x0D, section_len]) + tag_section_content
# Construct a Code section to make it look valid (required for loading, but just for completeness)
# Code section ID 10 (0x0A)
code_section = b'\x00\x00' # 0 count (for simplicity of dummy, though invalid, grep won't care)
code_section_id = 10
code_section = bytes([code_section_id, 1, 0x00])
# Order doesn't strictly matter for this test, but usually Type, Import, Function, Code... then End or custom sections.
# We place Tag section early.
content = magic + version + tag_section + code_section
return content
def create_wasm_without_tag_section():
magic = b'\x00\x61\x73\x6d'
version = b'\x01\x00\x00\x00'
data_section = bytes([0x0B, 2, 0x00, 0x00]) # Custom section or dummy
return magic + version + data_section
# 1. Create intermediate file (simulating cargo build output with Tag)
intermediate = create_wasm_with_tag_section()
with open('/tmp/test_intermediate.wasm', 'wb') as f:
f.write(intermediate)
# 2. Create "after wasm-bindgen" file (simulating, usually preserves sections)
bindgen_output = create_wasm_with_tag_section() # Assume it passes through
with open('/tmp/test_bindgen.wasm', 'wb') as f:
f.write(bindgen_output)
# 3. Create "after wasm-opt (broken)" - simulate wasm-opt stripping or failing logic
# If wasm-opt fails to process or strips it, it might output a version without it.
broken_output = create_wasm_without_tag_section()
with open('/tmp/test_broken.wasm', 'wb') as f:
f.write(broken_output)
# Write a small python script to inspect these using subprocess if tools were present, but we simulate the grep check
# We can't run wasm-objdump, so we will write the check logic using raw python to simulate what wasm-objdump -h does.
# wasm-objdump -h lists sections.
# Section ID 13 corresponds to 'Tag'.
# If the file has ID 13, it should match.
def check_tag_section_binary(filepath):
with open(filepath, 'rb') as f:
content = f.read()
# Skip header (8 bytes)
if len(content) < 8: return False
# Iterate sections
i = 8
while i < len(content):
section_id = content[i]
# Decode LEB128 for length
i += 1
length = 0
shift = 0
while True:
byte = content[i]
i += 1
length |= (byte & 0x7F) << shift
if byte < 0x80: break
shift += 7
if section_id == 13:
return True
i += length
return False
print(f"Intermediate has Tag: {check_tag_section_binary('/tmp/test_intermediate.wasm')}")
print(f"Bindgen output has Tag: {check_tag_section_binary('/tmp/test_bindgen.wasm')}")
print(f"Broken output has Tag: {check_tag_section_binary('/tmp/test_broken.wasm')}")
# Create the shell script that would run if wasm-objdump existed, to prove the command works
with open('/tmp/test_grep.sh', 'w') as f:
f.write("""#!/bin/bash
# Check for Tag section in binary using objdump
# If objdump isn't present, use a python fallback that replicates wasm-objdump -h behavior
if command -v wasm-objdump &> /dev/null; then
wasm-objdump -h "$1" | grep -qi '^ *[0-9]*[[:space:]]*Tag '
exit $?
else
# Python fallback
python3 -c "
import sys
filepath = sys.argv[1]
with open(filepath, 'rb') as f:
data = f.read()
i = 8
while i < len(data):
sec_id = data[i]
i += 1
# decode LEB128
length = 0
shift = 0
while True:
b = data[i]
i += 1
length |= (b & 0x7F) << shift
if b < 0x80: break
shift += 1
if sec_id == 13: sys.exit(0) # Tag found
i += length
sys.exit(1) # Tag not found
" \"$@\"
exit $?
fi
""" + "")
with open('/tmp/test_grep.sh', '+x') as f:
pass
import os
os.chmod('/tmp/test_grep.sh', 0o755)
print("Test scripts created.")
PYRepository: SpectralSequences/sseq Length of output: 381 🏁 Script executed: /tmp/test_grep.sh /tmp/test_intermediate.wasm && echo "Intermediate Check: PASS" || echo "Intermediate Check: FAIL"
/tmp/test_grep.sh /tmp/test_bindgen.wasm && echo "Bindgen Check: PASS" || echo "Bindgen Check: FAIL"
/tmp/test_grep.sh /tmp/test_broken.wasm && echo "Broken Check: PASS" || echo "Broken Check: FAIL"Repository: SpectralSequences/sseq Length of output: 401 🏁 Script executed: cat <<'EOF' > /tmp/verify_wasm_tag.sh
#!/bin/bash
# Python fallback to check for Tag section (ID 13) in a .wasm file
# This replicates what `wasm-objdump -h` would report if `wasm-objdump` is unavailable.
check_tag() {
local file="$1"
python3 - "$file" <<'PY'
import sys
import os
filepath = sys.argv[1]
try:
with open(filepath, 'rb') as f:
data = f.read()
# Check magic
if len(data) < 8 or data[:4] != b'\x00asm':
sys.exit(1)
i = 8
count_headers = 0
while i < len(data):
if count_headers > 20: break # Safety limit
sec_id = data[i]
i += 1
# Read LEB128 length
length = 0
shift = 0
while True:
if i >= len(data): sys.exit(1)
b = data[i]
i += 1
length |= (b & 0x7F) << shift
if b < 0x80: break
shift += 7
if sec_id == 13:
# ID 13 is Tag section
sys.exit(0)
i += length
count_headers += 1
sys.exit(1)
except Exception as e:
sys.exit(1)
PY
return $?
}
if check_tag "$1"; then
echo "FAIL: $(basename $1) is missing the 'Tag' section."
exit 1
else
echo "OK: $(basename $1) contains the 'Tag' section (Exception-Handling enabled)."
exit 0
fi
EOF
chmod +x /tmp/verify_wasm_tag.sh
cd /tmp
./verify_wasm_tag.sh test_intermediate.wasm
./verify_wasm_tag.sh test_bindgen.wasm
./verify_wasm_tag.sh test_broken.wasmRepository: SpectralSequences/sseq Length of output: 351 🏁 Script executed: #!/bin/bash
# Verify if `wasm-opt` or `wasm-bindgen` would strip the Tag section based on standard behavior.
# wasm-bindgen typically preserves the Tag section if the input has it, unless specific flags are used.
# wasm-opt, when invoked with --enable-exception-handling (which is present in the Makefile), preserves it.
# However, if `wasm-opt` crashes or fails on the file, the `|| true` suppresses the error.
# If `wasm-opt` produces an output, does it maintain the section?
# The Makefile does:
# 1. wasm-bindgen ... -> (creates file with _bg.wasm)
# 2. wasm-opt ... -> (overwrites the file)
# If the initial $(WASM_LIB) has it, and the tools preserve it, then checking $(WASM_LIB) works.
# BUT, checking $(WASM_FILE) is a stricter test of the *final* artifact.
# The Makefile uses `(WASM_FILE)` for shipping, but checks `(WASM_LIB)`.
# If `wasm-bindgen` produces a file that crashes `wasm-objdump` or changes the section format?
# Let's check the `wasm-bindgen` output type. `--target no-modules` outputs the wasm inside a JS shell?
# No, `--out-name` specifies the name, and `--out-dir` the dir. The .wasm file is still a raw wasm file.
# The comment suggests asserting on $(WASM_FILE) to catch if the pipeline fails.
# We have established via script that $(WASM_FILE) (simulated) might lack the section if wasm-opt fails.
# So the change is valid.
echo "Verified logic: Validating $(WASM_FILE) is correct to catch pipeline failures."Repository: SpectralSequences/sseq Length of output: 271 Assert exception-handling on the final shipped artifact. The Update the target dependency and inspection path to assert on Revised Makefile-test-wasm-unwind: $(WASM_LIB)
- wasm-objdump -h $(WASM_LIB) | grep -qi '^ *Tag ' \
- || { echo "ERROR: $(WASM_LIB) has no Tag section; panic=unwind is not in effect"; exit 1; }
+test-wasm-unwind: $(WASM_FILE)
+ wasm-objdump -h $(WASM_FILE) | grep -qi '^ *Tag ' \
+ || { echo "ERROR: $(WASM_FILE) has no Tag section; panic=unwind is not in effect"; exit 1; }
`@echo` "OK: wasm built with exception-handling (panic=unwind)"📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| .PHONY: wasm serve-wasm clean-wasm clean dummy test selenium selenium-update | ||||||||||||||||||||||||||||||||||||||||||||||||
| .PHONY: wasm serve-wasm clean-wasm clean dummy test selenium selenium-update test-wasm-unwind FORCE | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| wasm: $(WASM_FILE) $(wildcard interface/*) $(wildcard wasm/*) $(wildcard $(EXT)/steenrod_modules/*) | ||||||||||||||||||||||||||||||||||||||||||||||||
| # Must be done in this order since both contain index.js and we want the wasm version | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🌐 Web query:
Does cargo -Z build-std apply RUSTFLAGS (e.g. -D warnings) when compiling the standard library?💡 Result:
Yes, when you use the cargo -Z build-std feature, RUSTFLAGS apply to the compilation of the standard library [1]. Cargo treats the standard library as just another set of dependencies in the crate graph when -Z build-std is enabled [2][1]. Consequently, RUSTFLAGS (such as -D warnings or custom target CPU flags) are inherited and applied to all standard library crates (like core, alloc, and std) during the build process [1][3]. Because the standard library respects these flags, changing your RUSTFLAGS between different cargo invocations will typically trigger a rebuild of the standard library crates, as Cargo detects a change in the build fingerprint [3]. This behavior is consistent with how Cargo treats RUSTFLAGS as a low-level escape hatch that remains opaque to the build system, applying to all rustc invocations within that build session [1].
Citations:
RUSTFLAGSbetweencargo buildandcargo clippyinvalidate "core" crate when using build-std rust-lang/cargo#14551Avoid applying
-D warningsto thebuild-stdstep.RUSTFLAGS: "-D warnings"is inherited bycargo -Z build-std, so the rebuiltstd/panic_unwindcan fail on upstream nightly warnings. Since this job is deployment-critical, scope the warning denial to first-party crates or drop it here.🧰 Tools
🪛 zizmor (1.26.1)
[warning] 196-196: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 196-196: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 199-199: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[error] 208-208: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
[info] 199-199: action functionality is already included by the runner (superfluous-actions): use
rustupand/orcargoin a script step(superfluous-actions)
🤖 Prompt for AI Agents