diff options
| -rw-r--r-- | runtime/interpreter/shadow_frame.h | 8 | ||||
| -rw-r--r-- | runtime/stack.cc | 31 | ||||
| -rw-r--r-- | test/686-get-this/expected.txt | 1 | ||||
| -rw-r--r-- | test/686-get-this/info.txt | 2 | ||||
| -rw-r--r-- | test/686-get-this/smali/Test.smali | 45 | ||||
| -rw-r--r-- | test/686-get-this/src/Main.java | 47 | ||||
| -rw-r--r-- | test/common/stack_inspect.cc | 20 | ||||
| -rw-r--r-- | test/knownfailures.json | 1 |
8 files changed, 143 insertions, 12 deletions
diff --git a/runtime/interpreter/shadow_frame.h b/runtime/interpreter/shadow_frame.h index 91371d1e4e..88eb413ec7 100644 --- a/runtime/interpreter/shadow_frame.h +++ b/runtime/interpreter/shadow_frame.h @@ -179,12 +179,8 @@ class ShadowFrame { 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 eb9c661d18..25939d2f2c 100644 --- a/runtime/stack.cc +++ b/runtime/stack.cc @@ -139,9 +139,9 @@ mirror::Object* StackVisitor::GetThisObject() const { } 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 @@ bool StackVisitor::GetVRegFromOptimizedCode(ArtMethod* m, uint16_t vreg, VRegKin 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 0000000000..6a5618ebc6 --- /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 0000000000..7227bad602 --- /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 0000000000..533f60774e --- /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 0000000000..4ea5301f2d --- /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 d74d2efa12..581aa74d4e 100644 --- a/test/common/stack_inspect.cc +++ b/test/common/stack_inspect.cc @@ -196,4 +196,24 @@ extern "C" JNIEXPORT void JNICALL Java_Main_assertCallerIsManaged(JNIEnv* env, j } } +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 e27a4d6f44..db06c4e69d 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", |