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

CMake check_type_size returns wrong value in Node 18 #17268

Closed
CoalZombik opened this issue Jun 19, 2022 · 1 comment
Closed

CMake check_type_size returns wrong value in Node 18 #17268

CoalZombik opened this issue Jun 19, 2022 · 1 comment

Comments

@CoalZombik
Copy link

Cmake command check_type_size returns 7 for all variable types with using node 18 as CMAKE_CROSSCOMPILING_EMULATOR, but with node 16 works well.

Tested on static binaries of node downloaded from official site:
Node 16.15.1
Node 18.4.0

Testing sources

CMakeLists.txt

cmake_minimum_required(VERSION 3.10)

project(sizeof-test)

include(CheckTypeSize)

check_type_size("char" SIZEOF_CHAR)
message(STATUS "SIZEOF_CHAR: ${SIZEOF_CHAR}")

check_type_size("short" SIZEOF_SHORT)
message(STATUS "SIZEOF_SHORT: ${SIZEOF_SHORT}")

check_type_size("int" SIZEOF_INT)
message(STATUS "SIZEOF_INT: ${SIZEOF_INT}")

check_type_size("long" SIZEOF_LONG)
message(STATUS "SIZEOF_LONG: ${SIZEOF_LONG}")

check_type_size("long long" SIZEOF_LONG_LONG)
message(STATUS "SIZEOF_LONG_LONG: ${SIZEOF_LONG_LONG}")


check_type_size("off_t" SIZEOF_OFF_T)
message(STATUS "SIZEOF_OFF_T: ${SIZEOF_OFF_T}")

check_type_size("size_t" SIZEOF_SIZE_T)
message(STATUS "SIZEOF_SIZE_T: ${SIZEOF_SIZE_T}")

add_executable(${PROJECT_NAME} "test.cpp")
set_target_properties(${PROJECT_NAME} PROPERTIES LINK_FLAGS "--bind -s SINGLE_FILE=1 -s MODULARIZE=1")

test.cpp

#include <emscripten/bind.h>
#include <iostream>

void test()
{
	std::cout << "char sizeof: " << sizeof(char) << std::endl;
	std::cout << "short sizeof: " << sizeof(short) << std::endl;
	std::cout << "int sizeof: " << sizeof(int) << std::endl;
	std::cout << "long sizeof: " << sizeof(long) << std::endl;
	std::cout << "long long sizeof: " << sizeof(long long) << std::endl;
	
	std::cout << "off_t sizeof: " << sizeof(off_t) << std::endl;
	std::cout << "size_t sizeof: " << sizeof(size_t) << std::endl;
}

EMSCRIPTEN_BINDINGS(libzip_wasm)
{
	emscripten::function("test", &test);
}

test.js

const wasm = require('./sizeof-test.js');
wasm().then((module) => module.test());

Invalid behaviour (Node.js v18.4.0)

cmake log

-- Looking for sys/types.h
-- Looking for sys/types.h - found
-- Looking for stdint.h
-- Looking for stdint.h - found
-- Looking for stddef.h
-- Looking for stddef.h - found
-- Check size of char
-- Check size of char - done
-- SIZEOF_CHAR: 7
-- Check size of short
-- Check size of short - done
-- SIZEOF_SHORT: 7
-- Check size of int
-- Check size of int - done
-- SIZEOF_INT: 7
-- Check size of long
-- Check size of long - done
-- SIZEOF_LONG: 7
-- Check size of long long
-- Check size of long long - done
-- SIZEOF_LONG_LONG: 7
-- Check size of off_t
-- Check size of off_t - done
-- SIZEOF_OFF_T: 7
-- Check size of size_t
-- Check size of size_t - done
-- SIZEOF_SIZE_T: 7
-- Configuring done
-- Generating done

Expected behaviour (Node.js v16.15.1)

cmake log

-- Looking for sys/types.h
-- Looking for sys/types.h - found
-- Looking for stdint.h
-- Looking for stdint.h - found
-- Looking for stddef.h
-- Looking for stddef.h - found
-- Check size of char
-- Check size of char - done
-- SIZEOF_CHAR: 1
-- Check size of short
-- Check size of short - done
-- SIZEOF_SHORT: 2
-- Check size of int
-- Check size of int - done
-- SIZEOF_INT: 4
-- Check size of long
-- Check size of long - done
-- SIZEOF_LONG: 4
-- Check size of long long
-- Check size of long long - done
-- SIZEOF_LONG_LONG: 8
-- Check size of off_t
-- Check size of off_t - done
-- SIZEOF_OFF_T: 8
-- Check size of size_t
-- Check size of size_t - done
-- SIZEOF_SIZE_T: 4
-- Configuring done
-- Generating done

output from test() call

char sizeof: 1
short sizeof: 2
int sizeof: 4
long sizeof: 4
long long sizeof: 8
off_t sizeof: 8
size_t sizeof: 4

How to reproduce

  • Download sources and static binaries of node 16 and 18 (save as node_16 and node_18)
  • Run with node 18
    • mkdir build_18 && cd build_18
    • cmake .. -DCMAKE_TOOLCHAIN_FILE=/usr/lib/emscripten/cmake/Modules/Platform/Emscripten.cmake -DCMAKE_CROSSCOMPILING_EMULATOR=../node_18
  • Run with node 16
    • mkdir build_16 && cd build_16
    • cmake .. -DCMAKE_TOOLCHAIN_FILE=/usr/lib/emscripten/cmake/Modules/Platform/Emscripten.cmake -DCMAKE_CROSSCOMPILING_EMULATOR=../node_16

Versions of used programs

Version of emscripten/emsdk:

emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.1.9-git (54675bac246e84ca024c182e477665d21fe2e2f7)
clang version 15.0.0 (/startdir/llvm-project faef447e72a5c63dfb12bb7b02d44c3c916d31cd)
Target: wasm32-unknown-emscripten
Thread model: posix
InstalledDir: /opt/emscripten-llvm/bin

Version of cmake: 3.23.2

Version of node: 18.4.0 and 16.15.1

@CoalZombik
Copy link
Author

I was looking for source of this problem and found that is related with issue #16913. Node 18 failed to load a wasm file and returned error code 7 that was treated as sizeof value.

Works well in main branch, closing this issue.

sbc100 added a commit that referenced this issue Dec 2, 2022
Update the CheckTypeSize files to v3.20.0 (mostly arbitrary version).

Local patch to CheckTypeSize is not much smaller.  Instead of executing
the program and using the return value (doesn't work for return values
greater than 255), we simply inject `-oformat=wasm` onto the link
command, and then we can use `strings` on the binary just like the
upstream check.  This is basically a single line patch against upstream
now.

Fixes: #18278 #18238 #17268 #18084 #17811
sbc100 added a commit that referenced this issue Dec 2, 2022
Update the CheckTypeSize files to v3.10.2 (mostly arbitrary version, but
this is what we use in CI).

Local patch to CheckTypeSize is not much smaller.  Instead of executing
the program and using the return value (doesn't work for return values
greater than 255), we simply inject `-oformat=wasm` onto the link
command, and then we can use `strings` on the binary just like the
upstream check.  This is basically a single line patch against upstream
now.

Fixes: #18278 #18238 #17268 #18084 #17811
sbc100 added a commit that referenced this issue Dec 5, 2022
Update the CheckTypeSize files to v3.10.2 (mostly arbitrary version, but
this is what we use in CI).

Local patch to CheckTypeSize is not much smaller.  Instead of executing
the program and using the return value (doesn't work for return values
greater than 255), we simply inject `-oformat=wasm` onto the link
command, and then we can use `strings` on the binary just like the
upstream check.  This is basically a single line patch against upstream
now.

Fixes: #18278 #18238 #17268 #18084 #17811
sbc100 added a commit that referenced this issue Dec 5, 2022
Update the CheckTypeSize files to v3.10.2 (mostly arbitrary version, but
this is what we use in CI).

Local patch to CheckTypeSize is not much smaller.  Instead of executing
the program and using the return value (doesn't work for return values
greater than 255), we simply inject `-oformat=wasm` onto the link
command, and then we can use `strings` on the binary just like the
upstream check.  This is basically a single line patch against upstream
now.

This change also avoids a second issue which is that node failures (non-zero
return codes) are indistinguishable from non-zero return codes from user
code.

Fixes: #18278 #18238 #17268 #18084 #17811
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

No branches or pull requests

1 participant