wasm-ctor-eval: Remove the start function when we succeed

src/tools/wasm-ctor-eval.cpp

@@ -312,9 +312,22 @@ struct CtorEvalExternalInterface : EvallingModuleRunner::ExternalInterface {
     linkedInstances.swap(linkedInstances_);
   }
 
+  bool firstApplication = true;
+
   // Called when we want to apply the current state of execution to the Module.
   // Until this is called the Module is never changed.
   void applyToModule() {
+    if (firstApplication) {
+      // The first time we apply things to the module, we can remove the start
+      // function: we evalled it successfully, if we got to here (and we must
+      // not execute it again later, which would mean it runs twice). We do not
+      // do this after the first application because we start to build up a new
+      // start function with the things we need, unrelated to the original one
+      // (see addStartFixup).
+      wasm->start = Name();
+      firstApplication = false;
+    }
+
     clearApplyState();
 
     // If nothing was ever written to memories then there is nothing to update.

test/lit/ctor-eval/gc-cycle.wast

@@ -1158,7 +1158,9 @@
 ;; CHECK-NEXT:  (nop)
 ;; CHECK-NEXT: )
 (module
- ;; The start function already exists here. We must prepend to it.
+ ;; The start function already exists here. We must *not* prepend to it: it gets
+ ;; evalled away too (we execute it before the first ctor, and we should not
+ ;; eval those contents twice).
 
  ;; CHECK:      (type $A (struct (field (mut (ref null $A))) (field i32)))
  (type $A (struct (field (mut (ref null $A))) (field i32)))
@@ -1178,11 +1180,6 @@
  ;; CHECK:      (global $b (mut (ref null $A)) (ref.null none))
  (global $b (mut (ref null $A)) (ref.null $A))
 
- ;; CHECK:      (export "test" (func $test_3))
-
- ;; CHECK:      (export "keepalive" (func $keepalive))
-
- ;; CHECK:      (start $start)
  (start $start)
 
  (func $test (export "test")
@@ -1201,6 +1198,12 @@
   )
  )
 
+ ;; CHECK:      (export "test" (func $test_4))
+
+ ;; CHECK:      (export "keepalive" (func $keepalive))
+
+ ;; CHECK:      (start $start_3)
+
  ;; CHECK:      (func $keepalive (type $2) (result i32)
  ;; CHECK-NEXT:  (i32.add
  ;; CHECK-NEXT:   (struct.get $A 1
@@ -1222,23 +1225,21 @@
   )
  )
 
- ;; CHECK:      (func $start (type $1)
- ;; CHECK-NEXT:  (struct.set $A 0
- ;; CHECK-NEXT:   (global.get $ctor-eval$global_4)
- ;; CHECK-NEXT:   (global.get $ctor-eval$global_4)
- ;; CHECK-NEXT:  )
- ;; CHECK-NEXT:  (global.set $b
- ;; CHECK-NEXT:   (global.get $a)
- ;; CHECK-NEXT:  )
- ;; CHECK-NEXT: )
  (func $start
   (global.set $b
    (global.get $a)
   )
  )
 )
 
-;; CHECK:      (func $test_3 (type $1)
+;; CHECK:      (func $start_3 (type $1)
+;; CHECK-NEXT:  (struct.set $A 0
+;; CHECK-NEXT:   (global.get $ctor-eval$global_4)
+;; CHECK-NEXT:   (global.get $ctor-eval$global_4)
+;; CHECK-NEXT:  )
+;; CHECK-NEXT: )
+
+;; CHECK:      (func $test_4 (type $1)
 ;; CHECK-NEXT:  (local $a (ref $A))
 ;; CHECK-NEXT:  (nop)
 ;; CHECK-NEXT: )

test/lit/ctor-eval/start-bad-2.wast

@@ -0,0 +1,65 @@
+;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
+;; RUN: foreach %s %t wasm-ctor-eval --ctors=test --kept-exports=test --quiet -all -S -o - | filecheck %s
+
+;; We fail to eval away test (due to infinite recursion). As a result, we do
+;; not update either global - not the one it modifies or even the one that the
+;; start function modifies, and the start function remains as the start.
+;; TODO: We could perhaps eval away the start in such cases, even when nothing
+;; else gets optimized.
+
+(module
+ ;; CHECK:      (type $0 (func))
+
+ ;; CHECK:      (type $1 (func (result i32)))
+
+ ;; CHECK:      (global $global1 (mut i32) (i32.const 0))
+ (global $global1 (mut i32) (i32.const 0))
+
+ ;; CHECK:      (global $global2 (mut i32) (i32.const 0))
+ (global $global2 (mut i32) (i32.const 0))
+
+ ;; CHECK:      (export "test" (func $test))
+
+ ;; CHECK:      (export "keepalive" (func $keepalive))
+
+ ;; CHECK:      (start $start)
+ (start $start)
+
+ ;; CHECK:      (func $start (type $0)
+ ;; CHECK-NEXT:  (global.set $global2
+ ;; CHECK-NEXT:   (i32.const 42)
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $start
+  (global.set $global2
+   (i32.const 42)
+  )
+ )
+
+ ;; CHECK:      (func $test (type $0)
+ ;; CHECK-NEXT:  (call $test)
+ ;; CHECK-NEXT:  (global.set $global1
+ ;; CHECK-NEXT:   (i32.const 1337)
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $test (export "test")
+  (call $test)
+  (global.set $global1
+   (i32.const 1337)
+  )
+ )
+
+ ;; CHECK:      (func $keepalive (type $1) (result i32)
+ ;; CHECK-NEXT:  (i32.add
+ ;; CHECK-NEXT:   (global.get $global1)
+ ;; CHECK-NEXT:   (global.get $global2)
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $keepalive (export "keepalive") (result i32)
+  ;; Keep the globals alive to show changes.
+  (i32.add
+   (global.get $global1)
+   (global.get $global2)
+  )
+ )
+)

test/lit/ctor-eval/start-bad.wast

@@ -0,0 +1,62 @@
+;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
+;; RUN: foreach %s %t wasm-ctor-eval --ctors=test --kept-exports=test --quiet -all -S -o - | filecheck %s
+
+;; The start function traps here, so we cannot eval anything. None of the
+;; globals should change.
+
+(module
+ ;; CHECK:      (type $0 (func))
+
+ ;; CHECK:      (type $1 (func (result i32)))
+
+ ;; CHECK:      (global $global1 (mut i32) (i32.const 0))
+ (global $global1 (mut i32) (i32.const 0))
+
+ ;; CHECK:      (global $global2 (mut i32) (i32.const 0))
+ (global $global2 (mut i32) (i32.const 0))
+
+ ;; CHECK:      (export "test" (func $test))
+
+ ;; CHECK:      (export "keepalive" (func $keepalive))
+
+ ;; CHECK:      (start $start)
+ (start $start)
+
+ ;; CHECK:      (func $start (type $0)
+ ;; CHECK-NEXT:  (global.set $global2
+ ;; CHECK-NEXT:   (i32.const 42)
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT:  (unreachable)
+ ;; CHECK-NEXT: )
+ (func $start
+  (global.set $global2
+   (i32.const 42)
+  )
+  (unreachable)
+ )
+
+ ;; CHECK:      (func $test (type $0)
+ ;; CHECK-NEXT:  (global.set $global1
+ ;; CHECK-NEXT:   (i32.const 1337)
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $test (export "test")
+  (global.set $global1
+   (i32.const 1337)
+  )
+ )
+
+ ;; CHECK:      (func $keepalive (type $1) (result i32)
+ ;; CHECK-NEXT:  (i32.add
+ ;; CHECK-NEXT:   (global.get $global1)
+ ;; CHECK-NEXT:   (global.get $global2)
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $keepalive (export "keepalive") (result i32)
+  ;; Keep the globals alive to show changes.
+  (i32.add
+   (global.get $global1)
+   (global.get $global2)
+  )
+ )
+)

test/lit/ctor-eval/start-rerun.wast

@@ -0,0 +1,64 @@
+;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
+;; RUN: foreach %s %t wasm-ctor-eval --ctors=t,s --kept-exports=t,s --quiet -all -S -o - | filecheck %s
+
+;; A corner case where we export the start function and consider it a ctor.
+;; That it executes twice should not cause an internal error.
+
+(module
+ (rec
+  ;; CHECK:      (rec
+  ;; CHECK-NEXT:  (type $A (sub (shared (struct (field (mut (ref null $B)))))))
+  (type $A (sub (shared (struct (field (mut (ref null $B)))))))
+  ;; CHECK:       (type $B (sub (shared (struct (field (mut (ref null (shared any))))))))
+  (type $B (sub (shared (struct (field (mut (ref null (shared any))))))))
+ )
+
+ (global $global (ref $A) (struct.new $A
+  (struct.new_default $B)
+ ))
+
+ (export "s" (func $s))
+
+ (export "t" (func $t))
+
+ (start $s)
+
+ (func $s
+  (nop)
+ )
+
+ (func $t
+  (nop)
+ )
+)
+
+;; CHECK:      (type $2 (func))
+
+;; CHECK:      (global $ctor-eval$global_3 (ref (exact $A)) (struct.new $A
+;; CHECK-NEXT:  (ref.null (shared none))
+;; CHECK-NEXT: ))
+
+;; CHECK:      (global $ctor-eval$global_4 (ref (exact $B)) (struct.new $B
+;; CHECK-NEXT:  (ref.null (shared none))
+;; CHECK-NEXT: ))
+
+;; CHECK:      (export "s" (func $s_4))
+
+;; CHECK:      (export "t" (func $t_3))
+
+;; CHECK:      (start $start)
+
+;; CHECK:      (func $start (type $2)
+;; CHECK-NEXT:  (struct.set $A 0
+;; CHECK-NEXT:   (global.get $ctor-eval$global_3)
+;; CHECK-NEXT:   (global.get $ctor-eval$global_4)
+;; CHECK-NEXT:  )
+;; CHECK-NEXT: )
+
+;; CHECK:      (func $t_3 (type $2)
+;; CHECK-NEXT:  (nop)
+;; CHECK-NEXT: )
+
+;; CHECK:      (func $s_4 (type $2)
+;; CHECK-NEXT:  (nop)
+;; CHECK-NEXT: )

test/lit/ctor-eval/start.wast

@@ -0,0 +1,68 @@
+;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited.
+;; RUN: foreach %s %t wasm-ctor-eval --ctors=test --kept-exports=test --quiet -all -S -o - | filecheck %s
+
+;; This code does the following:
+;;
+;;  * start writes global2, and traps if global1 is set.
+;;  * test sets global1.
+;;
+;; We must eval away the start function, when we eval away the other. That is,
+;; there should be no start function afterwards. Otherwise, if it remains as the
+;; start, it will trap when it reads the modified global.
+;;
+;; While doing so we must apply the changes of the start function, to global2.
+;; So both globals end up modified.
+
+(module
+ ;; CHECK:      (type $0 (func (result i32)))
+
+ ;; CHECK:      (type $1 (func))
+
+ ;; CHECK:      (global $global1 (mut i32) (i32.const 1337))
+ (global $global1 (mut i32) (i32.const 0))
+
+ ;; CHECK:      (global $global2 (mut i32) (i32.const 42))
+ (global $global2 (mut i32) (i32.const 0))
+
+ (start $start)
+
+ (func $start
+  (global.set $global2
+   (i32.const 42)
+  )
+  (if
+   (global.get $global1)
+   (then
+    (unreachable)
+   )
+  )
+ )
+
+ (func $test (export "test")
+  (global.set $global1
+   (i32.const 1337)
+  )
+ )
+
+ ;; CHECK:      (export "test" (func $test_3))
+
+ ;; CHECK:      (export "keepalive" (func $keepalive))
+
+ ;; CHECK:      (func $keepalive (type $0) (result i32)
+ ;; CHECK-NEXT:  (i32.add
+ ;; CHECK-NEXT:   (global.get $global1)
+ ;; CHECK-NEXT:   (global.get $global2)
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $keepalive (export "keepalive") (result i32)
+  ;; Keep the globals alive to show changes.
+  (i32.add
+   (global.get $global1)
+   (global.get $global2)
+  )
+ )
+)
+
+;; CHECK:      (func $test_3 (type $1)
+;; CHECK-NEXT:  (nop)
+;; CHECK-NEXT: )