[aarch64-port-dev ] [NEW BUG]Missing LoadStore barrier in interpreter?

[Andrew Haley]

On 1/30/19 8:36 PM, Lun Liu wrote:

> I noticed in the template interpreter only a StoreStore barrier is
> used before a volatile write. According to JMM cookbook I think a
> LoadStore barrier is also needed to prevent reordering between a
> normal read and a volatile write.

Yes, you're right. When I translated the code from x86, I mistakenly did
this:


@@ -2382,13 +2382,16 @@
   jvmti_post_field_mod(rcpool, index, is_static);
   load_field_cp_cache_entry(obj, cache, index, off, flags, is_static);

-  // [jk] not needed currently
-  // volatile_barrier(Assembler::Membar_mask_bits(Assembler::LoadStore |
-  //                                              Assembler::StoreStore));
-
-  Label notVolatile, Done;
+  Label Done;
   __ mov(r5, flags);

+  {
+    Label notVolatile;
+    __ tbz(r5, ConstantPoolCacheEntry::is_volatile_shift, notVolatile);
+    __ membar(MacroAssembler::Membar_mask_bits(MacroAssembler::StoreStore));
+    __ bind(notVolatile);
+  }
+
   // field address
   const Address field(obj, off);


As you can see, the x86 comment is correct, but I didn't follow it.

> In the template interpreter it seems like LoadLoad | LoadStore
> barriers are inserted after both normal read and volatile read so it
> should be fine but I think this might cause a problem if the
> compiler generates a normal read using dmb barriers and the
> interpreter is executing the volatile write. 

I'm not sure. The LoadStore should prevent a load (any kind) from
being reordered with the volatile store. This only matters if another
thread stores to the memory in the load, and that store in the other
thread is sequenced after the volatile store in this thread.

> Could this be a potential bug?

We've never seen any test failures, but that doesn't prove
anything. It is a bug, so we should fix it. Thank you.

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671