[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-compiler-dev
Subject:    Re: RFR (XXS): 8016474: Crash in sun.reflect.UnsafeObjectFieldAccessorImpl.get
From:       Igor Veresov <iggy.veresov () gmail ! com>
Date:       2013-07-26 21:59:34
Message-ID: C2335A96-0AE2-45F1-970E-F5574CCADAFA () gmail ! com
[Download RAW message or body]


On Jul 26, 2013, at 2:00 PM, Christian Thalinger <christian.thalinger@oracle.com> \
wrote:
> > > 
> > > > 
> > > > The code dereferences src_klass (line 2301), which is of type T_OBJECT. The \
> > > > value of src_klass is read from memory of type T_ADDRESS. The result of the \
> > > > move seems to have the type of the first argument, so it would seem that if \
> > > > it's T_ADDRESS no decompression is going to happen and the deference is going \
> > > > to be problematic. Or does it work differently?
> > > 
> > > Why would there be no decoding?  As mentioned above the logic of decoding klass \
> > > pointers is all in LIR_Assembler::mem2reg.  Maybe we should also change this, \
> > > though: 
> > > -      LIR_Opr src_klass = new_register(T_OBJECT);
> > > +      LIR_Opr src_klass = new_register(T_ADDRESS);
> > > 
> > 
> > It probably should be T_METADATA? It can't be T_ADDRESS because it wouldn't be \
> > exposed in register maps for GC.
> 
> By register maps you mean oop maps?  The read is the G1 pre-barrier code and is \
> only used for a type check. It doesn't live beyond that point. 

You're right it doesn't, the T_ADDRESS will work in the given context. However \
T_METADATA is the type properly describing the value, also in case someone decides to \
move things around it will be GC-safe. It also makes places like that easier to find. \
But T_OBJECT seems to be incorrect, since it may get spilled and be in the frame for \
a longer time and GC will consider it to be an object on the heap, which it is not.

Either way with T_ADDRESS or T_METADATA here the code looks good.

Thanks!
igor


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic