Handle more cases of super calls in the compiler.
Add support for calling super methods that are not referenced within the
compiling dex file.
Test: 808-checker-invoke-super
Test: 809-checker-invoke-super-bss
Change-Id: Ib103f818ac8b612a79b6b18cc8eda81131bb3149
diff --git a/compiler/optimizing/instruction_builder.cc b/compiler/optimizing/instruction_builder.cc
index 3401e65..9fb9c4e 100644
--- a/compiler/optimizing/instruction_builder.cc
+++ b/compiler/optimizing/instruction_builder.cc
@@ -957,18 +957,6 @@
actual_method = compiling_class->GetSuperClass()->GetVTableEntry(
vtable_index, class_linker->GetImagePointerSize());
}
- if (actual_method != resolved_method &&
- !IsSameDexFile(*actual_method->GetDexFile(), *dex_compilation_unit.GetDexFile())) {
- // The back-end code generator relies on this check in order to ensure that it will not
- // attempt to read the dex_cache with a dex_method_index that is not from the correct
- // dex_file. If we didn't do this check then the dex_method_index will not be updated in the
- // builder, which means that the code-generator (and sharpening and inliner, maybe)
- // might invoke an incorrect method.
- // TODO: The actual method could still be referenced in the current dex file, so we
- // could try locating it.
- // TODO: Remove the dex_file restriction.
- return nullptr;
- }
if (!actual_method->IsInvokable()) {
// Fail if the actual method cannot be invoked. Otherwise, the runtime resolution stub
// could resolve the callee to the wrong method.
@@ -1092,15 +1080,31 @@
HInvoke* invoke = nullptr;
if (invoke_type == kDirect || invoke_type == kStatic || invoke_type == kSuper) {
+ // For sharpening, we create another MethodReference, to account for the
+ // kSuper case below where we cannot find a dex method index.
+ bool has_method_id = true;
if (invoke_type == kSuper) {
+ uint32_t dex_method_index = method_reference.index;
if (IsSameDexFile(*method_info.dex_file, *dex_compilation_unit_->GetDexFile())) {
// Update the method index to the one resolved. Note that this may be a no-op if
// we resolved to the method referenced by the instruction.
- method_reference.index = method_info.index;
+ dex_method_index = method_info.index;
+ } else {
+ // Try to find a dex method index in this caller's dex file.
+ ScopedObjectAccess soa(Thread::Current());
+ dex_method_index = resolved_method->FindDexMethodIndexInOtherDexFile(
+ *dex_compilation_unit_->GetDexFile(), method_idx);
+ }
+ if (dex_method_index == dex::kDexNoIndex) {
+ has_method_id = false;
+ } else {
+ method_reference.index = dex_method_index;
}
}
HInvokeStaticOrDirect::DispatchInfo dispatch_info =
- HSharpening::SharpenInvokeStaticOrDirect(resolved_method, code_generator_);
+ HSharpening::SharpenInvokeStaticOrDirect(resolved_method,
+ has_method_id,
+ code_generator_);
if (dispatch_info.code_ptr_location ==
HInvokeStaticOrDirect::CodePtrLocation::kCallCriticalNative) {
graph_->SetHasDirectCriticalNativeCall(true);
diff --git a/compiler/optimizing/instruction_simplifier.cc b/compiler/optimizing/instruction_simplifier.cc
index d586306..d616912 100644
--- a/compiler/optimizing/instruction_simplifier.cc
+++ b/compiler/optimizing/instruction_simplifier.cc
@@ -2310,7 +2310,8 @@
// the invoke, as we would need to look it up in the current dex file, and it
// is unlikely that it exists. The most usual situation for such typed
// arraycopy methods is a direct pointer to the boot image.
- invoke->SetDispatchInfo(HSharpening::SharpenInvokeStaticOrDirect(method, codegen_));
+ invoke->SetDispatchInfo(
+ HSharpening::SharpenInvokeStaticOrDirect(method, /* has_method_id= */ true, codegen_));
}
}
}
diff --git a/compiler/optimizing/sharpening.cc b/compiler/optimizing/sharpening.cc
index 04a8eab..f658f8a 100644
--- a/compiler/optimizing/sharpening.cc
+++ b/compiler/optimizing/sharpening.cc
@@ -58,7 +58,7 @@
}
HInvokeStaticOrDirect::DispatchInfo HSharpening::SharpenInvokeStaticOrDirect(
- ArtMethod* callee, CodeGenerator* codegen) {
+ ArtMethod* callee, bool has_method_id, CodeGenerator* codegen) {
if (kIsDebugBuild) {
ScopedObjectAccess soa(Thread::Current()); // Required for GetDeclaringClass below.
DCHECK(callee != nullptr);
@@ -96,6 +96,8 @@
method_load_kind = HInvokeStaticOrDirect::MethodLoadKind::kBootImageRelRo;
} else if (BootImageAOTCanEmbedMethod(callee, compiler_options)) {
method_load_kind = HInvokeStaticOrDirect::MethodLoadKind::kBootImageLinkTimePcRelative;
+ } else if (!has_method_id) {
+ method_load_kind = HInvokeStaticOrDirect::MethodLoadKind::kRuntimeCall;
} else {
// Use PC-relative access to the .bss methods array.
method_load_kind = HInvokeStaticOrDirect::MethodLoadKind::kBssEntry;
@@ -118,6 +120,9 @@
// Use PC-relative access to the .data.bimg.rel.ro methods array.
method_load_kind = HInvokeStaticOrDirect::MethodLoadKind::kBootImageRelRo;
code_ptr_location = HInvokeStaticOrDirect::CodePtrLocation::kCallArtMethod;
+ } else if (!has_method_id) {
+ method_load_kind = HInvokeStaticOrDirect::MethodLoadKind::kRuntimeCall;
+ code_ptr_location = HInvokeStaticOrDirect::CodePtrLocation::kCallArtMethod;
} else {
// Use PC-relative access to the .bss methods array.
method_load_kind = HInvokeStaticOrDirect::MethodLoadKind::kBssEntry;
diff --git a/compiler/optimizing/sharpening.h b/compiler/optimizing/sharpening.h
index b818672..b48cd4b 100644
--- a/compiler/optimizing/sharpening.h
+++ b/compiler/optimizing/sharpening.h
@@ -31,7 +31,7 @@
public:
// Used by the builder and InstructionSimplifier.
static HInvokeStaticOrDirect::DispatchInfo SharpenInvokeStaticOrDirect(
- ArtMethod* callee, CodeGenerator* codegen);
+ ArtMethod* callee, bool has_method_id, CodeGenerator* codegen);
// Used by the builder and the inliner.
static HLoadClass::LoadKind ComputeLoadClassKind(HLoadClass* load_class,
diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index 6be095b..dc990ab 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -1322,9 +1322,11 @@
if (called != nullptr) {
if (invoke_type == kSuper) {
if (called->GetDexFile() == called_method.dex_file) {
- // When the target is in a different dex file, the compiler makes sure
- // the index is relative to the caller's dex file.
called_method.index = called->GetDexMethodIndex();
+ } else {
+ called_method.index = called->FindDexMethodIndexInOtherDexFile(
+ *called_method.dex_file, called_method.index);
+ DCHECK_NE(called_method.index, dex::kDexNoIndex);
}
}
MaybeUpdateBssMethodEntry(called, called_method);
diff --git a/test/808-checker-invoke-super/expected.txt b/test/808-checker-invoke-super/expected.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/808-checker-invoke-super/expected.txt
diff --git a/test/808-checker-invoke-super/info.txt b/test/808-checker-invoke-super/info.txt
new file mode 100644
index 0000000..015ce5c
--- /dev/null
+++ b/test/808-checker-invoke-super/info.txt
@@ -0,0 +1,2 @@
+Test that even if a supercall cannot directly reference the resolved method
+within the dex file, we still generate a HInvokeStaticOrDirect.
diff --git a/test/808-checker-invoke-super/smali-multidex/Class.smali b/test/808-checker-invoke-super/smali-multidex/Class.smali
new file mode 100644
index 0000000..e312c45
--- /dev/null
+++ b/test/808-checker-invoke-super/smali-multidex/Class.smali
@@ -0,0 +1,24 @@
+# Copyright 2020 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.
+
+.class LClass;
+.super LSuperClass;
+
+## CHECK-START: void Class.$noinline$foo() register (after)
+## CHECK-DAG: InvokeStaticOrDirect method_name:SuperClass.$noinline$foo
+.method public $noinline$foo()V
+.registers 1
+ invoke-super {p0}, LClass;->$noinline$foo()V
+ return-void
+.end method
diff --git a/test/808-checker-invoke-super/smali/Main.smali b/test/808-checker-invoke-super/smali/Main.smali
new file mode 100644
index 0000000..b9122b3
--- /dev/null
+++ b/test/808-checker-invoke-super/smali/Main.smali
@@ -0,0 +1,25 @@
+# Copyright 2020 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.
+
+.class LMain;
+.super Ljava/lang/Object;
+
+.method public static main([Ljava/lang/String;)V
+.registers 1
+ new-instance v0, LClass;
+ invoke-direct {v0}, LClass;-><init>()V
+ invoke-virtual {v0}, LClass;->$noinline$foo()V
+ invoke-virtual {v0}, LClass;->$noinline$foo()V
+ return-void
+.end method
diff --git a/test/808-checker-invoke-super/smali/SuperClass.smali b/test/808-checker-invoke-super/smali/SuperClass.smali
new file mode 100644
index 0000000..506cf56
--- /dev/null
+++ b/test/808-checker-invoke-super/smali/SuperClass.smali
@@ -0,0 +1,21 @@
+# Copyright 2020 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.
+
+.class LSuperClass;
+.super Ljava/lang/Object;
+
+.method public $noinline$foo()V
+.registers 1
+ return-void
+.end method
diff --git a/test/809-checker-invoke-super-bss/smali-multidex/OtherClass.smali b/test/809-checker-invoke-super-bss/smali-multidex/OtherClass.smali
index 92770fe..2a87f4b 100644
--- a/test/809-checker-invoke-super-bss/smali-multidex/OtherClass.smali
+++ b/test/809-checker-invoke-super-bss/smali-multidex/OtherClass.smali
@@ -16,10 +16,11 @@
.super LSuperClass;
## CHECK-START: void OtherClass.$noinline$foo() register (after)
-# TODO(ngeoffray): We should emit HInvokeStaticDirect
-## CHECK-DAG: InvokeUnresolved method_name:OtherClass.$noinline$foo
+## CHECK-DAG: InvokeStaticOrDirect dex_file_index:<<Index1:\d+>> method_name:SuperSuperClass.$noinline$foo
## CHECK-DAG: InvokeStaticOrDirect dex_file_index:<<Index2:\d+>> method_name:SuperSuperClass.$noinline$foo
## CHECK-DAG: InvokeStaticOrDirect dex_file_index:<<Index3:\d+>> method_name:SuperSuperClass.$noinline$foo
+## CHECK-EVAL: <<Index1>> == <<Index2>>
+## CHECK-EVAL: <<Index1>> == <<Index3>>
.method public $noinline$foo()V
.registers 1
invoke-super {p0}, LOtherClass;->$noinline$foo()V
diff --git a/test/knownfailures.json b/test/knownfailures.json
index 36cfd19..edaea07 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1131,6 +1131,7 @@
"692-vdex-inmem-loader",
"693-vdex-inmem-loader-evict",
"723-string-init-range",
+ "808-checker-invoke-super",
"809-checker-invoke-super-bss",
"999-redefine-hiddenapi",
"1000-non-moving-space-stress",