[Fwd: Request for review (S): CR 6889740 - G1: OpenDS fails with "unhandled exception in compiled code"]

[Vladimir Kozlov]

Thank you, John, for hunting down this difficult to find problem.

Changes looks good. Did you thought to use int32_t imm32
instead of intptr_t src?

I wonder if we have other places like this. For example,
the next code in generate_exception_handler_common()
uses NULL_WORD without casting to (int32_t):

      __ movptr(c_rarg2, NULL_WORD);

but there is no void* dummy declaration for movptr():

   void movptr(Register dst, intptr_t src);
   void movptr(Register dst, Register src);
#ifdef _LP64
   void movptr(Register dst, int32_t imm32);
#endif // _LP64

Vladimir

john cuthbertson - Sun Microsystems wrote:
> Hi Everyone,
> 
> I should have included the compiler alias on the attached review 
> request. Can someone look over the changes?
> 
> Thanks,
> 
> JohnC
> 
> -------- Original Message --------
> Subject: 	Request for review (S): CR 6889740 - G1: OpenDS fails with 
> "unhandled exception in compiled code"
> Date: 	Tue, 27 Oct 2009 14:51:46 -0700
> From: 	john cuthbertson - Sun Microsystems <John.Cuthbertson at Sun.COM>
> To: 	hotspot-gc-dev at openjdk.java.net
> 
> 
> 
> Hi Everyone,
> 
> Can I have a couple of volunteers to review the proposed fix for this 
> bug? The webrev can be found at 
> http://cr.openjdk.java.net/~johnc/6889740/webrev.0/.
> 
> The issue is that bad code was being generated for the store operation 
> in the null case of the aastore bytecode template. The bad code was 
> caused by there being only one version of the store_heap_oop routine 
> that took a Register as the second argument. When the calling code 
> passed in NULL_WORD (0) to this routine the value was used as a Register 
> encoding and converted to Register(0), which is rax. Thus the generated 
> store was "mov (dst), $rax" instead of "mov (dst), $0x0". This is 
> normally not a problem as the preceding code in the template fetches the 
> value to be stored into rax. When the G1 pre-barrier code calls the 
> runtime, however, the value in rax can be overwritten and the heap can 
> become corrupted.
> 
> Testing: OpenDS, jprt, refworkload, and the GC test suite.
> 
> Thanks,
> 
> JohnC
>