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

[CP] [dart2wasm] Fix array.new_fixed calls with too large size #55873

Closed
mkustermann opened this issue May 30, 2024 · 4 comments
Closed

[CP] [dart2wasm] Fix array.new_fixed calls with too large size #55873

mkustermann opened this issue May 30, 2024 · 4 comments
Assignees
Labels
cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve

Comments

@mkustermann
Copy link
Member

Commit(s) to merge

c7834b6

Target

stable

Prepared changelist for beta/stable

https://dart-review.googlesource.com/c/sdk/+/368644

Issue Description

Certain large apps can result in dart2wasm emitting an array.new_fixed instruction with more than 10000 elements, which causes a runtime error when running in browsers.

What is the fix

Avoid using array.new_fixed if the array is larger than 10000

Why cherry-pick

Users are hitting this on stable channel.

Risk

Low

Issue link(s)

#55396, #55872

Extra Info

No response

@itsjustkevin
Copy link
Contributor

@mraleph could you take a look at this cherry-pick request?

@mraleph
Copy link
Member

mraleph commented May 30, 2024

LGTM

@mkustermann
Copy link
Member Author

@itsjustkevin if you approve, I'll merge the CL

@itsjustkevin
Copy link
Contributor

Approved! Merge away @mkustermann!

@itsjustkevin itsjustkevin added the cherry-pick-approved Label for approved cherrypick request label May 30, 2024
copybara-service bot pushed a commit that referenced this issue May 30, 2024
Currently `array.new_fixed` has the upper size limit of 10,000.

Larger arrays need to be initialized with `array.new` or
`array.new_default`.

We handle this correctly in constant list and strings, but we didn't
handle it in `WasmArray.literal` consturctor. This CL fixes it.

Bug: #55396
Bug: #55872

Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/361780
Cherry-pick-request: #55873
Change-Id: I97c8dc104cb4231ba0508d9cd87690cf1bbe95a9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/368644
Reviewed-by: Slava Egorov <[email protected]>
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Ömer Ağacan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve
Projects
None yet
Development

No branches or pull requests

6 participants