Unsafe.storeFence produces a floating fence

[Doug Simon]

Hi Andrew,

Can you please create an issue on github for this.

-Doug

> On 18 Nov 2016, at 16:45, Andrew Haley <aph at redhat.com> wrote:
> 
> Here's a jcstress test:
> 
> public class ShortFencedTest {
> 
>    Shell shell;
> 
>    public static class Shell {
>        short x;
> 
>        public Shell() {
>            this.x = (short) 0xFFFF;
>            UnsafeHolder.U.storeFence();
>        }
>    }
> 
>    @Actor
>    public void actor1() {
>        shell = new Shell();
>    }
> 
>    @Actor
>    public void actor2(ShortResult1 r) {
>        Shell sh = shell;
>        r.r1 = (sh == null) ? 42 : sh.x;
>    }
> 
> }
> 
> the problem is that the call to storeFence() floats above the store to
> this.x in Shell().  In fact, it even floats past the object creation.
> 
> Here's the generated assembly code for actor1.  The storeFence has floated past
> the creation of the object, all the way to the entry point of actor1:
> 
> [Verified Entry Point]
>  0x0000007fa5593900: nop
>  0x0000007fa5593904: sub	x8, sp, #0x4, lsl #12
>  0x0000007fa5593908: str	xzr, [x8]
>  0x0000007fa559390c: sub	sp, sp, #0x30
>  0x0000007fa5593910: stp	x29, x30, [sp,#32]
>  0x0000007fa5593914: dmb	ish             ;*invokevirtual storeFence {reexecute=0 rethrow=0 return_oop=0}
>                                                ; - sun.misc.Unsafe::storeFence at 3 (line 1215)
>                                                ; - org.openjdk.jcstress.tests.init.primitives.fenced.ShortFencedTest$Shell::<init>@12 (line 47)
>                                                ; - org.openjdk.jcstress.tests.init.primitives.fenced.ShortFencedTest::actor1 at 5 (line 53)
> 
>  0x0000007fa5593918: orr	x0, xzr, #0x60
>  0x0000007fa559391c: ldr	x2, [x28,x0]
>  0x0000007fa5593920: orr	x3, xzr, #0x70
>  0x0000007fa5593924: ldr	x3, [x28,x3]
>  0x0000007fa5593928: add	x4, x2, #0x10
>  0x0000007fa559392c: cmp	x4, x3
>  0x0000007fa5593930: b.hi	0x0000007fa55939bc
>  0x0000007fa5593934: str	x4, [x28,x0]
>  0x0000007fa5593938: orr	x0, xzr, #0x180
>  0x0000007fa559393c: nop
>  0x0000007fa5593940: mov	x0, xzr
>  0x0000007fa5593944: orr	x3, xzr, #0x1
>  0x0000007fa5593948: str	x3, [x2,x0]
>  0x0000007fa559394c: orr	x0, xzr, #0x8
>  0x0000007fa5593950: mov	x3, #0xd0000               	// #851968
>                                                ;   {metadata('org/openjdk/jcstress/tests/init/primitives/fenced/ShortFencedTest$Shell')}
>  0x0000007fa5593954: movk	x3, #0x83f8
>  0x0000007fa5593958: str	w3, [x2,x0]
>  0x0000007fa559395c: orr	x0, xzr, #0xc
>  0x0000007fa5593960: str	wzr, [x2,x0]
>  0x0000007fa5593964: dmb	ish
>  0x0000007fa5593968: mov	x0, x2
>  0x0000007fa559396c: orr	x2, xzr, #0xc
>  0x0000007fa5593970: orr	x3, xzr, #0xffff
>  0x0000007fa5593974: strh	w3, [x0,x2]
>  0x0000007fa5593978: orr	x2, xzr, #0xc
>  0x0000007fa559397c: lsr	x0, x0, #3
>  0x0000007fa5593980: str	w0, [x1,x2]     ;*putfield shell {reexecute=0 rethrow=0 return_oop=0}
>                                                ; - org.openjdk.jcstress.tests.init.primitives.fenced.ShortFencedTest::actor1 at 8 (line 53)
> 
>  0x0000007fa5593984: lsr	x1, x1, #9
>  0x0000007fa5593988: mov	x0, #0xf000                	// #61440
>  0x0000007fa559398c: movk	x0, #0xafe5, lsl #16
>  0x0000007fa5593990: movk	x0, #0x7f, lsl #32
>  0x0000007fa5593994: add	x1, x1, x0
>  0x0000007fa5593998: mov	x0, xzr
>  0x0000007fa559399c: strb	wzr, [x1,x0]
>  0x0000007fa55939a0: ldp	x29, x30, [sp,#32]
>  0x0000007fa55939a4: add	sp, sp, #0x30
>  0x0000007fa55939a8: mov	x8, #0x5000                	// #20480
>                                                ;   {poll_return}
>  0x0000007fa55939ac: movk	x8, #0xb4fd, lsl #16
>  0x0000007fa55939b0: movk	x8, #0x7f, lsl #32
>  0x0000007fa55939b4: ldr	wzr, [x8]       ;   {poll_return}
>  0x0000007fa55939b8: ret                       ;*return {reexecute=0 rethrow=0 return_oop=0}
>                                                ; - org.openjdk.jcstress.tests.init.primitives.fenced.ShortFencedTest::actor1 at 11 (line 54)
> 
> UnsafeFencePlugin looks like:
> 
>    public static class UnsafeFencePlugin implements InvocationPlugin {
> 
>        private final int barriers;
> 
>        public UnsafeFencePlugin(int barriers) {
>            this.barriers = barriers;
>        }
> 
>        @Override
>        public boolean apply(GraphBuilderContext b, ResolvedJavaMethod targetMethod, Receiver unsafe) {
>            // Emits a null-check for the otherwise unused receiver
>            unsafe.get();
>            b.add(new MembarNode(barriers));
>            return true;
>        }
>    }
> 
> MembarNode is a FixedWithNextNode, but in this case it's simply added to the
> graph.
> 
> I'd love to have an opportunity to fix this bug, but I'd like some guidance.
> 
> Andrew.
>