-
Notifications
You must be signed in to change notification settings - Fork 16
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
Fixing thread start_routine signatures in tests #2
base: main
Are you sure you want to change the base?
Fixing thread start_routine signatures in tests #2
Conversation
pthread_create function requires that start_routine has void*(void*) signature. However many tests defined this function as void*(void). When compiling such test to native code such pointer was implicitly casted. In case of Emscripten such casts are won't work and resulted in exception thrown at JavaScript side. This patch adjust tests start_routine to have proper signature.
51ea54e
to
21f5aa8
Compare
I this still needed? are all those tests currently failing? |
I remembered that this was necessary when compiling with ASan, see for example: But perhaps this can also manifest when compiling without optimizations (or with assertions enabled)? |
fwiw, there are still some non-matching signatures on this branch. Issue #6 might also be relevant here. sed -i 's/CFLAGS =/& -fpermissive/' Makefile
sed -i '/check_gcc/,+2d' Makefile
for test in `ls -d conformance/interfaces/pthread_*`; do POSIX_TARGET=$test CC=g++ make build-tests LDFLAGS='-pthread'; done
cat logfile | grep -A 4 -- "-Werror=permissive" Details (filtered)
|
It looks that those tests passes. So for that purpose this patch is not needed. However as @kleisauke mentioned it is necessary for ASAN, so I can rebase it and fix other test signatures if this still will be useful. WDYT? |
Ah! The reason it works is because we run the thread entry points from JS code (which can handle signature mismatches): https://github.com/emscripten-core/emscripten/blob/d8ec08a37915c5a7a358b324627abe5a00c0f632/src/library_pthread.js#L1281-L1283 |
Perhaps we could instread patch asan to launch threads via JS like this? (To avoid the signature mismatch issue). |
Patching ASan to invoke the pthread entry points via JS sounds good to me. I think it could be done by using However, this wouldn't resolve the signature mismatches of the signal handlers, but perhaps that can be resolved in a similar way. |
Yes, I already worked around that here: |
pthread_create
function requires that start_routine hasvoid*(void*)
signature. However many tests defined this function asvoid*(void)
. When compiling such test to native code such pointer was implicitly casted. In case of Emscripten such casts are won't work and resulted in exception thrown at JavaScript side.This PR adjust tests start_routine to have proper signature.