Skip to content

workflow: re-enable ubsan #53142

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

RafaelGSS
Copy link
Member

Attempt to solve #52753

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/actions

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label May 24, 2024
@RafaelGSS
Copy link
Member Author

RafaelGSS commented May 24, 2024

We are getting:

../deps/v8/src/base/small-vector.h:25:3: error: static_assert failed due to requirement '::v8::base::is_trivially_copyable<std::pair<const v8::internal::compiler::turboshaft::PhiOp *, const v8::internal::compiler::turboshaft::OpIndex>>::value' "T should be trivially copyable"

More specifically because of:

../deps/v8/src/compiler/turboshaft/loop-unrolling-reducer.h:431:65: note: in instantiation of template class 'v8::base::SmallVector<std::pair<const v8::internal::compiler::turboshaft::PhiOp *, const v8::internal::compiler::turboshaft::OpIndex>, 16>' requested here

But, shouldn't this static assert happen regardless if ubsan is enabled? Why this doesn't fail on a regular test-linux for instance? In theory is_trivially_copyable runs on build time in any set of configs.

cc: @nodejs/v8

@targos
Copy link
Member

targos commented May 24, 2024

This might be a clang bug. Can you try to change to Ubuntu 24.04 ? It has a much more recent version preinstalled

@RafaelGSS RafaelGSS force-pushed the re-enable-ubsan branch 2 times, most recently from 1ddf96c to e81b5f0 Compare May 29, 2024 18:54
@targos
Copy link
Member

targos commented May 30, 2024

test-ubsan was skipped 🤔

@gengjiawen
Copy link
Member

delete if should make ubsan work.

@RafaelGSS
Copy link
Member Author

delete if should make ubsan work.

It shouldn't make a difference once this PR is not a Draft. But, I've commented on the line anyway.

@targos
Copy link
Member

targos commented Jun 9, 2024

I see a lot of test failures due to the same OpenSSL function:

SUMMARY: UndefinedBehaviorSanitizer: function-type-mismatch ../deps/openssl/openssl/crypto/lhash/lhash.c:297:12 
../deps/openssl/openssl/crypto/lhash/lhash.c:311:13: runtime error: call to function err_string_data_cmp through pointer to incorrect function type 'int (*)(const void *, const void *)'
/home/runner/work/node/node/out/../deps/openssl/openssl/crypto/err/err.c:183: note: err_string_data_cmp defined here
    #0 0x563b7631f9d9 in getrn /home/runner/work/node/node/out/../deps/openssl/openssl/crypto/lhash/lhash.c:311:13
    #1 0x563b7631f219 in OPENSSL_LH_insert /home/runn
2024-06-08T18:12:31.4664031Z === release node-options ===
2024-06-08T18:12:31.4664691Z Path: node-api/test_uv_threadpool_size/node-options
2024-06-08T18:12:31.4916567Z ##[error]--- stderr ---
../deps/openssl/openssl/crypto/lhash/lhash.c:297:12: runtime error: call to function property_hash through pointer to incorrect function type 'unsigned long (*)(const void *)'
/home/runner/work/node/node/out/../deps/openssl/openssl/crypto/property/property_string.c:46: note: property_hash defined here
    #0 0x55a81c31fbac in getrn /home/runner/work/node/node/out/../deps/openssl/openssl/crypto/lhash/lhash.c:297:12
    #1 0x55a81c320ba7 in OPENSSL_LH_retrieve /home/runner/work/node/node/out/../deps/openssl/openssl/crypto/lhash/lhash.c:171:10
    #2 0x55a81c3db31c in lh_PROPERTY_STRING_retrieve /home/runner/work/node/node/out/../deps/openssl/openssl/crypto/property/property_string.c:34:1
    #3 0x55a81c3db31c in ossl_property_string /home/runner/work/node/node/out/../deps/openssl/openssl/crypto/property/property_string.c:149:10
    #4 0x55a81c3d883f in ossl_property_parse_init /home/runner/work/node/node/out/../deps/openssl/openssl/crypto/property/property_parse.c:587:13
    #5 0x55a81c323443 in context_init /home/runner/work/node/node/out/../deps/openssl/openssl/crypto/context.c:102:10
    #6 0x55a81c323dc8 in default_context_do_init /home/runner/work/node/node/out/../deps/openssl/openssl/crypto/context.c:155:12
    #7 0x55a81c323dc8 in default_context_do_init_ossl_ /home/runner/work/node/node/out/../deps/openssl/openssl/crypto/context.c:152:1
    #8 0x7f8121ea1ec2  (/lib/x86_64-linux-gnu/libc.so.6+0xa1ec2) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
    #9 0x55a81c35eb2a in CRYPTO_THREAD_run_once /home/runner/work/node/node/out/../deps/openssl/openssl/crypto/threads_pthread.c:154:9
    #10 0x55a81c323d7b in OSSL_LIB_CTX_get0_global_default /home/runner/work/node/node/out/../deps/openssl/openssl/crypto/context.c:257:10
    #11 0x55a81c0a6b32 in ossl_config_int /home/runner/work/node/node/out/../deps/openssl/openssl/crypto/conf/conf_sap.c:68:37
    #12 0x55a81c32e7c4 in ossl_init_config_settings /home/runner/work/node/node/out/../deps/openssl/openssl/crypto/init.c:256:15
    #13 0x55a81c32e7c4 in ossl_init_config_settings_ossl_ /home/runner/work/node/node/out/../deps/openssl/openssl/crypto/init.c:254:1
    #14 0x7f8121ea1ec2  (/lib/x86_64-linux-gnu/libc.so.6+0xa1ec2) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
    #15 0x55a81c35eb2a in CRYPTO_THREAD_run_once /home/runner/work/node/node/out/../deps/openssl/openssl/crypto/threads_pthread.c:154:9
    #16 0x55a81c32e54e in OPENSSL_init_crypto /home/runner/work/node/node/out/../deps/openssl/openssl/crypto/init.c:593:23
    #17 0x55a8197460be in node::InitializeOncePerProcessInternal(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>> const&, node::ProcessInitializationFlags::Flags) /home/runner/work/node/node/out/../src/node.cc:1096:5
    #18 0x55a819748787 in node::StartInternal(int, char**) /home/runner/work/node/node/out/../src/node.cc:1408:7
    #19 0x55a819748787 in node::Start(int, char**) /home/runner/work/node/node/out/../src/node.cc:1468:27
    #20 0x7f8121e2a1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
    #21 0x7f8121e2a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
    #22 0x55a8195b15d4 in _start (/home/runner/work/node/node/out/Release/node+0x37b15d4) (BuildId: 8ec883732b4448543eff7356a897f332a5972617)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants