Use indirect call effects in LinearExecutionWalker

src/ir/effects.h

@@ -23,6 +23,7 @@
 #include "ir/intrinsics.h"
 #include "pass.h"
 #include "support/name.h"
+#include "support/utilities.h"
 #include "wasm-traversal.h"
 #include "wasm-type.h"
 #include "wasm.h"
@@ -670,27 +671,34 @@ class EffectAnalyzer {
       }
     }
 
+    // Handle effects due to a null type arriving in a place where a null input
+    // causes trapping. That is, handle the case of the type proving that the
+    // input is null.
+    // Returns true iff there is no need to consider further effects.
+    bool trapOnNull(Type type) {
+      if (type == Type::unreachable) {
+        return true;
+      }
+      assert(type.isRef());
+      if (type.isNull()) {
+        parent.trap = true;
+        return true;
+      }
+      if (type.isNullable()) {
+        parent.implicitTrap = true;
+      }
+
+      return false;
+    }
+
     // Handle effects due to an explicit null check of the operands in `exprs`.
     // Returns true iff there is no need to consider further effects.
     bool trapOnNull(std::initializer_list<Expression*> exprs) {
       for (auto* expr : exprs) {
-        if (expr && expr->type == Type::unreachable) {
+        if (expr && trapOnNull(expr->type)) {
           return true;
         }
       }
-      for (auto* expr : exprs) {
-        assert(!expr || expr->type.isRef());
-        if (expr && expr->type.isNull()) {
-          parent.trap = true;
-          return true;
-        }
-      }
-      for (auto* expr : exprs) {
-        if (expr && expr->type.isNullable()) {
-          parent.implicitTrap = true;
-          break;
-        }
-      }
       return false;
     }
 
@@ -716,71 +724,52 @@ class EffectAnalyzer {
     }
 
     void visitCall(Call* curr) {
-      // call.without.effects has no effects.
       if (Intrinsics(parent.module).isCallWithoutEffects(curr)) {
         return;
       }
 
-      // Get the target's effects, if they exist. Note that we must handle the
-      // case of the function not yet existing (we may be executed in the middle
-      // of a pass, which may have built up calls but not the targets of those
-      // calls; in such a case, we do not find the targets and therefore assume
-      // we know nothing about the effects, which is safe).
-      const EffectAnalyzer* targetEffects = nullptr;
-      if (auto* target = parent.module.getFunctionOrNull(curr->target)) {
-        targetEffects = target->effects.get();
+      const EffectAnalyzer* callTargetEffects = nullptr;
+      if (auto* target = parent.module.getFunctionOrNull(curr->target);
+          target && target->effects) {
+        callTargetEffects = target->effects.get();
       }
-
-      if (curr->isReturn) {
-        parent.branchesOut = true;
-        // When EH is enabled, any call can throw.
-        if (parent.features.hasExceptionHandling() &&
-            (!targetEffects || targetEffects->throws())) {
-          parent.hasReturnCallThrow = true;
-        }
+      addCallEffects(curr, callTargetEffects);
+    }
+    void visitCallIndirect(CallIndirect* curr) {
+      auto* table = parent.module.getTable(curr->table);
+      if (trapOnNull(table->type)) {
+        return;
       }
 
-      if (targetEffects) {
-        // We have effect information for this call target, and can just use
-        // that. The one change we may want to make is to remove throws_, if the
-        // target function throws and we know that will be caught anyhow, the
-        // same as the code below for the general path. We can always filter out
-        // throws for return calls because they are already more precisely
-        // captured by `branchesOut`, which models the return, and
-        // `hasReturnCallThrow`, which models the throw that will happen after
-        // the return.
-        if (targetEffects->throws_ && (parent.tryDepth > 0 || curr->isReturn)) {
-          auto filteredEffects = *targetEffects;
-          filteredEffects.throws_ = false;
-          parent.mergeIn(filteredEffects);
-        } else {
-          // Just merge in all the effects.
-          parent.mergeIn(*targetEffects);
-        }
+      if (!Type::isSubType(Type(curr->heapType, Nullability::NonNullable),
+                           table->type)) {
+        parent.trap = true;
         return;
       }
 
-      parent.calls = true;
-      // When EH is enabled, any call can throw. Skip this for return calls
-      // because the throw is already more precisely captured by the combination
-      // of `hasReturnCallThrow` and `branchesOut`.
-      if (parent.features.hasExceptionHandling() && parent.tryDepth == 0 &&
-          !curr->isReturn) {
-        parent.throws_ = true;
+      // Due to index out of bounds. Type-related traps are handled above and
+      // may set either implicitTrap or trap (or neither).
+      parent.implicitTrap = true;
+
+      const EffectAnalyzer* callTargetEffects = nullptr;
+      if (auto it = parent.module.indirectCallEffects.find(curr->heapType);
+          it != parent.module.indirectCallEffects.end()) {
+        callTargetEffects = it->second.get();
       }
+      addCallEffects(curr, callTargetEffects);
     }
-    void visitCallIndirect(CallIndirect* curr) {
-      parent.calls = true;
-      if (curr->isReturn) {
-        parent.branchesOut = true;
-        if (parent.features.hasExceptionHandling()) {
-          parent.hasReturnCallThrow = true;
-        }
+    void visitCallRef(CallRef* curr) {
+      if (trapOnNull(curr->target)) {
+        return;
       }
-      if (parent.features.hasExceptionHandling() &&
-          (parent.tryDepth == 0 && !curr->isReturn)) {
-        parent.throws_ = true;
+
+      const EffectAnalyzer* callTargetEffects = nullptr;
+      if (auto it = parent.module.indirectCallEffects.find(
+            curr->target->type.getHeapType());
+          it != parent.module.indirectCallEffects.end()) {
+        callTargetEffects = it->second.get();
       }
+      addCallEffects(curr, callTargetEffects);
     }
     void visitLocalGet(LocalGet* curr) {
       parent.localsRead.insert(curr->index);
@@ -1038,22 +1027,6 @@ class EffectAnalyzer {
     void visitTupleExtract(TupleExtract* curr) {}
     void visitRefI31(RefI31* curr) {}
     void visitI31Get(I31Get* curr) { trapOnNull(curr->i31); }
-    void visitCallRef(CallRef* curr) {
-      if (trapOnNull(curr->target)) {
-        return;
-      }
-      if (curr->isReturn) {
-        parent.branchesOut = true;
-        if (parent.features.hasExceptionHandling()) {
-          parent.hasReturnCallThrow = true;
-        }
-      }
-      parent.calls = true;
-      if (parent.features.hasExceptionHandling() &&
-          (parent.tryDepth == 0 && !curr->isReturn)) {
-        parent.throws_ = true;
-      }
-    }
     void visitRefTest(RefTest* curr) {}
 
     void visitRefCast(RefCast* curr) {
@@ -1335,6 +1308,66 @@ class EffectAnalyzer {
         parent.throws_ = true;
       }
     }
+
+  private:
+    // Populate a call's effects using effects computed from GlobalEffects. Note
+    // that calls may have other effects that aren't captured by the function
+    // body of the target (e.g. a call_ref may trap on null refs).
+    template<typename CallType>
+    void addCallEffectsFromGlobalEffects(const CallType* curr,
+                                         const EffectAnalyzer& funcEffects) {
+      if (curr->isReturn) {
+        if (funcEffects.throws()) {
+          parent.hasReturnCallThrow = true;
+        }
+      }
+
+      if (funcEffects.throws_ && (parent.tryDepth > 0 || curr->isReturn)) {
+        // We can ignore a throw here, as the parent catches it.
+        //
+        // Also, we can filter out throws for return calls because they are
+        // already more precisely captured by `branchesOut`, which models the
+        // return, and `hasReturnCallThrow`, which models the throw that will
+        // happen after the return.
+        //
+        // TODO: we check for throws_ here instead of throws() and only clear
+        // throws_. So throws() can remain true in the case that this expression
+        // is the target of a delegate statement. We should consider clearing
+        // filteredEffects.delegateTargets here.
+        auto filteredEffects = funcEffects;
+        filteredEffects.throws_ = false;
+        parent.mergeIn(filteredEffects);
+      } else {
+        parent.mergeIn(funcEffects);
+      }
+    }
+
+    // Common effects logic for the 3 types of call: `call`, `call_indirect`,
+    // and `call_ref`.
+    template<typename CallType>
+    void addCallEffects(const CallType* curr,
+                        const EffectAnalyzer* callTargetEffects) {
+      if (curr->isReturn) {
+        parent.branchesOut = true;
+      }
+
+      if (callTargetEffects) {
+        addCallEffectsFromGlobalEffects(curr, *callTargetEffects);
+        return;
+      }
+
+      parent.calls = true;
+      // If EH is enabled and we don't have global effects information,
+      // assume that the call target may throw.
+      if (parent.features.hasExceptionHandling()) {
+        if (curr->isReturn) {
+          parent.hasReturnCallThrow = true;
+        }
+        if (parent.tryDepth == 0 && !curr->isReturn) {
+          parent.throws_ = true;
+        }
+      }
+    }
   };
 
 public:

src/ir/linear-execution.h

@@ -80,7 +80,12 @@ struct LinearExecutionWalker : public PostWalker<SubType, VisitorType> {
   static void scan(SubType* self, Expression** currp) {
     Expression* curr = *currp;
 
-    auto handleCall = [&](bool mayThrow, bool isReturn) {
+    auto handleCall = [&](bool isReturn, const EffectAnalyzer* effects) {
+      bool refutesThrowEffect = effects && !effects->throws_;
+      bool mayThrow = !self->getModule() ||
+                      self->getModule()->features.hasExceptionHandling();
+      mayThrow = mayThrow && !refutesThrowEffect;
+
       if (!self->connectAdjacentBlocks) {
         // Control is nonlinear if we return or throw. Traps don't need to be
         // taken into account since they don't break control flow in a way
@@ -156,40 +161,52 @@ struct LinearExecutionWalker : public PostWalker<SubType, VisitorType> {
       case Expression::Id::CallId: {
         auto* call = curr->cast<Call>();
 
-        bool mayThrow = !self->getModule() ||
-                        self->getModule()->features.hasExceptionHandling();
-        if (mayThrow && self->getModule()) {
-          auto* effects =
-            self->getModule()->getFunction(call->target)->effects.get();
-
-          if (effects && !effects->throws_) {
-            mayThrow = false;
+        const EffectAnalyzer* effects = nullptr;
+        if (self->getModule()) {
+          auto* func = self->getModule()->getFunctionOrNull(call->target);
+          if (func) {
+            effects = func->effects.get();
           }
         }
 
-        handleCall(mayThrow, call->isReturn);
+        handleCall(call->isReturn, effects);
         break;
       }
       case Expression::Id::CallRefId: {
         auto* callRef = curr->cast<CallRef>();
 
-        // TODO: Effect analysis for indirect calls isn't implemented yet.
-        // Assume any indirect call may throw for now.
-        bool mayThrow = !self->getModule() ||
-                        self->getModule()->features.hasExceptionHandling();
+        const EffectAnalyzer* effects = [&]() -> const EffectAnalyzer* {
+          if (!self->getModule()) {
+            return nullptr;
+          }
+          if (!callRef->target->type.isRef()) {
+            return nullptr;
+          }
 
-        handleCall(mayThrow, callRef->isReturn);
+          auto* effects_ptr =
+            find_or_null(self->getModule()->indirectCallEffects,
+                         callRef->target->type.getHeapType());
+          if (!effects_ptr) {
+            return nullptr;
+          }
+          return effects_ptr->get();
+        }();
+
+        handleCall(callRef->isReturn, effects);
         break;
       }
       case Expression::Id::CallIndirectId: {
         auto* callIndirect = curr->cast<CallIndirect>();
 
-        // TODO: Effect analysis for indirect calls isn't implemented yet.
-        // Assume any indirect call may throw for now.
-        bool mayThrow = !self->getModule() ||
-                        self->getModule()->features.hasExceptionHandling();
-
-        handleCall(mayThrow, callIndirect->isReturn);
+        const EffectAnalyzer* effects = nullptr;
+        if (self->getModule()) {
+          if (const auto& effects_ptr =
+                find_or_null(self->getModule()->indirectCallEffects,
+                             callIndirect->heapType)) {
+            effects = effects_ptr->get();
+          }
+        }
+        handleCall(callIndirect->isReturn, effects);
         break;
       }
       case Expression::Id::TryId: {

src/ir/type-updating.cpp

@@ -324,6 +324,29 @@ void GlobalTypeRewriter::mapTypes(const TypeMap& oldToNewTypes) {
   for (auto& tag : wasm.tags) {
     tag->type = updater.getNew(tag->type);
   }
+
+  // Update indirect call effects per type.
+  // When A is rewritten to B, B inherits the effects of A and A loses its
+  // effects.
+  std::unordered_map<HeapType, std::shared_ptr<const EffectAnalyzer>>
+    newTypeEffects;
+  for (auto& [oldType, oldEffects] : wasm.indirectCallEffects) {
+    if (!oldEffects) {
+      continue;
+    }
+
+    auto newType = updater.getNew(oldType);
+    std::shared_ptr<const EffectAnalyzer>& targetEffects =
+      newTypeEffects[newType];
+    if (!targetEffects) {
+      targetEffects = oldEffects;
+    } else {
+      auto merged = std::make_shared<EffectAnalyzer>(*targetEffects);
+      merged->mergeIn(*oldEffects);
+      targetEffects = merged;
+    }
+  }
+  wasm.indirectCallEffects = std::move(newTypeEffects);
 }
 
 void GlobalTypeRewriter::mapTypeNamesAndIndices(const TypeMap& oldToNewTypes) {