Fix lock visiting for synchronized native methods.
The `GetGenericJniSynchronizationObject()` function was used
in the wrong context. As documented, it can be used only for
a method with a GenericJni frame and also on the top of the
stack. When visiting locks, we can have a non-GenericJni
method frame as well as a method deeper in the stack.
Replace the wrong use with specialized code.
(cherry picked from commit 654f01cd509ca11eae22177d4e764f1241fb3a53)
Test: Added regression test to 178-app-image-native-methods
Test: testrunner.py --host --debug --ndebug
Bug: 172332525
Bug: 189235039
Merged-In: Ia26f0b980c04a766e31b1588a1c011bcf46c90d8
Change-Id: I48af0e2838cda4774b8129927ea04cee9cf3a9f8
diff --git a/runtime/entrypoints/entrypoint_utils-inl.h b/runtime/entrypoints/entrypoint_utils-inl.h
index 01e8911..84299d5 100644
--- a/runtime/entrypoints/entrypoint_utils-inl.h
+++ b/runtime/entrypoints/entrypoint_utils-inl.h
@@ -765,10 +765,13 @@
DCHECK(self->GetManagedStack()->GetTopQuickFrame() != nullptr);
DCHECK_EQ(*self->GetManagedStack()->GetTopQuickFrame(), called);
if (called->IsStatic()) {
+ // Static methods synchronize on the declaring class object.
// The `jclass` is a pointer to the method's declaring class.
return reinterpret_cast<jobject>(called->GetDeclaringClassAddressWithoutBarrier());
} else {
+ // Instance methods synchronize on the `this` object.
// The `this` reference is stored in the first out vreg in the caller's frame.
+ // The `jobject` is a pointer to the spill slot.
uint8_t* sp = reinterpret_cast<uint8_t*>(self->GetManagedStack()->GetTopQuickFrame());
size_t frame_size = RuntimeCalleeSaveFrame::GetFrameSize(CalleeSaveType::kSaveRefsAndArgs);
return reinterpret_cast<jobject>(sp + frame_size + static_cast<size_t>(kRuntimePointerSize));
diff --git a/runtime/monitor.cc b/runtime/monitor.cc
index 2f59022..f2189e1 100644
--- a/runtime/monitor.cc
+++ b/runtime/monitor.cc
@@ -1452,9 +1452,21 @@
// TODO: use the JNI implementation's table of explicit MonitorEnter calls and dump those too.
if (m->IsNative()) {
if (m->IsSynchronized()) {
- Thread* thread = stack_visitor->GetThread();
- jobject lock = GetGenericJniSynchronizationObject(thread, m);
- callback(thread->DecodeJObject(lock), callback_context);
+ DCHECK(!m->IsCriticalNative());
+ DCHECK(!m->IsFastNative());
+ ObjPtr<mirror::Object> lock;
+ if (m->IsStatic()) {
+ // Static methods synchronize on the declaring class object.
+ lock = m->GetDeclaringClass();
+ } else {
+ // Instance methods synchronize on the `this` object.
+ // The `this` reference is stored in the first out vreg in the caller's frame.
+ uint8_t* sp = reinterpret_cast<uint8_t*>(stack_visitor->GetCurrentQuickFrame());
+ size_t frame_size = stack_visitor->GetCurrentQuickFrameInfo().FrameSizeInBytes();
+ lock = reinterpret_cast<StackReference<mirror::Object>*>(
+ sp + frame_size + static_cast<size_t>(kRuntimePointerSize))->AsMirrorPtr();
+ }
+ callback(lock, callback_context);
}
return;
}
diff --git a/test/178-app-image-native-method/native_methods.cc b/test/178-app-image-native-method/native_methods.cc
index 0669deb..709c5df 100644
--- a/test/178-app-image-native-method/native_methods.cc
+++ b/test/178-app-image-native-method/native_methods.cc
@@ -14,8 +14,16 @@
* limitations under the License.
*/
+#include <sstream>
+
#include "jni.h"
+#include "arch/context.h"
+#include "monitor.h"
+#include "scoped_thread_state_change-inl.h"
+#include "stack.h"
+#include "thread-current-inl.h"
+
namespace art {
static inline bool VerifyManyParameters(
@@ -638,4 +646,60 @@
return ok ? 42 : -1;
}
+extern "C" JNIEXPORT jint JNICALL Java_Main_b189235039CallThrough(JNIEnv* env, jobject m) {
+ jclass main_klass = env->GetObjectClass(m);
+ jmethodID checkLocks = env->GetStaticMethodID(main_klass, "b189235039CheckLocks", "(ILMain;)I");
+ if (checkLocks == nullptr) {
+ return -1;
+ }
+ return env->CallStaticIntMethod(main_klass, checkLocks, 42, m);
+}
+
+extern "C" JNIEXPORT jint JNICALL Java_Main_b189235039CheckLocks(JNIEnv*,
+ jclass,
+ int arg,
+ jobject lock) {
+ // Check that we do not crash when visiting locks and that we find the right lock.
+ ScopedObjectAccess soa(Thread::Current());
+ class VisitLocks : public StackVisitor {
+ public:
+ VisitLocks(Thread* thread, Context* context, jobject expected_lock)
+ : StackVisitor(thread, context, StackWalkKind::kIncludeInlinedFrames),
+ expected_lock_(expected_lock) {
+ }
+
+ bool VisitFrame() override REQUIRES_SHARED(Locks::mutator_lock_) {
+ ArtMethod* m = GetMethod();
+
+ // Ignore runtime methods.
+ if (m == nullptr || m->IsRuntimeMethod()) {
+ return true;
+ }
+
+ if (m->IsSynchronized()) {
+ // Interesting frame.
+ Monitor::VisitLocks(this, Callback, expected_lock_);
+ return false;
+ }
+
+ return true;
+ }
+
+ private:
+ static void Callback(ObjPtr<mirror::Object> obj, void* expected_lock)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ CHECK(obj != nullptr);
+ CHECK(obj == Thread::Current()->DecodeJObject(reinterpret_cast<jobject>(expected_lock)));
+ }
+
+ jobject expected_lock_;
+ };
+ {
+ std::unique_ptr<Context> context(Context::Create());
+ VisitLocks vl(soa.Self(), context.get(), lock);
+ vl.WalkStack();
+ }
+ return arg;
+}
+
} // namespace art
diff --git a/test/178-app-image-native-method/src/Main.java b/test/178-app-image-native-method/src/Main.java
index e6c76ff..294ad47 100644
--- a/test/178-app-image-native-method/src/Main.java
+++ b/test/178-app-image-native-method/src/Main.java
@@ -52,6 +52,7 @@
$noinline$opt$testCriticalSignatures();
$noinline$regressionTestB181736463();
+ $noinline$regressionTestB189235039();
new CriticalClinitCheck();
sTestCriticalClinitCheckOtherThread.join();
@@ -380,6 +381,10 @@
}
}
+ static void $noinline$regressionTestB189235039() {
+ assertEquals(42, new Main().b189235039CallThrough());
+ }
+
static void initializingCriticalClinitCheck() {
// Called from CriticalClinitCheck.<clinit>().
// Test @CriticalNative calls on the initializing thread.
@@ -425,6 +430,9 @@
}
public static native void makeVisiblyInitialized();
+
+ public native synchronized int b189235039CallThrough();
+ public static native int b189235039CheckLocks(int placeholder, Main m);
}
class Test {