Bug 7162645: JVMTI Field Modification Callback Sideeffect

[David Holmes]

Hi Stephen,

This is a known bug: 7158988 and is already fixed in hs24-b08:

http://hg.openjdk.java.net/hsx/hotspot-main/hotspot/rev/19e197e2a1af

I will close your bug as a duplicate.

David

On 24/04/2012 10:00 AM, Stephen Nelson wrote:
> My apologies if this is not the appropriate place for this.
>
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7162645
>
> I submitted a bug to bugs.sun.com a few days ago, and I have since
> found the problematic code which produces this bug. However, I am
> unable to post a fix to the bug database (I get an error about
> upgrades and I'm told to come back in a few days).
>
> I have identified [templateTable_x86_64.cpp:2707] in the function
> [void TemplateTable::jvmti_post_fast_field_mod()] as the cause of the
> bug:
>
> ====
>      // Save rax because call_VM() will clobber it, then use it for
>      // JVMTI purposes
>      __ push(rax);
> ====
>
> This code is erroneous because it only saves the value of $rax; field
> values are also stored in $xmm0, which can be also be clobbered. This
> is apparent from the switch statement immediately above this line
> which reads $xmm0 to get the value of floats and doubles.
>
> I'm not very familiar with hotspot, so I'm not comfortable submitting
> a patch myself. However, I believe that there are two possible
> solutions:
>
> (a) add switch statements before and after the VM call to push/pop the
> appropriate register based on the value of bytecode() -- similar to
> the one at line 2602.
>
> (b) modify the signature of [jvmti_post_fast_field_mod] to add a
> TosState parameter. This would allow the push/pop(TosState) methods to
> be used, which do the right thing for each type of parameter.
>
> I think (b) is superior as the purpose of the push/pop becomes clearer
> without referencing particular registers. This might help avoid
> similar bugs on other platforms. However, this would require changing
> the equivalent methods for all platforms. I'm not sure if this is
> appropriate.
>
> As far as I can tell, this method is only called from
> [fast_storefield], which already has a TosState parameter.
>
> Please let me know if there is a more appropriate mailing list.
>
> Best Regards,
>
> Stephen Nelson