JDK-4879051 : Internal Error occurs during offet conversion of byte code in rewrite/relocate
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 1.3.1_03
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • OS: windows_2000
  • CPU: x86
  • Submitted: 2003-06-16
  • Updated: 2004-06-10
  • Resolved: 2004-02-02
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.
Other Other
1.3.1_12 12Fixed 1.4.2_06Fixed
Description
A licensee reported the JVM ABENDs in 1.3.1_03(-server) with Internal Error, 

# ShouldNotReachHere()
#
# Error ID: 52454C4F4341544F520E43505001DB

(This code means "src/share/vm/runtime/relocator.cpp, 475".)

Although they can not send the test code, the licensee sent the following
detail report.
Their report also says this will occur in 1.4.X.(and tiger ?).
So, I filed this issue.

==== Their investigation ====

This reported issue occurs in Windows2000 and JDK1.3.1_03(-server).
However, as the following report says, that might not depend on
specific platform and JDK version.

CRASH SCENARIO :

 This issue occurs at rewrite/relocate operation which is achieved at 
 initializing process of byte code interpreter.
 According to our investigation, the source code and functions related to 
 the rewrite/relocate process are as follows.
 
- rewrite :
  - src/share/vm/oops/generateOopMap.cpp
     void GenerateOopMap::do_interpretation()
     void GenerateOopMap::rewrite_refval_conflicts()
     void GenerateOopMap::rewrite_refval_conflict()
     bool GenerateOopMap::rewrite_refval_conflict_inst()
     bool GenerateOopMap::rewrite_load_or_store()
     bool GenerateOopMap::expand_current_instr()
     
- relocate:
  -src/share/vm/runtime/relocator.cpp
    methodHandle Relocator::insert_space_at()
    bool Relocator::handle_code_changes()
    bool ChangeJumpWiden::handle_code_change()
    void Relocator::change_jumps()
    void Relocator::change_jump()
    bool Relocator::handle_jump_widen()
 
The rewrite operation broadens the index area of aload/astore op. code
under some condition.(To insert wide op. code and expand the 1-byte area to
2-bytes area)
When this rewrite operation occurs, the relocate occurs at the same time 
in order to adjust the offset in byte-codes. 
The problem occurs during the change for the branch offset value of "if" 
code in relocate operation. 

Specifically speaking, the branch offset value of goto/jsr can take 65535
as maximun(than's when goto_w/jsr_w is used).
However, the offset of "if" op. code can take 32767(signed int 16 bits) 
as maximum value. If the length of byte code area becomes over 32767 
after the above rewrite operation, there might be the case when the program 
can not branch because of the offset short.

When the conversion from signed 16 bit offset to signed 32 bit offset 
for each  op. code like goto/jsr/if(ex. goto =>goto_w, jsr => jsr_w ) 
is needed and the op. code is "if", this abnormal end occurs in 
Relocator::handle_jump_widen() as Internal error.

=== Their investigation End ======


REQUEST:
 We need a workaround or fix.
 
 
NOTE:
 According to the source code of 1.4.X(and 1.5 ?), the same issue occurs
 possibly.
 ===================================

Comments
CONVERTED DATA BugTraq+ Release Management Values COMMIT TO FIX: 1.3.1_12 1.4.2_06 generic tiger-beta2 FIXED IN: 1.3.1_12 1.4.2_06 tiger-beta2 INTEGRATED IN: 1.3.1_12 1.4.2_06 tiger-beta2 VERIFIED IN: 1.3.1_12
21-07-2004

EVALUATION The Fujitsu evaluation sounds correct. An "if" bytecode cannot currently be rewritten to a wide version, since none exists. I am working on a relocator change that will change the code from: #1 if <cond> <wide delta> #2 <else code> to: #1 if <cont> #2 #2 goto #4 #3 goto_w <wide delta> #4 <else code> I don't really have anything to test it on. Can Fujitsu send a class file sample? Or could I send them my changes and have them test them out? Thanks! ###@###.### 2003-10-24 I marked this incomplete to get it off the hot list until Fujitsu can try out the fix I put in "suggested fix". ###@###.### 2003-11-18 Correction, the new bytecode sequence is: #1 if <cond> #3 #2 goto #4 #3 goto_w <wide delta> #4 <else code> I have a test case to test it on now and the code is debugged and working. ###@###.### 2003-12-09 Putback comments: Several users in the field have been getting the ShouldNotReachHere() from relocator.cpp because support for the branch destination of "if" bytecodes could not be widened past a short. In this fix, I have rewritten the bytecode stream for widening "if" from: #1 if <cond> <wide delta> #2 <else code> to: #1 if <cont> #3 #2 goto #4 #3 goto_w <wide delta> #4 <else code> This has been debugged and tested for a testcase that Mingyao wrote that meets all the conditions for this rewriting (appropriately placed jsr's and an if branch needing widening to 32 bits). I was hoping Fujitsu would test this out but I never heard from them. Risk assessment: This is a corner case, which previously asserted. There have been lots of customer complaints about this assertion so this fix should be backported. Also, fixed was the bitfield length calculation for representing bits for each bci in basic blocks. If there was >1 rewriting towards the end of the method, the bit vector would overflow and assert. Also fixed: When pushing jump relocations on the _changes worklist, the delta offset to use was wrong if the jump relocation was already on the worklist. The second request was ignored if it had already been relocated. This was wrong because if the bci delta needed further adjustment, that adjustment was never made. This was seen with another of Mingyao's test cases where two branches needed adjustment and the adjustment of the first (+2) required that the second delta be further adjusted. The fix was to scan the worklist and modify the delta rather than pushing a duplicate request on the list with a delta that was derived from the original non-adjusted code stream. See suggested fix for webrev. ###@###.### 2003-12-19
19-12-2003

SUGGESTED FIX *** /src/share/vm/runtime/relocator.cpp- Tue Nov 18 14:23:57 2003 --- relocator.cpp Tue Nov 18 14:19:48 2003 *** 1,7 **** #ifdef USE_PRAGMA_IDENT_SRC ! #pragma ident "@(#)relocator.cpp 1.27 03/01/23 12:24:46 JVM" #endif /* * Copyright 2003 Sun Microsystems, Inc. All rights reserved. * SUN PROPRIETARY/CONFIDENTIAL. Use is subject to license terms. */ --- 1,7 ---- #ifdef USE_PRAGMA_IDENT_SRC ! #pragma ident "@(#)relocator.cpp 1.28 03/11/18 14:19:48 JVM" #endif /* * Copyright 2003 Sun Microsystems, Inc. All rights reserved. * SUN PROPRIETARY/CONFIDENTIAL. Use is subject to license terms. */ *** 469,491 **** // handle jump_widen instruction. Called be ChangeJumpWiden class bool Relocator::handle_jump_widen(int bci, int delta) { int ilen = rc_instr_len(bci); if (ilen != 3) { // Request already handled assert(code_at(bci) == Bytecodes::_goto_w || code_at(bci) == Bytecodes::_jsr_w, "sanity check"); return true; } - assert(ilen == 3, "check length"); if (!relocate_code(bci, 3, 2)) return false; - Bytecodes::Code bc = code_at(bci); - switch (bc) { - case Bytecodes::_goto: code_at_put(bci, Bytecodes::_goto_w); break; - case Bytecodes::_jsr: code_at_put(bci, Bytecodes::_jsr_w); break; default: ShouldNotReachHere(); } // If it's a forward jump, add 2 for the widening. if (delta > 0) delta += 2; --- 469,540 ---- // handle jump_widen instruction. Called be ChangeJumpWiden class bool Relocator::handle_jump_widen(int bci, int delta) { int ilen = rc_instr_len(bci); + Bytecodes::Code bc = code_at(bci); + switch (bc) { + case Bytecodes::_ifeq: + case Bytecodes::_ifne: + case Bytecodes::_iflt: + case Bytecodes::_ifge: + case Bytecodes::_ifgt: + case Bytecodes::_ifle: + case Bytecodes::_if_icmpeq: + case Bytecodes::_if_icmpne: + case Bytecodes::_if_icmplt: + case Bytecodes::_if_icmpge: + case Bytecodes::_if_icmpgt: + case Bytecodes::_if_icmple: + case Bytecodes::_if_acmpeq: { + // If 'if' points to the next bytecode, it's already handled. + if (short_at(bci+1) == ilen) { + return true; + } + assert(ilen == 3, "check length"); + + // Convert to 0 if <cond> goto 6 + // 3 _goto 10 + // 6 _goto_w <wide delta offset> + // 11 <else code> + const int goto_bci = Bytecodes::length_for(Bytecodes::_goto); + const int goto_w_bci = Bytecodes::length_for(Bytecodes::_goto_w); + const int add_bci = goto_bci + goto_w_bci; + + if (!relocate_code(bci, 3, /*delta*/add_bci)) return false; + + short_at_put(bci + 1, ilen); + + int cbci = bci + ilen; + // goto around + code_at_put(cbci, Bytecodes::_goto); + short_at_put(cbci + 1, add_bci); + // goto_w <wide delta> + cbci = cbci + goto_bci; + code_at_put(cbci, Bytecodes::_goto_w); + if (delta > 0) delta += 2; + int_at_put(cbci + 1, delta); + break; + + } + case Bytecodes::_if_acmpne: + case Bytecodes::_goto: + case Bytecodes::_jsr: if (ilen != 3) { // Request already handled assert(code_at(bci) == Bytecodes::_goto_w || code_at(bci) == Bytecodes::_jsr_w, "sanity check"); return true; } assert(ilen == 3, "check length"); + if (!relocate_code(bci, 3, 2)) return false; + if (bc == Bytecodes::_goto) + code_at_put(bci, Bytecodes::_goto_w); + else + code_at_put(bci, Bytecodes::_jsr_w); + break; default: ShouldNotReachHere(); } // If it's a forward jump, add 2 for the widening. if (delta > 0) delta += 2; ###@###.### 2003-12-19 See webrev: http://jruntime.east/~coleenp/net/philli.east/files/coleenp/webrev/relocation/index.html Or: http://jruntime.east/~coleenp/webrev/4879051/ for potential backport.
19-12-2003