3 files changed, 67 insertions, 1 deletions
diff --git a/src/ir/branch-utils.h b/src/ir/branch-utils.h
index f2fa43f18..57bad2fb9 100644
--- a/src/ir/branch-utils.h
+++ b/src/ir/branch-utils.h
@@ -238,6 +238,28 @@ inline NameSet getBranchTargets(Expression* ast) {
   return scanner.targets;
 }
 
+// Check if an expression defines a particular name as a branch target anywhere
+// inside it.
+inline bool hasBranchTarget(Expression* ast, Name target) {
+  struct Scanner
+    : public PostWalker<Scanner, UnifiedExpressionVisitor<Scanner>> {
+    Name target;
+    bool has = false;
+
+    void visitExpression(Expression* curr) {
+      operateOnScopeNameDefs(curr, [&](Name& name) {
+        if (name == target) {
+          has = true;
+        }
+      });
+    }
+  };
+  Scanner scanner;
+  scanner.target = target;
+  scanner.walk(ast);
+  return scanner.has;
+}
+
 // Get the name of the branch target that is defined in the expression, or an
 // empty name if there is none.
 inline Name getDefinedName(Expression* curr) {
diff --git a/src/passes/Inlining.cpp b/src/passes/Inlining.cpp
index 7467d2559..0360974f0 100644
--- a/src/passes/Inlining.cpp
+++ b/src/passes/Inlining.cpp
@@ -299,6 +299,23 @@ doInlining(Module* module, Function* into, const InliningAction& action) {
   Builder builder(*module);
   auto* block = builder.makeBlock();
   block->name = Name(std::string("__inlined_func$") + from->name.str);
+  // In the unlikely event that the function already has a branch target with
+  // this name, fix that up, as otherwise we can get unexpected capture of our
+  // branches, that is, we could end up with this:
+  //
+  //  (block $X             ;; a new block we add as the target of returns
+  //    (from's contents
+  //      (block $X         ;; a block in from's contents with a colliding name
+  //        (br $X          ;; a new br we just added that replaces a return
+  //
+  // Here the br wants to go to the very outermost block, to represent a
+  // return from the inlined function's code, but it ends up captured by an
+  // internal block.
+  if (BranchUtils::hasBranchTarget(from->body, block->name)) {
+    auto existingNames = BranchUtils::getBranchTargets(from->body);
+    block->name = Names::getValidName(
+      block->name, [&](Name test) { return !existingNames.count(test); });
+  }
   if (call->isReturn) {
     if (retType.isConcrete()) {
       *action.callSite = builder.makeReturn(block);
diff --git a/test/lit/passes/inlining_optimize-level=3.wast b/test/lit/passes/inlining_optimize-level=3.wast
index c9e60fc47..ec39b9030 100644
--- a/test/lit/passes/inlining_optimize-level=3.wast
+++ b/test/lit/passes/inlining_optimize-level=3.wast
@@ -423,4 +423,31 @@
     (drop (i32.const 1))
   )
 )
-.
+
+(module
+ (func $bar
+  (drop
+   (block $__inlined_func$bar (result i32)
+    (return) ;; After inlining, this return will be replaced with a br to a
+             ;; new block. That block's name must not collide with the name
+             ;; of the outer block here, which has been chosen so as to
+             ;; potentially collide. If it collides, we will fail to validate
+             ;; as the new outer block will have type none.
+   )
+  )
+ )
+ ;; CHECK:      (type $none_=>_none (func))
+
+ ;; CHECK:      (func $foo
+ ;; CHECK-NEXT:  (block $__inlined_func$bar_0
+ ;; CHECK-NEXT:   (drop
+ ;; CHECK-NEXT:    (block $__inlined_func$bar (result i32)
+ ;; CHECK-NEXT:     (br $__inlined_func$bar_0)
+ ;; CHECK-NEXT:    )
+ ;; CHECK-NEXT:   )
+ ;; CHECK-NEXT:  )
+ ;; CHECK-NEXT: )
+ (func $foo
+  (call $bar)
+ )
+)