-
Notifications
You must be signed in to change notification settings - Fork 240
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
Curl extension for the Node.js build of PHP.wasm #1273
Curl extension for the Node.js build of PHP.wasm #1273
Conversation
I already tried some things but nothing was efficient enough. Here is what I did : I explored the different methods called in the So after a lot of trial and error, I added this new list in the
But it always stops at |
I've spent some time tracing it down. These RAND_ functions come from libopenssl and I bet this is either because |
Or maybe I'm wrong. I rebuilt both with the additional wrapping and it still didn't work. Also, the ASYNCIFY having effect on some Openssl functions tells me maybe the linking shouldn't matter after all. I'm going through the openssl source code to find even more functions to add to ASYNCIFY_ONLY. |
Aha, this got me to the next function call (at +"Curl_infof",\
+"Curl_debug",\
+"ossl_connect_step1",\
+"rand_enough",\
+"RAND_get_rand_method",\
+"RAND_OpenSSL",\
+"RAND_get0_primary",\
+"EVP_RAND_get_state",\
+"OPENSSL_init_ssl",\
+"lh_SSL_SESSION_new",\
+"CRYPTO_THREAD_lock_new",\
+"CRYPTO_NEW_REF",\
+"SSL_get_ex_data_X509_STORE_CTX_idx",\
+"OPENSSL_zalloc",\
+"OPENSSL_strdup",\
+"ssl_load_ciphers",\
+"X509_STORE_new",\
+"SSL_get_ex_data_X509_STORE_CTX_idx",\
+"ssl_load_groups",\
+"ssl_setup_sigalgs",\
+"ssl_cert_new",\
+"SSL_CTX_set_ciphersuites",\
+"sk_SSL_CIPHER_num",\
+"ssl_create_cipher_list",\
+"OSSL_default_cipher_list",\
+"OPENSSL_zalloc",\
+"X509_VERIFY_PARAM_new",\
+"CRYPTO_NEW_REF",\
+"ssl_evp_md_fetch",\
+"sk_X509_NAME_new_null",\
+"OPENSSL_free",\
+"CRYPTO_new_ex_data",\
+"CRYPTO_THREAD_lock_new",\
+"ERR_raise",\
+"OPENSSL_secure_zalloc",\
+"ossl_comp_has_alg",\
+"RAND_priv_bytes_ex",\
+"RAND_bytes_ex",\
+"SSL_CTX_set_client_cert_engine",\
+"ssl_ctx_srp_ctx_init_intern",\
+"SSL_CTX_new",\
+"SSL_CTX_new_ex",\
+"RAND_bytes",\
+"rand_nopseudo_bytes",\
+"rand_bytes",\ So it's going to be all about finding the right functions manually. Yikes! I wonder why aren't these functions showing up in the stack traces, it would've simplified a lot. |
@adamziel This looks like a job for me ! I found out the next step + "ERR_raise",\
+ "OPENSSL_init_crypto",\
+ "RUN_ONCE",\
+ "RUN_ONCE_ALT",\ |
Yay! I couldn't crack the next one, tossing some |
@adamziel I tried to It worked before in Any idea ? Your suggestions above could work with |
@adamziel I found out something extremely strange to me while trying things you suggested : If I replace line with
The current one we face. But if I replace this by
The @adamziel @bgrgicak @brandonpayton At this point I probably need some insights from you |
Some functions probably get inlined with -O3 and don't show up in the stack trace. I'm confused, though, why it's so different. |
I remember that @adamziel and I experienced the same behavior while working on another task, but we couldn't find out why that was happening. |
We redirect Emscripten stdout stream to /internal/stdout and PHP reads the output from there, unless it crashes of course. It's a good question why that output was visible in the browser – I'd have to step through the code to answer as I'm not sure off the top of my head. One way to explain it would be that There's two things you could do from here:
|
@adamziel I made some tests in browser, alongside the message
The two first prints showed before The problem is, when I am on browser, I have a different error as in node.
No I also tried to comment out these lines earlier :
but nothing different happened in terminal. My main goal here is to find out how to print anything in the terminal when using |
Just to be sure – are you reproducing the same setup as here? Or did that also work without the unreachable error? If yes, it's a curious case where the networking workaround we use in the browser changes something about the code execution flow as compared to node.
I may have more historical context, but I'm far from being a WASM expert and am just as confused as you :D |
@adamziel I am sorry about that, I thought you digged in the files changed in this PR : I added the strict necessary in this PR to reproduce the error I had in the I wanted to have an environment as "light" as possible compared to the other PR. I suppose When on desktop everything seems to be working in |
I didn't get a chance to review in details at that time, sorry, I was just basing on the conversation here.
👍 Node has its own networking support, it opens a WebSocket to a local server that translates the received traffic to a raw TCP socket.
Yeah without that network handler it's very likely that curl figures out it cannot open the socket and short-circuits before the line where the crash happens. Debugging this in the browser without that HTTPS handler might not be possible.
This is weird and I don't have a good answer. The easiest way to debug this, then, might not be via prints but attaching a debugger and stepping through the execution.
I don't think the code execution gets to the point where it calls the same functions that crash on node. |
@adamziel I don't think
When But anyways. How, and thank you for your patience explaining these things to me but, how could I use that debugger and step through the methods used in C while using the |
I had some luck with the VS Code debugger by running a test script via its debug configurations. I never got it to step through the actual C source code, though, it was always wasm bytecode. I know @brandonpayton got the source map and symbol names to work in the browser so I'm CCing him for insights. Perhaps you could build PHP with a separate DWARF, plug it in some VS Code panel, and have it "just work"? |
@adamziel I managed to generate the sourcemaps of ![]() Unfortunately a lot of files are missing, and it seems like the files are also missing a lot of functions. Looking at file
Meaning the only curl files available in sourcemaps are those used by I assume this is still interesting. @brandonpayton I hope you could help here. |
When I was debugging, I only enabled DWARF symbols and not source maps, but it seems like they could co-exist with no issues. Unfortunately, when I was looking, utilizing DWARF symbols only worked with Chrome Canary with the CPP/WASM extension installed. I made a note to experiment with debugging in VS Code and will let you know if I find anything more.
@mho22 In this case, when libcurl.a is built with Emscripten, is debug info included? I'm not sure if Emscripten will leverage that or not, but if libcurl.a isn't built with debug info, then it does not seem Emscripten has any place to find debug info when building php-wasm:node. 🤔 @adamziel I wonder whether we could build all our pre-built libs with debug info and trust Emscripten to exclude it when linking normal php-wasm builds. Then, if debug info is desired and configured in the php-wasm build, maybe it would just be included from the pre-built libs automatically. |
If the linker can do tree shaking, minification etc and get us the same bundle size regardless of the optimized/unoptimized inputs, I'm all for this! Otherwise we could ship "prod" and "dev" libraries in the repo. |
@brandonpayton @adamziel I followed these steps :
-> This added the 2 files :
But only 7 occurences of
-> After
But right now, when I run my script ![]()
So, I decided to build this directory. How ? By adding this line in
Now I have And these lines in
After recompiling and building node I get this : ![]() So I decide to debug in VS Code and add a marker on the first line of method
![]() Nothing. But I still get two infos :
This indicates it goes logically through the steps but after step But what now? What can we do to make the debug work correctly in VSCode? Should I push my current code on this PR ? |
Invalid state looks like an Asyncify error so we may be getting somewhere:
|
@adamziel It looks like it is the I had to add ![]() Two new
Let's investigate further. |
@adamziel New step :
But we now have a new
I added the new elements from this stack trace :
But this won't change the new stack trace. Stuck again. What we can define is the step where we are :
Is called from Next step in the stack trace is |
@mho22 I think PHP 8.0 unit tests will pass now. I'm happy to merge this PR as soon as it ships all the rebuilt PHP versions with green unit tests and a few tests to confirm curl works. I found the missing functions that tripped up the multisite unit tests by doing this on the cd packages/playground/blueprints
node --experimental-wasm-stack-switching ../../../node_modules/.bin/vitest src/lib/steps/enable-multisite.spec.ts And by adding this bit to the for (const fn of global.asyncifyFunctions) {
console.log(`"${fn}",`);
} |
@adamziel I am trying to build
While with
Something is probably missing in |
PHP 7.3 uses the following check:
PHP 7.4, however, doesn't seem to test for the |
@adamziel I don't think this will solve the issue. It seems on
if not, then it will avoid the code you pasted above, and there is no The next message printed is And the last message
While in
We then should probably try to add |
@mho22 I wrote the libcurl.pc by hand. It satisfied one check, but the version check kept failing. I removed that version check, too, and the build worked for all PHP versions! :) I found one more ASYNCIFY_ONLY entry, added a fee more unit tests, and I'm rebuilding everything right now. |
@adamziel Haha fantastic! |
php-wasm:node
with Curl extension
Let's ship! 🎉 |
What is this PR doing?
This PR builds PHP with
--with-libcurl
option to support the curl extension.It also changes two nuances in the overall PHP build process:
select(2)
function using-Wl,--wrap=select
emcc option instead of patching PHP source code – this enables supporting asynchronousselect(2)
in curl without additional patches.__wrap_select
implementation more in line withselect(2)
, add support forPOLLERR
.poll(2)
– that's becauselibcurl
polls/dev/urandom
.Builds on top of and supersedes #1133
Debugging Asyncify problems
The typical way of resolving Asyncify crashes didn't work during the work on this PR. Functions didn't come up in the error messages and even raw stack traces. The reasons are unclear.
The JSPI build of PHP was more helpful as it enabled logging the current stack trace in all the asynchronous calls, which quickly revealed all the missing
ASYNCIFY_ONLY
functions. This is the way to debug any future issues until we fully migrate to JSPI.Testing Instructions
Confirm the CI checks pass. This PR ships a few new tests specifically targeting networking with curl.
Related resources