Assume flat string when checking CompileFunctionInContext arguments.

hashseed · Commit Bot · commit 61f4c2251e10 · 2019-01-07T07:19:06.000Z
R=jkummerow@chromium.org Change-Id: I54c6137a3c6e14d4102188f154aa7216e7414dbc Reviewed-on: https://chromium-review.googlesource.com/c/1388533 Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Commit-Queue: Yang Guo <yangguo@chromium.org> Cr-Commit-Position: refs/heads/master@{#58562}
diff --git a/src/api.cc b/src/api.cc
@@ -2432,43 +2432,28 @@ MaybeLocal<Module> ScriptCompiler::CompileModule(
   return ToApiHandle<Module>(i_isolate->factory()->NewModule(shared));
 }
 
-
-class IsIdentifierHelper {
- public:
-  IsIdentifierHelper() : is_identifier_(false), first_char_(true) {}
-
-  bool Check(i::String string) {
-    i::ConsString cons_string = i::String::VisitFlat(this, string, 0);
-    if (cons_string.is_null()) return is_identifier_;
-    // We don't support cons strings here.
-    return false;
-  }
-  void VisitOneByteString(const uint8_t* chars, int length) {
-    for (int i = 0; i < length; ++i) {
-      if (first_char_) {
-        first_char_ = false;
-        is_identifier_ = i::IsIdentifierStart(chars[0]);
-      } else {
-        is_identifier_ &= i::IsIdentifierPart(chars[i]);
-      }
+namespace {
+bool IsIdentifier(i::Isolate* isolate, i::Handle<i::String> string) {
+  string = i::String::Flatten(isolate, string);
+  const int length = string->length();
+  if (length == 0) return false;
+  if (!i::IsIdentifierStart(string->Get(0))) return false;
+  i::DisallowHeapAllocation no_gc;
+  i::String::FlatContent flat = string->GetFlatContent(no_gc);
+  if (flat.IsOneByte()) {
+    auto vector = flat.ToOneByteVector();
+    for (int i = 1; i < length; i++) {
+      if (!i::IsIdentifierPart(vector[i])) return false;
     }
-  }
-  void VisitTwoByteString(const uint16_t* chars, int length) {
-    for (int i = 0; i < length; ++i) {
-      if (first_char_) {
-        first_char_ = false;
-        is_identifier_ = i::IsIdentifierStart(chars[0]);
-      } else {
-        is_identifier_ &= i::IsIdentifierPart(chars[i]);
-      }
+  } else {
+    auto vector = flat.ToUC16Vector();
+    for (int i = 1; i < length; i++) {
+      if (!i::IsIdentifierPart(vector[i])) return false;
     }
   }
-
- private:
-  bool is_identifier_;
-  bool first_char_;
-  DISALLOW_COPY_AND_ASSIGN(IsIdentifierHelper);
-};
+  return true;
+}
+}  // anonymous namespace
 
 MaybeLocal<Function> ScriptCompiler::CompileFunctionInContext(
     Local<Context> v8_context, Source* source, size_t arguments_count,
@@ -2493,9 +2478,8 @@ MaybeLocal<Function> ScriptCompiler::CompileFunctionInContext(
   i::Handle<i::FixedArray> arguments_list =
       isolate->factory()->NewFixedArray(static_cast<int>(arguments_count));
   for (int i = 0; i < static_cast<int>(arguments_count); i++) {
-    IsIdentifierHelper helper;
     i::Handle<i::String> argument = Utils::OpenHandle(*arguments[i]);
-    if (!helper.Check(*argument)) return Local<Function>();
+    if (!IsIdentifier(isolate, argument)) return Local<Function>();
     arguments_list->set(i, *argument);
   }
 
diff --git a/test/cctest/test-compiler.cc b/test/cctest/test-compiler.cc
@@ -486,8 +486,8 @@ TEST(CompileFunctionInContextArgs) {
   v8::Local<v8::Object> ext[1];
   ext[0] = v8::Local<v8::Object>::Cast(
       env->Global()->Get(env.local(), v8_str("a")).ToLocalChecked());
-  v8::ScriptCompiler::Source script_source(v8_str("result = x + b"));
-  v8::Local<v8::String> arg = v8_str("b");
+  v8::ScriptCompiler::Source script_source(v8_str("result = x + abc"));
+  v8::Local<v8::String> arg = v8_str("abc");
   v8::Local<v8::Function> fun =
       v8::ScriptCompiler::CompileFunctionInContext(env.local(), &script_source,
                                                    1, &arg, 1, ext)
@@ -497,8 +497,8 @@ TEST(CompileFunctionInContextArgs) {
                   ->ToInt32(env.local())
                   .ToLocalChecked()
                   ->Value());
-  v8::Local<v8::Value> b_value = v8::Number::New(CcTest::isolate(), 42.0);
-  fun->Call(env.local(), env->Global(), 1, &b_value).ToLocalChecked();
+  v8::Local<v8::Value> arg_value = v8::Number::New(CcTest::isolate(), 42.0);
+  fun->Call(env.local(), env->Global(), 1, &arg_value).ToLocalChecked();
   CHECK(env->Global()->Has(env.local(), v8_str("result")).FromJust());
   v8::Local<v8::Value> result =
       env->Global()->Get(env.local(), v8_str("result")).ToLocalChecked();
@@ -515,16 +515,17 @@ TEST(CompileFunctionInContextComments) {
   v8::Local<v8::Object> ext[1];
   ext[0] = v8::Local<v8::Object>::Cast(
       env->Global()->Get(env.local(), v8_str("a")).ToLocalChecked());
-  v8::ScriptCompiler::Source script_source(
-      v8_str("result = /* y + */ x + b // + z"));
-  v8::Local<v8::String> arg = v8_str("b");
+  v8::Local<v8::String> source =
+      CompileRun("'result = /* y + */ x + a\\u4e00 // + z'").As<v8::String>();
+  v8::ScriptCompiler::Source script_source(source);
+  v8::Local<v8::String> arg = CompileRun("'a\\u4e00'").As<v8::String>();
   v8::Local<v8::Function> fun =
       v8::ScriptCompiler::CompileFunctionInContext(env.local(), &script_source,
                                                    1, &arg, 1, ext)
           .ToLocalChecked();
   CHECK(!fun.IsEmpty());
-  v8::Local<v8::Value> b_value = v8::Number::New(CcTest::isolate(), 42.0);
-  fun->Call(env.local(), env->Global(), 1, &b_value).ToLocalChecked();
+  v8::Local<v8::Value> arg_value = v8::Number::New(CcTest::isolate(), 42.0);
+  fun->Call(env.local(), env->Global(), 1, &arg_value).ToLocalChecked();
   CHECK(env->Global()->Has(env.local(), v8_str("result")).FromJust());
   v8::Local<v8::Value> result =
       env->Global()->Get(env.local(), v8_str("result")).ToLocalChecked();
