Closed Bug 1357911 Opened 3 years ago Closed 3 years ago
Baldr: update to new name section format
59 bytes, text/x-review-board-request
Alon provided a test case: http://kripken.github.io/talks/demangle_stacks.html which will currently print 'wasm-function[i]' but, once we have the fix, should print the names and we should confirm match Chrome's names (i.e., we're getting the indexes right).
FYI, the update is now landed in Binaryen: https://github.com/WebAssembly/binaryen/commit/57a2bb96c7b8a98433446828aca845a9e9be8c3d#diff-4cded05b66b5e51b39ab209ab97af507 as well as V8 https://chromium-review.googlesource.com/c/470247/
Attachment #8861214 - Attachment is obsolete: true
As we're not using the locals anywhere the idea is to leave the code as simple as possible; that's why binary.js:466 has been scraped, along with the support for the old "legacy" name section. Also, some of the identifiers in MaybeDecodeNameSectionBody are named like that to be as close as possible to the actual names in the spec. And yes, the test in comment 1 looks good.:)
Comment on attachment 8862747 [details] Bug 1357911 - [WASM] Update to new name section format. https://reviewboard.mozilla.org/r/134612/#review143680 Since you're on PTO, I'll make changes and land. Unfortunately, since the patch was written, a new Module name subsection was added which goes in front of the function subsection. This requires a bit more disciplined approach to decoding subsections. ::: js/src/wasm/WasmValidate.cpp:1609 (Diff revision 1) > + func.length = nameLen; > + > + if (!d.readBytes(nameLen)) > - return; > + return; > + > + funcNames[funcIndex] = func; funcNames is resized above to nameCount but indexed here with funcIndex but funcIndex has no relation to nameCount. Instead, funcNames must be lazily resized to grow on each funcIndex. Also, function indices are supposed to be in ascending order in BinaryEncoding.md which should be checked.
Attachment #8862747 - Flags: review?(luke) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c67aff0a734d Baldr: update names section format (r=luke)
3 years ago
Depends on: 1382040
You need to log in before you can comment on or make changes to this bug.