Remove CHECK that GetThis() must always work.
Ensures GetVReg of a reference on compiled code checks that the
value is indeed a reference from the stack maps.
bug: 116683601
bug: 117452149
Test: 686-get-this
Change-Id: I3a370c45786a4892c7888491bec6d5ae64672f6c
diff --git a/runtime/interpreter/shadow_frame.h b/runtime/interpreter/shadow_frame.h
index 91371d1..88eb413 100644
--- a/runtime/interpreter/shadow_frame.h
+++ b/runtime/interpreter/shadow_frame.h
@@ -179,12 +179,8 @@
mirror::Object* GetVRegReference(size_t i) const REQUIRES_SHARED(Locks::mutator_lock_) {
DCHECK_LT(i, NumberOfVRegs());
mirror::Object* ref;
- if (HasReferenceArray()) {
- ref = References()[i].AsMirrorPtr();
- } else {
- const uint32_t* vreg_ptr = &vregs_[i];
- ref = reinterpret_cast<const StackReference<mirror::Object>*>(vreg_ptr)->AsMirrorPtr();
- }
+ DCHECK(HasReferenceArray());
+ ref = References()[i].AsMirrorPtr();
ReadBarrier::MaybeAssertToSpaceInvariant(ref);
if (kVerifyFlags & kVerifyReads) {
VerifyObject(ref);
diff --git a/runtime/stack.cc b/runtime/stack.cc
index eb9c661..25939d2 100644
--- a/runtime/stack.cc
+++ b/runtime/stack.cc
@@ -139,9 +139,9 @@
} else {
uint16_t reg = accessor.RegistersSize() - accessor.InsSize();
uint32_t value = 0;
- bool success = GetVReg(m, reg, kReferenceVReg, &value);
- // We currently always guarantee the `this` object is live throughout the method.
- CHECK(success) << "Failed to read the this object in " << ArtMethod::PrettyMethod(m);
+ if (!GetVReg(m, reg, kReferenceVReg, &value)) {
+ return nullptr;
+ }
return reinterpret_cast<mirror::Object*>(value);
}
}
@@ -223,20 +223,39 @@
switch (location_kind) {
case DexRegisterLocation::Kind::kInStack: {
const int32_t offset = dex_register_map[vreg].GetStackOffsetInBytes();
+ BitMemoryRegion stack_mask = code_info.GetStackMaskOf(stack_map);
+ if (kind == kReferenceVReg && !stack_mask.LoadBit(offset / kFrameSlotSize)) {
+ return false;
+ }
const uint8_t* addr = reinterpret_cast<const uint8_t*>(cur_quick_frame_) + offset;
*val = *reinterpret_cast<const uint32_t*>(addr);
return true;
}
- case DexRegisterLocation::Kind::kInRegister:
+ case DexRegisterLocation::Kind::kInRegister: {
+ uint32_t register_mask = code_info.GetRegisterMaskOf(stack_map);
+ uint32_t reg = dex_register_map[vreg].GetMachineRegister();
+ if (kind == kReferenceVReg && !(register_mask & (1 << reg))) {
+ return false;
+ }
+ return GetRegisterIfAccessible(reg, kind, val);
+ }
case DexRegisterLocation::Kind::kInRegisterHigh:
case DexRegisterLocation::Kind::kInFpuRegister:
case DexRegisterLocation::Kind::kInFpuRegisterHigh: {
+ if (kind == kReferenceVReg) {
+ return false;
+ }
uint32_t reg = dex_register_map[vreg].GetMachineRegister();
return GetRegisterIfAccessible(reg, kind, val);
}
- case DexRegisterLocation::Kind::kConstant:
- *val = dex_register_map[vreg].GetConstant();
+ case DexRegisterLocation::Kind::kConstant: {
+ uint32_t result = dex_register_map[vreg].GetConstant();
+ if (kind == kReferenceVReg && result != 0) {
+ return false;
+ }
+ *val = result;
return true;
+ }
case DexRegisterLocation::Kind::kNone:
return false;
default:
diff --git a/test/686-get-this/expected.txt b/test/686-get-this/expected.txt
new file mode 100644
index 0000000..6a5618e
--- /dev/null
+++ b/test/686-get-this/expected.txt
@@ -0,0 +1 @@
+JNI_OnLoad called
diff --git a/test/686-get-this/info.txt b/test/686-get-this/info.txt
new file mode 100644
index 0000000..7227bad
--- /dev/null
+++ b/test/686-get-this/info.txt
@@ -0,0 +1,2 @@
+Test that we can successfully call StackVisitor.GetThis() even when
+'this' gets overwritten.
diff --git a/test/686-get-this/smali/Test.smali b/test/686-get-this/smali/Test.smali
new file mode 100644
index 0000000..533f607
--- /dev/null
+++ b/test/686-get-this/smali/Test.smali
@@ -0,0 +1,45 @@
+# Copyright (C) 2018 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 LTest;
+.super Ljava/lang/Object;
+
+.method public constructor <init>()V
+ .registers 2
+ invoke-direct {p0}, Ljava/lang/Object;-><init>()V
+ const/4 v0, 0x1
+ sput v0, LTest;->field:I
+ return-void
+.end method
+
+
+.method public testEmpty()V
+ .registers 2
+ const/4 p0, 0x1
+ invoke-static {}, LMain;->getThisOfCaller()Ljava/lang/Object;
+ move-result-object v0
+ sput-object v0, LMain;->field:Ljava/lang/Object;
+ return-void
+.end method
+
+.method public testPrimitive()I
+ .registers 2
+ sget p0, LTest;->field:I
+ invoke-static {}, LMain;->getThisOfCaller()Ljava/lang/Object;
+ move-result-object v0
+ sput-object v0, LMain;->field:Ljava/lang/Object;
+ return p0
+.end method
+
+.field static public field:I
diff --git a/test/686-get-this/src/Main.java b/test/686-get-this/src/Main.java
new file mode 100644
index 0000000..4ea5301
--- /dev/null
+++ b/test/686-get-this/src/Main.java
@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2018 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.
+ */
+
+import java.lang.reflect.Method;
+
+public class Main {
+ public static void main(String[] args) throws Exception {
+ System.loadLibrary(args[0]);
+
+ Class<?> c = Class.forName("Test");
+ ensureJitCompiled(c, "testEmpty");
+ ensureJitCompiled(c, "testPrimitive");
+
+ Method m = c.getMethod("testEmpty");
+ m.invoke(c.newInstance());
+ if (field != null) {
+ throw new Error("Expected null");
+ }
+
+ m = c.getMethod("testPrimitive");
+ int a = (Integer)m.invoke(c.newInstance());
+ if (a != 1) {
+ throw new Error("Expected 1, got " + a);
+ }
+ if (field != null) {
+ throw new Error("Expected null");
+ }
+ }
+
+ public static Object field;
+
+ private static native void ensureJitCompiled(Class<?> itf, String method_name);
+ public static native Object getThisOfCaller();
+}
diff --git a/test/common/stack_inspect.cc b/test/common/stack_inspect.cc
index d74d2ef..581aa74 100644
--- a/test/common/stack_inspect.cc
+++ b/test/common/stack_inspect.cc
@@ -196,4 +196,24 @@
}
}
+struct GetCallingFrameVisitor : public StackVisitor {
+ GetCallingFrameVisitor(Thread* thread, Context* context)
+ REQUIRES_SHARED(Locks::mutator_lock_)
+ : StackVisitor(thread, context, StackVisitor::StackWalkKind::kIncludeInlinedFrames) {}
+
+ bool VisitFrame() override NO_THREAD_SAFETY_ANALYSIS {
+ // Discard stubs and Main.getThisOfCaller.
+ return GetMethod() == nullptr || GetMethod()->IsNative();
+ }
+};
+
+extern "C" JNIEXPORT jobject JNICALL Java_Main_getThisOfCaller(
+ JNIEnv* env, jclass cls ATTRIBUTE_UNUSED) {
+ ScopedObjectAccess soa(env);
+ std::unique_ptr<art::Context> context(art::Context::Create());
+ GetCallingFrameVisitor visitor(soa.Self(), context.get());
+ visitor.WalkStack();
+ return soa.AddLocalReference<jobject>(visitor.GetThisObject());
+}
+
} // namespace art
diff --git a/test/knownfailures.json b/test/knownfailures.json
index e27a4d6..db06c4e 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -947,6 +947,7 @@
"676-resolve-field-type",
"685-deoptimizeable",
"685-shifts",
+ "686-get-this",
"706-checker-scheduler",
"707-checker-invalid-profile",
"714-invoke-custom-lambda-metafactory",
