Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Implement a FNV1-a style checksum to the binary decoded Wasm data in debug builds#25910

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
juj wants to merge4 commits intoemscripten-core:main
base:main
Choose a base branch
Loading
fromjuj:binary_encode_utf8_helper

Conversation

@juj
Copy link
Collaborator

@jujjuj commentedDec 6, 2025

In ASSERTIONS-enabled builds, implement a FNV1-a style checksum to the binary decoded Wasm data, and display a help notice about needing to serve the HTML/JS file with UTF-8 encoding. This helps developers identify if their own shell files might not have the necessary encoding.

@sbc100
Copy link
Collaborator

Are we sure this usefull though? In particular it would not have helped with#25906 would it?

Adding more magic<<<>>> replacements is not something I want to do lightly since its outside the normal JS generation mechanisms.

@juj
Copy link
CollaboratorAuthor

juj commentedDec 9, 2025

In particular it would not have helped with#25906 would it?

It would help, but that issue has additional complexity to the problem that the developer is not able to reproduce, but it is only an end user that is experiencing the problem.

In that issue, the developer was claiming that the issue would be a problem with binary encoding, so after this PR, If the developer were able to reproduce the issue locally, then they could make an ASSERTIONS-enabled build to have this PR prove that there is nothing wrong with binary encoding. They could entertain the possibility of shipping an ASSERTIONS-enabled build to the customer however.

This PR would also help this complaint:https://groups.google.com/g/emscripten-discuss/c/E_HmYqXGjN8 , where the developer reported a bug about binary encoding, but the root issue was that they were missing proper UTF-8 encoding on the file. With this PR, Emscripten would have directly stated so, and the developer would not have needed to raise an emscripten-discuss thread in the first place.

Copy link
Collaborator

@sbc100sbc100 left a comment

Choose a reason for hiding this comment

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

Ok sounds good!

lgtm % comment

…d display a help notice about needing to serve the HTML/JS file with UTF-8 encoding. This helps developers identify if their own shell files might not have the necessary encoding.
This is an automatic change generated by tools/maint/rebaseline_tests.py.The following (4) test expectation files were updated byrunning the tests with `--rebaseline`:```codesize/test_codesize_hello_O0.json: 39332 => 39358 [+26 bytes / +0.07%]test/codesize/test_codesize_minimal_O0.expected.js updatedcodesize/test_codesize_minimal_O0.json: 20603 => 20629 [+26 bytes / +0.13%]codesize/test_unoptimized_code_size.json: 180926 => 180997 [+71 bytes / +0.04%]Average change: +0.08% (+0.04% - +0.13%)```
@jujjujforce-pushed thebinary_encode_utf8_helper branch from1ddca1c toab8f793CompareDecember 10, 2025 23:50
@jujjujenabled auto-merge (squash)December 10, 2025 23:51
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@sbc100sbc100sbc100 left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@juj@sbc100

[8]ページ先頭

©2009-2025 Movatter.jp