summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlon Zakai <azakai@google.com>2020-01-24 11:28:18 -0800
committerGitHub <noreply@github.com>2020-01-24 11:28:18 -0800
commit1a0530da9c0217e7118965aeb4ee1f59f68df73c (patch)
treea20d1d84a9fd0b5d12a7ae4de76a02559a1005dc
parent1ca6394d34c827904e6ececb6463ef4899c95fe9 (diff)
downloadbinaryen-1a0530da9c0217e7118965aeb4ee1f59f68df73c.tar.gz
binaryen-1a0530da9c0217e7118965aeb4ee1f59f68df73c.tar.bz2
binaryen-1a0530da9c0217e7118965aeb4ee1f59f68df73c.zip
Handle indirect calls in CallGraphPropertyAnalysis (#2624)
We ignored them, which is a bad default, as typically they imply we can call anything in the table (and the table might change). Instead, notice indirect calls during traversal, and force the user to decide whether to ignore them or not. This was only an issue in PostEmscripten because the other user, Asyncify, already had indirect call analysis because it needed it for other things. Fixes a bug uncovered by #2619 and fixes the current binaryen roll.
-rw-r--r--src/ir/module-utils.h15
-rw-r--r--src/passes/Asyncify.cpp3
-rw-r--r--src/passes/PostEmscripten.cpp4
-rw-r--r--test/passes/post-emscripten.txt24
-rw-r--r--test/passes/post-emscripten.wast23
5 files changed, 64 insertions, 5 deletions
diff --git a/src/ir/module-utils.h b/src/ir/module-utils.h
index 703ae3e53..93d110120 100644
--- a/src/ir/module-utils.h
+++ b/src/ir/module-utils.h
@@ -341,6 +341,7 @@ template<typename T> struct CallGraphPropertyAnalysis {
struct FunctionInfo {
std::set<Function*> callsTo;
std::set<Function*> calledBy;
+ bool hasIndirectCall = false;
};
typedef std::map<Function*, T> Map;
@@ -362,6 +363,10 @@ template<typename T> struct CallGraphPropertyAnalysis {
info.callsTo.insert(module->getFunction(curr->target));
}
+ void visitCallIndirect(CallIndirect* curr) {
+ info.hasIndirectCall = true;
+ }
+
private:
Module* module;
T& info;
@@ -382,14 +387,20 @@ template<typename T> struct CallGraphPropertyAnalysis {
}
}
+ enum IndirectCalls { IgnoreIndirectCalls, IndirectCallsHaveProperty };
+
// Propagate a property from a function to those that call it.
void propagateBack(std::function<bool(const T&)> hasProperty,
std::function<bool(const T&)> canHaveProperty,
- std::function<void(T&)> addProperty) {
+ std::function<void(T&)> addProperty,
+ IndirectCalls indirectCalls) {
// The work queue contains items we just learned can change the state.
UniqueDeferredQueue<Function*> work;
for (auto& func : wasm.functions) {
- if (hasProperty(map[func.get()])) {
+ if (hasProperty(map[func.get()]) ||
+ (indirectCalls == IndirectCallsHaveProperty &&
+ map[func.get()].hasIndirectCall)) {
+ addProperty(map[func.get()]);
work.push(func.get());
}
}
diff --git a/src/passes/Asyncify.cpp b/src/passes/Asyncify.cpp
index b7e8c90e0..a4ff5794b 100644
--- a/src/passes/Asyncify.cpp
+++ b/src/passes/Asyncify.cpp
@@ -549,7 +549,8 @@ public:
return !info.isBottomMostRuntime &&
!info.inBlacklist;
},
- [](Info& info) { info.canChangeState = true; });
+ [](Info& info) { info.canChangeState = true; },
+ scanner.IgnoreIndirectCalls);
map.swap(scanner.map);
diff --git a/src/passes/PostEmscripten.cpp b/src/passes/PostEmscripten.cpp
index e6eeb8e2a..ccf985c50 100644
--- a/src/passes/PostEmscripten.cpp
+++ b/src/passes/PostEmscripten.cpp
@@ -154,9 +154,11 @@ struct PostEmscripten : public Pass {
}
});
+ // Assume an indirect call might throw.
analyzer.propagateBack([](const Info& info) { return info.canThrow; },
[](const Info& info) { return true; },
- [](Info& info) { info.canThrow = true; });
+ [](Info& info) { info.canThrow = true; },
+ analyzer.IndirectCallsHaveProperty);
// Apply the information.
struct OptimizeInvokes : public WalkerPass<PostWalker<OptimizeInvokes>> {
diff --git a/test/passes/post-emscripten.txt b/test/passes/post-emscripten.txt
index 13fb3e0de..7b8567cb1 100644
--- a/test/passes/post-emscripten.txt
+++ b/test/passes/post-emscripten.txt
@@ -152,3 +152,27 @@
(nop)
)
)
+(module
+ (type $none_=>_none (func))
+ (type $i32_i32_f32_=>_none (func (param i32 i32 f32)))
+ (type $i32_f32_=>_none (func (param i32 f32)))
+ (type $f64_f64_=>_f64 (func (param f64 f64) (result f64)))
+ (import "env" "glob" (global $glob i32))
+ (import "global.Math" "pow" (func $Math_pow (param f64 f64) (result f64)))
+ (import "env" "invoke_vif" (func $invoke_vif (param i32 i32 f32)))
+ (memory $0 256 256)
+ (table $0 7 7 funcref)
+ (elem (i32.const 0) $other_safe)
+ (func $exc (; 2 ;)
+ (call $invoke_vif
+ (i32.const 0)
+ (i32.const 42)
+ (f32.const 3.141590118408203)
+ )
+ )
+ (func $other_safe (; 3 ;) (param $0 i32) (param $1 f32)
+ (call_indirect (type $none_=>_none)
+ (i32.const 0)
+ )
+ )
+)
diff --git a/test/passes/post-emscripten.wast b/test/passes/post-emscripten.wast
index fe60858f7..e5a4637cc 100644
--- a/test/passes/post-emscripten.wast
+++ b/test/passes/post-emscripten.wast
@@ -109,7 +109,7 @@
(call $call)
)
)
-(module
+(module ;; non-constant base for elem
(type $0 (func (param i32)))
(import "global.Math" "pow" (func $Math_pow (param f64 f64) (result f64)))
(import "env" "invoke_vif" (func $invoke_vif (param i32 i32 f32)))
@@ -127,3 +127,24 @@
(func $other_safe (param i32) (param f32)
)
)
+(module ;; indirect call in the invoke target, which we assume might throw
+ (type $none_=>_none (func))
+ (import "global.Math" "pow" (func $Math_pow (param f64 f64) (result f64)))
+ (import "env" "invoke_vif" (func $invoke_vif (param i32 i32 f32)))
+ (import "env" "glob" (global $glob i32)) ;; non-constant table offset
+ (memory 256 256)
+ (table 7 7 funcref)
+ (elem (i32.const 0) $other_safe)
+ (func $exc
+ (call $invoke_vif
+ (i32.const 0) ;; other_safe()
+ (i32.const 42)
+ (f32.const 3.14159)
+ )
+ )
+ (func $other_safe (param i32) (param f32)
+ (call_indirect (type $none_=>_none)
+ (i32.const 0)
+ )
+ )
+)