(original) (raw)
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"insteadof"mov(dst),rax" instead of "mov (dst), rax"insteadof"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