SAFE_HEAP: remove fastcomp, prepare for new emscripten approach #3078
Conversation
src/passes/SafeHeap.cpp
Outdated
| @@ -102,13 +109,14 @@ struct AccessInstrumenter : public WalkerPass<PostWalker<AccessInstrumenter>> { | |||
|
|
|||
| struct SafeHeap : public Pass { | |||
| PassOptions options; | |||
| Name funcToIgnore; | |||
There was a problem hiding this comment.
This simultaneously looks like a general mechanism for ignoring functions (due to the name) and also like a very specific mechanism (because there is only one name). Would it make more sense to name it something like shouldIgnoreGetSbrkPtr and make it a bool to make it clear that it is a very specific mechanism that is only used for that one thing?
There was a problem hiding this comment.
A bool would be inconvenient actually, because we need to know the name. If we just pass a bool to a function-parallel pass like this, then each instance (one per function) would need to look up the function name, and we need to look it up because we need to find an export with that name - there may be no Name section, but the export is guaranteed.
There was a problem hiding this comment.
(I can add a comment explaining this more, if you think that's useful?)
There was a problem hiding this comment.
I see. Perhaps a name like GetSbrkPtrFuncToIgnore would be more descriptive then?
There was a problem hiding this comment.
Good point, yeah, a better name could help here. How about getSbrkPtrName (which matches getSbrkPtr which is the actual function), which I pushed now?
There was a problem hiding this comment.
Oh wait... getSbrkPtr was already the name. So no new variable or name was needed! Fixed.
In fastcomp we implemented
emscripten_get_sbrk_ptrin wasm, andexported
_emscripten_get_sbrk_ptr. We don't need that anymore andcan remove it.
However I want to switch us to implementing
emscripten_get_sbrk_ptrin wasm in upstream too, as part of removing
DYNAMICTOP_PTRandother silliness that we have around link (#3043).
This makes us support an export of
emscripten_get_sbrk_ptr(noprefix), and also it makes sure not to instrument that function, which
may contain some memory operations itself, but if we SAFE_HEAP-ify
them we'd get infinite recursion, as the SAFE_HEAP methods need to
call that.