[jdk21u-dev] RFR: 8341715: PPC64: ObjectMonitor::_owner should be reset unconditionally in nmethod unlocking

[Richard Reingruber]

On Mon, 2 Dec 2024 16:45:46 GMT, Richard Reingruber <rrich at openjdk.org> wrote:

> Backport of https://github.com/openjdk/jdk/commit/f9208fadde8141e18a025ddb6ce28423861ba391
> 
> The change removes instructions that check if an ObjectMonitor is owned by the current thread before unlocking it in a c1/c2 compiled nmethod.
> 
> The check is imprecise. It can fail if the lock was inflated into an ObjectMonitor by another thread.
> It is also redundant since only bytecode with symmetric locking operations is compiled.
> See JBS-Issue for details.
> 
> Note that there existed a special diagnostic locking mode to do the same check also on x86. Purpose was to protect against missbehaving JNI code. It got removed though with [JDK-8210381](https://bugs.openjdk.org/browse/JDK-8210381) in jdk12. On ppc64le it was not removed as the check was done in default mode as well (unclear why).
> 
> The fix didn't apply because in jdk21 there is still just one method to handle LM_LEGACY and LM_LIGHTWEIGHT and LM_LIGHTWEIGHT requires the owner check to handle ANONYMOUS owner. Also RTM is still supported in jdk21.
> 
> I'd consider the risk low. Aside from the assertions, the change is very small and it just brings ppc64 in line with how unlocking is done on other platforms.
> 
> The fix passed our CI testing: JTReg tests: tier1-4 of hotspot and jdk. All of Langtools and jaxp. JCK, SPECjvm2008, SPECjbb2015, Renaissance Suite, and SAP specific tests.
> Testing was done with fastdebug builds on the main platforms and also on Linux/PPC64le and AIX.

Danke für Deinen Review mit Approval. Eins reicht, oder?

Grüße, Richard.

> Looks correct.
Oh, oh, nicht korrekt, aber nicht unbedingt gut... 😉


From: Martin ***@***.***>
Date: Wednesday, 4. December 2024 at 18:49
To: openjdk/jdk21u-dev ***@***.***>
Cc: Reingruber, Richard ***@***.***>, Author ***@***.***>
Subject: Re: [openjdk/jdk21u-dev] 8341715: PPC64: ObjectMonitor::_owner should be reset unconditionally in nmethod unlocking (PR #1191)

@TheRealMDoerr approved this pull request.

Looks correct.

—
Reply to this email directly, view it on GitHub<https://github.com/openjdk/jdk21u-dev/pull/1191#pullrequestreview-2479479345>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AH7IPDMZBRLLPVU3EOSXWHD2D46CRAVCNFSM6AAAAABS324GE2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDINZZGQ3TSMZUGU>.
You are receiving this because you authored the thread.Message ID: ***@***.***>

-------------

PR Comment: https://git.openjdk.org/jdk21u-dev/pull/1191#issuecomment-2519521381