[WebAssembly] Narrow the scope of WebAssemblyFixFunctionBitcasts

Summary:
The pass to fix function bitcasts generates thunks for functions that
are called directly with a mismatching signature. It was also generating
thunks in cases where the function was address-taken, causing aliasing
problems in otherwise valid cases.
This patch tightens the restrictions for when the pass runs.

Reviewers: sunfish, dschuff

Subscribers: jfb, sbc100, llvm-commits, aheejin

Differential Revision: https://reviews.llvm.org/D38640

llvm-svn: 315326

llvm/lib/Target/WebAssembly/WebAssemblyFixFunctionBitcasts.cpp

@@ -24,6 +24,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "WebAssembly.h"
+#include "llvm/IR/CallSite.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Module.h"
@@ -68,10 +69,19 @@ static void FindUses(Value *V, Function &F,
     if (BitCastOperator *BC = dyn_cast<BitCastOperator>(U.getUser()))
       FindUses(BC, F, Uses, ConstantBCs);
     else if (U.get()->getType() != F.getType()) {
+      CallSite CS(U.getUser());
+      if (!CS)
+        // Skip uses that aren't immediately called
+        continue;
+      Value *Callee = CS.getCalledValue();
+      if (Callee != V)
+        // Skip calls where the function isn't the callee
+        continue;
       if (isa<Constant>(U.get())) {
         // Only add constant bitcasts to the list once; they get RAUW'd
         auto c = ConstantBCs.insert(cast<Constant>(U.get()));
-        if (!c.second) continue;
+        if (!c.second)
+          continue;
       }
       Uses.push_back(std::make_pair(&U, &F));
     }

llvm/test/CodeGen/WebAssembly/function-bitcasts.ll

@@ -1,10 +1,20 @@
-; RUN: llc < %s -asm-verbose=false -disable-wasm-explicit-locals | FileCheck %s
+; RUN: llc < %s -asm-verbose=false -disable-wasm-explicit-locals -enable-emscripten-cxx-exceptions | FileCheck %s
 
 ; Test that function pointer casts are replaced with wrappers.
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown-wasm"
 
+declare void @has_i32_arg(i32)
+declare i32 @has_i32_ret()
+declare void @vararg(...)
+declare void @plain(i32)
+
+declare void @foo0()
+declare void @foo1()
+declare void @foo2()
+declare void @foo3()
+
 ; CHECK-LABEL: test:
 ; CHECK-NEXT: call        .Lbitcast@FUNCTION{{$}}
 ; CHECK-NEXT: call        .Lbitcast@FUNCTION{{$}}
@@ -21,6 +31,25 @@ target triple = "wasm32-unknown-unknown-wasm"
 ; CHECK-NEXT: call        foo1@FUNCTION{{$}}
 ; CHECK-NEXT: call        foo3@FUNCTION{{$}}
 ; CHECK-NEXT: end_function
+define void @test() {
+entry:
+  call void bitcast (void (i32)* @has_i32_arg to void ()*)()
+  call void bitcast (void (i32)* @has_i32_arg to void ()*)()
+  call void bitcast (i32 ()* @has_i32_ret to void ()*)()
+  call void bitcast (void ()* @foo0 to void (i32)*)(i32 0)
+  %p = bitcast void ()* @foo0 to void (i32)*
+  call void %p(i32 0)
+  %q = bitcast void ()* @foo0 to void (i32)*
+  call void %q(i32 0)
+  %r = bitcast void (i32)* %q to void ()*
+  call void %r()
+  %t = call i32 bitcast (void ()* @foo1 to i32 ()*)()
+  call void bitcast (void ()* @foo2 to void ()*)()
+  call void @foo1()
+  call void @foo3()
+
+  ret void
+}
 
 ; CHECK-LABEL: test_varargs:
 ; CHECK:      set_global
@@ -29,6 +58,85 @@ target triple = "wasm32-unknown-unknown-wasm"
 ; CHECK-NEXT: i32.const   $push[[L4:[0-9]+]]=, 0{{$}}
 ; CHECK-NEXT: i32.store   0($[[L5:[0-9]+]]), $pop[[L4]]{{$}}
 ; CHECK-NEXT: call        plain@FUNCTION, $[[L5]]{{$}}
+define void @test_varargs() {
+  call void bitcast (void (...)* @vararg to void (i32)*)(i32 0)
+  call void (...) bitcast (void (i32)* @plain to void (...)*)(i32 0)
+  ret void
+}
+
+; Don't use wrappers when the value is stored in memory
+
+@global_func = hidden local_unnamed_addr global void ()* null
+
+; CHECK-LABEL: test_store:
+; CHECK-NEXT: i32.const   $push[[L0:[0-9]+]]=, 0{{$}}
+; CHECK-NEXT: i32.const   $push[[L1:[0-9]+]]=, has_i32_ret@FUNCTION{{$}}
+; CHECK-NEXT: i32.store   global_func($pop[[L0]]), $pop[[L1]]{{$}}
+define void @test_store() {
+  %1 = bitcast i32 ()* @has_i32_ret to void ()*
+  store void ()* %1, void ()** @global_func
+  ret void
+}
+
+; CHECK-LABEL: test_load:
+; CHECK-NEXT: result      i32{{$}}
+; CHECK-NEXT: i32.const   $push[[L0:[0-9]+]]=, 0{{$}}
+; CHECK-NEXT: i32.load    $push[[L1:[0-9]+]]=, global_func($pop[[L0]]){{$}}
+; CHECK-NEXT: i32.call_indirect $push{{[0-9]+}}=, $pop[[L1]]{{$}}
+define i32 @test_load() {
+  %1 = load i32 ()*, i32 ()** bitcast (void ()** @global_func to i32 ()**)
+  %2 = call i32 %1()
+  ret i32 %2
+}
+
+; Don't use wrappers when the value is passed to a function call
+
+declare void @call_func(i32 ()*)
+
+; CHECK-LABEL: test_argument:
+; CHECK-NEXT: i32.const   $push[[L0:[0-9]+]]=, has_i32_ret@FUNCTION{{$}}
+; CHECK-NEXT: call        call_func@FUNCTION, $pop[[L0]]{{$}}
+; CHECK-NEXT: i32.const   $push[[L1:[0-9]+]]=, has_i32_arg@FUNCTION{{$}}
+; CHECK-NEXT: call        call_func@FUNCTION, $pop[[L1]]{{$}}
+define void @test_argument() {
+  call void @call_func(i32 ()* @has_i32_ret)
+  call void @call_func(i32 ()* bitcast (void (i32)* @has_i32_arg to i32 ()*))
+  ret void
+}
+
+; Invokes should be treated like calls
+
+; CHECK-LABEL: test_invoke:
+; CHECK:      i32.const   $push[[L1:[0-9]+]]=, call_func@FUNCTION{{$}}
+; CHECK-NEXT: i32.const   $push[[L0:[0-9]+]]=, has_i32_ret@FUNCTION{{$}}
+; CHECK-NEXT: call        "__invoke_void_i32()*"@FUNCTION, $pop[[L1]], $pop[[L0]]{{$}}
+; CHECK:      i32.const   $push[[L3:[0-9]+]]=, call_func@FUNCTION{{$}}
+; CHECK-NEXT: i32.const   $push[[L2:[0-9]+]]=, has_i32_arg@FUNCTION{{$}}
+; CHECK-NEXT: call        "__invoke_void_i32()*"@FUNCTION, $pop[[L3]], $pop[[L2]]{{$}}
+; CHECK:      i32.const   $push[[L4:[0-9]+]]=, .Lbitcast@FUNCTION{{$}}
+; CHECK-NEXT: call        __invoke_void@FUNCTION, $pop[[L4]]{{$}}
+declare i32 @personality(...)
+define void @test_invoke() personality i32 (...)* @personality {
+entry:
+  invoke void @call_func(i32 ()* @has_i32_ret)
+          to label %cont unwind label %lpad
+
+cont:
+  invoke void @call_func(i32 ()* bitcast (void (i32)* @has_i32_arg to i32 ()*))
+          to label %cont2 unwind label %lpad
+
+cont2:
+  invoke void bitcast (void (i32)* @has_i32_arg to void ()*)()
+          to label %end unwind label %lpad
+
+lpad:
+  %0 = landingpad { i8*, i32 }
+          catch i8* null
+  br label %end
+
+end:
+  ret void
+}
 
 ; CHECK-LABEL: .Lbitcast:
 ; CHECK-NEXT: call        has_i32_arg@FUNCTION, $0{{$}}
@@ -48,39 +156,3 @@ target triple = "wasm32-unknown-unknown-wasm"
 ; CHECK-NEXT: call        foo1@FUNCTION{{$}}
 ; CHECK-NEXT: copy_local  $push0=, $0
 ; CHECK-NEXT: end_function
-
-declare void @has_i32_arg(i32)
-declare i32 @has_i32_ret()
-declare void @vararg(...)
-declare void @plain(i32)
-
-declare void @foo0()
-declare void @foo1()
-declare void @foo2()
-declare void @foo3()
-
-define void @test() {
-entry:
-  call void bitcast (void (i32)* @has_i32_arg to void ()*)()
-  call void bitcast (void (i32)* @has_i32_arg to void ()*)()
-  call void bitcast (i32 ()* @has_i32_ret to void ()*)()
-  call void bitcast (void ()* @foo0 to void (i32)*)(i32 0)
-  %p = bitcast void ()* @foo0 to void (i32)*
-  call void %p(i32 0)
-  %q = bitcast void ()* @foo0 to void (i32)*
-  call void %q(i32 0)
-  %r = bitcast void (i32)* %q to void ()*
-  call void %r()
-  %t = call i32 bitcast (void ()* @foo1 to i32 ()*)()
-  call void bitcast (void ()* @foo2 to void ()*)()
-  call void @foo1()
-  call void @foo3()
-
-  ret void
-}
-
-define void @test_varargs() {
-  call void bitcast (void (...)* @vararg to void (i32)*)(i32 0)
-  call void (...) bitcast (void (i32)* @plain to void (...)*)(i32 0)
-  ret void
-}