[aarch64-port-dev ] RFR: JDK-8143584 Aarch64: Load constant pool tag and class status with load acquire

[Andrew Haley]

On 11/24/2015 01:23 PM, Hui Shi wrote:

> Checking similar places for aarch64 runtime, more places needs similar fix:
> 1. Instanceof is similar with checkcast
> 2. "ldc", similar with load resolved class in checkcast, need load acquire
> when loading tag in constant pool
> 3. "new" loads tag for resolved class.
> 4. "new" Loads and checks class initialized status, class init status
> update is guarded with orderaccess::storestore, need guarantee order
> between load class initialize status and load "instance_size in
> InstanceKlass".
> 5. C1 runtime checks if class initialize status when generate code for
> fast_new_instance_init_check_id, similar with "new".

I'm looking at 4, the InstanceKlass::init_state change:

@@ -3326,7 +3328,8 @@

   // make sure klass is initialized & doesn't have finalizer
   // make sure klass is fully initialized
-  __ ldrb(rscratch1, Address(r4, InstanceKlass::init_state_offset()));
+  __ lea(rscratch1, Address(r4, InstanceKlass::init_state_offset()));
+  __ ldarb(rscratch1, rscratch1);
   __ cmp(rscratch1, InstanceKlass::fully_initialized);
   __ br(Assembler::NE, slow_case);

While this looks reasonable enough, the init_state accessors in
InstanceKlass don't seem to use fences. Therefore, as far as I can
see, the set_init_state (fully_initialized) doesn't have a
happens-before relationship with the load of init_state above.

This is in eager_initialize_impl():

    // linking successfull, mark class as initialized
    this_k->set_init_state (fully_initialized);
    this_k->fence_and_clear_init_lock();

Note that the call to fence_and_clear_init_lock() (which contains a
storestore) is *after* the set_init_state().  If set_init_state()
needs to synchronize with the reading code, a storestore must come
*before* set_init_state() is called.

The rest look OK.

Andrew.