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.

Sign up for GitHub

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

Jump to bottom

test: use InitializeNodeWithArgs in cctest #32406

Closed
wants to merge 1 commit into from
Closed

test: use InitializeNodeWithArgs in cctest #32406

wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

Refs: d7f1107
Fixes: #30257

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Mar 21, 2020
@addaleax addaleax added c++ Issues and PRs that require attention from people who are familiar with C++. and removed test Issues and PRs related to the tests. labels Mar 21, 2020
@addaleax addaleax mentioned this pull request Mar 21, 2020
Closed
@gengjiawen
Copy link
Member

How many asan failure on your machine now ? I remember some crypto test failed due to performance regression for ASAN build.

@addaleax
Copy link
Member Author

@gengjiawen Like in #32405, I didn’t run/verify this with ASAN, only valgrind, so I don’t know what the current state is overall

@gengjiawen
Copy link
Member

Thanks for the info :) I will find a time to do a full run.

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 21, 2020
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/29967/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/29970/

@gengjiawen
Copy link
Member

@addaleax I applied the patch locally. ASAN still not works.

@addaleax
Copy link
Member Author

@gengjiawen If there’s a specific issue, could you share the error output?

@gengjiawen
Copy link
Member

gengjiawen commented Mar 23, 2020 •
edited
Loading

@gengjiawen If there’s a specific issue, could you share the error output?

After the ASAN build, test with python3 tools/test.py -J --mode=debug

==77788==ERROR: AddressSanitizer: new-delete-type-mismatch on 0x60400001b050 in thread T0:
  object passed to delete has wrong type:
  size of the allocated type:   48 bytes;
  size of the deallocated type: 1 bytes.
    #0 0x7f329cb93f45 in operator delete(void*, unsigned long) (/lib/x86_64-linux-gnu/libasan.so.5+0x110f45)
    #1 0x55e327ffeece in std::default_delete<v8::BackingStore>::operator()(v8::BackingStore*) const /usr/include/c++/9/bits/unique_ptr.h:81
    #2 0x55e3280030c6 in std::_Sp_counted_deleter<v8::BackingStore*, std::default_delete<v8::BackingStore>, std::allocator<void>, (__gnu_cxx::_Lock_policy)2>::_M_dispose() /usr/include/c++/9/bits/shared_ptr_base.h:471
    #3 0x55e3294b9d8c in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() /usr/include/c++/9/bits/shared_ptr_base.h:155
    #4 0x55e3294b9d8c in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() /usr/include/c++/9/bits/shared_ptr_base.h:148
    #5 0x55e3294b9d8c in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() /usr/include/c++/9/bits/shared_ptr_base.h:730
    #6 0x55e3294b9d8c in std::__shared_ptr<v8::internal::BackingStore, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() /usr/include/c++/9/bits/shared_ptr_base.h:1169
    #7 0x55e3294b9d8c in std::shared_ptr<v8::internal::BackingStore>::~shared_ptr() /usr/include/c++/9/bits/shared_ptr.h:103
    #8 0x55e3294b9d8c in std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >::~pair() /usr/include/c++/9/bits/stl_pair.h:208
    #9 0x55e3294b9d8c in void __gnu_cxx::new_allocator<std::__detail::_Hash_node<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >, true> >::destroy<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> > >(std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >*) /usr/include/c++/9/ext/new_allocator.h:153
    #10 0x55e3294b9d8c in void std::allocator_traits<std::allocator<std::__detail::_Hash_node<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >, true> > >::destroy<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> > >(std::allocator<std::__detail::_Hash_node<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >, true> >&, std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >*) /usr/include/c++/9/bits/alloc_traits.h:497
    #11 0x55e3294b9d8c in std::__detail::_Hashtable_alloc<std::allocator<std::__detail::_Hash_node<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >, true> > >::_M_deallocate_node(std::__detail::_Hash_node<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >, true>*) /usr/include/c++/9/bits/hashtable_policy.h:2102
    #12 0x55e3294ba37d in std::_Hashtable<v8::internal::JSArrayBuffer, std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >, std::allocator<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> > >, std::__detail::_Select1st, std::equal_to<v8::internal::JSArrayBuffer>, v8::internal::LocalArrayBufferTracker::Hasher, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true> >::_M_erase(unsigned long, std::__detail::_Hash_node_base*, std::__detail::_Hash_node<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >, true>*) /usr/include/c++/9/bits/hashtable.h:1886
    #13 0x55e3294ba37d in std::_Hashtable<v8::internal::JSArrayBuffer, std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >, std::allocator<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> > >, std::__detail::_Select1st, std::equal_to<v8::internal::JSArrayBuffer>, v8::internal::LocalArrayBufferTracker::Hasher, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true> >::erase(std::__detail::_Node_const_iterator<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >, false, true>) /usr/include/c++/9/bits/hashtable.h:1861
    #14 0x55e3294ba37d in std::_Hashtable<v8::internal::JSArrayBuffer, std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >, std::allocator<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> > >, std::__detail::_Select1st, std::equal_to<v8::internal::JSArrayBuffer>, v8::internal::LocalArrayBufferTracker::Hasher, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true> >::erase(std::__detail::_Node_iterator<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >, false, true>) /usr/include/c++/9/bits/hashtable.h:768
    #15 0x55e3294ba37d in std::unordered_map<v8::internal::JSArrayBuffer, std::shared_ptr<v8::internal::BackingStore>, v8::internal::LocalArrayBufferTracker::Hasher, std::equal_to<v8::internal::JSArrayBuffer>, std::allocator<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> > > >::erase(std::__detail::_Node_iterator<std::pair<v8::internal::JSArrayBuffer const, std::shared_ptr<v8::internal::BackingStore> >, false, true>) /usr/include/c++/9/bits/unordered_map.h:798
    #16 0x55e3294ba37d in Free<v8::internal::ArrayBufferTracker::FreeAll(v8::internal::Page*)::<lambda(v8::internal::JSArrayBuffer)> > ../../deps/v8/src/heap/array-buffer-tracker-inl.h:108
    #17 0x55e3294ba37d in v8::internal::ArrayBufferTracker::FreeAll(v8::internal::Page*) ../../deps/v8/src/heap/array-buffer-tracker.cc:106
    #18 0x55e3294bb014 in v8::internal::ArrayBufferTracker::TearDown(v8::internal::Heap*) ../../deps/v8/src/heap/array-buffer-tracker.cc:148
    #19 0x55e3296e7355 in v8::internal::Heap::TearDown() ../../deps/v8/src/heap/heap.cc:5395
    #20 0x55e3293e8783 in v8::internal::Isolate::Deinit() ../../deps/v8/src/execution/isolate.cc:2994
    #21 0x55e3293ef1f8 in v8::internal::Isolate::Delete(v8::internal::Isolate*) ../../deps/v8/src/execution/isolate.cc:2815
    #22 0x55e328221dee in node::NodeMainInstance::~NodeMainInstance() ../../src/node_main_instance.cc:106
    #23 0x55e32802a28e in node::Start(int, char**) ../../src/node.cc:1053
    #24 0x55e32c67e69f in main ../../src/node_main.cc:129
    #25 0x7f329c5371e2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x271e2)
    #26 0x55e327dd545d in _start (/root/node/out/Debug/node+0x13d745d)

0x60400001b050 is located 0 bytes inside of 48-byte region [0x60400001b050,0x60400001b080)
allocated by thread T0 here:
    #0 0x7f329cb92867 in operator new(unsigned long) (/lib/x86_64-linux-gnu/libasan.so.5+0x10f867)
    #1 0x55e329d2a772 in v8::internal::BackingStore::WrapAllocation(void*, unsigned long, void (*)(void*, unsigned long, void*), void*, v8::internal::SharedFlag) ../../deps/v8/src/objects/backing-store.cc:543
    #2 0x55e3287d4a71 in v8::ArrayBuffer::NewBackingStore(void*, unsigned long, void (*)(void*, unsigned long, void*), void*) ../../deps/v8/src/api/api.cc:7526
    #3 0x55e328066f7e in Initialize ../../src/node_buffer.cc:1209
    #4 0x55e328050ca3 in InitModule ../../src/node_binding.cc:564
    #5 0x55e3280512a7 in node::binding::GetInternalBinding(v8::FunctionCallbackInfo<v8::Value> const&) ../../src/node_binding.cc:585
    #6 0x55e328b560fd in v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) ../../deps/v8/src/api/api-arguments-inl.h:158
    #7 0x55e328b5c02f in HandleApiCallHelper<false> ../../deps/v8/src/builtins/builtins-api.cc:111
    #8 0x55e328b713a3 in Builtin_Impl_HandleApiCall ../../deps/v8/src/builtins/builtins-api.cc:141
    #9 0x55e328b750c6 in v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) ../../deps/v8/src/builtins/builtins-api.cc:129
    #10 0x55e32bdf015f in Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit (/root/node/out/Debug/node+0x53f215f)
    #11 0x55e32bbeda17 in Builtins_InterpreterEntryTrampoline (/root/node/out/Debug/node+0x51efa17)
    #12 0x55e32bbeda17 in Builtins_InterpreterEntryTrampoline (/root/node/out/Debug/node+0x51efa17)
    #13 0x55e32bbeda17 in Builtins_InterpreterEntryTrampoline (/root/node/out/Debug/node+0x51efa17)
    #14 0x55e32bbeda17 in Builtins_InterpreterEntryTrampoline (/root/node/out/Debug/node+0x51efa17)
    #15 0x55e32bbeda17 in Builtins_InterpreterEntryTrampoline (/root/node/out/Debug/node+0x51efa17)
    #16 0x55e32bbeda17 in Builtins_InterpreterEntryTrampoline (/root/node/out/Debug/node+0x51efa17)
    #17 0x55e32bbe49b9 in Builtins_JSEntryTrampoline (/root/node/out/Debug/node+0x51e69b9)
    #18 0x55e32bbe4797 in Builtins_JSEntry (/root/node/out/Debug/node+0x51e6797)
    #19 0x55e32931f159 in v8::internal::GeneratedCode<unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, long, unsigned long**>::Call(unsigned long, unsigned long, unsigned long, unsigned long, long, unsigned long**) ../../deps/v8/src/execution/simulator.h:142
    #20 0x55e32931f159 in Invoke ../../deps/v8/src/execution/execution.cc:372
    #21 0x55e329322e70 in v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*) ../../deps/v8/src/execution/execution.cc:467
    #22 0x55e32890d14c in v8::Function::Call(v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) ../../deps/v8/src/api/api.cc:4921
    #23 0x55e328022898 in node::ExecuteBootstrapper(node::Environment*, char const*, std::vector<v8::Local<v8::String>, std::allocator<v8::Local<v8::String> > >*, std::vector<v8::Local<v8::Value>, std::allocator<v8::Local<v8::Value> > >*) ../../src/node.cc:182
    #24 0x55e328024719 in node::Environment::BootstrapNode() ../../src/node.cc:305
    #25 0x55e3280251c8 in node::Environment::RunBootstrapping() ../../src/node.cc:353
    #26 0x55e327e9112d in node::CreateEnvironment(node::IsolateData*, v8::Local<v8::Context>, 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&, 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::EnvironmentFlags::Flags, node::ThreadId) ../../src/api/environment.cc:368
    #27 0x55e328222ffc in node::NodeMainInstance::CreateMainEnvironment(int*) ../../src/node_main_instance.cc:208
    #28 0x55e3282220ea in node::NodeMainInstance::Run() ../../src/node_main_instance.cc:116
    #29 0x55e32802a244 in node::Start(int, char**) ../../src/node.cc:1054
    #30 0x55e32c67e69f in main ../../src/node_main.cc:129

SUMMARY: AddressSanitizer: new-delete-type-mismatch (/lib/x86_64-linux-gnu/libasan.so.5+0x110f45) in operator delete(void*, unsigned long)
==77788==HINT: if you don't care about these errors you may set ASAN_OPTIONS=new_delete_type_mismatch=0

@addaleax
Copy link
Member Author

@gengjiawen I’d suggest opening a separate issue about that, as it’s unrelated to this PR. That being said, I don’t quite understand how that’s happening, and additionally it’s almost certainly a V8 bug and not a Node.js bug.

@gengjiawen
Copy link
Member

gengjiawen commented Mar 23, 2020 •
edited
Loading

@gengjiawen I’d suggest opening a separate issue about that, as it’s unrelated to this PR. That being said, I don’t quite understand how that’s happening, and additionally it’s almost certainly a V8 bug and not a Node.js bug.

Maybe uncovered case by ASAN in v8. I will open a issue for it (See #32435).

@gengjiawen gengjiawen mentioned this pull request Mar 23, 2020
Closed
@mmarchini
Copy link
Contributor

Landed in f2cc28a

@mmarchini mmarchini closed this Mar 23, 2020
mmarchini pushed a commit that referenced this pull request Mar 23, 2020
@addaleax @mmarchini
test: use InitializeNodeWithArgs in cctest
f2cc28a 
Refs: d7f1107
Fixes: #30257

PR-URL: #32406
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@addaleax addaleax deleted the cctest-InitializeNodeWithArgs branch March 23, 2020 18:02
@addaleax addaleax mentioned this pull request Mar 30, 2020
Closed
4 tasks
@BethGriggs BethGriggs mentioned this pull request Apr 14, 2020
Merged
@addaleax addaleax mentioned this pull request Sep 17, 2020
Closed
4 tasks
addaleax added a commit to addaleax/node that referenced this pull request Sep 23, 2020
@addaleax
test: use InitializeNodeWithArgs in cctest
72f42bf 
Refs: nodejs@d7f1107
Fixes: nodejs#30257

PR-URL: nodejs#32406
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax added a commit that referenced this pull request Sep 23, 2020
@addaleax
test: use InitializeNodeWithArgs in cctest
bdf6d41 
Refs: d7f1107
Fixes: #30257

Backport-PR-URL: #35241
PR-URL: #32406
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@codebytere codebytere mentioned this pull request Sep 28, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@targos targos targos approved these changes

@cjihrig cjihrig cjihrig approved these changes

@gengjiawen gengjiawen gengjiawen approved these changes

@mmarchini mmarchini mmarchini approved these changes

@devnexen devnexen devnexen approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Current issue with asan
8 participants
@addaleax @gengjiawen @nodejs-github-bot @mmarchini @jasnell @targos @cjihrig @devnexen