Visit targets of proxy methods when visiting thread roots.
The target of a non-static proxy method (`this` object), stored in the
proxy method's stack frame, needs to be visited as GC root. This is
especially important in the case of a moving GC, where the proxy
instance may be moved like any object.
Fix initially provided by Robert Vollmer.
Test: m test-art-host
Test: art/test/testrunner/testrunner.py --gcstress -t 1939-proxy-frames
Test: art/test/testrunner/testrunner.py --gcstress -t 1914-get-local-instance
Bug: 70216372
Bug: 67679263
Change-Id: Iea27a8eba51ccd9c9055efaf6b263892830170b5
diff --git a/runtime/art_method-inl.h b/runtime/art_method-inl.h
index e6e35c8..8b48aa2 100644
--- a/runtime/art_method-inl.h
+++ b/runtime/art_method-inl.h
@@ -328,6 +328,7 @@
}
inline bool ArtMethod::IsProxyMethod() {
+ DCHECK(!IsRuntimeMethod()) << "ArtMethod::IsProxyMethod called on a runtime method";
// Avoid read barrier since the from-space version of the class will have the correct proxy class
// flags since they are constant for the lifetime of the class.
return GetDeclaringClass<kWithoutReadBarrier>()->IsProxyClass();
diff --git a/runtime/art_method.h b/runtime/art_method.h
index cec2ec4..21ee8f1 100644
--- a/runtime/art_method.h
+++ b/runtime/art_method.h
@@ -551,7 +551,7 @@
// Is this a CalleSaveMethod or ResolutionMethod and therefore doesn't adhere to normal
// conventions for a method of managed code. Returns false for Proxy methods.
ALWAYS_INLINE bool IsRuntimeMethod() {
- return dex_method_index_ == kRuntimeMethodDexMethodIndex;;
+ return dex_method_index_ == kRuntimeMethodDexMethodIndex;
}
// Is this a hand crafted method used for something like describing callee saves?
diff --git a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
index c5157ce..a8c328f 100644
--- a/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_trampoline_entrypoints.cc
@@ -319,7 +319,7 @@
// 'this' object is the 1st argument. They also have the same frame layout as the
// kRefAndArgs runtime method. Since 'this' is a reference, it is located in the
// 1st GPR.
- static mirror::Object* GetProxyThisObject(ArtMethod** sp)
+ static StackReference<mirror::Object>* GetProxyThisObjectReference(ArtMethod** sp)
REQUIRES_SHARED(Locks::mutator_lock_) {
CHECK((*sp)->IsProxyMethod());
CHECK_GT(kNumQuickGprArgs, 0u);
@@ -327,7 +327,7 @@
size_t this_arg_offset = kQuickCalleeSaveFrame_RefAndArgs_Gpr1Offset +
GprIndexToGprOffset(kThisGprIndex);
uint8_t* this_arg_address = reinterpret_cast<uint8_t*>(sp) + this_arg_offset;
- return reinterpret_cast<StackReference<mirror::Object>*>(this_arg_address)->AsMirrorPtr();
+ return reinterpret_cast<StackReference<mirror::Object>*>(this_arg_address);
}
static ArtMethod* GetCallingMethod(ArtMethod** sp) REQUIRES_SHARED(Locks::mutator_lock_) {
@@ -647,7 +647,11 @@
// allows to use the QuickArgumentVisitor constants without moving all the code in its own module.
extern "C" mirror::Object* artQuickGetProxyThisObject(ArtMethod** sp)
REQUIRES_SHARED(Locks::mutator_lock_) {
- return QuickArgumentVisitor::GetProxyThisObject(sp);
+ return QuickArgumentVisitor::GetProxyThisObjectReference(sp)->AsMirrorPtr();
+}
+extern "C" StackReference<mirror::Object>* artQuickGetProxyThisObjectReference(ArtMethod** sp)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ return QuickArgumentVisitor::GetProxyThisObjectReference(sp);
}
// Visits arguments on the stack placing them into the shadow frame.
diff --git a/runtime/thread.cc b/runtime/thread.cc
index 9dc92f3..7136fee 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -3435,6 +3435,9 @@
return object != nullptr && object->GetLockOwnerThreadId() == GetThreadId();
}
+extern "C" StackReference<mirror::Object>* artQuickGetProxyThisObjectReference(ArtMethod** sp)
+ REQUIRES_SHARED(Locks::mutator_lock_);
+
// RootVisitor parameters are: (const Object* obj, size_t vreg, const StackVisitor* visitor).
template <typename RootVisitor, bool kPrecise = false>
class ReferenceMapVisitor : public StackVisitor {
@@ -3535,7 +3538,7 @@
if (!m->IsNative() && !m->IsRuntimeMethod() && (!m->IsProxyMethod() || m->IsConstructor())) {
const OatQuickMethodHeader* method_header = GetCurrentOatQuickMethodHeader();
DCHECK(method_header->IsOptimized());
- auto* vreg_base = reinterpret_cast<StackReference<mirror::Object>*>(
+ StackReference<mirror::Object>* vreg_base = reinterpret_cast<StackReference<mirror::Object>*>(
reinterpret_cast<uintptr_t>(cur_quick_frame));
uintptr_t native_pc_offset = method_header->NativeQuickPcOffset(GetCurrentQuickFramePc());
CodeInfo code_info = method_header->GetOptimizedCodeInfo();
@@ -3550,7 +3553,7 @@
BitMemoryRegion stack_mask = code_info.GetStackMaskOf(encoding, map);
for (size_t i = 0; i < number_of_bits; ++i) {
if (stack_mask.LoadBit(i)) {
- auto* ref_addr = vreg_base + i;
+ StackReference<mirror::Object>* ref_addr = vreg_base + i;
mirror::Object* ref = ref_addr->AsMirrorPtr();
if (ref != nullptr) {
mirror::Object* new_ref = ref;
@@ -3579,6 +3582,19 @@
}
}
}
+ } else if (!m->IsStatic() && !m->IsRuntimeMethod() && m->IsProxyMethod()) {
+ // If this is a non-static proxy method, visit its target (`this` object).
+ DCHECK(!m->IsNative());
+ StackReference<mirror::Object>* ref_addr =
+ artQuickGetProxyThisObjectReference(cur_quick_frame);
+ mirror::Object* ref = ref_addr->AsMirrorPtr();
+ if (ref != nullptr) {
+ mirror::Object* new_ref = ref;
+ visitor_(&new_ref, -1, this);
+ if (ref != new_ref) {
+ ref_addr->Assign(new_ref);
+ }
+ }
}
}
@@ -3773,9 +3789,9 @@
void Thread::VisitRoots(RootVisitor* visitor, VisitRootFlags flags) {
if ((flags & VisitRootFlags::kVisitRootFlagPrecise) != 0) {
- VisitRoots<true>(visitor);
+ VisitRoots</* kPrecise */ true>(visitor);
} else {
- VisitRoots<false>(visitor);
+ VisitRoots</* kPrecise */ false>(visitor);
}
}
diff --git a/test/knownfailures.json b/test/knownfailures.json
index 2b28409..980d605 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1,12 +1,5 @@
[
{
- "tests": [ "1939-proxy-frames", "1914-get-local-instance" ],
- "description": ["Test 1939 & 1914 seems to consistently fail in gcstress on 64 bit with",
- "a proxy this object having no associated class!"],
- "variant": "gcstress",
- "bug": "http://b/67679263"
- },
- {
"tests": "1934-jvmti-signal-thread",
"description": ["Disables 1934-jvmti-signal-thread in tracing configurations"],
"variant": "trace | stream",