Revert "Turn off cross-dex inlining while we take a look at 207329152"
This reverts commit f6bb80661e2a4f2940fc899dd1474aa457e1b45a.
Reason for revert: Found fix. We were using method instead of
outer_method to get the oat_dex_files vector. Fix can be seen between
patchsets 2 and 3.
Bug: 154012332, 206513969, 206992606
Change-Id: I0a1b3a3c6e9670400afe6e673e5a6ddc510fd1a4
diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index f8bd436..a40218d 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -67,9 +67,6 @@
// Controls the use of inline caches in AOT mode.
static constexpr bool kUseAOTInlineCaches = true;
-// Enables the use of cross-dex inlining in AOT mode.
-static constexpr bool kEnableAotCrossDexInlining = false;
-
// We check for line numbers to make sure the DepthString implementation
// aligns the output nicely.
#define LOG_INTERNAL(msg) \
@@ -1719,10 +1716,6 @@
return true;
}
- if (!kEnableAotCrossDexInlining) {
- return false;
- }
-
// Inline across dexfiles if the callee's DexFile is:
// 1) in the bootclasspath, or
if (callee->GetDeclaringClass()->GetClassLoader() == nullptr) {
diff --git a/runtime/entrypoints/entrypoint_utils-inl.h b/runtime/entrypoints/entrypoint_utils-inl.h
index a160a7b..d3370af 100644
--- a/runtime/entrypoints/entrypoint_utils-inl.h
+++ b/runtime/entrypoints/entrypoint_utils-inl.h
@@ -50,6 +50,57 @@
namespace art {
+inline std::string GetResolvedMethodErrorString(ClassLinker* class_linker,
+ ArtMethod* inlined_method,
+ ArtMethod* parent_method,
+ ArtMethod* outer_method,
+ ObjPtr<mirror::DexCache> dex_cache,
+ MethodInfo method_info)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ const uint32_t method_index = method_info.GetMethodIndex();
+
+ std::stringstream error_ss;
+ std::string separator = "";
+ error_ss << "BCP vector {";
+ for (const DexFile* df : class_linker->GetBootClassPath()) {
+ error_ss << separator << df << "(" << df->GetLocation() << ")";
+ separator = ", ";
+ }
+ error_ss << "}. oat_dex_files vector: {";
+ separator = "";
+ for (const OatDexFile* odf_value :
+ parent_method->GetDexFile()->GetOatDexFile()->GetOatFile()->GetOatDexFiles()) {
+ error_ss << separator << odf_value << "(" << odf_value->GetDexFileLocation() << ")";
+ separator = ", ";
+ }
+ error_ss << "}. ";
+ if (inlined_method != nullptr) {
+ error_ss << "Inlined method: " << inlined_method->PrettyMethod() << " ("
+ << inlined_method->GetDexFile()->GetLocation() << "/"
+ << static_cast<const void*>(inlined_method->GetDexFile()) << "). ";
+ } else if (dex_cache != nullptr) {
+ error_ss << "Could not find an inlined method from an .oat file, using dex_cache to print the "
+ "inlined method: "
+ << dex_cache->GetDexFile()->PrettyMethod(method_index) << " ("
+ << dex_cache->GetDexFile()->GetLocation() << "/"
+ << static_cast<const void*>(dex_cache->GetDexFile()) << "). ";
+ } else {
+ error_ss << "Both inlined_method and dex_cache are null. This means that we had an OOB access "
+ << "to either bcp_dex_files or oat_dex_files. ";
+ }
+ error_ss << "The outer method is: " << parent_method->PrettyMethod() << " ("
+ << parent_method->GetDexFile()->GetLocation() << "/"
+ << static_cast<const void*>(parent_method->GetDexFile())
+ << "). The outermost method in the chain is: " << outer_method->PrettyMethod() << " ("
+ << outer_method->GetDexFile()->GetLocation() << "/"
+ << static_cast<const void*>(outer_method->GetDexFile())
+ << "). MethodInfo: method_index=" << std::dec << method_index
+ << ", is_in_bootclasspath=" << std::boolalpha
+ << (method_info.GetDexFileIndexKind() == MethodInfo::kKindBCP)
+ << ", dex_file_index=" << std::dec << method_info.GetDexFileIndex() << ".";
+ return error_ss.str();
+}
+
inline ArtMethod* GetResolvedMethod(ArtMethod* outer_method,
const CodeInfo& code_info,
const BitTableRange<InlineInfo>& inline_infos)
@@ -85,41 +136,28 @@
DCHECK_NE(inline_info.GetDexPc(), static_cast<uint32_t>(-1));
MethodInfo method_info = code_info.GetMethodInfoOf(inline_info);
uint32_t method_index = method_info.GetMethodIndex();
- ArtMethod* inlined_method;
- ObjPtr<mirror::DexCache> dex_cache;
+ const uint32_t dex_file_index = method_info.GetDexFileIndex();
+ ArtMethod* inlined_method = nullptr;
+ ObjPtr<mirror::DexCache> dex_cache = nullptr;
if (method_info.HasDexFileIndex()) {
if (method_info.GetDexFileIndexKind() == MethodInfo::kKindBCP) {
ArrayRef<const DexFile* const> bcp_dex_files(class_linker->GetBootClassPath());
- const DexFile* dex_file = bcp_dex_files[method_info.GetDexFileIndex()];
+ DCHECK_LT(dex_file_index, bcp_dex_files.size())
+ << "OOB access to bcp_dex_files. Dumping info: "
+ << GetResolvedMethodErrorString(
+ class_linker, inlined_method, method, outer_method, dex_cache, method_info);
+ const DexFile* dex_file = bcp_dex_files[dex_file_index];
+ DCHECK_NE(dex_file, nullptr);
dex_cache = class_linker->FindDexCache(Thread::Current(), *dex_file);
} else {
ArrayRef<const OatDexFile* const> oat_dex_files(
- method->GetDexFile()->GetOatDexFile()->GetOatFile()->GetOatDexFiles());
- // TODO(solanes, 206992606): Remove check when bug is solved.
- const uint32_t index = method_info.GetDexFileIndex();
- if (UNLIKELY(index >= oat_dex_files.size())) {
- std::stringstream error_ss;
-
- error_ss << "Wanted to retrieve DexFile index " << method_info.GetDexFileIndex()
- << " from the oat_dex_files vector {";
- for (const OatDexFile* odf_value : oat_dex_files) {
- error_ss << odf_value << ",";
- }
- error_ss << "}. The outer method is: " << method->PrettyMethod() << " ("
- << method->GetDexFile()->GetLocation() << "/"
- << static_cast<const void*>(method->GetDexFile())
- << "). The outermost method in the chain is: " << outer_method->PrettyMethod()
- << " (" << outer_method->GetDexFile()->GetLocation() << "/"
- << static_cast<const void*>(outer_method->GetDexFile())
- << "). MethodInfo: method_index=" << std::dec << method_index
- << ", is_in_bootclasspath=" << std::boolalpha
- << (method_info.GetDexFileIndexKind() == MethodInfo::kKindBCP)
- << ", dex_file_index=" << std::dec << method_info.GetDexFileIndex() << ".";
- LOG(FATAL) << error_ss.str();
- UNREACHABLE();
- }
- const OatDexFile* odf = oat_dex_files[method_info.GetDexFileIndex()];
- DCHECK(odf != nullptr);
+ outer_method->GetDexFile()->GetOatDexFile()->GetOatFile()->GetOatDexFiles());
+ DCHECK_LT(dex_file_index, oat_dex_files.size())
+ << "OOB access to oat_dex_files. Dumping info: "
+ << GetResolvedMethodErrorString(
+ class_linker, inlined_method, method, outer_method, dex_cache, method_info);
+ const OatDexFile* odf = oat_dex_files[dex_file_index];
+ DCHECK_NE(odf, nullptr);
dex_cache = class_linker->FindDexCache(Thread::Current(), *odf);
}
} else {
@@ -129,33 +167,15 @@
class_linker->LookupResolvedMethod(method_index, dex_cache, dex_cache->GetClassLoader());
if (UNLIKELY(inlined_method == nullptr)) {
- LOG(FATAL) << "Could not find an inlined method from an .oat file: "
- << dex_cache->GetDexFile()->PrettyMethod(method_index) << " ("
- << dex_cache->GetDexFile()->GetLocation() << "/"
- << static_cast<const void*>(dex_cache->GetDexFile()) << ") in "
- << method->PrettyMethod() << " (" << method->GetDexFile()->GetLocation() << "/"
- << static_cast<const void*>(method->GetDexFile())
- << "). The outermost method in the chain is: " << outer_method->PrettyMethod()
- << " (" << outer_method->GetDexFile()->GetLocation() << "/"
- << static_cast<const void*>(outer_method->GetDexFile())
- << "). MethodInfo: method_index=" << std::dec << method_index
- << ", is_in_bootclasspath=" << std::boolalpha
- << (method_info.GetDexFileIndexKind() == MethodInfo::kKindBCP)
- << ", dex_file_index=" << std::dec << method_info.GetDexFileIndex() << ".";
+ LOG(FATAL) << GetResolvedMethodErrorString(
+ class_linker, inlined_method, method, outer_method, dex_cache, method_info);
UNREACHABLE();
}
DCHECK(!inlined_method->IsRuntimeMethod());
DCHECK_EQ(inlined_method->GetDexFile() == outer_method->GetDexFile(),
- method_info.GetDexFileIndex() == MethodInfo::kSameDexFile)
- << "Inlined method: " << inlined_method->PrettyMethod() << " ("
- << inlined_method->GetDexFile()->GetLocation() << "/"
- << static_cast<const void*>(inlined_method->GetDexFile()) << ") in "
- << method->PrettyMethod() << " (" << method->GetDexFile()->GetLocation() << "/"
- << static_cast<const void*>(method->GetDexFile()) << ")."
- << "The outermost method in the chain is: " << outer_method->PrettyMethod() << " ("
- << outer_method->GetDexFile()->GetLocation() << "/"
- << static_cast<const void*>(outer_method->GetDexFile());
-
+ dex_file_index == MethodInfo::kSameDexFile)
+ << GetResolvedMethodErrorString(
+ class_linker, inlined_method, method, outer_method, dex_cache, method_info);
method = inlined_method;
}
diff --git a/test/2237-checker-inline-multidex/expected-stderr.txt b/test/2237-checker-inline-multidex/expected-stderr.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/2237-checker-inline-multidex/expected-stderr.txt
diff --git a/test/2237-checker-inline-multidex/expected-stdout.txt b/test/2237-checker-inline-multidex/expected-stdout.txt
new file mode 100644
index 0000000..571349a
--- /dev/null
+++ b/test/2237-checker-inline-multidex/expected-stdout.txt
@@ -0,0 +1,4 @@
+abc
+def
+ghi
+class Multi$Multi2
diff --git a/test/2237-checker-inline-multidex/info.txt b/test/2237-checker-inline-multidex/info.txt
new file mode 100644
index 0000000..fc6dc60
--- /dev/null
+++ b/test/2237-checker-inline-multidex/info.txt
@@ -0,0 +1 @@
+Checks that we inline across dex files, even when we need an environment.
diff --git a/test/2237-checker-inline-multidex/src-multidex/Multi.java b/test/2237-checker-inline-multidex/src-multidex/Multi.java
new file mode 100644
index 0000000..cd234c3
--- /dev/null
+++ b/test/2237-checker-inline-multidex/src-multidex/Multi.java
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class Multi {
+ public static String $inline$NeedsEnvironmentMultiDex(String str) {
+ // StringBuilderAppend needs an environment but it doesn't need a .bss entry.
+ StringBuilder sb = new StringBuilder();
+ return sb.append(str).toString();
+ }
+
+ public static String $inline$NeedsBssEntryStringMultiDex() {
+ return "def";
+ }
+
+ private static String $noinline$InnerInvokeMultiDex() {
+ return "ghi";
+ }
+
+ public static String $inline$NeedsBssEntryInvokeMultiDex() {
+ return $noinline$InnerInvokeMultiDex();
+ }
+
+ public static Class<?> NeedsBssEntryClassMultiDex() {
+ return Multi2.class;
+ }
+
+ private class Multi2 {}
+}
diff --git a/test/2237-checker-inline-multidex/src/Main.java b/test/2237-checker-inline-multidex/src/Main.java
new file mode 100644
index 0000000..7ab2e7f
--- /dev/null
+++ b/test/2237-checker-inline-multidex/src/Main.java
@@ -0,0 +1,72 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class Main {
+ public static void main(String[] args) {
+ System.out.println(testNeedsEnvironment());
+ System.out.println(testNeedsBssEntryString());
+ System.out.println(testNeedsBssEntryInvoke());
+ System.out.println(testClass());
+ }
+
+ /// CHECK-START: java.lang.String Main.testNeedsEnvironment() inliner (before)
+ /// CHECK: InvokeStaticOrDirect method_name:Multi.$inline$NeedsEnvironmentMultiDex
+
+ /// CHECK-START: java.lang.String Main.testNeedsEnvironment() inliner (after)
+ /// CHECK-NOT: InvokeStaticOrDirect method_name:Multi.$inline$NeedsEnvironmentMultiDex
+
+ /// CHECK-START: java.lang.String Main.testNeedsEnvironment() inliner (after)
+ /// CHECK: StringBuilderAppend
+ public static String testNeedsEnvironment() {
+ return Multi.$inline$NeedsEnvironmentMultiDex("abc");
+ }
+
+ /// CHECK-START: java.lang.String Main.testNeedsBssEntryString() inliner (before)
+ /// CHECK: InvokeStaticOrDirect method_name:Multi.$inline$NeedsBssEntryStringMultiDex
+
+ /// CHECK-START: java.lang.String Main.testNeedsBssEntryString() inliner (after)
+ /// CHECK-NOT: InvokeStaticOrDirect method_name:Multi.$inline$NeedsBssEntryStringMultiDex
+
+ /// CHECK-START: java.lang.String Main.testNeedsBssEntryString() inliner (after)
+ /// CHECK: LoadString load_kind:BssEntry
+ public static String testNeedsBssEntryString() {
+ return Multi.$inline$NeedsBssEntryStringMultiDex();
+ }
+
+ /// CHECK-START: java.lang.String Main.testNeedsBssEntryInvoke() inliner (before)
+ /// CHECK: InvokeStaticOrDirect method_name:Multi.$inline$NeedsBssEntryInvokeMultiDex
+
+ /// CHECK-START: java.lang.String Main.testNeedsBssEntryInvoke() inliner (after)
+ /// CHECK-NOT: InvokeStaticOrDirect method_name:Multi.$inline$NeedsBssEntryInvokeMultiDex
+
+ /// CHECK-START: java.lang.String Main.testNeedsBssEntryInvoke() inliner (after)
+ /// CHECK: InvokeStaticOrDirect method_name:Multi.$noinline$InnerInvokeMultiDex method_load_kind:BssEntry
+ public static String testNeedsBssEntryInvoke() {
+ return Multi.$inline$NeedsBssEntryInvokeMultiDex();
+ }
+
+ /// CHECK-START: java.lang.Class Main.testClass() inliner (before)
+ /// CHECK: InvokeStaticOrDirect method_name:Multi.NeedsBssEntryClassMultiDex
+
+ /// CHECK-START: java.lang.Class Main.testClass() inliner (after)
+ /// CHECK-NOT: InvokeStaticOrDirect method_name:Multi.NeedsBssEntryClassMultiDex
+
+ /// CHECK-START: java.lang.Class Main.testClass() inliner (after)
+ /// CHECK: LoadClass load_kind:BssEntry class_name:Multi$Multi2
+ public static Class<?> testClass() {
+ return Multi.NeedsBssEntryClassMultiDex();
+ }
+}
diff --git a/test/462-checker-inlining-dex-files/src/Main.java b/test/462-checker-inlining-dex-files/src/Main.java
index c6fff41..f426b40 100644
--- a/test/462-checker-inlining-dex-files/src/Main.java
+++ b/test/462-checker-inlining-dex-files/src/Main.java
@@ -47,15 +47,14 @@
return OtherDex.returnIntMethod();
}
- /// CHECK-START: int Main.dontInlineOtherDexStatic() inliner (before)
- /// CHECK-DAG: <<Invoke:i\d+>> InvokeStaticOrDirect
+ /// CHECK-START: int Main.inlineOtherDexStatic() inliner (before)
+ /// CHECK-DAG: <<Invoke:i\d+>> InvokeStaticOrDirect method_name:OtherDex.returnOtherDexStatic
/// CHECK-DAG: Return [<<Invoke>>]
- /// CHECK-START: int Main.dontInlineOtherDexStatic() inliner (after)
- /// CHECK-DAG: <<Invoke:i\d+>> InvokeStaticOrDirect
- /// CHECK-DAG: Return [<<Invoke>>]
+ /// CHECK-START: int Main.inlineOtherDexStatic() inliner (after)
+ /// CHECK-NOT: InvokeStaticOrDirect method_name:OtherDex.returnOtherDexStatic
- public static int dontInlineOtherDexStatic() {
+ public static int inlineOtherDexStatic() {
return OtherDex.returnOtherDexStatic();
}
@@ -86,28 +85,25 @@
return OtherDex.recursiveCall();
}
- /// CHECK-START: java.lang.String Main.dontInlineReturnString() inliner (before)
- /// CHECK-DAG: <<Invoke:l\d+>> InvokeStaticOrDirect
+ /// CHECK-START: java.lang.String Main.inlineReturnString() inliner (before)
+ /// CHECK-DAG: <<Invoke:l\d+>> InvokeStaticOrDirect method_name:OtherDex.returnString
/// CHECK-DAG: Return [<<Invoke>>]
- /// CHECK-START: java.lang.String Main.dontInlineReturnString() inliner (after)
- /// CHECK-DAG: <<Invoke:l\d+>> InvokeStaticOrDirect
- /// CHECK-DAG: Return [<<Invoke>>]
+ /// CHECK-START: java.lang.String Main.inlineReturnString() inliner (after)
+ /// CHECK-NOT: InvokeStaticOrDirect method_name:OtherDex.returnString
- public static String dontInlineReturnString() {
+ public static String inlineReturnString() {
return OtherDex.returnString();
}
-
- /// CHECK-START: java.lang.Class Main.dontInlineOtherDexClass() inliner (before)
- /// CHECK-DAG: <<Invoke:l\d+>> InvokeStaticOrDirect
+ /// CHECK-START: java.lang.Class Main.inlineOtherDexClass() inliner (before)
+ /// CHECK-DAG: <<Invoke:l\d+>> InvokeStaticOrDirect method_name:OtherDex.returnOtherDexClass
/// CHECK-DAG: Return [<<Invoke>>]
- /// CHECK-START: java.lang.Class Main.dontInlineOtherDexClass() inliner (after)
- /// CHECK-DAG: <<Invoke:l\d+>> InvokeStaticOrDirect
- /// CHECK-DAG: Return [<<Invoke>>]
+ /// CHECK-START: java.lang.Class Main.inlineOtherDexClass() inliner (after)
+ /// CHECK-NOT: InvokeStaticOrDirect method_name:OtherDex.returnOtherDexClass
- public static Class<?> dontInlineOtherDexClass() {
+ public static Class<?> inlineOtherDexClass() {
return OtherDex.returnOtherDexClass();
}
@@ -128,15 +124,14 @@
return OtherDex.returnMainClass();
}
- /// CHECK-START: java.lang.Class Main.dontInlineOtherDexClassStaticCall() inliner (before)
- /// CHECK-DAG: <<Invoke:l\d+>> InvokeStaticOrDirect
+ /// CHECK-START: java.lang.Class Main.inlineOtherDexClassStaticCall() inliner (before)
+ /// CHECK-DAG: <<Invoke:l\d+>> InvokeStaticOrDirect method_name:OtherDex.returnOtherDexClassStaticCall
/// CHECK-DAG: Return [<<Invoke>>]
- /// CHECK-START: java.lang.Class Main.dontInlineOtherDexClassStaticCall() inliner (after)
- /// CHECK-DAG: <<Invoke:l\d+>> InvokeStaticOrDirect
- /// CHECK-DAG: Return [<<Invoke>>]
+ /// CHECK-START: java.lang.Class Main.inlineOtherDexClassStaticCall() inliner (after)
+ /// CHECK-NOT: InvokeStaticOrDirect method_name:OtherDex.returnOtherDexClassStaticCall
- public static Class<?> dontInlineOtherDexClassStaticCall() {
+ public static Class<?> inlineOtherDexClassStaticCall() {
return OtherDex.returnOtherDexClassStaticCall();
}
@@ -167,7 +162,7 @@
throw new Error("Expected 38");
}
- if (dontInlineOtherDexStatic() != 1) {
+ if (inlineOtherDexStatic() != 1) {
throw new Error("Expected 1");
}
@@ -175,15 +170,15 @@
throw new Error("Expected 42");
}
- if (dontInlineReturnString() != "OtherDex") {
+ if (inlineReturnString() != "OtherDex") {
throw new Error("Expected OtherDex");
}
- if (dontInlineOtherDexClass() != OtherDex.class) {
+ if (inlineOtherDexClass() != OtherDex.class) {
throw new Error("Expected " + OtherDex.class);
}
- if (dontInlineOtherDexClassStaticCall() != OtherDex.class) {
+ if (inlineOtherDexClassStaticCall() != OtherDex.class) {
throw new Error("Expected " + OtherDex.class);
}
diff --git a/test/580-checker-string-fact-intrinsics/src-art/Main.java b/test/580-checker-string-fact-intrinsics/src-art/Main.java
index d0750f9..34af579 100644
--- a/test/580-checker-string-fact-intrinsics/src-art/Main.java
+++ b/test/580-checker-string-fact-intrinsics/src-art/Main.java
@@ -40,16 +40,14 @@
// java.lang.StringFactory.newStringFromChars(char[] data)
//
// which contains a call to the former (non-public) native method.
- // However, this call will not be inlined (because it is a method in
- // another Dex file and which contains a call, which needs an
- // environment), so we cannot use Checker here to ensure the native
- // call was intrinsified either.
+ // After the inliner runs, we can see the inlined call and check
+ // that the compiler intrinsifies it.
/// CHECK-START: void Main.testNewStringFromChars() builder (after)
/// CHECK-DAG: InvokeStaticOrDirect method_name:java.lang.StringFactory.newStringFromChars intrinsic:None
/// CHECK-START: void Main.testNewStringFromChars() inliner (after)
- /// CHECK-DAG: InvokeStaticOrDirect method_name:java.lang.StringFactory.newStringFromChars intrinsic:None
+ /// CHECK-DAG: InvokeStaticOrDirect method_name:java.lang.StringFactory.newStringFromChars intrinsic:StringNewStringFromChars
public static void testNewStringFromChars() {
char[] chars = { 'b', 'a', 'r' };