JDK-4351343 : create_itable_stub() inconsistent with start_of_itable()
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 2.0
  • Priority: P4
  • Status: Closed
  • Resolution: Duplicate
  • OS: windows_nt
  • CPU: x86
  • Submitted: 2000-07-07
  • Updated: 2004-04-30
  • Resolved: 2004-04-30
Related Reports
Duplicate :  
Description
Using the Hotspot 2.0 Windows sources from
hotspot-2_0-fcs-src-win-01_may_2000.zip

The code in VtableStubs::create_itable_stub in vtableStubs_i486.cpp
is inconsistent with the implementation of start_of_itable() in instanceKlass.hpp.

The implementation of start_of_itable() is
  int* start_of_itable() const        { return start_of_vtable() + align_object_offset(vtable_length()); }

The implementation of create_itable_stub needs something like this change.
==================
cvs diff -r1.2 vtableStubs_i486.cpp
146,147c146,151
<   assert(vtableEntry::size() * wordSize == 4, "adjust the scaling in the code below");
<   __ movl(edx, Address(ebx, instanceKlass::vtable_length_offset() * wordSize)); // Get length of vtable
---
>   __ movl(edx, Address(ebx, instanceKlass::vtable_length_offset() * wordSize)); // Get length of vtable (Gemstone, in words)
>   // Gemstone fix for alignment, to go with the code in instanceKlass.hpp , see sparc interp.
>   if (align_object_offset(1) > 1) {  // Gemstone, 
>     __ round_to(edx, align_object_offset(1));  // add 2 instructions at 6 bytes each
>   }
>   assert(vtableEntry::size() * wordSize == 4, "adjust the scaling in the leal code below");
252,253c256,257
<     COMPILER1_ONLY(return (DebugVtables ? 300 : 61) + (CountCompiledCalls ? 6 : 0); )
<     COMPILER2_ONLY(return (DebugVtables ? 300 : 56) + (CountCompiledCalls ? 6 : 0);  )
---
>     COMPILER1_ONLY(return (DebugVtables ? 300 : (61 + 12) ) + (CountCompiledCalls ? 6 : 0); )
>     COMPILER2_ONLY(return (DebugVtables ? 300 : (56 + 12/*Gemstone alignment fix*/)) + (CountCompiledCalls ? 6 : 0);  )
==================

On Windows, this inconsistency only causes problems if the size of oopDesc is changed

Also, on Windows,  in templateTable_i486.cpp, 
TemplateTable::fast_invokeinterface is missing the alignment code, like this:
=====
1974c2260,2265
<   __ movl(esi, Address(edx, instanceKlass::vtable_length_offset() * wordSize)); // Get length
 of vtable
---
>   __ movl(esi, Address(edx, instanceKlass::vtable_length_offset() * wordSize)); // Get length
 of vtable in words
> 
>   // Gemstone fix for alignment, to go with the code in instanceKlass.hpp , see sparc interp.
>   if (align_object_offset(1) > 1) {
>     __ round_to(esi, align_object_offset(1));
>   }
====


On Solaris, a similar problem exists, I believe in the 
hotsparc-2_0-pre-fcs-b-src-sol-03_may_2000.zip  sources.

The implementation in cpu/sparc/vm/vtableStubs_sparc.cpp appears to
attempt to round after multiplying by  (vtableEntry::size() * wordSize),
and I don't think this rounding is working .  
Since the and3 to "isolate the odd bit" is done after the sll, I think
the and3 will always have a zero result. I changed the  sparc code
to use the round_to() from the assembler and some of the SEGV problems I
was having went away.  The instruction count in create_itable_stub is unchanged
after this fix


========
diff -r1.3 vtableStubs_sparc.cpp
153d152
<   __ ld(Address(RklassOop, 0, instanceKlass::vtable_length_offset() * wordSize), Rscratch);
155,165c154,178
<   // %%% Could store the aligned, prescaled offset in the klassoop.
<   __ sll(Rscratch, exact_log2(vtableEntry::size() * wordSize), Rscratch);
<   // see code for instanceKlass::start_of_itable!
<   const int vtable_alignment = align_object_offset(1);
<   assert(vtable_alignment == 1 || vtable_alignment == 2, "");
<   const int odd_bit = vtableEntry::size() * wordSize;
<   if (vtable_alignment == 2)
<     __ and3(Rscratch, odd_bit, Rtemp);  // isolate the odd bit
<   __ add(RklassOop, Rscratch, Rscratch);
<   if (vtable_alignment == 2)
<     __ add(Rscratch, Rtemp, Rscratch);  // double the odd bit, to align up
---
> // OLD JAVASOFT CODE , I think this alignment code is bad.
> //  __ ld(Address(RklassOop, 0, instanceKlass::vtable_length_offset() * wordSize), Rscratch);
> //
> //  // %%% Could store the aligned, prescaled offset in the klassoop.
> //  __ sll(Rscratch, exact_log2(vtableEntry::size() * wordSize), Rscratch);
> //  // see code for instanceKlass::start_of_itable!
> //  const int vtable_alignment = align_object_offset(1);
> //  assert(vtable_alignment == 1 || vtable_alignment == 2, "");
> //  const int odd_bit = vtableEntry::size() * wordSize;
> //  if (vtable_alignment == 2)
> //    __ and3(Rscratch, odd_bit, Rtemp);  // isolate the odd bit
> //  __ add(RklassOop, Rscratch, Rscratch);
> //  if (vtable_alignment == 2)
> //    __ add(Rscratch, Rtemp, Rscratch);  // double the odd bit, to align up
> // END OLD CODE
>
> // GEMSTONE, use this alignment code to agree with code in fast_invokeinterface
>   __ ld(Address(RklassOop, 0, instanceKlass::vtable_length_offset() * wordSize), Rscratch);  // load vtable length in words into Rscratch
>   if (align_object_offset(1) > 1) {
>     __ round_to(Rscratch, align_object_offset(1));
>   }
>   __ sll(Rscratch, exact_log2(vtableEntry::size() * wordSize), Rscratch); // convert to byte offset
>   __ add(RklassOop, Rscratch, Rscratch);
>
> // END GEMSTONE fix

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

Other licensees probably won't see the bug unless they
do something to disturb alignment of things in instanceKlass.hpp

It is a fairly difficult bug to diagnose, at least
on Sparc, because the SEGV's produced appear to be outside of any
intepreter or C2 generated code, and cause a direct core dump,
since the hotspot SEGV handler can't run successfully for some reason.

On NT, I had to enhance the printing logic
in hotspot's unexpected exception handler to have it tell me what kind
of stub the fault was occurring in.



Comments
EVALUATION Is this still an issue for customer in latest mantis beta or 1.4.1_03 release? ###@###.### 2003-04-10 The alignment aspect of this bug was fixed long ago. The apparent error in VtableStubs::create_itable_stub of file cpu/sparc/vm/vtableStubs_sparc.cpp (i.e. worrying that the and3 is doing nothing) is a simple misunderstanding, as the "mask" involved with the and3 is based on wordsize while the alignment (in the case of concern) is wordsize*2. That is, the code in question is correct as written. ###@###.### 2004-04-30
30-04-2004