Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

asyncify WHITELIST wildcard matching #9381

Open
Beuc opened this issue Sep 4, 2019 · 9 comments

Comments

@Beuc
Copy link
Contributor

commented Sep 4, 2019

Hi,

In my project some function names are not predictable, so I need some wildcard system in ASYNCIFY_WHITELIST.

With Emterpreter that could be due to naming conflicts in the various object files; with asyncify/upstream I cannot tell for sure, llvm-nm shows me 28 __Pyx_PyObject_CallNoArg homonym functions.
EDIT: wasm-dis shows me 20 __Pyx_PyObject_CallNoArg.XX, so wildcard still needed.

It can also be due to cython-generated functions which AFAICS may change depending on line number or function ordering. Here I want to match __pyx_pw_5renpy_2gl_6gldraw_6GLDraw_*draw_screen among 56 functions that start with __pyx_pw_5renpy_2gl_6gldraw_6GLDraw_, so I believe full wildcard (as I implemented for Emterpreter, not just prefix matching) would be justified.

@Beuc

This comment has been minimized.

Copy link
Contributor Author

commented Sep 5, 2019

EDIT: wasm-dis shows me 20 __Pyx_PyObject_CallNoArg.XX, so wildcard still needed to support homonym functions.

@kripken

This comment has been minimized.

Copy link
Member

commented Sep 6, 2019

I see, yeah, name conflicts do make this unpredictable.

We'd need some way to do arbitrary wildcards in binaryen then. Perhaps we should use C++ regex? No idea what's a good solution in that space, help welcome.

@hostilefork

This comment has been minimized.

Copy link

commented Sep 7, 2019

I also hit this when converting an emterpreter project to asyncify...the wildcards came up as errors.

On another incompatibility note: the names for the EMTERPRETIFY_WHITELIST had leading underscores...and the ASYNCIFY_WHITELIST seemed to not want those underscores. That may be worth explaining/mentioning in "Migrating from Older APIs" on the Asyncify page

Perhaps we should use C++ regex? No idea what's a good solution in that space, help welcome.

I often forget that the "technical" term for wildcard matching with asterisks in a file-path type way is "globbing":

https://en.wikipedia.org/wiki/Glob_(programming)

Seems there's a lot of pre-written glob code already, and it's handy enough, so including that seems nice. OTOH, RegEx is practically against my religion (and I like Rebol's PARSE). So my leaning is that if a problem is something that simple globbing can't handle, it's best to offer a callout/hook where people with problems that complex can use tools of their choice.

@Beuc

This comment has been minimized.

Copy link
Contributor Author

commented Sep 8, 2019

For the record this is Emterpreter's pull request discussion.
#8056
We implemented glogging back then, was it an oversight or is there something different in Asyncify's functions list?

@kripken

This comment has been minimized.

Copy link
Member

commented Sep 10, 2019

The main difference is that Asyncify is written in C++ (it's a Binaryen pass), so we need a globbing library in that language (maybe there is one in the C++ stdlib?).

@Beuc

This comment has been minimized.

Copy link
Contributor Author

commented Sep 10, 2019

I see. There's <glob.h> in libc and mingw and gnulib; apparently there's also a GlobPattern in LLVM support.
What is the target build environment?
-- though a simple matching on * would probably be enough, do we really need support for ? and [] ?

@kripken

This comment has been minimized.

Copy link
Member

commented Sep 11, 2019

Hmm, glob.h looks like it only works on the filesystem, not arbitrary strings?

The target here is to be portable C++ code, binaryen tests building on linux using gcc and clang and msvc for windows.

But yeah, simple matching on * is probably enough. We can just write such a helper - I wrote one for suffixes with *, but prefixes etc. are more work. Help welcome!

@Beuc

This comment has been minimized.

Copy link
Contributor Author

commented Sep 12, 2019

Right, that would be fnmatch or PathMatchSpec.
I was hoping for mingw, I won't be able to test msvc.

@Beuc

This comment has been minimized.

Copy link
Contributor Author

commented Sep 18, 2019

I pushed a first version at WebAssembly/binaryen#2344

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.