Do not encode out of range dex pcs in the profile.

Test: 2271-profile-inline-cache
Bug: 330376053
Change-Id: Ib67013586c60712ccf894405f4fa8254643b273b

6 files changed, 78 insertions, 13 deletions

diff --git a/libprofile/profile/profile_compilation_info.cc b/libprofile/profile/profile_compilation_info.cc
index 21dc675e54..39c1438d69 100644
--- a/libprofile/profile/profile_compilation_info.cc
+++ b/libprofile/profile/profile_compilation_info.cc
@@ -57,6 +57,7 @@
 #include "base/unix_file/fd_file.h"
 #include "base/utils.h"
 #include "base/zip_archive.h"
+#include "dex/code_item_accessors-inl.h"
 #include "dex/descriptors_names.h"
 #include "dex/dex_file_loader.h"
 
@@ -676,9 +677,10 @@ ProfileCompilationInfo::ProfileSampleAnnotation ProfileCompilationInfo::GetAnnot
 
 bool ProfileCompilationInfo::AddMethods(const std::vector<ProfileMethodInfo>& methods,
                                         MethodHotness::Flag flags,
-                                        const ProfileSampleAnnotation& annotation) {
+                                        const ProfileSampleAnnotation& annotation,
+                                        bool is_test) {
   for (const ProfileMethodInfo& method : methods) {
-    if (!AddMethod(method, flags, annotation)) {
+    if (!AddMethod(method, flags, annotation, is_test)) {
       return false;
     }
   }
@@ -1316,7 +1318,8 @@ ProfileCompilationInfo::ExtraDescriptorIndex ProfileCompilationInfo::AddExtraDes
 
 bool ProfileCompilationInfo::AddMethod(const ProfileMethodInfo& pmi,
                                        MethodHotness::Flag flags,
-                                       const ProfileSampleAnnotation& annotation) {
+                                       const ProfileSampleAnnotation& annotation,
+                                       bool is_test) {
   DexFileData* const data = GetOrAddDexFileData(pmi.ref.dex_file, annotation);
   if (data == nullptr) {  // checksum mismatch
     return false;
@@ -1333,12 +1336,37 @@ bool ProfileCompilationInfo::AddMethod(const ProfileMethodInfo& pmi,
   InlineCacheMap* inline_cache = data->FindOrAddHotMethod(pmi.ref.index);
   DCHECK(inline_cache != nullptr);
 
+  const dex::MethodId& mid = pmi.ref.GetMethodId();
+  const DexFile& dex_file = *pmi.ref.dex_file;
+  const dex::ClassDef* class_def = dex_file.FindClassDef(mid.class_idx_);
+  // If `is_test` is true, we don't try to look at whether dex_pc fit in the
+  // code item of that method.
+  uint32_t dex_pc_max = 0u;
+  if (is_test) {
+    dex_pc_max = std::numeric_limits<uint32_t>::max();
+  } else {
+    if (class_def == nullptr || dex_file.GetClassData(*class_def) == nullptr) {
+      return true;
+    }
+    std::optional<uint32_t> offset = dex_file.GetCodeItemOffset(*class_def, pmi.ref.index);
+    if (!offset.has_value()) {
+      return true;
+    }
+    CodeItemInstructionAccessor accessor(dex_file, dex_file.GetCodeItem(offset.value()));
+    dex_pc_max = accessor.InsnsSizeInCodeUnits();
+  }
+
   for (const ProfileMethodInfo::ProfileInlineCache& cache : pmi.inline_caches) {
     if (cache.dex_pc >= std::numeric_limits<uint16_t>::max()) {
       // Discard entries that don't fit the encoding. This should only apply to
       // inlined inline caches. See also `HInliner::GetInlineCacheAOT`.
       continue;
     }
+    if (cache.dex_pc >= dex_pc_max) {
+      // Discard entries for inlined inline caches. We don't support them in
+      // profiles yet.
+      continue;
+    }
     if (cache.is_missing_types) {
       FindOrAddDexPc(inline_cache, cache.dex_pc)->SetIsMissingTypes();
       continue;
diff --git a/libprofile/profile/profile_compilation_info.h b/libprofile/profile/profile_compilation_info.h
index 745b9a2401..bb16cf8dea 100644
--- a/libprofile/profile/profile_compilation_info.h
+++ b/libprofile/profile/profile_compilation_info.h
@@ -322,9 +322,12 @@ class ProfileCompilationInfo {
   //
   // Note: if an annotation is provided, the methods/classes will be associated with the group
   // (dex_file, sample_annotation). Each group keeps its unique set of methods/classes.
+  // `is_test` should be set to true for unit tests which create artificial dex
+  // files.
   bool AddMethods(const std::vector<ProfileMethodInfo>& methods,
                   MethodHotness::Flag flags,
-                  const ProfileSampleAnnotation& annotation = ProfileSampleAnnotation::kNone);
+                  const ProfileSampleAnnotation& annotation = ProfileSampleAnnotation::kNone,
+                  bool is_test = false);
 
   // Find a type index in the `dex_file` if there is a `TypeId` for it. Otherwise,
   // find or insert the descriptor in "extra descriptors" and return an artificial
@@ -411,7 +414,8 @@ class ProfileCompilationInfo {
   // Note: see AddMethods docs for the handling of annotations.
   bool AddMethod(const ProfileMethodInfo& pmi,
                  MethodHotness::Flag flags,
-                 const ProfileSampleAnnotation& annotation = ProfileSampleAnnotation::kNone);
+                 const ProfileSampleAnnotation& annotation = ProfileSampleAnnotation::kNone,
+                 bool is_test = false);
 
   // Bulk add sampled methods and/or hot methods for a single dex, fast since it only has one
   // GetOrAddDexFileData call.
diff --git a/libprofile/profile/profile_compilation_info_test.cc b/libprofile/profile/profile_compilation_info_test.cc
index 627d05d7dd..25c560b78e 100644
--- a/libprofile/profile/profile_compilation_info_test.cc
+++ b/libprofile/profile/profile_compilation_info_test.cc
@@ -1498,10 +1498,16 @@ TEST_F(ProfileCompilationInfoTest, AddMethodsProfileMethodInfoInlineCaches) {
 
   // Add inline caches with the methods. The profile should record only the one for the hot method.
   std::vector<TypeReference> types = {};
-  ProfileMethodInfo::ProfileInlineCache ic(/*dex_pc*/ 0, /*missing_types*/true, types);
+  ProfileMethodInfo::ProfileInlineCache ic(/*dex_pc=*/ 0, /*missing_types=*/ true, types);
   std::vector<ProfileMethodInfo::ProfileInlineCache> inline_caches = {ic};
-  info.AddMethod(ProfileMethodInfo(hot, inline_caches), Hotness::kFlagHot);
-  info.AddMethod(ProfileMethodInfo(startup, inline_caches), Hotness::kFlagStartup);
+  info.AddMethod(ProfileMethodInfo(hot, inline_caches),
+                 Hotness::kFlagHot,
+                 ProfileSampleAnnotation::kNone,
+                 /*is_test=*/ true);
+  info.AddMethod(ProfileMethodInfo(startup, inline_caches),
+                 Hotness::kFlagStartup,
+                 ProfileSampleAnnotation::kNone,
+                 /*is_test=*/ true);
 
   // Check the hot method's inline cache.
   ProfileCompilationInfo::MethodHotness hot_hotness = GetMethod(info, dex1, hot.index);
diff --git a/libprofile/profile/profile_test_helper.h b/libprofile/profile/profile_test_helper.h
index fdb062b744..82c3aadd5d 100644
--- a/libprofile/profile/profile_test_helper.h
+++ b/libprofile/profile/profile_test_helper.h
@@ -48,8 +48,10 @@ class ProfileTestHelper {
       uint16_t method_idx,
       Hotness::Flag flags,
       const ProfileSampleAnnotation& annotation = ProfileSampleAnnotation::kNone) {
-    return info->AddMethod(
-        ProfileMethodInfo(MethodReference(dex, method_idx)), flags, annotation);
+    return info->AddMethod(ProfileMethodInfo(MethodReference(dex, method_idx)),
+                           flags,
+                           annotation,
+                           /*is_test=*/ true);
   }
 
   static bool AddMethod(
@@ -68,8 +70,10 @@ class ProfileTestHelper {
       const std::vector<ProfileInlineCache>& inline_caches,
       Hotness::Flag flags,
       const ProfileSampleAnnotation& annotation = ProfileSampleAnnotation::kNone) {
-    return info->AddMethod(
-        ProfileMethodInfo(MethodReference(dex, method_idx), inline_caches), flags, annotation);
+    return info->AddMethod(ProfileMethodInfo(MethodReference(dex, method_idx), inline_caches),
+                           flags,
+                           annotation,
+                           /*is_test=*/ true);
   }
 
   static bool AddClass(ProfileCompilationInfo* info,
diff --git a/runtime/jit/profiling_info_test.cc b/runtime/jit/profiling_info_test.cc
index 674cb73dcd..9b91c28ac5 100644
--- a/runtime/jit/profiling_info_test.cc
+++ b/runtime/jit/profiling_info_test.cc
@@ -144,7 +144,10 @@ class ProfileCompilationInfoTest : public CommonRuntimeTest {
       profile_methods_map->Put(method, pmi);
     }
 
-    if (!info.AddMethods(profile_methods, flags)
+    if (!info.AddMethods(profile_methods,
+                         flags,
+                         ProfileCompilationInfo::ProfileSampleAnnotation::kNone,
+                         /*is_test=*/ true)
         || info.GetNumberOfMethods() != profile_methods.size()) {
       return false;
     }
diff --git a/test/2271-profile-inline-cache/src/Main.java b/test/2271-profile-inline-cache/src/Main.java
index a95d4acd91..b51c9b49d4 100644
--- a/test/2271-profile-inline-cache/src/Main.java
+++ b/test/2271-profile-inline-cache/src/Main.java
@@ -26,6 +26,7 @@ public class Main {
     private static Method sMethod2 = null;
     private static Method sMethod3 = null;
     private static Method sMethod4 = null;
+    private static Method sMethod5 = null;
 
     public static void main(String[] args) throws Exception {
         System.loadLibrary(args[0]);
@@ -39,6 +40,7 @@ public class Main {
         sMethod2 = Main.class.getDeclaredMethod("$noinline$method2", Base.class);
         sMethod3 = Main.class.getDeclaredMethod("$noinline$method3", Base.class);
         sMethod4 = Main.class.getDeclaredMethod("$noinline$method4", Base.class);
+        sMethod5 = Main.class.getDeclaredMethod("$noinline$method5", Base.class);
 
         sFile = createTempFile();
         sFile.deleteOnExit();
@@ -106,6 +108,16 @@ public class Main {
         }
         ensureMethodJitCompiled(sMethod4);
         checkMethodHasInlineCache(sFile, sMethod4, Derived1.class, Derived2.class);
+
+        // This method is above the JIT threshold.
+        ensureJitBaselineCompiled(sMethod5);
+        try (ScopedAssertNoGc noGc = new ScopedAssertNoGc()) {
+            $noinline$method5(derived1);
+            $noinline$method5(derived2);
+        }
+        ensureMethodJitCompiled(sMethod5);
+        // We currently do not encode inlined inline caches.
+        checkMethodHasNoInlineCache(sFile, sMethod5);
     }
 
     private static void reset() {
@@ -131,6 +143,14 @@ public class Main {
         obj.f();
     }
 
+    public static void $noinline$method5(Base obj) {
+        $inline$callF(obj);
+    }
+
+    public static void $inline$callF(Base obj) {
+      obj.f();
+    }
+
     public static class Base {
         public void f() {}
     }