@@ -67,9 +67,23 @@ class EffectAnalyzer {
6767 // noticeable from the perspective of the caller, that is, effects that are
6868 // only noticeable during the call, but "vanish" when the call stack is
6969 // unwound.
70+ //
71+ // Unlike walking just the body, walking the function will also
72+ // include the effects of any return calls the function makes. For that
73+ // reason, it is a bug if a user of this code calls walk(Expression*) and not
74+ // walk(Function*) if their intention is to scan an entire function body.
75+ // Putting it another way, a return_call is syntax sugar for a return and a
76+ // call, where the call executes at the function scope, so there is a
77+ // meaningful difference between scanning an expression and scanning
78+ // the entire function body.
7079 void walk (Function* func) {
7180 walk (func->body );
7281
82+ // Effects of return-called functions will be visible to the caller.
83+ if (hasReturnCallThrow) {
84+ throws_ = true ;
85+ }
86+
7387 // We can ignore branching out of the function body - this can only be
7488 // a return, and that is only noticeable in the function, not outside.
7589 branchesOut = false ;
@@ -143,6 +157,22 @@ class EffectAnalyzer {
143157 // or a continuation that is never continued, are examples of that.
144158 bool mayNotReturn = false ;
145159
160+ // Since return calls return out of the body of the function before performing
161+ // their call, they are indistinguishable from normal returns from the
162+ // perspective of their surrounding code, and the return-callee's effects only
163+ // become visible when considering the effects of the whole function
164+ // containing the return call. To model this correctly, stash the callee's
165+ // effects on the side and only merge them in after walking a full function
166+ // body.
167+ //
168+ // We currently do this stashing only for the throw effect, but in principle
169+ // we could do it for all effects if it made a difference. (Only throw is
170+ // noticeable now because the only thing that can change between doing the
171+ // call here and doing it outside at the function exit is the scoping of
172+ // try-catch blocks. If future wasm scoping additions are added, we may need
173+ // more here.)
174+ bool hasReturnCallThrow = false ;
175+
146176 // Helper functions to check for various effect types
147177
148178 bool accessesLocal () const {
@@ -466,43 +496,63 @@ class EffectAnalyzer {
466496 return ;
467497 }
468498
499+ const EffectAnalyzer* targetEffects = nullptr ;
500+ if (parent.funcEffectsMap ) {
501+ auto iter = parent.funcEffectsMap ->find (curr->target );
502+ if (iter != parent.funcEffectsMap ->end ()) {
503+ targetEffects = &iter->second ;
504+ }
505+ }
506+
469507 if (curr->isReturn ) {
470508 parent.branchesOut = true ;
509+ // When EH is enabled, any call can throw.
510+ if (parent.features .hasExceptionHandling () &&
511+ (!targetEffects || targetEffects->throws ())) {
512+ parent.hasReturnCallThrow = true ;
513+ }
471514 }
472515
473- if (parent.funcEffectsMap ) {
474- auto iter = parent.funcEffectsMap ->find (curr->target );
475- if (iter != parent.funcEffectsMap ->end ()) {
476- // We have effect information for this call target, and can just use
477- // that. The one change we may want to make is to remove throws_, if
478- // the target function throws and we know that will be caught anyhow,
479- // the same as the code below for the general path.
480- const auto & targetEffects = iter->second ;
481- if (targetEffects.throws_ && parent.tryDepth > 0 ) {
482- auto filteredEffects = targetEffects;
483- filteredEffects.throws_ = false ;
484- parent.mergeIn (filteredEffects);
485- } else {
486- // Just merge in all the effects.
487- parent.mergeIn (targetEffects);
488- }
489- return ;
516+ if (targetEffects) {
517+ // We have effect information for this call target, and can just use
518+ // that. The one change we may want to make is to remove throws_, if the
519+ // target function throws and we know that will be caught anyhow, the
520+ // same as the code below for the general path. We can always filter out
521+ // throws for return calls because they are already more precisely
522+ // captured by `branchesOut`, which models the return, and
523+ // `hasReturnCallThrow`, which models the throw that will happen after
524+ // the return.
525+ if (targetEffects->throws_ && (parent.tryDepth > 0 || curr->isReturn )) {
526+ auto filteredEffects = *targetEffects;
527+ filteredEffects.throws_ = false ;
528+ parent.mergeIn (filteredEffects);
529+ } else {
530+ // Just merge in all the effects.
531+ parent.mergeIn (*targetEffects);
490532 }
533+ return ;
491534 }
492535
493536 parent.calls = true ;
494- // When EH is enabled, any call can throw.
495- if (parent.features .hasExceptionHandling () && parent.tryDepth == 0 ) {
537+ // When EH is enabled, any call can throw. Skip this for return calls
538+ // because the throw is already more precisely captured by the combination
539+ // of `hasReturnCallThrow` and `branchesOut`.
540+ if (parent.features .hasExceptionHandling () && parent.tryDepth == 0 &&
541+ !curr->isReturn ) {
496542 parent.throws_ = true ;
497543 }
498544 }
499545 void visitCallIndirect (CallIndirect* curr) {
500546 parent.calls = true ;
501- if (parent.features .hasExceptionHandling () && parent.tryDepth == 0 ) {
502- parent.throws_ = true ;
503- }
504547 if (curr->isReturn ) {
505548 parent.branchesOut = true ;
549+ if (parent.features .hasExceptionHandling ()) {
550+ parent.hasReturnCallThrow = true ;
551+ }
552+ }
553+ if (parent.features .hasExceptionHandling () &&
554+ (parent.tryDepth == 0 && !curr->isReturn )) {
555+ parent.throws_ = true ;
506556 }
507557 }
508558 void visitLocalGet (LocalGet* curr) {
@@ -745,21 +795,26 @@ class EffectAnalyzer {
745795 }
746796 }
747797 void visitCallRef (CallRef* curr) {
798+ if (curr->isReturn ) {
799+ parent.branchesOut = true ;
800+ if (parent.features .hasExceptionHandling ()) {
801+ parent.hasReturnCallThrow = true ;
802+ }
803+ }
748804 if (curr->target ->type .isNull ()) {
749805 parent.trap = true ;
750806 return ;
751807 }
752- parent.calls = true ;
753- if (parent.features .hasExceptionHandling () && parent.tryDepth == 0 ) {
754- parent.throws_ = true ;
755- }
756- if (curr->isReturn ) {
757- parent.branchesOut = true ;
758- }
759808 // traps when the call target is null
760809 if (curr->target ->type .isNullable ()) {
761810 parent.implicitTrap = true ;
762811 }
812+
813+ parent.calls = true ;
814+ if (parent.features .hasExceptionHandling () &&
815+ (parent.tryDepth == 0 && !curr->isReturn )) {
816+ parent.throws_ = true ;
817+ }
763818 }
764819 void visitRefTest (RefTest* curr) {}
765820 void visitRefCast (RefCast* curr) {
0 commit comments