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

[Christian Thalinger]

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).

> 
> 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);

-- 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
>> 
>