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

[john cuthbertson - Sun Microsystems]

Hi Christian,

The main reason was just to avoid changing the call-sites and make it 
consistent with the other generator routines. I don't have a strong 
preference either way.

Thanks for looking at the code,

JohnC

On 10/28/09 01:36, Christian Thalinger wrote:
> On Tue, 2009-10-27 at 15:10 -0700, john cuthbertson - Sun Microsystems
> wrote:
>   
>> 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.
>>     
>
> Why do you actually pass in a src and then assert on it's value?
>
> +void MacroAssembler::store_heap_oop(Address dst, intptr_t src) {
> +  assert(src == NULL_WORD, "use something else otherwise");
>
> It seems it must be null anyway and we could use something like:
>
> void MacroAssembler::store_heap_oop_null(Address dst) {
>
> -- Christian
>
>   

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20091028/7c2946ea/attachment.html