Don't store copied methods in BSS.
Otherwise, we can end up in a state where the method on the stack is
unrelated to the receiver.
Also fix a comment related to GetCanonicalMethod and
StackVisitor::ValidateFrame.
Test: 810-checker-invoke-super-default
Change-Id: I3030e4af6059f7a4a7a1f046f2aabae8ce9057da
diff --git a/compiler/optimizing/graph_visualizer.cc b/compiler/optimizing/graph_visualizer.cc
index d5840fc..3f6215a 100644
--- a/compiler/optimizing/graph_visualizer.cc
+++ b/compiler/optimizing/graph_visualizer.cc
@@ -23,6 +23,7 @@
#include "android-base/stringprintf.h"
#include "art_method.h"
+#include "art_method-inl.h"
#include "base/intrusive_forward_list.h"
#include "bounds_check_elimination.h"
#include "builder.h"
@@ -470,6 +471,9 @@
StartAttributeStream("always_throws") << std::boolalpha
<< invoke->AlwaysThrows()
<< std::noboolalpha;
+ if (method != nullptr) {
+ StartAttributeStream("method_index") << method->GetMethodIndex();
+ }
}
void VisitInvokeUnresolved(HInvokeUnresolved* invoke) override {
diff --git a/compiler/optimizing/instruction_builder.cc b/compiler/optimizing/instruction_builder.cc
index ab28e4b..e9d1ee2 100644
--- a/compiler/optimizing/instruction_builder.cc
+++ b/compiler/optimizing/instruction_builder.cc
@@ -964,7 +964,9 @@
// could resolve the callee to the wrong method.
return nullptr;
}
- resolved_method = actual_method;
+ // Call GetCanonicalMethod in case the resolved method is a copy: for super calls, the encoding
+ // of ArtMethod in BSS relies on not having copies there.
+ resolved_method = actual_method->GetCanonicalMethod(class_linker->GetImagePointerSize());
}
if (*invoke_type == kInterface) {
diff --git a/compiler/optimizing/sharpening.cc b/compiler/optimizing/sharpening.cc
index 67cd200..393369d 100644
--- a/compiler/optimizing/sharpening.cc
+++ b/compiler/optimizing/sharpening.cc
@@ -99,6 +99,7 @@
} else if (!has_method_id) {
method_load_kind = MethodLoadKind::kRuntimeCall;
} else {
+ DCHECK(!callee->IsCopied());
// Use PC-relative access to the .bss methods array.
method_load_kind = MethodLoadKind::kBssEntry;
}
@@ -124,6 +125,7 @@
method_load_kind = MethodLoadKind::kRuntimeCall;
code_ptr_location = CodePtrLocation::kCallArtMethod;
} else {
+ DCHECK(!callee->IsCopied());
// Use PC-relative access to the .bss methods array.
method_load_kind = MethodLoadKind::kBssEntry;
code_ptr_location = CodePtrLocation::kCallArtMethod;
diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index dc990ab..45c50b0 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -1320,6 +1320,10 @@
// If successful, update .bss entry in oat file if any.
if (called != nullptr) {
+ // We only put non copied methods in the BSS. Putting a copy can lead to an
+ // odd situation where the ArtMethod being executed is unrelated to the
+ // receiver of the method.
+ called = called->GetCanonicalMethod();
if (invoke_type == kSuper) {
if (called->GetDexFile() == called_method.dex_file) {
called_method.index = called->GetDexMethodIndex();
diff --git a/runtime/stack.cc b/runtime/stack.cc
index a20f40c..094c25b 100644
--- a/runtime/stack.cc
+++ b/runtime/stack.cc
@@ -711,8 +711,8 @@
LinearAlloc* const linear_alloc = runtime->GetLinearAlloc();
if (!linear_alloc->Contains(method)) {
// Check class linker linear allocs.
- // We get the canonical method as copied methods may have their declaring
- // class from another class loader.
+ // We get the canonical method as copied methods may have been allocated
+ // by a different class loader.
const PointerSize ptrSize = runtime->GetClassLinker()->GetImagePointerSize();
ArtMethod* canonical = method->GetCanonicalMethod(ptrSize);
ObjPtr<mirror::Class> klass = canonical->GetDeclaringClass();
diff --git a/test/810-checker-invoke-super-default/expected.txt b/test/810-checker-invoke-super-default/expected.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/810-checker-invoke-super-default/expected.txt
diff --git a/test/810-checker-invoke-super-default/info.txt b/test/810-checker-invoke-super-default/info.txt
new file mode 100644
index 0000000..f5c5dbf
--- /dev/null
+++ b/test/810-checker-invoke-super-default/info.txt
@@ -0,0 +1 @@
+Test that we don't store copied methods in BSS.
diff --git a/test/810-checker-invoke-super-default/smali/AbstractClass.smali b/test/810-checker-invoke-super-default/smali/AbstractClass.smali
new file mode 100644
index 0000000..c7be9c7
--- /dev/null
+++ b/test/810-checker-invoke-super-default/smali/AbstractClass.smali
@@ -0,0 +1,17 @@
+# 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 abstract LAbstractClass;
+.super Ljava/lang/Object;
+.implements LItf;
diff --git a/test/810-checker-invoke-super-default/smali/Class.smali b/test/810-checker-invoke-super-default/smali/Class.smali
new file mode 100644
index 0000000..807b839
--- /dev/null
+++ b/test/810-checker-invoke-super-default/smali/Class.smali
@@ -0,0 +1,30 @@
+# 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 LAbstractClass;
+
+## CHECK-START: void Class.$noinline$foo() register (after)
+# We check that the method_index is 0, as that's the method index for the interface method.
+# The copied method would have a different method index.
+## CHECK-DAG: InvokeStaticOrDirect method_name:Itf.$noinline$foo method_index:0
+## CHECK-DAG: InvokeStaticOrDirect method_name:Itf.$noinline$foo method_index:0
+## CHECK-DAG: InvokeStaticOrDirect method_name:Itf.$noinline$foo method_index:0
+.method public $noinline$foo()V
+.registers 1
+ invoke-super {p0}, LClass;->$noinline$foo()V
+ invoke-super {p0}, LAbstractClass;->$noinline$foo()V
+ invoke-super {p0}, LItf;->$noinline$foo()V
+ return-void
+.end method
diff --git a/test/810-checker-invoke-super-default/smali/Itf.smali b/test/810-checker-invoke-super-default/smali/Itf.smali
new file mode 100644
index 0000000..350c85d
--- /dev/null
+++ b/test/810-checker-invoke-super-default/smali/Itf.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 public abstract interface LItf;
+.super Ljava/lang/Object;
+
+.method public $noinline$foo()V
+.registers 1
+ return-void
+.end method
diff --git a/test/810-checker-invoke-super-default/smali/Main.smali b/test/810-checker-invoke-super-default/smali/Main.smali
new file mode 100644
index 0000000..6d67f08
--- /dev/null
+++ b/test/810-checker-invoke-super-default/smali/Main.smali
@@ -0,0 +1,29 @@
+# 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
+ new-instance v0, LOtherClass;
+ invoke-direct {v0}, LOtherClass;-><init>()V
+ invoke-virtual {v0}, LOtherClass;->$noinline$foo()V
+ invoke-virtual {v0}, LOtherClass;->$noinline$foo()V
+ return-void
+.end method
diff --git a/test/810-checker-invoke-super-default/smali/OtherAbstractClass.smali b/test/810-checker-invoke-super-default/smali/OtherAbstractClass.smali
new file mode 100644
index 0000000..496484c
--- /dev/null
+++ b/test/810-checker-invoke-super-default/smali/OtherAbstractClass.smali
@@ -0,0 +1,17 @@
+# 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 abstract LOtherAbstractClass;
+.super Ljava/lang/Object;
+.implements LItf;
diff --git a/test/810-checker-invoke-super-default/smali/OtherClass.smali b/test/810-checker-invoke-super-default/smali/OtherClass.smali
new file mode 100644
index 0000000..e6e18fe
--- /dev/null
+++ b/test/810-checker-invoke-super-default/smali/OtherClass.smali
@@ -0,0 +1,30 @@
+# 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 LOtherClass;
+.super LOtherAbstractClass;
+
+## CHECK-START: void OtherClass.$noinline$foo() register (after)
+# We check that the method_index is 0, as that's the method index for the interface method.
+# The copied method would have a different method index.
+## CHECK-DAG: InvokeStaticOrDirect method_name:Itf.$noinline$foo method_index:0
+## CHECK-DAG: InvokeStaticOrDirect method_name:Itf.$noinline$foo method_index:0
+## CHECK-DAG: InvokeStaticOrDirect method_name:Itf.$noinline$foo method_index:0
+.method public $noinline$foo()V
+.registers 1
+ invoke-super {p0}, LOtherClass;->$noinline$foo()V
+ invoke-super {p0}, LOtherAbstractClass;->$noinline$foo()V
+ invoke-super {p0}, LItf;->$noinline$foo()V
+ return-void
+.end method
diff --git a/test/knownfailures.json b/test/knownfailures.json
index edaea07..f5a825c 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1133,6 +1133,7 @@
"723-string-init-range",
"808-checker-invoke-super",
"809-checker-invoke-super-bss",
+ "810-checker-invoke-super-default",
"999-redefine-hiddenapi",
"1000-non-moving-space-stress",
"1001-app-image-regions",