Skip to content

Commit

Permalink
Include string size when passing from WASM to JS
Browse files Browse the repository at this point in the history
Summary:
This is the first part of a fix for unicode handling within the Hermes parser compiled to WASM. JS strings may contain null bytes (e.g. if they are escaped using `\0`), but the current decoding utility (emscripten's `UTF8ToString`) is built to stop at the first null byte.

In the next diff we will be introducing a new decoder to handle this issue with null bytes (along with handling of surrogate pairs from Hermes). To set up for this we need to refactor the portion of the JS/WASM bridge logic that pertains to strings. Instead of just sending a string's pointer over from WASM to JS, we must now send both the pointer and its size in bytes. These will be passed to a new `buildString` method in the JS library that decodes the string of a given length and returns a reference to it on the JS side. This reference can then be passed to an AST node builder function, just like references for nodes and node lists.

Reviewed By: gkz

Differential Revision: D24521408

fbshipit-source-id: a69d4c284aad1bba437ba6976d73f4879ba1585d
  • Loading branch information
Hans Halverson authored and facebook-github-bot committed Oct 28, 2020
1 parent fb0e241 commit 480d588
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 18 deletions.
14 changes: 8 additions & 6 deletions tools/hermes-parser/HermesParserJSBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,17 @@ class HermesParserJSBuilder {
return num;
}

/// Pointers to null-terminated strings can be passed directly to a JS library
/// call. The caller in JS must then read the null-terminated string off the
/// WASM heap.
const char *buildNode(NodeLabel label) {
/// Decode a string within JS and return a reference to it from this function.
/// We must pass the size along with the pointer as the string may contain
/// null bytes.
JSReference buildNode(NodeLabel label) {
if (label == nullptr) {
return nullptr;
return 0;
}

return label->c_str();
auto str = label->str();

return buildString(str.begin(), str.size());
}

/// Arrays cannot be passed directly to a JS library call. Instead, an array
Expand Down
3 changes: 2 additions & 1 deletion tools/hermes-parser/HermesParserJSLibrary.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

using JSReference = int;

using NodeLabel = const char *;
using NodeLabel = JSReference;
using NodeBoolean = bool;
using NodeNumber = double;
using NodePtr = JSReference;
Expand All @@ -19,6 +19,7 @@ using NodeList = JSReference;
/// Definitions of JS Library functions that can be called from WASM

extern "C" {
JSReference buildString(const char *, size_t);
JSReference buildArray();
void appendToArray(JSReference, JSReference);
JSReference buildSourceLocation(int, int, int, int, int, int);
Expand Down
22 changes: 11 additions & 11 deletions tools/hermes-parser/HermesParserJSLibrary.js.template
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,18 @@ const HermesParserJSLibrary = {

return value;
},
},

/**
* Return the string at a given pointer, returning null if the pointer is 0
* (corresponding to the null pointer).
*/
getString: function(pointer) {
if (pointer == 0) {
return null;
}
/**
* Return a reference to the decoded string at a given pointer, returning 0
* if the pointer is 0 (corresponding to the null pointer).
*/
buildString: function(pointer, size) {
if (pointer == 0) {
return 0;
}

return UTF8ToString(pointer);
},
return JSReferences.store(UTF8ToString(pointer, size));
},

buildArray: function() {
Expand Down Expand Up @@ -104,7 +104,7 @@ const HermesParserJSLibrary = {

// Define wrapper functions for each node type. Numbers do not need to be wrapped.
#define NodeNumber
#define NodeLabel JSReferences.getString
#define NodeLabel JSReferences.pop
#define NodeBoolean Boolean
#define NodePtr JSReferences.pop
#define NodeList JSReferences.pop
Expand Down

0 comments on commit 480d588

Please sign in to comment.