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

New CMake sizeof check breaks pthread builds #18354

Closed
RReverser opened this issue Dec 12, 2022 · 4 comments · Fixed by #18355
Closed

New CMake sizeof check breaks pthread builds #18354

RReverser opened this issue Dec 12, 2022 · 4 comments · Fixed by #18355

Comments

@RReverser
Copy link
Collaborator

RReverser commented Dec 12, 2022

I'm seeing an issue when using #18309 (via emscripten 3.1.28) when building libjpeg as part of my project build which uses -pthread and SIMD.

This is the generated jconfigint.h:

/* The size of `size_t', as computed by sizeof. */
#define SIZEOF_SIZE_T  

Here's CMakeCache.txt:

//CHECK_TYPE_SIZE: size_t unknown
SIZE_T:INTERNAL=

Looks like it failed due to -pthread in flags (note the "STANDALONE_WASM does not support shared memories yet"):

Building C object CMakeFiles/cmTC_aee2f.dir/SIZE_T.c.o
/emsdk/upstream/emscripten/emcc   -O3 -pthread -flto -mnontrapping-fptoint -msimd128 -DWASM_SIMD_COMPAT_SLOW -DWASM_BIGINT -DNO_GETENV -DNO_PUTENV  -MD -MT CMakeFiles/cmTC_aee2f.dir/SIZE_T.c.o -MF CMakeFiles/cmTC_aee2f.dir/SIZE_T.c.o.d -o CMakeFiles/cmTC_aee2f.dir/SIZE_T.c.o -c /src/build/deps/jpeg/_build/CMakeFiles/CheckTypeSize/SIZE_T.c
Linking C executable cmTC_aee2f.js
/usr/bin/cmake -E cmake_link_script CMakeFiles/cmTC_aee2f.dir/link.txt --verbose=1
/emsdk/upstream/emscripten/emcc -O3 -pthread -flto -mnontrapping-fptoint -msimd128 -DWASM_SIMD_COMPAT_SLOW -DWASM_BIGINT -DNO_GETENV -DNO_PUTENV  -O3 -pthread -flto -L/src/build/target/lib -sAUTO_JS_LIBRARIES=0 -sAUTO_NATIVE_LIBRARIES=0 -sWASM_BIGINT  @CMakeFiles/cmTC_aee2f.dir/objects1.rsp -o cmTC_aee2f.js @CMakeFiles/cmTC_aee2f.dir/linklibs.rsp
cache:INFO: generating system asset: sysroot/lib/wasm32-emscripten/lto/crt1.o... (this will be cached in "/src/build/deps/emcache/sysroot/lib/wasm32-emscripten/lto/crt1.o" for subsequent builds)
system_libs:INFO: compiled 1 inputs
cache:INFO:  - ok
cache:INFO: generating system library: sysroot/lib/wasm32-emscripten/lto/libstandalonewasm.a... (this will be cached in "/src/build/deps/emcache/sysroot/lib/wasm32-emscripten/lto/libstandalonewasm.a" for subsequent builds)
system_libs:INFO: compiled 20 inputs
cache:INFO:  - ok
cache:INFO: generating system asset: symbol_lists/70de70b14b316e42625f6158cce62c3c32a6f961.txt... (this will be cached in "/src/build/deps/emcache/symbol_lists/70de70b14b316e42625f6158cce62c3c32a6f961.txt" for subsequent builds)
error: library_pthread.js:29: #error "STANDALONE_WASM does not support shared memories yet"
cache:INFO:  - ok
error: library_pthread.js:29: #error "STANDALONE_WASM does not support shared memories yet"
Error: Aborting compilation due to previous errors
emcc: error: '/usr/bin/node /emsdk/upstream/emscripten/src/compiler.js /tmp/tmpdp5fx9nz.json' failed (returned 1)
gmake[1]: *** [CMakeFiles/cmTC_aee2f.dir/build.make:102: cmTC_aee2f.js] Error 1
gmake[1]: *** Deleting file 'cmTC_aee2f.js'
gmake[1]: Leaving directory '/src/build/deps/jpeg/_build/CMakeFiles/CMakeTmp'
gmake: *** [Makefile:127: cmTC_aee2f/fast] Error 2


/src/build/deps/jpeg/_build/CMakeFiles/CheckTypeSize/SIZE_T.c:
#include <sys/types.h>
#include <stdint.h>
#include <stddef.h>


#undef KEY
#if defined(__i386)
# define KEY '_','_','i','3','8','6'
#elif defined(__x86_64)
# define KEY '_','_','x','8','6','_','6','4'
#elif defined(__ppc__)
# define KEY '_','_','p','p','c','_','_'
#elif defined(__ppc64__)
# define KEY '_','_','p','p','c','6','4','_','_'
#elif defined(__aarch64__)
# define KEY '_','_','a','a','r','c','h','6','4','_','_'
#elif defined(__ARM_ARCH_7A__)
# define KEY '_','_','A','R','M','_','A','R','C','H','_','7','A','_','_'
#elif defined(__ARM_ARCH_7S__)
# define KEY '_','_','A','R','M','_','A','R','C','H','_','7','S','_','_'
#endif

#define SIZE (sizeof(size_t))
char info_size[] =  {'I', 'N', 'F', 'O', ':', 's','i','z','e','[',
  ('0' + ((SIZE / 10000)%10)),
  ('0' + ((SIZE / 1000)%10)),
  ('0' + ((SIZE / 100)%10)),
  ('0' + ((SIZE / 10)%10)),
  ('0' +  (SIZE    % 10)),
  ']',
#ifdef KEY
  ' ','k','e','y','[', KEY, ']',
#endif
  '\0'};

#ifdef __CLASSIC_C__
int main(argc, argv) int argc; char *argv[];
#else
int main(int argc, char *argv[])
#endif
{
  int require = 0;
  require += info_size[argc];
  (void)argv;
  return require;
}


Determining size of unsigned long failed with the following output:
Change Dir: /src/build/deps/jpeg/_build/CMakeFiles/CMakeTmp

@sbc100

@RReverser
Copy link
Collaborator Author

It's probably possible to move flags around so that they don't apply to configure stage, but I suspect this might break more projects than #18309 fixed and might be better to revert until we have a better solution.

@sbc100
Copy link
Collaborator

sbc100 commented Dec 12, 2022

I think maybe we should make STANDALONE_WASM works with shared memory. Either that or inject -sUSE_PTHREADS=0 when building this test, but that seems less correct.

@RReverser
Copy link
Collaborator Author

I think maybe we should make STANDALONE_WASM works with shared memory.

Sure, that would be ideal, but I'm not sure how much effort it is, so a revert for now to unbreak builds seemed like a safer suggestion.

@sbc100
Copy link
Collaborator

sbc100 commented Dec 12, 2022

Oh sorry, I didn't see the suggestions of a revert. I'm fine with reverting for now.

sbc100 added a commit that referenced this issue Dec 12, 2022
Using these two options together will most likely result in some
non-standard JS imports (e.g. `_pthread_create_js`), but the point of
`STANDALONE_WASM` is not to completely eliminate those, only to avoid
them where possible.

Fixes: #18354
sbc100 added a commit that referenced this issue Dec 12, 2022
Using these two options together will most likely result in some
non-standard JS imports (e.g. `_pthread_create_js`), but the point of
`STANDALONE_WASM` is not to completely eliminate those, only to avoid
them where possible.

Fixes: #18354
sbc100 added a commit that referenced this issue Dec 12, 2022
Using these two options together will most likely result in some
non-standard JS imports (e.g. `_pthread_create_js`), but the point of
`STANDALONE_WASM` is not to completely eliminate those, only to avoid
them where possible.

Fixes: #18354
sbc100 added a commit that referenced this issue Dec 12, 2022
Using these two options together will most likely result in some
non-standard JS imports (e.g. `_pthread_create_js`), but the point of
`STANDALONE_WASM` is not to completely eliminate those, only to avoid
them where possible.

Fixes: #18354
sbc100 added a commit that referenced this issue Dec 12, 2022
Using these two options together will most likely result in some
non-standard JS imports (e.g. `_pthread_create_js`), but the point of
`STANDALONE_WASM` is not to completely eliminate those, only to avoid
them where possible.

Fixes: #18354
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants