JDK-8256757 : Incorrect MachCallRuntimeNode::ret_addr_offset() for CallLeafNoFP on x86_32
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 8,11,15,16
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: linux
  • CPU: x86
  • Submitted: 2020-11-20
  • Updated: 2021-01-08
  • Resolved: 2020-11-27
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 11 JDK 16
11.0.11Fixed 16 b27Fixed
Related Reports
Duplicate :  
Relates :  
Relates :  
Description
JDK-8254231 adds [2] the following assert to output.cpp

    assert(!is_mcall || (call_returns[block->_pre_order] <= (uint) current_offset))

Which verifies that the offset returned by MachCallNode::ret_addr_offset() (and sub-types) at [1] matches the emitted code, to avoid potential conflicts between oop maps of different calls.

This assert is failing when running test/hotspot/jtreg/compiler/intrinsics/string/TestStringLatin1IndexOfChar.java on Linux-x86 (32-bits), because it forces lower SSE settings. The real issue is MachCallRuntimeNode::ret_addr_offset() computing the offset incorrectly for CallLeafNoFP: it does not include FP stack cleaning.

[1] : https://github.com/openjdk/jdk/blob/master/src/hotspot/share/opto/output.cpp#L1530

[2]:

diff --git a/src/hotspot/share/opto/output.cpp b/src/hotspot/share/opto/output.cpp
index 8e78481a491..681cd205044 100644
--- a/src/hotspot/share/opto/output.cpp
+++ b/src/hotspot/share/opto/output.cpp
@@ -1683,6 +1683,8 @@ void PhaseOutput::fill_buffer(CodeBuffer* cb, uint* blk_starts) {
       n->emit(*cb, C->regalloc());
       current_offset = cb->insts_size();
 
+      assert(!is_mcall || (call_returns[block->_pre_order] <= (uint) current_offset), "ret_addr_offset() not within emitted code");
+
       // Above we only verified that there is enough space in the instruction section.
       // However, the instruction may emit stubs that cause code buffer expansion.
       // Bail out here if expansion failed due to a lack of code cache space.
Comments
Fix Request (11u) This fixes the same issue in 11u. Patch applies cleanly to 11u, tier{1,2} passes for both x86_32 and x86_64. I manually inspected the code to see the issue is there.
06-01-2021

Changeset: 9a468d85 Author: Aleksey Shipilev <shade@openjdk.org> Date: 2020-11-27 06:47:30 +0000 URL: https://git.openjdk.java.net/jdk/commit/9a468d85
27-11-2020

FYI: CallLeafNoFPDirect [1] don't need to emit FFree_Float_Stack_All [2] code. But MachCallRuntimeNode::ret_addr_offset() always adds sizeof_FFree_Float_Stack_All [3] for CallLeafNoFPDirect. If UseSSE < 2 [4], MachCallRuntimeNode::ret_addr_offset() will be larger than the code size generated by CallLeafNoFPDirect. [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/x86_32.ad#L13260 [2] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/x86_32.ad#L1756 [3] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/x86_32.ad#L311 [4] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/x86_32.ad#L1790
26-11-2020

I concluded the same as Jie above, and I have a fix. Let me RFR it.
26-11-2020

ILW = HLM = P3
23-11-2020

[~jtatton] The test that triggers the assert was recently added by you, so assigning to you initially. Could you take a look? FWIW, this issue is a heads-up for if JDK-8254231 is integrated, since the added assert triggers in a tier1 test. Either way, even without JDK-8254231 which adds the assert, there is still a potential problem.
20-11-2020

See also similar fix on AArch64: https://bugs.openjdk.java.net/browse/JDK-8256025
20-11-2020