Source hooks need to be able to support UTF-8 and UTF-16 source text both

RESOLVED FIXED in Firefox 68

Status

()

enhancement
P1
normal
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: Waldo, Assigned: Waldo)

Tracking

Trunk
mozilla68
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(13 attachments)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

The current source hook mechanism can reconstitute scripts as UTF-16. But if you've compiled a UTF-8 script, coordinates of things like function toString start are in UTF-16 space. So if your script contains anything non-ASCII, coordinates of functions that begin or end after it will be skew. Source hooks need to be able to reconstitute either UTF-8 or UTF-16, according to how the script was originally loaded.

Patch series to follow, once I have a bug-number to splat into the commit messages.

Priority: -- → P1
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2af8656bd3c0
Make |JSScript::loadSource| applicable to any |ScriptSource| by folding its precondition into its behavior.  r=arai
https://hg.mozilla.org/integration/mozilla-inbound/rev/74774a7f1a4d
Rename JSScript::tryLoadSource back to JSScript::loadSource now that all users have recognized the semantics change.  r=arai
https://hg.mozilla.org/integration/mozilla-inbound/rev/8509f60299df
Move JSScript::loadSource into ScriptSource, because it only acts on ScriptSource and so only makes sense there.  r=arai
https://hg.mozilla.org/integration/mozilla-inbound/rev/14248be0eba9
Reformulate ScriptSource::loadSource into Variant-matching code.  r=arai
https://hg.mozilla.org/integration/mozilla-inbound/rev/762b78b66248
Make BytecodeCompiler::assignSource<Unit>(...) directly defer to a new ScriptSource::assignSource<Unit>(...) function.  r=arai
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e238465037a
Split ScriptSource::setCompressedSource into ScriptSource::{convertTo,initializeWith}CompressedSource so that ScriptSource::data's state transitions are clearer.  r=arai
https://hg.mozilla.org/integration/mozilla-inbound/rev/d4d8e4bf895f
Add ScriptSource::setRetrievedSource to clarify the ScriptSource::data state transition when retrieved source is set.  r=arai
https://hg.mozilla.org/integration/mozilla-inbound/rev/d137eb73db95
Inline ScriptSource::setSource<Unit>(SourceTypeTraits<Unit>::SharedImmutableString) into its two callers for clarity.  r=arai
https://hg.mozilla.org/integration/mozilla-inbound/rev/8be1bf5f1e97
Convert ScriptSource::setSource to ScriptSource::setUncompressedSourceHelper as a helper function that more-precise functions can call.  r=arai

Landed the reviewed parts here, did not land the unreviewed parts for the obvious reason but also because I told arai not to review them because they're broken. :-)

I've gotten those revs to work, with some substantial additional work done before those two revs -- will post those several things (including revised versions of the unreviewed stuff here) shortly.

Keywords: leave-open

Okay, so the ordering of applicability theoretically is preserved in phabricator, but for clarity,

@ 472703:9040e21842c5 jwalden sourcehook-either-source-type Bug 1544882 - Make js::SourceHook load either UTF-8 or UTF-16 source text, depending upon the source type of the ScriptSource in question. r=arai
o 472702:4190742aa399 jwalden Bug 1544882 - Split ScriptSource::Retrievable<Unit> out of Missing to record sources that are retrievable even tho they're missing. (Only UTF-16 is supported for now.) r=arai
o 472701:9831832d9177 jwalden Bug 1544882 - XDR ScriptSource data almost entirely by switching on the type of data stored in |ScriptSource::data|. r=arai
o 472699:df8ff981a7d9 jwalden Bug 1544882 - Move XDR of ScriptSource::data and a couple related values into its own standalone function, for simplicity/to reduce complexity when SS::data is XDR'd using variant-matching code. r=arai

starting at bottom and going to top is the order in which these revs should be reviewed.

Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/mozilla-inbound/rev/641a364dcb54
Move XDR of ScriptSource::data and a couple related values into its own standalone function, for simplicity/to reduce complexity when SS::data is XDR'd using variant-matching code.  r=arai
https://hg.mozilla.org/integration/mozilla-inbound/rev/808585ce1598
XDR ScriptSource data almost entirely by switching on the type of data stored in |ScriptSource::data|.  r=arai
https://hg.mozilla.org/integration/mozilla-inbound/rev/b3053863705d
Split ScriptSource::Retrievable<Unit> out of Missing to record sources that are retrievable even tho they're missing.  (Only UTF-16 is supported for now.)  r=arai
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b05b45b9abe
Make js::SourceHook load either UTF-8 or UTF-16 source text, depending upon the source type of the ScriptSource in question.  r=arai
Keywords: leave-open
Regressions: 1547471
Regressions: 1547478
Regressions: 1547809
Duplicate of this bug: 1543802
You need to log in before you can comment on or make changes to this bug.