Check that external references are registered in the serializer.

Inspired by a patch by Slava Chigrin <vchigrin@yandex-team.ru>

R=jkummerow@chromium.org, vchigrin@yandex-team.ru

Review URL: https://codereview.chromium.org/441983002

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@22896 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

PRESUBMIT.py

@@ -42,6 +42,7 @@ def _V8PresubmitChecks(input_api, output_api):
   from presubmit import CppLintProcessor
   from presubmit import SourceProcessor
   from presubmit import CheckGeneratedRuntimeTests
+  from presubmit import CheckExternalReferenceRegistration
 
   results = []
   if not CppLintProcessor().Run(input_api.PresubmitLocalPath()):
@@ -53,6 +54,9 @@ def _V8PresubmitChecks(input_api, output_api):
   if not CheckGeneratedRuntimeTests(input_api.PresubmitLocalPath()):
     results.append(output_api.PresubmitError(
         "Generated runtime tests check failed"))
+  if not CheckExternalReferenceRegistration(input_api.PresubmitLocalPath()):
+    results.append(output_api.PresubmitError(
+        "External references registration check failed"))
   return results
 
 

src/arm/codegen-arm.cc

@@ -797,6 +797,7 @@ void MathExpGenerator::EmitMathExp(MacroAssembler* masm,
   DCHECK(!temp1.is(temp3));
   DCHECK(!temp2.is(temp3));
   DCHECK(ExternalReference::math_exp_constants(0).address() != NULL);
+  DCHECK(!masm->serializer_enabled());  // External references not serializable.
 
   Label zero, infinity, done;
 

src/arm64/codegen-arm64.cc

@@ -520,6 +520,7 @@ void MathExpGenerator::EmitMathExp(MacroAssembler* masm,
                      double_temp1, double_temp2,
                      temp1, temp2, temp3));
   DCHECK(ExternalReference::math_exp_constants(0).address() != NULL);
+  DCHECK(!masm->serializer_enabled());  // External references not serializable.
 
   Label done;
   DoubleRegister double_temp3 = result;

src/assembler.cc

@@ -105,9 +105,6 @@ struct DoubleConstant BASE_EMBEDDED {
 double min_int;
 double one_half;
 double minus_one_half;
-double minus_zero;
-double zero;
-double uint8_max_value;
 double negative_infinity;
 double canonical_non_hole_nan;
 double the_hole_nan;
@@ -905,9 +902,6 @@ void ExternalReference::SetUp() {
   double_constants.min_int = kMinInt;
   double_constants.one_half = 0.5;
   double_constants.minus_one_half = -0.5;
-  double_constants.minus_zero = -0.0;
-  double_constants.uint8_max_value = 255;
-  double_constants.zero = 0.0;
   double_constants.canonical_non_hole_nan = base::OS::nan_value();
   double_constants.the_hole_nan = BitCast<double>(kHoleNanInt64);
   double_constants.negative_infinity = -V8_INFINITY;
@@ -1165,13 +1159,6 @@ ExternalReference ExternalReference::new_space_allocation_top_address(
 }
 
 
-ExternalReference ExternalReference::heap_always_allocate_scope_depth(
-    Isolate* isolate) {
-  Heap* heap = isolate->heap();
-  return ExternalReference(heap->always_allocate_scope_depth_address());
-}
-
-
 ExternalReference ExternalReference::new_space_allocation_limit_address(
     Isolate* isolate) {
   return ExternalReference(isolate->heap()->NewSpaceAllocationLimitAddress());
@@ -1264,23 +1251,6 @@ ExternalReference ExternalReference::address_of_minus_one_half() {
 }
 
 
-ExternalReference ExternalReference::address_of_minus_zero() {
-  return ExternalReference(
-      reinterpret_cast<void*>(&double_constants.minus_zero));
-}
-
-
-ExternalReference ExternalReference::address_of_zero() {
-  return ExternalReference(reinterpret_cast<void*>(&double_constants.zero));
-}
-
-
-ExternalReference ExternalReference::address_of_uint8_max_value() {
-  return ExternalReference(
-      reinterpret_cast<void*>(&double_constants.uint8_max_value));
-}
-
-
 ExternalReference ExternalReference::address_of_negative_infinity() {
   return ExternalReference(
       reinterpret_cast<void*>(&double_constants.negative_infinity));

src/assembler.h

@@ -866,8 +866,6 @@ class ExternalReference BASE_EMBEDDED {
   // Static variable Heap::NewSpaceStart()
   static ExternalReference new_space_start(Isolate* isolate);
   static ExternalReference new_space_mask(Isolate* isolate);
-  static ExternalReference heap_always_allocate_scope_depth(Isolate* isolate);
-  static ExternalReference new_space_mark_bits(Isolate* isolate);
 
   // Write barrier.
   static ExternalReference store_buffer_top(Isolate* isolate);
@@ -901,9 +899,6 @@ class ExternalReference BASE_EMBEDDED {
   static ExternalReference address_of_min_int();
   static ExternalReference address_of_one_half();
   static ExternalReference address_of_minus_one_half();
-  static ExternalReference address_of_minus_zero();
-  static ExternalReference address_of_zero();
-  static ExternalReference address_of_uint8_max_value();
   static ExternalReference address_of_negative_infinity();
   static ExternalReference address_of_canonical_non_hole_nan();
   static ExternalReference address_of_the_hole_nan();

src/ia32/codegen-ia32.cc

@@ -929,6 +929,7 @@ void MathExpGenerator::EmitMathExp(MacroAssembler* masm,
   DCHECK(!result.is(double_scratch));
   DCHECK(!temp1.is(temp2));
   DCHECK(ExternalReference::math_exp_constants(0).address() != NULL);
+  DCHECK(!masm->serializer_enabled());  // External references not serializable.
 
   Label done;
 

src/ia32/macro-assembler-ia32.cc

@@ -3128,6 +3128,7 @@ void MacroAssembler::CheckPageFlagForMap(
     Label::Distance condition_met_distance) {
   DCHECK(cc == zero || cc == not_zero);
   Page* page = Page::FromAddress(map->address());
+  DCHECK(!serializer_enabled());  // Serializer cannot match page_flags.
   ExternalReference reference(ExternalReference::page_flags(page));
   // The inlined static address check of the page's flags relies
   // on maps never being compacted.

src/mips/codegen-mips.cc

@@ -1099,6 +1099,7 @@ void MathExpGenerator::EmitMathExp(MacroAssembler* masm,
   DCHECK(!temp1.is(temp3));
   DCHECK(!temp2.is(temp3));
   DCHECK(ExternalReference::math_exp_constants(0).address() != NULL);
+  DCHECK(!masm->serializer_enabled());  // External references not serializable.
 
   Label zero, infinity, done;
 

src/serialize.h

@@ -17,17 +17,16 @@ namespace internal {
 // A TypeCode is used to distinguish different kinds of external reference.
 // It is a single bit to make testing for types easy.
 enum TypeCode {
-  UNCLASSIFIED,        // One-of-a-kind references.
+  UNCLASSIFIED,  // One-of-a-kind references.
+  C_BUILTIN,
   BUILTIN,
   RUNTIME_FUNCTION,
   IC_UTILITY,
   STATS_COUNTER,
   TOP_ADDRESS,
-  C_BUILTIN,
-  EXTENSION,
   ACCESSOR,
-  RUNTIME_ENTRY,
   STUB_CACHE_TABLE,
+  RUNTIME_ENTRY,
   LAZY_DEOPTIMIZATION
 };
 
@@ -81,8 +80,12 @@ class ExternalReferenceTable {
   // For other types of references, the caller will figure out the address.
   void Add(Address address, TypeCode type, uint16_t id, const char* name);
 
+  void Add(Address address, const char* name) {
+    Add(address, UNCLASSIFIED, ++max_id_[UNCLASSIFIED], name);
+  }
+
   List<ExternalReferenceEntry> refs_;
-  int max_id_[kTypeCodeCount];
+  uint16_t max_id_[kTypeCodeCount];
 };
 
 

src/x64/codegen-x64.cc

@@ -585,6 +585,7 @@ void MathExpGenerator::EmitMathExp(MacroAssembler* masm,
   DCHECK(!result.is(double_scratch));
   DCHECK(!temp1.is(temp2));
   DCHECK(ExternalReference::math_exp_constants(0).address() != NULL);
+  DCHECK(!masm->serializer_enabled());  // External references not serializable.
 
   Label done;
 

test/cctest/test-serialize.cc

@@ -116,21 +116,21 @@ TEST(ExternalReferenceEncoder) {
            encoder.Encode(total_compile_size.address()));
   ExternalReference stack_limit_address =
       ExternalReference::address_of_stack_limit(isolate);
-  CHECK_EQ(make_code(UNCLASSIFIED, 4),
+  CHECK_EQ(make_code(UNCLASSIFIED, 2),
            encoder.Encode(stack_limit_address.address()));
   ExternalReference real_stack_limit_address =
       ExternalReference::address_of_real_stack_limit(isolate);
-  CHECK_EQ(make_code(UNCLASSIFIED, 5),
+  CHECK_EQ(make_code(UNCLASSIFIED, 3),
            encoder.Encode(real_stack_limit_address.address()));
-  CHECK_EQ(make_code(UNCLASSIFIED, 16),
+  CHECK_EQ(make_code(UNCLASSIFIED, 8),
            encoder.Encode(ExternalReference::debug_break(isolate).address()));
-  CHECK_EQ(make_code(UNCLASSIFIED, 10),
-           encoder.Encode(
-               ExternalReference::new_space_start(isolate).address()));
-  CHECK_EQ(make_code(UNCLASSIFIED, 3),
-           encoder.Encode(
-               ExternalReference::roots_array_start(isolate).address()));
-  CHECK_EQ(make_code(UNCLASSIFIED, 52),
+  CHECK_EQ(
+      make_code(UNCLASSIFIED, 4),
+      encoder.Encode(ExternalReference::new_space_start(isolate).address()));
+  CHECK_EQ(
+      make_code(UNCLASSIFIED, 1),
+      encoder.Encode(ExternalReference::roots_array_start(isolate).address()));
+  CHECK_EQ(make_code(UNCLASSIFIED, 34),
            encoder.Encode(ExternalReference::cpu_features().address()));
 }
 
@@ -153,13 +153,13 @@ TEST(ExternalReferenceDecoder) {
                make_code(STATS_COUNTER,
                          Counters::k_total_compile_size)));
   CHECK_EQ(ExternalReference::address_of_stack_limit(isolate).address(),
-           decoder.Decode(make_code(UNCLASSIFIED, 4)));
+           decoder.Decode(make_code(UNCLASSIFIED, 2)));
   CHECK_EQ(ExternalReference::address_of_real_stack_limit(isolate).address(),
-           decoder.Decode(make_code(UNCLASSIFIED, 5)));
+           decoder.Decode(make_code(UNCLASSIFIED, 3)));
   CHECK_EQ(ExternalReference::debug_break(isolate).address(),
-           decoder.Decode(make_code(UNCLASSIFIED, 16)));
+           decoder.Decode(make_code(UNCLASSIFIED, 8)));
   CHECK_EQ(ExternalReference::new_space_start(isolate).address(),
-           decoder.Decode(make_code(UNCLASSIFIED, 10)));
+           decoder.Decode(make_code(UNCLASSIFIED, 4)));
 }
 
 

tools/external-reference-check.py

+#!/usr/bin/env python
+# Copyright 2014 the V8 project authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import re
+import os
+import sys
+
+DECLARE_FILE = "src/assembler.h"
+REGISTER_FILE = "src/serialize.cc"
+DECLARE_RE = re.compile("\s*static ExternalReference ([^(]+)\(")
+REGISTER_RE = re.compile("\s*Add\(ExternalReference::([^(]+)\(")
+
+WORKSPACE = os.path.abspath(os.path.join(os.path.dirname(sys.argv[0]), ".."))
+
+# Ignore those.
+BLACKLISTED = [
+  "page_flags",
+  "math_exp_constants",
+  "math_exp_log_table",
+  "ForDeoptEntry",
+]
+
+def Find(filename, re):
+  references = []
+  with open(filename, "r") as f:
+    for line in f:
+      match = re.match(line)
+      if match:
+        references.append(match.group(1))
+  return references
+
+def Main():
+  declarations = Find(DECLARE_FILE, DECLARE_RE)
+  registrations = Find(REGISTER_FILE, REGISTER_RE)
+  difference = list(set(declarations) - set(registrations) - set(BLACKLISTED))
+  for reference in difference:
+    print("Declared but not registered: ExternalReference::%s" % reference)
+  return len(difference) > 0
+
+if __name__ == "__main__":
+  sys.exit(Main())

tools/presubmit.py

@@ -423,6 +423,12 @@ def CheckGeneratedRuntimeTests(workspace):
   return code == 0
 
 
+def CheckExternalReferenceRegistration(workspace):
+  code = subprocess.call(
+      [sys.executable, join(workspace, "tools", "external-reference-check.py")])
+  return code == 0
+
+
 def GetOptions():
   result = optparse.OptionParser()
   result.add_option('--no-lint', help="Do not run cpplint", default=False,
@@ -442,6 +448,7 @@ def Main():
         "two empty lines between declarations check..."
   success = SourceProcessor().Run(workspace) and success
   success = CheckGeneratedRuntimeTests(workspace) and success
+  success = CheckExternalReferenceRegistration(workspace) and success
   if success:
     return 0
   else: