RFR (XXS): 8016474: Crash in sun.reflect.UnsafeObjectFieldAccessorImpl.get

[Christian Thalinger]

On Jul 25, 2013, at 11:36 AM, Igor Veresov <iggy.veresov at gmail.com> wrote:

> 
> On Jul 25, 2013, at 9:34 AM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
> 
>> 
>> On Jul 24, 2013, at 10:46 PM, Igor Veresov <iggy.veresov at gmail.com> wrote:
>> 
>>> I haven't looked at it after the perm gen removal but shouldn't all klasses be of type T_METADATA or something?
>>> May be the code wasn't updated since then?
>> 
>> Yes, I think that's the case but T_ADDRESS handles klass pointers right now (see LIR_Assembler::mem2reg).
> 
> Ah, indeed it is. I didn't notice that. Thanks for pointing this out. It's dangerously relying on a specific offset though.
> 
>> 
>>> 
>>> 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.

-- Chris

> 
> igor 
> 
> 
> 
>> -- Chris
>> 
>>> 
>>> igor
>>> 
>>> On Jul 24, 2013, at 2:37 PM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>>> 
>>>> http://cr.openjdk.java.net/~twisti/8016474
>>>> 
>>>> 8016474: Crash in sun.reflect.UnsafeObjectFieldAccessorImpl.get
>>>> Summary: C1's GetUnsafeObject G1 pre-barrier uses the wrong type to read the klass pointer.
>>>> Reviewed-by:
>>>> 
>>>> There is a bug in C1's GetUnsafeObject G1 pre-barrier code.  If UseCompressedKlassPointers is on we use T_OBJECT to read the klass pointer of the object.  If we also use a different object alignment like 16 or 32 (-XX:ObjectAlignmentInBytes=16) the klass pointer gets decoded with the wrong shift resulting in a wrong pointer and a crash.
>>>> 
>>>> The fix is to always use T_ADDRESS for klass pointer reads.
>>>> 
>>>> src/share/vm/c1/c1_LIRGenerator.cpp
>>>> test/compiler/unsafe/GetUnsafeObjectG1PreBarrier.java
>>>> 
>>> 
>> 
>