Bug 1532517 - Check identifier syntax in BinTokenReaderMultipart. r=Yoric
authorTooru Fujisawa <arai_a@mac.com>
Mon, 11 Mar 2019 15:23:30 +0000
changeset 521398 f1c6fa74a15e
parent 521397 0a192c810854
child 521399 9b356aa135ac
push id10866
push usernerli@mozilla.com
push dateTue, 12 Mar 2019 18:59:09 +0000
treeherdermozilla-beta@445c24a51727 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersYoric
bugs1532517
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1532517 - Check identifier syntax in BinTokenReaderMultipart. r=Yoric There was some case that IsIdentifier check is missing after readIdentifierName. Moved the check to readIdentifierName itself. Differential Revision: https://phabricator.services.mozilla.com/D22647
js/src/frontend/BinASTParser.cpp
js/src/frontend/BinSource.yaml
js/src/frontend/BinTokenReaderMultipart.cpp
js/src/jit-test/tests/binast/invalid/identifier-assign-target.binjs
js/src/jit-test/tests/binast/invalid/identifier-assign-target.dir
js/src/jit-test/tests/binast/invalid/identifier-binding.binjs
js/src/jit-test/tests/binast/invalid/identifier-binding.dir
js/src/jit-test/tests/binast/invalid/identifier-catch.binjs
js/src/jit-test/tests/binast/invalid/identifier-catch.dir
js/src/jit-test/tests/binast/invalid/identifier-declared-name.binjs
js/src/jit-test/tests/binast/invalid/identifier-declared-name.dir
js/src/jit-test/tests/binast/invalid/identifier-expression.binjs
js/src/jit-test/tests/binast/invalid/identifier-expression.dir
js/src/jit-test/tests/binast/invalid/identifier-function-name.binjs
js/src/jit-test/tests/binast/invalid/identifier-function-name.dir
js/src/jit-test/tests/binast/invalid/identifier-positional-parameter.binjs
js/src/jit-test/tests/binast/invalid/identifier-positional-parameter.dir
js/src/jsapi-tests/binast/invalid/tests/identifier-assign-target.js
js/src/jsapi-tests/binast/invalid/tests/identifier-assign-target.py
js/src/jsapi-tests/binast/invalid/tests/identifier-binding.js
js/src/jsapi-tests/binast/invalid/tests/identifier-binding.py
js/src/jsapi-tests/binast/invalid/tests/identifier-catch.js
js/src/jsapi-tests/binast/invalid/tests/identifier-catch.py
js/src/jsapi-tests/binast/invalid/tests/identifier-declared-name.js
js/src/jsapi-tests/binast/invalid/tests/identifier-declared-name.py
js/src/jsapi-tests/binast/invalid/tests/identifier-expression.js
js/src/jsapi-tests/binast/invalid/tests/identifier-expression.py
js/src/jsapi-tests/binast/invalid/tests/identifier-function-name.js
js/src/jsapi-tests/binast/invalid/tests/identifier-function-name.py
js/src/jsapi-tests/binast/invalid/tests/identifier-positional-parameter.js
js/src/jsapi-tests/binast/invalid/tests/identifier-positional-parameter.py
--- a/js/src/frontend/BinASTParser.cpp
+++ b/js/src/frontend/BinASTParser.cpp
@@ -1779,19 +1779,16 @@ BinASTParser<Tok>::parseInterfaceAssignm
 #if defined(DEBUG)
   const BinField expected_fields[1] = {BinField::Name};
   MOZ_TRY(tokenizer_->checkFields(kind, fields, expected_fields));
 #endif  // defined(DEBUG)
 
   RootedAtom name(cx_);
   MOZ_TRY_VAR(name, tokenizer_->readIdentifierName());
 
-  if (!IsIdentifier(name)) {
-    return raiseError("Invalid identifier");
-  }
   BINJS_TRY(usedNames_.noteUse(cx_, name, pc_->scriptId(),
                                pc_->innermostScope()->id()));
   BINJS_TRY_DECL(result, handler_.newName(name->asPropertyName(),
                                           tokenizer_->pos(start), cx_));
   return result;
 }
 
 template <typename Tok>
@@ -1946,19 +1943,16 @@ JS::Result<ParseNode*> BinASTParser<Tok>
 #if defined(DEBUG)
   const BinField expected_fields[1] = {BinField::Name};
   MOZ_TRY(tokenizer_->checkFields(kind, fields, expected_fields));
 #endif  // defined(DEBUG)
 
   RootedAtom name(cx_);
   MOZ_TRY_VAR(name, tokenizer_->readIdentifierName());
 
-  if (!IsIdentifier(name)) {
-    return raiseError("Invalid identifier");
-  }
   BINJS_TRY_DECL(result, handler_.newName(name->asPropertyName(),
                                           tokenizer_->pos(start), cx_));
   return result;
 }
 
 template <typename Tok>
 JS::Result<ParseNode*> BinASTParser<Tok>::parseInterfaceBindingWithInitializer(
     const size_t start, const BinKind kind, const BinFields& fields) {
@@ -3185,19 +3179,16 @@ JS::Result<ParseNode*> BinASTParser<Tok>
 #if defined(DEBUG)
   const BinField expected_fields[1] = {BinField::Name};
   MOZ_TRY(tokenizer_->checkFields(kind, fields, expected_fields));
 #endif  // defined(DEBUG)
 
   RootedAtom name(cx_);
   MOZ_TRY_VAR(name, tokenizer_->readIdentifierName());
 
-  if (!IsIdentifier(name)) {
-    return raiseError("Invalid identifier");
-  }
   BINJS_TRY(usedNames_.noteUse(cx_, name, pc_->scriptId(),
                                pc_->innermostScope()->id()));
   BINJS_TRY_DECL(result, handler_.newName(name->asPropertyName(),
                                           tokenizer_->pos(start), cx_));
   return result;
 }
 
 template <typename Tok>
--- a/js/src/frontend/BinSource.yaml
+++ b/js/src/frontend/BinSource.yaml
@@ -493,30 +493,24 @@ AssertedVarScope:
 AssignmentExpression:
   build: |
     BINJS_TRY_DECL(result,
                    handler_.newAssignment(ParseNodeKind::AssignExpr,
                                           binding, expression));
 
 AssignmentTargetIdentifier:
   build: |
-    if (!IsIdentifier(name)) {
-      return raiseError("Invalid identifier");
-    }
     BINJS_TRY(usedNames_.noteUse(cx_, name, pc_->scriptId(),
                                  pc_->innermostScope()->id()));
     BINJS_TRY_DECL(result,
                    handler_.newName(name->asPropertyName(),
                                     tokenizer_->pos(start), cx_));
 
 BindingIdentifier:
   build: |
-    if (!IsIdentifier(name)) {
-      return raiseError("Invalid identifier");
-    }
     BINJS_TRY_DECL(result,
                    handler_.newName(name->asPropertyName(),
                                     tokenizer_->pos(start), cx_));
 
 BinaryExpression:
   build: |
     ParseNodeKind pnk;
     switch (operator_) {
@@ -1112,19 +1106,16 @@ ForStatement:
       BINJS_TRY_VAR(result, handler_.newLexicalScope(*bindings, result));
     }
 
 FunctionBody:
   inherits: ListOfStatement
 
 IdentifierExpression:
   build: |
-    if (!IsIdentifier(name)) {
-      return raiseError("Invalid identifier");
-    }
     BINJS_TRY(usedNames_.noteUse(cx_, name, pc_->scriptId(),
                                  pc_->innermostScope()->id()));
     BINJS_TRY_DECL(result,
                    handler_.newName(name->asPropertyName(),
                                     tokenizer_->pos(start), cx_));
 
 IfStatement:
   build: |
--- a/js/src/frontend/BinTokenReaderMultipart.cpp
+++ b/js/src/frontend/BinTokenReaderMultipart.cpp
@@ -11,16 +11,17 @@
 #include "mozilla/EndianUtils.h"
 #include "mozilla/Maybe.h"
 #include "mozilla/ScopeExit.h"
 
 #include <utility>
 
 #include "frontend/BinSource-macros.h"
 #include "frontend/BinSourceRuntimeSupport.h"
+#include "frontend/BytecodeCompiler.h" // IsIdentifier
 
 #include "js/Result.h"
 
 namespace js {
 namespace frontend {
 
 // The magic header, at the start of every binjs file.
 const char MAGIC_HEADER[] = "BINJS";
@@ -277,21 +278,31 @@ JS::Result<JSAtom*> BinTokenReaderMultip
   if (!maybe) {
     return raiseError("Empty string");
   }
 
   return maybe;
 }
 
 JS::Result<JSAtom*> BinTokenReaderMultipart::readMaybeIdentifierName() {
-  return readMaybeAtom();
+  BINJS_MOZ_TRY_DECL(result, readMaybeAtom());
+  if (result) {
+    if (!IsIdentifier(result)) {
+      return raiseError("Invalid identifier");
+    }
+  }
+  return result;
 }
 
 JS::Result<JSAtom*> BinTokenReaderMultipart::readIdentifierName() {
-  return readAtom();
+  BINJS_MOZ_TRY_DECL(result, readMaybeAtom());
+  if (!IsIdentifier(result)) {
+    return raiseError("Invalid identifier");
+  }
+  return result;
 }
 
 JS::Result<JSAtom*> BinTokenReaderMultipart::readMaybePropertyKey() {
   return readMaybeAtom();
 }
 
 JS::Result<JSAtom*> BinTokenReaderMultipart::readPropertyKey() {
   return readAtom();
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..cbfcb4372da30f989a2203c4690a78ee74cce436
GIT binary patch
literal 209
zc$`I!K?{OF5QSw4iSTEHbgbysB?FO-^swtqI&3=0fUYu5P=7w!?6B`P?|saBXIWgd
zs4|u0d7^IEfMw+O@e}QnX#!$sbWJ#N)(v;l>3SGp;}UFb@6hx$61Z!y2XB^!T3|W9
zdLU5jDb0rA$84|2K&C66JrLHOi2ikzE|n}Y?H7eY?5a|wskh=);3Pt^2$sRD_2~;2
C2uHyH
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/binast/invalid/identifier-assign-target.dir
@@ -0,0 +1,1 @@
+// |jit-test| error:SyntaxError
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..5ed59ce641ff56ae4bbee3a4ed1ed4b5e770e23f
GIT binary patch
literal 244
zc${61O$&lR6h((YBqCbWKM0AI)wWGaB1*b&q>Z$A>MI_MA3Ue1e_s?8bXSLS;c~q&
ziImKP)Qw{|JyL~@A#>kM<#^^%Q&DpcU+zT6%RL!n(8;AnrqaK4jD1gS;~anq3XMA~
zH87Z#7jEAp8?4?6s|0FXf21mzIuq!&0IkZl!_nK!|A(IAxysTo3Doc0#Fk)Rf#oXC
bQs4g-(QqJbA%zffBrQw4@EK=o3G2%bJNrxK
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/binast/invalid/identifier-binding.dir
@@ -0,0 +1,1 @@
+// |jit-test| error:SyntaxError
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..99d2e012d5adb73d60138360af0052de36e12919
GIT binary patch
literal 241
zc${<c^z#a4igpij^!0TNip@+(%`3?)skGK+5>jz2E>0~fNlkIe$xqG>PR=h#<pML6
z;1c<zc`1I0xv6>>BE?|!MlkW<<f6=i68D_^q{JMsjFMAkUP@+Ox+mDew9M2ZG3UgR
z<P7JW#M0ta9*BCSkfKVkXmCklNop=oWpr>zkf)z}Fv6i+EKG(>snH=puC53{6Fv?G
V1_mYpCLqbc!^Xq{q_~*CJOEgrO_u-w
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/binast/invalid/identifier-catch.dir
@@ -0,0 +1,1 @@
+// |jit-test| error:SyntaxError
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..ea4081ffdedfb86e7a4b6b9a0e7d3b96278eb076
GIT binary patch
literal 236
zc${5My9&ZU5ImL;M8rb>K!RARr8Y(kA)+aA!6sO&$tnw%2b)v$?-fM^2d0@}9_uLH
zYRJOG-|zk8$OToF?PEEF{={!v5=+H~a)Uxej3v##qRxdI8=CrrM(1@yqcv9rSC{Br
zab}X~jx7nA_*#;19^4zDw&l9&eZu1d3ggMpX8`TX^8U42GMy$-9O~b>f$QE-0AZG;
aNf7*uMF$)ON-5_>1ziO_2VC`rw|WB2#Y=1e
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/binast/invalid/identifier-declared-name.dir
@@ -0,0 +1,1 @@
+// |jit-test| error:SyntaxError
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..35bb9f8129110d79257f36cc4634a6fc6af44a6c
GIT binary patch
literal 200
zc${<c^z#a4igpij^!0TNip@+(%`3?)skHvaW8_#|oLW?pni8B`lvz;Xo|B)Hm=m0w
zUy!N+l*vrb%LOWRttco;1@iLqOb~)0iACwDC7xiT(lS$v)DWtIOA<>`L25J*(g;}|
ph*i<SAwiyg?!gGxaIr8MG9^Wa1i88*1SPl_fPjgOiG_)S0RU^-Mh*Y~
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/binast/invalid/identifier-expression.dir
@@ -0,0 +1,1 @@
+// |jit-test| error:SyntaxError
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..e2b1228dc09e0a3c0cd2b55623a669a6104ad825
GIT binary patch
literal 281
zc$`I#O>4qH5QfJ^q!ba6{s8HzhaS|cUZTd3LQ=_!m*8Q{j=C6kWwI^w-*+`cPc!ca
z^Dy%`{gNV&vLH!<Y^N0&r`_N5EZ*QFs48MdYDq<jOe)0-eKzJA*-J+(3wxzWW6Of|
z)kUS<V|h=8O|r$h`aai2X>*M4tb-;Vg?J=h{g}dOYib(a*5S(X1<MDj8a1VFPRO0C
zT3F-!nW|fJbGgalG?LFp1|2aLht4l@zRAMy8O(elL>mA^7vR;;K6usY0lawe5AaW1
ARsaA1
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/binast/invalid/identifier-function-name.dir
@@ -0,0 +1,1 @@
+// |jit-test| error:SyntaxError
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..d39b81a95c072be3246894a1b50907c4aca4f3a7
GIT binary patch
literal 321
zc$`g7O-sW-5FLjSiWCu%o{J|B9$K%0kZKxJNG#dnC3qOK)4GuEmdPskoBWd|4Qaj1
z$9wbM%*>N)sj!IiFipe!-DtMn_@9Sg9N)~1)y8g<1GX_Na2wV&i)a1JWktd|qaEM%
z;EJSaJ&P*co*$2}tBp6#Qgs|?W%oU=N-=vMSMGzVgL2X{Z`4^$BigbozHHh0bi1i7
zCkrRLf4=p!A9|IP{ioAU&idy3dem(x)H+YHSe+gn4{?Jtq~M#RSm#l6qF%1gaR31T
V;03f1t{}i+li}<VVSu9s`~&JBY3cv~
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/binast/invalid/identifier-positional-parameter.dir
@@ -0,0 +1,1 @@
+// |jit-test| error:SyntaxError
new file mode 100644
--- /dev/null
+++ b/js/src/jsapi-tests/binast/invalid/tests/identifier-assign-target.js
@@ -0,0 +1,2 @@
+a = 10;
+
new file mode 100644
--- /dev/null
+++ b/js/src/jsapi-tests/binast/invalid/tests/identifier-assign-target.py
@@ -0,0 +1,21 @@
+def filter_ast(ast):
+    # AssignmentTargetIdentifier with non-identifier string.
+    import filter_utils as utils
+
+    utils.assert_interface(ast, 'Script')
+    global_stmts = utils.get_field(ast, 'statements')
+
+    expr_stmt = utils.get_element(global_stmts, 0)
+    utils.assert_interface(expr_stmt, 'ExpressionStatement')
+
+    assign_expr = utils.get_field(expr_stmt, 'expression')
+    utils.assert_interface(assign_expr, 'AssignmentExpression')
+
+    binding = utils.get_field(assign_expr, 'binding')
+    utils.assert_interface(binding, 'AssignmentTargetIdentifier')
+
+    name = utils.get_field(binding, 'name')
+
+    utils.set_identifier_name(name, '1')
+
+    return ast
new file mode 100644
--- /dev/null
+++ b/js/src/jsapi-tests/binast/invalid/tests/identifier-binding.js
@@ -0,0 +1,1 @@
+var a = 10;
new file mode 100644
--- /dev/null
+++ b/js/src/jsapi-tests/binast/invalid/tests/identifier-binding.py
@@ -0,0 +1,26 @@
+def filter_ast(ast):
+    # BindingIdentifier with non-identifier string.
+    import filter_utils as utils
+
+    utils.assert_interface(ast, 'Script')
+    global_stmts = utils.get_field(ast, 'statements')
+
+    var_decl = utils.get_element(global_stmts, 0)
+    utils.assert_interface(var_decl, 'VariableDeclaration')
+
+    decls = utils.get_field(var_decl, 'declarators')
+
+    decl = utils.get_element(decls, 0)
+    utils.assert_interface(decl, 'VariableDeclarator')
+
+    copied_decl = utils.copy_tagged_tuple(decl)
+    utils.append_element(decls, copied_decl)
+
+    binding = utils.get_field(copied_decl, 'binding')
+    utils.assert_interface(binding, 'BindingIdentifier')
+
+    name = utils.get_field(binding, 'name')
+
+    utils.set_identifier_name(name, '1')
+
+    return ast
new file mode 100644
--- /dev/null
+++ b/js/src/jsapi-tests/binast/invalid/tests/identifier-catch.js
@@ -0,0 +1,3 @@
+try {
+} catch (e) {
+}
new file mode 100644
--- /dev/null
+++ b/js/src/jsapi-tests/binast/invalid/tests/identifier-catch.py
@@ -0,0 +1,25 @@
+def filter_ast(ast):
+    # AssertedBoundName with non-identifier string.
+    import filter_utils as utils
+
+    utils.assert_interface(ast, 'Script')
+    global_stmts = utils.get_field(ast, 'statements')
+
+    try_stmt = utils.get_element(global_stmts, 0)
+    utils.assert_interface(try_stmt, 'TryCatchStatement')
+
+    catch = utils.get_field(try_stmt, 'catchClause')
+    utils.assert_interface(catch, 'CatchClause')
+
+    scope = utils.get_field(catch, 'bindingScope')
+    utils.assert_interface(scope, 'AssertedBoundNamesScope')
+
+    names = utils.get_field(scope, 'boundNames')
+    bound_name = utils.get_element(names, 0)
+    utils.assert_interface(bound_name, 'AssertedBoundName')
+
+    name = utils.get_field(bound_name, 'name')
+
+    utils.set_identifier_name(name, '1')
+
+    return ast
new file mode 100644
--- /dev/null
+++ b/js/src/jsapi-tests/binast/invalid/tests/identifier-declared-name.js
@@ -0,0 +1,2 @@
+var a = 10;
+
new file mode 100644
--- /dev/null
+++ b/js/src/jsapi-tests/binast/invalid/tests/identifier-declared-name.py
@@ -0,0 +1,20 @@
+def filter_ast(ast):
+    # AssertedDeclaredName with non-identifier string.
+    import filter_utils as utils
+
+    utils.assert_interface(ast, 'Script')
+    scope = utils.get_field(ast, 'scope')
+    utils.assert_interface(scope, 'AssertedScriptGlobalScope')
+
+    decl_names = utils.get_field(scope, 'declaredNames')
+    decl_name = utils.get_element(decl_names, 0)
+    utils.assert_interface(decl_name, 'AssertedDeclaredName')
+
+    copied_decl_name = utils.copy_tagged_tuple(decl_name)
+    utils.append_element(decl_names, copied_decl_name)
+
+    name = utils.get_field(copied_decl_name, 'name')
+
+    utils.set_identifier_name(name, '1')
+
+    return ast
new file mode 100644
--- /dev/null
+++ b/js/src/jsapi-tests/binast/invalid/tests/identifier-expression.js
@@ -0,0 +1,1 @@
+b = a;
new file mode 100644
--- /dev/null
+++ b/js/src/jsapi-tests/binast/invalid/tests/identifier-expression.py
@@ -0,0 +1,21 @@
+def filter_ast(ast):
+    # IdentifierExpression with non-identifier string.
+    import filter_utils as utils
+
+    utils.assert_interface(ast, 'Script')
+    global_stmts = utils.get_field(ast, 'statements')
+
+    expr_stmt = utils.get_element(global_stmts, 0)
+    utils.assert_interface(expr_stmt, 'ExpressionStatement')
+
+    assign_expr = utils.get_field(expr_stmt, 'expression')
+    utils.assert_interface(assign_expr, 'AssignmentExpression')
+
+    expr = utils.get_field(assign_expr, 'expression')
+    utils.assert_interface(expr, 'IdentifierExpression')
+
+    name = utils.get_field(expr, 'name')
+
+    utils.set_identifier_name(name, '1')
+
+    return ast
new file mode 100644
--- /dev/null
+++ b/js/src/jsapi-tests/binast/invalid/tests/identifier-function-name.js
@@ -0,0 +1,2 @@
+function f() {
+}
new file mode 100644
--- /dev/null
+++ b/js/src/jsapi-tests/binast/invalid/tests/identifier-function-name.py
@@ -0,0 +1,18 @@
+def filter_ast(ast):
+    # AssignmentTargetIdentifier with non-identifier string.
+    import filter_utils as utils
+
+    utils.assert_interface(ast, 'Script')
+    global_stmts = utils.get_field(ast, 'statements')
+
+    fun_decl = utils.get_element(global_stmts, 0)
+    utils.assert_interface(fun_decl, 'EagerFunctionDeclaration')
+
+    binding = utils.get_field(fun_decl, 'name')
+    utils.assert_interface(binding, 'BindingIdentifier')
+
+    name = utils.get_field(binding, 'name')
+
+    utils.set_identifier_name(name, '1')
+
+    return ast
new file mode 100644
--- /dev/null
+++ b/js/src/jsapi-tests/binast/invalid/tests/identifier-positional-parameter.js
@@ -0,0 +1,2 @@
+function f(a) {
+}
new file mode 100644
--- /dev/null
+++ b/js/src/jsapi-tests/binast/invalid/tests/identifier-positional-parameter.py
@@ -0,0 +1,25 @@
+def filter_ast(ast):
+    # AssertedPositionalParameterName with non-identifier string.
+    import filter_utils as utils
+
+    utils.assert_interface(ast, 'Script')
+    global_stmts = utils.get_field(ast, 'statements')
+
+    fun_decl = utils.get_element(global_stmts, 0)
+    utils.assert_interface(fun_decl, 'EagerFunctionDeclaration')
+
+    contents = utils.get_field(fun_decl, 'contents')
+    utils.assert_interface(contents, 'FunctionOrMethodContents')
+
+    scope = utils.get_field(contents, 'parameterScope')
+    utils.assert_interface(scope, 'AssertedParameterScope')
+
+    param_names = utils.get_field(scope, 'paramNames')
+    param_name = utils.get_element(param_names, 0)
+    utils.assert_interface(param_name, 'AssertedPositionalParameterName')
+
+    name = utils.get_field(param_name, 'name')
+
+    utils.set_identifier_name(name, '1')
+
+    return ast