RESOLVED FIXED 168857
WebAssembly: exporting a property with a name that's a number doesn't work
https://bugs.webkit.org/show_bug.cgi?id=168857
Summary WebAssembly: exporting a property with a name that's a number doesn't work
JF Bastien
Reported 2017-02-24 16:51:58 PST
The following test in test_Instance.js fails but shouldn't: (function ExportedNumber() { for (let i = 0; i <= 129; ++i) { const name = i.toString(); const builder = (new Builder()) .Type().End() .Function().End() .Export() .Function(name) .End() .Code() .Function(name, { params: [], ret: "i32" }) .I32Const(i) .Return() .End() .End(); const bin = builder.WebAssembly().get(); const module = new WebAssembly.Module(bin); const instance = new WebAssembly.Instance(module); const result = instance.exports[name](); assert.isA(result, "number"); assert.eq(result, i); } })(); If you prefix `name` with another string, everything works. I think the issue has to do with using AbstractModuleRecord, somewhere after: symbolTablePutTouchWatchpointSet(moduleEnvironment, state, exp.field, exportedValue, shouldThrowReadOnlyError, ignoreReadOnlyErrors, putResult); exp.field is being put in properly, but then the ModuleRecord mapping is sad at string such as "0". I'm adding this test, commented out, and leaving a FIXME. The spec tests have a similar failure.
Attachments
Patch (1.48 KB, patch)
2017-04-25 02:50 PDT, Yusuke Suzuki
no flags
Yusuke Suzuki
Comment 1 2017-02-26 23:43:34 PST
JF Bastien
Comment 2 2017-03-09 15:22:58 PST
(In reply to comment #1) > Is that the thing? https://bugs.webkit.org/show_bug.cgi?id=168870 Yeah WebAssembly doesn't limit the names. Right now they're just length + bytes. For JS we further limit to valid UTF-8, details still in flux. I expect numbers and weird Unicode will remain valid though. Maybe the AbstractModuleThing needs a virtual function for this?
Yusuke Suzuki
Comment 3 2017-04-25 02:44:06 PDT
I think r213453 already fixes this issue. I'll enable the test in wasm js-api/test_Instance.js.
Yusuke Suzuki
Comment 4 2017-04-25 02:50:53 PDT
JF Bastien
Comment 5 2017-04-25 08:06:48 PDT
Comment on attachment 308079 [details] Patch r=me
WebKit Commit Bot
Comment 6 2017-04-25 08:36:12 PDT
Comment on attachment 308079 [details] Patch Clearing flags on attachment: 308079 Committed r215733: <http://trac.webkit.org/changeset/215733>
WebKit Commit Bot
Comment 7 2017-04-25 08:36:13 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.