Avoid overflow when profiling builtins

The basic block instrumentation currently uses 32-bit integers, which
could overflow during a long profiling session. I considered upgrading
them to 64-bit integers, but generating the correct instrumentation code
for various architectures would be rather non-trivial. Instead, this
change uses 64-bit floating-point values, which are simple and also have
the nice behavior that they saturate rather than overflowing.

Bug: v8:10470
Change-Id: I60f7456cb750091809803c03a85dd348dc614b58
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2545573Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#71297}

src/builtins/profile-data-reader.cc

@@ -26,7 +26,7 @@ class ProfileDataFromFileInternal : public ProfileDataFromFile {
     hash_has_value_ = true;
   }
 
-  void AddCountToBlock(size_t block_id, uint32_t count) {
+  void AddCountToBlock(size_t block_id, double count) {
     if (block_counts_by_id_.size() <= block_id) {
       // std::vector initializes new data to zero when resizing.
       block_counts_by_id_.resize(block_id + 1);
@@ -67,7 +67,7 @@ EnsureInitProfileData() {
       CHECK(errno == 0 && end != token.c_str());
       std::getline(line_stream, token, ',');
       CHECK(line_stream.eof());
-      uint32_t count = static_cast<uint32_t>(strtoul(token.c_str(), &end, 0));
+      double count = strtod(token.c_str(), &end);
       CHECK(errno == 0 && end != token.c_str());
       ProfileDataFromFileInternal& counters_and_hash =
           (*data.get())[builtin_name];

src/builtins/profile-data-reader.h

@@ -20,7 +20,7 @@ class ProfileDataFromFile {
 
   // Returns how many times the block with the given ID was executed during
   // profiling.
-  uint32_t GetCounter(size_t block_id) const {
+  double GetCounter(size_t block_id) const {
     // The profile data is allowed to omit blocks which were never hit, so be
     // careful to avoid out-of-bounds access.
     return block_id < block_counts_by_id_.size() ? block_counts_by_id_[block_id]
@@ -38,7 +38,7 @@ class ProfileDataFromFile {
   // How many times each block was executed, indexed by block ID. This vector
   // may be shorter than the total number of blocks; any omitted block should be
   // treated as a zero.
-  std::vector<uint32_t> block_counts_by_id_;
+  std::vector<double> block_counts_by_id_;
 };
 
 // The following strings can't be static members of ProfileDataFromFile until

src/compiler/basic-block-instrumentor.cc

@@ -92,7 +92,7 @@ BasicBlockProfilerData* BasicBlockInstrumentor::Instrument(
   } else {
     counters_array = graph->NewNode(PointerConstant(&common, data->counts()));
   }
-  Node* one = graph->NewNode(common.Int32Constant(1));
+  Node* one = graph->NewNode(common.Float64Constant(1));
   BasicBlockVector* blocks = schedule->rpo_order();
   size_t block_number = 0;
   for (BasicBlockVector::iterator it = blocks->begin(); block_number < n_blocks;
@@ -104,18 +104,18 @@ BasicBlockProfilerData* BasicBlockInstrumentor::Instrument(
     // It is unnecessary to wire effect and control deps for load and store
     // since this happens after scheduling.
     // Construct increment operation.
-    int offset_to_counter_value = static_cast<int>(block_number) * kInt32Size;
+    int offset_to_counter_value = static_cast<int>(block_number) * kDoubleSize;
     if (on_heap_counters) {
       offset_to_counter_value += ByteArray::kHeaderSize - kHeapObjectTag;
     }
     Node* offset_to_counter =
         graph->NewNode(IntPtrConstant(&common, offset_to_counter_value));
     Node* load =
-        graph->NewNode(machine.Load(MachineType::Uint32()), counters_array,
+        graph->NewNode(machine.Load(MachineType::Float64()), counters_array,
                        offset_to_counter, graph->start(), graph->start());
-    Node* inc = graph->NewNode(machine.Int32Add(), load, one);
+    Node* inc = graph->NewNode(machine.Float64Add(), load, one);
     Node* store = graph->NewNode(
-        machine.Store(StoreRepresentation(MachineRepresentation::kWord32,
+        machine.Store(StoreRepresentation(MachineRepresentation::kFloat64,
                                           kNoWriteBarrier)),
         counters_array, offset_to_counter, inc, graph->start(), graph->start());
     // Insert the new nodes.

src/compiler/scheduler.cc

@@ -478,14 +478,14 @@ class CFGBuilder : public ZoneObject {
 
     BranchHint hint_from_profile = BranchHint::kNone;
     if (const ProfileDataFromFile* profile_data = scheduler_->profile_data()) {
-      uint32_t block_zero_count =
+      double block_zero_count =
           profile_data->GetCounter(successor_blocks[0]->id().ToSize());
-      uint32_t block_one_count =
+      double block_one_count =
           profile_data->GetCounter(successor_blocks[1]->id().ToSize());
       // If a branch is visited a non-trivial number of times and substantially
       // more often than its alternative, then mark it as likely.
-      constexpr uint32_t kMinimumCount = 100000;
-      constexpr uint32_t kThresholdRatio = 4000;
+      constexpr double kMinimumCount = 100000;
+      constexpr double kThresholdRatio = 4000;
       if (block_zero_count > kMinimumCount &&
           block_zero_count / kThresholdRatio > block_one_count) {
         hint_from_profile = BranchHint::kTrue;

src/diagnostics/basic-block-profiler.cc

@@ -59,58 +59,60 @@ Handle<String> CopyStringToJSHeap(const std::string& source, Isolate* isolate) {
                                                        AllocationType::kOld);
 }
 
-// Size of entries in both block_ids and counts.
-constexpr int kBasicBlockSlotSize = kInt32Size;
+constexpr int kBlockIdSlotSize = kInt32Size;
+constexpr int kBlockCountSlotSize = kDoubleSize;
 }  // namespace
 
 BasicBlockProfilerData::BasicBlockProfilerData(
     Handle<OnHeapBasicBlockProfilerData> js_heap_data, Isolate* isolate) {
-  function_name_ = js_heap_data->name().ToCString().get();
-  schedule_ = js_heap_data->schedule().ToCString().get();
-  code_ = js_heap_data->code().ToCString().get();
-  Handle<ByteArray> counts(js_heap_data->counts(), isolate);
-  for (int i = 0; i < counts->length() / kBasicBlockSlotSize; ++i) {
-    counts_.push_back(counts->get_uint32(i));
-  }
-  Handle<ByteArray> block_ids(js_heap_data->block_ids(), isolate);
-  for (int i = 0; i < block_ids->length() / kBasicBlockSlotSize; ++i) {
-    block_ids_.push_back(block_ids->get_int(i));
-  }
-  CHECK_EQ(block_ids_.size(), counts_.size());
-  hash_ = js_heap_data->hash();
+  DisallowHeapAllocation no_gc;
+  CopyFromJSHeap(*js_heap_data);
 }
 
 BasicBlockProfilerData::BasicBlockProfilerData(
     OnHeapBasicBlockProfilerData js_heap_data) {
+  CopyFromJSHeap(js_heap_data);
+}
+
+void BasicBlockProfilerData::CopyFromJSHeap(
+    OnHeapBasicBlockProfilerData js_heap_data) {
   function_name_ = js_heap_data.name().ToCString().get();
   schedule_ = js_heap_data.schedule().ToCString().get();
   code_ = js_heap_data.code().ToCString().get();
   ByteArray counts(js_heap_data.counts());
-  for (int i = 0; i < counts.length() / kBasicBlockSlotSize; ++i) {
-    counts_.push_back(counts.get_uint32(i));
+  for (int i = 0; i < counts.length() / kBlockCountSlotSize; ++i) {
+    counts_.push_back(
+        reinterpret_cast<double*>(counts.GetDataStartAddress())[i]);
   }
   ByteArray block_ids(js_heap_data.block_ids());
-  for (int i = 0; i < block_ids.length() / kBasicBlockSlotSize; ++i) {
+  for (int i = 0; i < block_ids.length() / kBlockIdSlotSize; ++i) {
     block_ids_.push_back(block_ids.get_int(i));
   }
   CHECK_EQ(block_ids_.size(), counts_.size());
+  hash_ = js_heap_data.hash();
 }
 
 Handle<OnHeapBasicBlockProfilerData> BasicBlockProfilerData::CopyToJSHeap(
     Isolate* isolate) {
-  int array_size_in_bytes = static_cast<int>(n_blocks() * kBasicBlockSlotSize);
-  CHECK(array_size_in_bytes >= 0 &&
-        static_cast<size_t>(array_size_in_bytes) / kBasicBlockSlotSize ==
+  int id_array_size_in_bytes = static_cast<int>(n_blocks() * kBlockIdSlotSize);
+  CHECK(id_array_size_in_bytes >= 0 &&
+        static_cast<size_t>(id_array_size_in_bytes) / kBlockIdSlotSize ==
             n_blocks());  // Overflow
   Handle<ByteArray> block_ids = isolate->factory()->NewByteArray(
-      array_size_in_bytes, AllocationType::kOld);
+      id_array_size_in_bytes, AllocationType::kOld);
   for (int i = 0; i < static_cast<int>(n_blocks()); ++i) {
     block_ids->set_int(i, block_ids_[i]);
   }
+
+  int counts_array_size_in_bytes =
+      static_cast<int>(n_blocks() * kBlockCountSlotSize);
+  CHECK(counts_array_size_in_bytes >= 0 &&
+        static_cast<size_t>(counts_array_size_in_bytes) / kBlockCountSlotSize ==
+            n_blocks());  // Overflow
   Handle<ByteArray> counts = isolate->factory()->NewByteArray(
-      array_size_in_bytes, AllocationType::kOld);
+      counts_array_size_in_bytes, AllocationType::kOld);
   for (int i = 0; i < static_cast<int>(n_blocks()); ++i) {
-    counts->set_uint32(i, counts_[i]);
+    reinterpret_cast<double*>(counts->GetDataStartAddress())[i] = counts_[i];
   }
   Handle<String> name = CopyStringToJSHeap(function_name_, isolate);
   Handle<String> schedule = CopyStringToJSHeap(schedule_, isolate);
@@ -130,8 +132,8 @@ void BasicBlockProfiler::ResetCounts(Isolate* isolate) {
   for (int i = 0; i < list->Length(); ++i) {
     Handle<ByteArray> counts(
         OnHeapBasicBlockProfilerData::cast(list->Get(i)).counts(), isolate);
-    for (int j = 0; j < counts->length() / kBasicBlockSlotSize; ++j) {
-      counts->set_uint32(j, 0);
+    for (int j = 0; j < counts->length() / kBlockCountSlotSize; ++j) {
+      reinterpret_cast<double*>(counts->GetDataStartAddress())[j] = 0;
     }
   }
 }
@@ -195,7 +197,8 @@ void BasicBlockProfilerData::Log(Isolate* isolate) {
 }
 
 std::ostream& operator<<(std::ostream& os, const BasicBlockProfilerData& d) {
-  int block_count_sum = std::accumulate(d.counts_.begin(), d.counts_.end(), 0);
+  double block_count_sum =
+      std::accumulate(d.counts_.begin(), d.counts_.end(), 0);
   if (block_count_sum == 0) return os;
   const char* name = "unknown function";
   if (!d.function_name_.empty()) {
@@ -207,14 +210,14 @@ std::ostream& operator<<(std::ostream& os, const BasicBlockProfilerData& d) {
     os << d.schedule_.c_str() << std::endl;
   }
   os << "block counts for " << name << ":" << std::endl;
-  std::vector<std::pair<size_t, uint32_t>> pairs;
+  std::vector<std::pair<size_t, double>> pairs;
   pairs.reserve(d.n_blocks());
   for (size_t i = 0; i < d.n_blocks(); ++i) {
     pairs.push_back(std::make_pair(i, d.counts_[i]));
   }
   std::sort(
       pairs.begin(), pairs.end(),
-      [=](std::pair<size_t, uint32_t> left, std::pair<size_t, uint32_t> right) {
+      [=](std::pair<size_t, double> left, std::pair<size_t, double> right) {
         if (right.second == left.second) return left.first < right.first;
         return right.second < left.second;
       });

src/diagnostics/basic-block-profiler.h

@@ -33,7 +33,7 @@ class BasicBlockProfilerData {
     DCHECK_EQ(block_ids_.size(), counts_.size());
     return block_ids_.size();
   }
-  const uint32_t* counts() const { return &counts_[0]; }
+  const double* counts() const { return &counts_[0]; }
 
   void SetCode(const std::ostringstream& os);
   void SetFunctionName(std::unique_ptr<char[]> name);
@@ -55,13 +55,15 @@ class BasicBlockProfilerData {
 
   V8_EXPORT_PRIVATE void ResetCounts();
 
+  void CopyFromJSHeap(OnHeapBasicBlockProfilerData js_heap_data);
+
   // These vectors are indexed by reverse post-order block number.
   std::vector<int32_t> block_ids_;
-  std::vector<uint32_t> counts_;
+  std::vector<double> counts_;
   std::string function_name_;
   std::string schedule_;
   std::string code_;
-  int hash_;
+  int hash_ = 0;
   DISALLOW_COPY_AND_ASSIGN(BasicBlockProfilerData);
 };
 
@@ -79,7 +81,7 @@ class BasicBlockProfiler {
   V8_EXPORT_PRIVATE void Print(std::ostream& os, Isolate* isolate);
 
   // Coverage bitmap in this context includes only on heap BasicBlockProfiler
-  // data It is used to export coverage of builtins function loaded from
+  // data. It is used to export coverage of builtins function loaded from
   // snapshot.
   V8_EXPORT_PRIVATE std::vector<bool> GetCoverageBitmap(Isolate* isolate);
 

src/logging/log.cc

@@ -1102,7 +1102,7 @@ void Logger::TimerEvent(Logger::StartEnd se, const char* name) {
 }
 
 void Logger::BasicBlockCounterEvent(const char* name, int block_id,
-                                    uint32_t count) {
+                                    double count) {
   if (!FLAG_turbo_profiling_log_builtins) return;
   std::unique_ptr<Log::MessageBuilder> msg_ptr = log_->NewMessageBuilder();
   if (!msg_ptr) return;

src/logging/log.h

@@ -247,7 +247,7 @@ class Logger : public CodeEventListener {
 
   V8_EXPORT_PRIVATE void TimerEvent(StartEnd se, const char* name);
 
-  void BasicBlockCounterEvent(const char* name, int block_id, uint32_t count);
+  void BasicBlockCounterEvent(const char* name, int block_id, double count);
 
   void BuiltinHashEvent(const char* name, int hash);
 

src/objects/shared-function-info.tq

@@ -85,7 +85,7 @@ class UncompiledDataWithPreparseData extends UncompiledData {
 @export
 class OnHeapBasicBlockProfilerData extends HeapObject {
   block_ids: ByteArray;  // Stored as 4-byte ints
-  counts: ByteArray;     // Stored as 4-byte ints
+  counts: ByteArray;     // Stored as 8-byte floats
   name: String;
   schedule: String;
   code: String;

test/cctest/compiler/test-basic-block-profiler.cc

@@ -28,9 +28,9 @@ class BasicBlockProfilerTest : public RawMachineAssemblerTester<int32_t> {
     CHECK_NE(0, static_cast<int>(l->size()));
     const BasicBlockProfilerData* data = l->back().get();
     CHECK_EQ(static_cast<int>(size), static_cast<int>(data->n_blocks()));
-    const uint32_t* counts = data->counts();
+    const double* counts = data->counts();
     for (size_t i = 0; i < size; ++i) {
-      CHECK_EQ(static_cast<int>(expected[i]), static_cast<int>(counts[i]));
+      CHECK_EQ(static_cast<double>(expected[i]), counts[i]);
     }
   }
 };