Skip to content

[emdawnwebgpu] Fix StringView argument signatures for MEMORY64 - #86

Open
slowriot wants to merge 2 commits into
google:mainfrom
slowriot:main
Open

slowriot wants to merge 2 commits into
google:mainfrom
slowriot:main

Conversation

@slowriot

Copy link
Copy Markdown

The signature generator treats by-value WGPUStringView arguments as unsupported structures, causing their generated __sig entries to be commented out.

Under MEMORY64, this prevents Emscripten from generating BigInt-to-Number pointer conversions for affected JavaScript bindings.

Calling PushDebugGroup or InsertDebugMarker consequently throws:

TypeError: Cannot mix BigInt and other types, use explicit conversions

The exception occurs in makeStringFromStringView when it performs Number arithmetic on the incoming BigInt address.

Recognise by-value WGPUStringView arguments as pointers, matching their indirect passing convention in the Wasm C ABI. This generates the required vpp signatures for the debug-group and marker functions. Structure return values and other structure types remain unchanged.

Validation:

  • Regenerated emdawnwebgpu signatures and verified all eight PushDebugGroup / InsertDebugMarker bindings emit vpp.
  • Checked that StringView return values and other structure types retain their existing handling.
  • Confirmed that applying the equivalent generated-signature changes fixes the crash in a wasm64 WebGPU application.

@github-actions

Copy link
Copy Markdown

👋 Thanks for your contribution! Your PR has been imported to Gerrit.
Please visit https://dawn-review.googlesource.com/c/dawn/+/342595 to see it and CC yourself on the change.
After iterating on feedback, please comment on the Gerrit review to notify reviewers.
All reviews are handled within Gerrit, any comments on the GitHub PR may be missed.
You can continue to upload commits to this PR, and they will be automatically imported
into Gerrit.

@kainino0x

Copy link
Copy Markdown
Member

Review comments on https://dawn-review.googlesource.com/c/dawn/+/342595

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.

2 participants