Skip to content

Fix bad access to detached ArrayBuffer in $growMemory. #24524

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

Merged
merged 1 commit into from
Jun 13, 2025

Conversation

juj
Copy link
Collaborator

@juj juj commented Jun 9, 2025

Consider:

var m = new WebAssembly.Memory({initial: 64});
var b = m.buffer;
m.grow(64);
console.log(b.byteLength);

This will print 0 instead of the old memory size before growing (or the new memory size after grow), because b got detached by the grow.

This caused bad assertions error print, and incorrect profiling.

To fix, avoid referencing a detached buffer.

@juj juj enabled auto-merge (squash) June 9, 2025 09:39
@RReverser
Copy link
Collaborator

Can we add a regression test that would catch this issue?

@sbc100
Copy link
Collaborator

sbc100 commented Jun 9, 2025

Maybe we lack testing for the onMemoryResize callback? Perhaps we could add a test to test_other.py that exercises -sMEMORYPROFILER? (I assume its this callback that is getting the wrong data in this case?)

@juj
Copy link
Collaborator Author

juj commented Jun 13, 2025

We recently had a "hackweek" at Unity, where we worked on rewriting the built-in --memoryprofiler, to replace it with a more robust pluggable one. I don't have much excitement to improve the old one, but I'm hoping to propose the new memory profiler as a replacement at some point.

@sbc100
Copy link
Collaborator

sbc100 commented Jun 13, 2025

Fair enough, this seems like a fairly obvious change.

Does your new profiler use the same callback system?

@juj juj merged commit 6114f02 into emscripten-core:main Jun 13, 2025
30 checks passed
@juj
Copy link
Collaborator Author

juj commented Jun 13, 2025

Does your new profiler use the same callback system?

Yes, effectively. It additionally adds two new JavaScript side tracing functions to enable tracking JS side object lifetimes, and revamps the visualization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants