JDK-8220351 : Cross-modifying code
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 13
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2019-03-08
  • Updated: 2022-01-27
  • Resolved: 2019-03-28
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 13
13 b15Fixed
Related Reports
Relates :  
JDK-8254264 - Remove redundant cross_modify_fence()
Relates :  
JDK-8226525 - HotSpot compile-time error for x86-32
Relates :  
JDK-8221554 - aarch64 cross-modifying code
Relates :  
JDK-8221555 - SPARC cross-modifying code
Relates :  
JDK-8221556 - arm32 cross-modifying code
Relates :  
JDK-8213961 - RIP values like 0xffffffff94bf7f80 due to patched NMethod
Description
After a JavaThread have been in a safepoint/(handshake) safe state it can start executing updated code.
E.g. an oop in the instruction stream can have been updated.

Most hardware's require a barrier or that the code cross modified is far away to guarantee that the thread executing the update instruction stream sees the modification.

What far away is and how far an update instruction stream is from a safepoint safe state is not clear.

To be compliant with those architectures an instruction stream barrier must be added when leaving the safepoint safe state.

In this issue we added the basic blocks and follow the x86 recommendations:
AMD64 Architecture Programmer's Manual Volume 2: System Programming, section 7.6.1
Intel® 64 and IA-32 Architectures Software Developer’s Manual, Volume 3A, section 8.1.3
Comments
URL: http://hg.openjdk.java.net/jdk/jdk/rev/846bc643f4ef User: rehn Date: 2019-03-28 10:18:48 +0000
28-03-2019
[~dlong] AMD said mfence or CPUID, while Intel only CPUID (IRET and RSM are not really an option). So changing to cpuid.
13-03-2019
See also JDK-8213961
12-03-2019
According to "Intel® 64 and IA-32 Architectures Software Developer’s Manual: Volume 3", Intel non-privileged serializing instructions are CPUID, IRET, and RSM.
12-03-2019
In a similar area: JDK-8219993
12-03-2019
I'm somewhat taken aback that we seem to have adopted a passive position in hotspot where we seem to rely on things taking "long enough" that this is not an issue. We also seem to have been implicitly relying on STW-safepoints to provide those "long enough" code paths. To quote Dave Dice: "I’d guess that the maximum store buffer latency plus prefetch queue plus out-of-order window on current CPUs remains _less than the length of the hotspot handshake/rendezvous exit path, so a missing serializing instruction wouldn’t manifest by any visible misbehavior. That is, we’d be "protected" by our current path lengths being sufficiently long that the store is recognized by the time we get back around to executing the patched location. But of course this is just speculation on my part, and even if accurate, Intel could release a new CPU tomorrow where things would fail visibly." Also note that mfence seems not to be the answer as "MFENCE does not serialize the instruction stream.". [1] [1] 4-22 Vol. 2B, INSTRUCTION SET REFERENCE, M-U
11-03-2019
From mail: ##### Immediate oop example, JavaThread going from native -> java. The JavaThread starts in native, thus safe, and any safepoint or handshake may be executed: store thread_state = _thread_in_native_trans (unsafe) StoreLoad // e.g. lock addl load thread_poll test thread_poll jcc slow_path store thread_state = _thread_in_Java ret return site: load <immediate oop> // No data If the <immediate oop> is updated we are not guaranteed to see it without mfence. The lock instruction, according to x86 manuals, have no effect on instruction stream. Even if the lock instruction would have effect on instruction stream, an on-going safepoint can update the <immediate oop> and finishes between the StoreLoad and test: JavaThread: | VMThread StoreLoad | | update <immediate oop> | disarm load thread_poll | This can also happen in the slow path (so there are is a bigger window also). By adding an instruction stream barrier (mfence) *after* the poll we are guaranteed to always see any updates to the instruction stream. #### The problem is that the 'patcher thread' cannot force the barrier on the target thread (JavaThread). Without a better polling mechanism we cannot provide such information, e.g. if instruction stream have been updated. A better per thread polling mechanism have been lightly discussed. [~dcubed] No, this is a really old issue. We are not compliant with the specs for cross-modifying code.
11-03-2019
If locking would fix this then I would assume the synchronization in safepoints/handshakes to still fix this. That said I'm dubious that locks would in fact sync the instruction stream. That said, only very specific things modify code, so only a handful of safepoint/handshake operations could be affected by this problem. So surely the fix should be restricted to such operations rather than applying it to every safepoint/handshake?
11-03-2019
[~rehn] - If I understand this correctly, the window for this issue was opened by the fix for faster safepoints: JDK-8203469 Faster safepoints because the lock operations on the Threads_lock and Safepoint_lock would have caused memory to be sync'ed. If that's correct, then we should add a link to JDK-8203469.
08-03-2019
http://cr.openjdk.java.net/~rehn/8220351/webrev/
08-03-2019