commit d82ceca9988efacd41ebf9582b1aebe1f333cb64
author Calin Juravle <calin@google.com> Tue Sep 19 18:20:37 2017 -0700
committer Calin Juravle <calin@google.com> Wed Sep 20 11:41:18 2017 -0700
tree 99d7ac435e150eb439fb0f3311e38cd279c79bed
parent 659a7dccd73c6b67e2d8bf56c84c09cbe69215c1

Add more error logs to profile validation

Replace a few DCHECKS with LOG(ERROR). It will avoid crashing and give
insights in what could cause the reported crash.

Bug: 65812889
Test: m test-art-host

(cherry picked from commit 1ad1e3f21d02d9f98a8cc527e756fe8795c20b6e)

Merged-In: I94a85997f865904208ef8e75c7aeb2c0911df765
Change-Id: I4bc2b6f7d7247d10f3ea20e6c254e3382fa0cbf5

runtime/jit/profile_compilation_info.cc

diff --git a/runtime/jit/profile_compilation_info.cc b/runtime/jit/profile_compilation_info.cc
index ee2c44a..77efce2 100644
--- a/runtime/jit/profile_compilation_info.cc
+++ b/runtime/jit/profile_compilation_info.cc
@@ -140,8 +140,7 @@
   if (data == nullptr) {
     return false;
   }
-  data->AddMethod(flags, ref.dex_method_index);
-  return true;
+  return data->AddMethod(flags, ref.dex_method_index);
 }
 
 bool ProfileCompilationInfo::AddMethodIndex(MethodHotness::Flag flags,
@@ -155,8 +154,7 @@
   if (data == nullptr) {
     return false;
   }
-  data->AddMethod(flags, method_idx);
-  return true;
+  return data->AddMethod(flags, method_idx);
 }
 
 bool ProfileCompilationInfo::AddMethods(const std::vector<ProfileMethodInfo>& methods) {
@@ -557,7 +555,14 @@
   // This should always be the case since since the cache map is managed by ProfileCompilationInfo.
   DCHECK_EQ(profile_key, result->profile_key);
   DCHECK_EQ(profile_index, result->profile_index);
-  DCHECK_EQ(num_method_ids, result->num_method_ids);
+
+  if (num_method_ids != result->num_method_ids) {
+    // This should not happen... added to help investigating b/65812889.
+    LOG(ERROR) << "num_method_ids mismatch for dex " << profile_key
+        << ", expected=" << num_method_ids
+        << ", actual=" << result->num_method_ids;
+    return nullptr;
+  }
 
   return result;
 }
@@ -1251,9 +1256,9 @@
   DexFileData* dex_data = GetOrAddDexFileData(method_ref.dex_file);
   if (dex_data != nullptr) {
     // TODO: Add inline caches.
-    dex_data->AddMethod(static_cast<MethodHotness::Flag>(hotness.GetFlags()),
-                        method_ref.dex_method_index);
-    return true;
+    return dex_data->AddMethod(
+        static_cast<MethodHotness::Flag>(hotness.GetFlags()),
+        method_ref.dex_method_index);
   }
   return false;
 }
@@ -1618,7 +1623,12 @@
 }
 
 // Mark a method as executed at least once.
-void ProfileCompilationInfo::DexFileData::AddMethod(MethodHotness::Flag flags, size_t index) {
+bool ProfileCompilationInfo::DexFileData::AddMethod(MethodHotness::Flag flags, size_t index) {
+  if (index >= num_method_ids) {
+    LOG(ERROR) << "Invalid method index " << index << ". num_method_ids=" << num_method_ids;
+    return false;
+  }
+
   if ((flags & MethodHotness::kFlagStartup) != 0) {
     method_bitmap.StoreBit(MethodBitIndex(/*startup*/ true, index), /*value*/ true);
   }
@@ -1630,6 +1640,7 @@
         index,
         InlineCacheMap(std::less<uint16_t>(), arena_->Adapter(kArenaAllocProfile)));
   }
+  return true;
 }
 
 ProfileCompilationInfo::MethodHotness ProfileCompilationInfo::DexFileData::GetHotnessInfo(

runtime/jit/profile_compilation_info.h

diff --git a/runtime/jit/profile_compilation_info.h b/runtime/jit/profile_compilation_info.h
index 4ab8be8..29fa79c 100644
--- a/runtime/jit/profile_compilation_info.h
+++ b/runtime/jit/profile_compilation_info.h
@@ -280,7 +280,9 @@
     }
     for (Iterator it = index_begin; it != index_end; ++it) {
       DCHECK_LT(*it, data->num_method_ids);
-      data->AddMethod(flags, *it);
+      if (!data->AddMethod(flags, *it)) {
+        return false;
+      }
     }
     return true;
   }
@@ -428,7 +430,7 @@
     }
 
     // Mark a method as executed at least once.
-    void AddMethod(MethodHotness::Flag flags, size_t index);
+    bool AddMethod(MethodHotness::Flag flags, size_t index);
 
     void MergeBitmap(const DexFileData& other) {
       DCHECK_EQ(bitmap_storage.size(), other.bitmap_storage.size());