J2CL: Add extra guardrails (#6171)

The patch puts a new guardrail that will only hoist the field
if it is initialized with the owner class.

The constant hoisting optimization in J2CL pass relies on the
assumption that clinit that will initialize the field will be
executed before the read of the field. That means the field
that is optimized is within the same class:

class Foo {
  public static final Object field = new Object();
}

Although it is possible to observe the initial value, that is
not intention of the developer (which the point of the
optimization).

However can also see a similar pattern in following:

class Foo {
  public static Object field;
}

class Zoo {
  static {
    Foo.field = new Object();
  }
}

Currently the pass also optimizes it as well since the field
is only initialized once and by a clinit. However Zoo clinit
is not guaranteed to be run before Foo.field access so it is
less safe to speculate on the intention of the developer here
hence it is not worth the risk.

FWIW, we haven't seen this issue. But this is something we
are also guarding in Closure Compiler so I decided it is
worthwhile to do here as well.

2 files changed, 84 insertions, 65 deletions

diff --git a/test/lit/passes/j2cl-inline.wast b/test/lit/passes/j2cl-inline.wast
index 9ccdad00e..dad35b80e 100644
--- a/test/lit/passes/j2cl-inline.wast
+++ b/test/lit/passes/j2cl-inline.wast
@@ -6,41 +6,41 @@
 (module
 
   ;; A once function that has become empty
-  (func $clinit-trivial-1_@once@_  )
+  (func $clinit-trivial-1_@once@_@Foo  )
 
   ;; A once function that just calls another
-  (func $clinit-trivial-2_@once@_
-    (call $clinit-trivial-1_@once@_)
+  (func $clinit-trivial-2_@once@_@Bar
+    (call $clinit-trivial-1_@once@_@Foo)
   )
 
   ;; CHECK:      (type $0 (func))
 
-  ;; CHECK:      (global $f_$initialized__ (mut i32) (i32.const 0))
-  (global $f_$initialized__ (mut i32) (i32.const 0))
+  ;; CHECK:      (global $$class-initialized@Zoo (mut i32) (i32.const 0))
+  (global $$class-initialized@Zoo (mut i32) (i32.const 0))
 
   ;; Not hoisted but trivial.
-  ;; CHECK:      (func $clinit-non-trivial_@once@_ (type $0)
+  ;; CHECK:      (func $clinit-non-trivial_@once@_@Zoo (type $0)
   ;; CHECK-NEXT:  (if
-  ;; CHECK-NEXT:   (global.get $f_$initialized__)
+  ;; CHECK-NEXT:   (global.get $$class-initialized@Zoo)
   ;; CHECK-NEXT:   (return)
   ;; CHECK-NEXT:  )
-  ;; CHECK-NEXT:  (global.set $f_$initialized__
+  ;; CHECK-NEXT:  (global.set $$class-initialized@Zoo
   ;; CHECK-NEXT:   (i32.const 1)
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
-  (func $clinit-non-trivial_@once@_
-    (if (global.get $f_$initialized__)
+  (func $clinit-non-trivial_@once@_@Zoo
+    (if (global.get $$class-initialized@Zoo)
      (return)
     )
-    (global.set $f_$initialized__ (i32.const 1))
+    (global.set $$class-initialized@Zoo (i32.const 1))
   )
 
   ;; CHECK:      (func $main (type $0)
-  ;; CHECK-NEXT:  (call $clinit-non-trivial_@once@_)
+  ;; CHECK-NEXT:  (call $clinit-non-trivial_@once@_@Zoo)
   ;; CHECK-NEXT: )
   (func $main
-    (call $clinit-trivial-1_@once@_)
-    (call $clinit-trivial-2_@once@_)
-    (call $clinit-non-trivial_@once@_)
+    (call $clinit-trivial-1_@once@_@Foo)
+    (call $clinit-trivial-2_@once@_@Bar)
+    (call $clinit-non-trivial_@once@_@Zoo)
   )
 )
diff --git a/test/lit/passes/j2cl.wast b/test/lit/passes/j2cl.wast
index 75c845ec5..0d37dd829 100644
--- a/test/lit/passes/j2cl.wast
+++ b/test/lit/passes/j2cl.wast
@@ -6,18 +6,18 @@
 (module
   ;; CHECK:      (type $0 (func))
 
-  ;; CHECK:      (global $field-f64 f64 (f64.const 1))
+  ;; CHECK:      (global $field-f64@Foo f64 (f64.const 1))
 
-  ;; CHECK:      (global $field-i32 i32 (i32.const 1))
-  (global $field-i32 (mut i32) (i32.const 0))
-  (global $field-f64 (mut f64) (f64.const 0))
+  ;; CHECK:      (global $field-i32@Foo i32 (i32.const 1))
+  (global $field-i32@Foo (mut i32) (i32.const 0))
+  (global $field-f64@Foo (mut f64) (f64.const 0))
 
-  ;; CHECK:      (func $clinit_@once@_ (type $0)
+  ;; CHECK:      (func $clinit_@once@_@Foo (type $0)
   ;; CHECK-NEXT:  (nop)
   ;; CHECK-NEXT: )
-  (func $clinit_@once@_
-    (global.set $field-i32 (i32.const 1))
-    (global.set $field-f64 (f64.const 1))
+  (func $clinit_@once@_@Foo
+    (global.set $field-i32@Foo (i32.const 1))
+    (global.set $field-f64@Foo (f64.const 1))
   )
 )
 
@@ -29,46 +29,46 @@
 
   ;; CHECK:      (type $1 (func))
 
-  ;; CHECK:      (global $field2 (mut anyref) (ref.null none))
+  ;; CHECK:      (global $field2@Foo (mut anyref) (ref.null none))
 
-  ;; CHECK:      (global $referredField i32 (i32.const 42))
-  (global $referredField (i32) (i32.const 42))
+  ;; CHECK:      (global $referredField@Foo i32 (i32.const 42))
+  (global $referredField@Foo (i32) (i32.const 42))
 
-  ;; CHECK:      (global $field1 anyref (struct.new $A
-  ;; CHECK-NEXT:  (global.get $referredField)
+  ;; CHECK:      (global $field1@Foo anyref (struct.new $A
+  ;; CHECK-NEXT:  (global.get $referredField@Foo)
   ;; CHECK-NEXT: ))
 
-  ;; CHECK:      (global $referredFieldMut (mut i32) (i32.const 42))
-  (global $referredFieldMut (mut i32) (i32.const 42))
+  ;; CHECK:      (global $referredFieldMut@Foo (mut i32) (i32.const 42))
+  (global $referredFieldMut@Foo (mut i32) (i32.const 42))
 
-  (global $field1 (mut anyref) (ref.null none))
+  (global $field1@Foo (mut anyref) (ref.null none))
 
-  (global $field2 (mut anyref) (ref.null none))
+  (global $field2@Foo (mut anyref) (ref.null none))
 
-  ;; CHECK:      (global $field3 anyref (global.get $field1))
-  (global $field3 (mut anyref) (ref.null none))
+  ;; CHECK:      (global $field3@Foo anyref (global.get $field1@Foo))
+  (global $field3@Foo (mut anyref) (ref.null none))
 
-  ;; CHECK:      (func $clinit_@once@_ (type $1)
-  ;; CHECK-NEXT:  (global.set $field2
+  ;; CHECK:      (func $clinit_@once@_@Foo (type $1)
+  ;; CHECK-NEXT:  (global.set $field2@Foo
   ;; CHECK-NEXT:   (struct.new $A
-  ;; CHECK-NEXT:    (global.get $referredFieldMut)
+  ;; CHECK-NEXT:    (global.get $referredFieldMut@Foo)
   ;; CHECK-NEXT:   )
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
-  (func $clinit_@once@_
+  (func $clinit_@once@_@Foo
     ;; Referred field is immutable, should hoist
-    (global.set $field1 (struct.new $A (
-      global.get $referredField)
+    (global.set $field1@Foo (struct.new $A (
+      global.get $referredField@Foo)
     ))
 
     ;; Referred field is mutable, should not hoist
-    (global.set $field2 (struct.new $A
-      (global.get $referredFieldMut)
+    (global.set $field2@Foo (struct.new $A
+      (global.get $referredFieldMut@Foo)
     ))
 
     ;; Referred field is mutable but hoistable hence also this one.
     ;; (Note that requires multiple iterations in a single run)
-    (global.set $field3 (global.get $field1))
+    (global.set $field3@Foo (global.get $field1@Foo))
   )
 )
 
@@ -78,24 +78,24 @@
   (type $A (struct))
   ;; CHECK:      (type $1 (func))
 
-  ;; CHECK:      (global $field-any (mut anyref) (struct.new_default $A))
+  ;; CHECK:      (global $field-any@Foo (mut anyref) (struct.new_default $A))
 
-  ;; CHECK:      (global $field-i32 (mut i32) (i32.const 2))
-  (global $field-i32 (mut i32) (i32.const 2))
+  ;; CHECK:      (global $field-i32@Foo (mut i32) (i32.const 2))
+  (global $field-i32@Foo (mut i32) (i32.const 2))
 
-  (global $field-any (mut anyref) (struct.new $A))
+  (global $field-any@Foo (mut anyref) (struct.new $A))
 
-  ;; CHECK:      (func $clinit_@once@_ (type $1)
-  ;; CHECK-NEXT:  (global.set $field-i32
+  ;; CHECK:      (func $clinit_@once@_@Foo (type $1)
+  ;; CHECK-NEXT:  (global.set $field-i32@Foo
   ;; CHECK-NEXT:   (i32.const 1)
   ;; CHECK-NEXT:  )
-  ;; CHECK-NEXT:  (global.set $field-any
+  ;; CHECK-NEXT:  (global.set $field-any@Foo
   ;; CHECK-NEXT:   (struct.new_default $A)
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
-  (func $clinit_@once@_
-    (global.set $field-i32 (i32.const 1))
-    (global.set $field-any (struct.new $A))
+  (func $clinit_@once@_@Foo
+    (global.set $field-i32@Foo (i32.const 1))
+    (global.set $field-any@Foo (struct.new $A))
   )
 )
 
@@ -104,31 +104,50 @@
 
   ;; CHECK:      (type $0 (func))
 
-  ;; CHECK:      (global $field-i32 i32 (i32.const 1))
-  (global $field-i32 (mut i32) (i32.const 0))
+  ;; CHECK:      (global $field@Foo i32 (i32.const 1))
+  (global $field@Foo (mut i32) (i32.const 0))
 
-  ;; CHECK:      (func $clinit_@once@_ (type $0)
+  ;; CHECK:      (func $clinit_@once@_@Foo (type $0)
   ;; CHECK-NEXT:  (nop)
   ;; CHECK-NEXT: )
-  (func $clinit_@once@_
-    (global.set $field-i32 (i32.const 1))
+  (func $clinit_@once@_@Foo
+    (global.set $field@Foo (i32.const 1))
   )
 )
 
-;; f_$initialized__ are not hoisted
+;; $$class-initialized are not hoisted
 (module
 
   ;; CHECK:      (type $0 (func))
 
-  ;; CHECK:      (global $f_$initialized__ (mut i32) (i32.const 0))
-  (global $f_$initialized__ (mut i32) (i32.const 0))
+  ;; CHECK:      (global $$class-initialized@Foo (mut i32) (i32.const 0))
+  (global $$class-initialized@Foo (mut i32) (i32.const 0))
 
-  ;; CHECK:      (func $clinit_@once@_ (type $0)
-  ;; CHECK-NEXT:  (global.set $f_$initialized__
+  ;; CHECK:      (func $clinit_@once@_@Foo (type $0)
+  ;; CHECK-NEXT:  (global.set $$class-initialized@Foo
   ;; CHECK-NEXT:   (i32.const 1)
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
-  (func $clinit_@once@_
-    (global.set $f_$initialized__ (i32.const 1))
+  (func $clinit_@once@_@Foo
+    (global.set $$class-initialized@Foo (i32.const 1))
+  )
+)
+
+;; Fields from different classes are not hoisted.
+(module
+
+  ;; CHECK:      (type $0 (func))
+
+  ;; CHECK:      (global $field@Foo (mut i32) (i32.const 0))
+  (global $field@Foo (mut i32) (i32.const 0))
+
+  ;; CHECK:      (func $clinit_@once@_@Bar (type $0)
+  ;; CHECK-NEXT:  (global.set $field@Foo
+  ;; CHECK-NEXT:   (i32.const 1)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $clinit_@once@_@Bar
+    ;; Note that $clinit is @Bar and field is @Foo.
+    (global.set $field@Foo (i32.const 1))
   )
 )