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

asan-test failure #35669

Closed
danbev opened this issue Oct 16, 2020 · 1 comment
Closed

asan-test failure #35669

danbev opened this issue Oct 16, 2020 · 1 comment
Labels
build Issues and PRs related to build files or the CI.

Comments

@danbev
Copy link
Contributor

danbev commented Oct 16, 2020 •
edited
Loading

  • Version:$ v15.0.0-pre
  • Platform: Linux localhost.localdomain 5.6.13-200.fc31.x86_64 deps: update openssl to 1.0.1j #1 SMP Thu May 14 23:26:14 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
  • Subsystem:

What steps will reproduce the bug?

Install asan:

$ sudo dnf install libasan libasan-static

Configure node to enable asan:

$ ./configure --enable-asan && make -j8

Run the following test to reproduce the issue:

$ out/Release/cctest --gtest_filter=EnvironmentTest.BufferWithFreeCallbackIsDetached

How often does it reproduce? Is there a required condition?

Consistently on my local machine

What is the expected behavior?

There should be no errors reported from address sanitizer.

What do you see instead?

Running main() from ../test/cctest/gtest/gtest_main.cc
Note: Google Test filter = EnvironmentTest.BufferWithFreeCallbackIsDetached
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from EnvironmentTest
[ RUN      ] EnvironmentTest.BufferWithFreeCallbackIsDetached
6
=================================================================
==2773765==ERROR: AddressSanitizer: new-delete-type-mismatch on 0x60400001af50 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 0x7f0a3a306175 in operator delete(void*, unsigned long) (/lib64/libasan.so.5+0x111175)
    #1 0x38328c2 in v8::internal::JSArrayBuffer::Detach(bool) (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x38328c2)
    #2 0x29bf88a in v8::ArrayBuffer::Detach() (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x29bf88a)
    #3 0x151adae in node::Buffer::(anonymous namespace)::CallbackInfo::CleanupHook(void*) (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x151adae)
    #4 0x144a5ff in node::Environment::RunCleanup() (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x144a5ff)
    #5 0x131a6f7 in node::FreeEnvironment(node::Environment*) (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x131a6f7)
    #6 0x11d7730 in EnvironmentTest_BufferWithFreeCallbackIsDetached_Test::TestBody() (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x11d7730)
    #7 0x1114cfd in testing::Test::Run() [clone .part.0] (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x1114cfd)
    #8 0x1115af0 in testing::TestInfo::Run() [clone .part.0] (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x1115af0)
    #9 0x11162c9 in testing::TestSuite::Run() [clone .part.0] (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x11162c9)
    #10 0x1137831 in testing::internal::UnitTestImpl::RunAllTests() (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x1137831)
    #11 0x11383bf in testing::UnitTest::Run() (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x11383bf)
    #12 0xa4d5b3 in main (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0xa4d5b3)
    #13 0x7f0a39ccf1a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)
    #14 0xa7f09d in _start (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0xa7f09d)

0x60400001af50 is located 0 bytes inside of 48-byte region [0x60400001af50,0x60400001af80)
allocated by thread T0 here:
    #0 0x7f0a3a304a97 in operator new(unsigned long) (/lib64/libasan.so.5+0x10fa97)
    #1 0x34cb1a8 in v8::internal::BackingStore::WrapAllocation(void*, unsigned long, void (*)(void*, unsigned long, void*), void*, v8::internal::SharedFlag) (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x34cb1a8)
    #2 0x29c18d5 in v8::ArrayBuffer::NewBackingStore(void*, unsigned long, void (*)(void*, unsigned long, void*), void*) (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x29c18d5)
    #3 0x1547193 in node::Buffer::New(node::Environment*, char*, unsigned long, void (*)(char*, void*), void*) (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x1547193)
    #4 0x1548e95 in node::Buffer::New(v8::Isolate*, char*, unsigned long, void (*)(char*, void*), void*) (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x1548e95)
    #5 0x11d7536 in EnvironmentTest_BufferWithFreeCallbackIsDetached_Test::TestBody() (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x11d7536)
    #6 0x1114cfd in testing::Test::Run() [clone .part.0] (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x1114cfd)
    #7 0x1115af0 in testing::TestInfo::Run() [clone .part.0] (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x1115af0)
    #8 0x11162c9 in testing::TestSuite::Run() [clone .part.0] (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x11162c9)
    #9 0x1137831 in testing::internal::UnitTestImpl::RunAllTests() (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x1137831)
    #10 0x11383bf in testing::UnitTest::Run() (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0x11383bf)
    #11 0xa4d5b3 in main (/home/danielbevenius/work/nodejs/node/out/Release/cctest+0xa4d5b3)
    #12 0x7f0a39ccf1a2 in __libc_start_main (/lib64/libc.so.6+0x271a2)

SUMMARY: AddressSanitizer: new-delete-type-mismatch (/lib64/libasan.so.5+0x111175) in operator delete(void*, unsigned long)
==2773765==HINT: if you don't care about these errors you may set ASAN_OPTIONS=new_delete_type_mismatch=0
==2773765==ABORTING

Additional information

A write up of this can be found here.

There is an open change set in V8 that I've tested which fixes this issue:
https://chromium-review.googlesource.com/c/v8/v8/+/2506712
@watilde watilde added the build Issues and PRs related to build files or the CI. label Oct 16, 2020
@danbev danbev mentioned this issue Oct 16, 2020
Closed
2 tasks
danbev added a commit to danbev/node that referenced this issue Oct 20, 2020
@danbev
test: ignore asan new_delete_type_mismatch
6e63ce4 
This commit adds ignore for new_delete_type_mismatch in EnvironmentTest
as this is currently generating an error when address sanitizer is
enabled and gcc is used as the compiler.

This might be a little risky as we run the risk of missing other errors
of this same type but I'm still looking into if this could be solved in
some other way in V8 and perhaps this could be a short term fix.

Fixes: nodejs#35669
@danbev danbev mentioned this issue Oct 20, 2020
Closed
2 tasks
@mmomtchev
Copy link
Contributor

This a duplicate of #32435
I am getting it even with node -e "let a = 1"

danbev added a commit to danbev/node that referenced this issue Nov 3, 2020
@danbev
deps: cherry-pick 9a49b22 from V8 upstream
d9e2331 
Original commit message:
  Fix alloc/dealloc size mismatch for v8::BackingStore

  On newer compilers the {operator delete} with explicit {size_t}
  argument would be instantiated for {v8::BackingStore} and used
  in the destructor of {std::unique_ptr<v8::BackingStore>}. The {size_t}
  argument is wrong though, since the pointer actually points
  to a {v8::internal::BackingStore} object.
  The solution is to explicitly provide a {operator delete}, preventing
  an implicitly generated {size_t} operator.

  Bug:v8:11081

  Change-Id: Iee0aa47a67f0e41000bea628942f7e3d70198b83
  Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2506712
  Commit-Queue: Ulan Degenbaev <[email protected]>
  Reviewed-by: Camillo Bruni <[email protected]>
  Cr-Commit-Position: refs/heads/master@{#70916}

Refs: v8/v8@9a49b22
Fixes: nodejs#35669
@danbev danbev mentioned this issue Nov 3, 2020
Closed
2 tasks
@danbev danbev closed this as completed in 1bba8c8 Nov 9, 2020
danielleadams pushed a commit that referenced this issue Nov 9, 2020
@danbev @danielleadams
deps: cherry-pick 9a49b22 from V8 upstream
7cb5c09 
Original commit message:
  Fix alloc/dealloc size mismatch for v8::BackingStore

  On newer compilers the {operator delete} with explicit {size_t}
  argument would be instantiated for {v8::BackingStore} and used
  in the destructor of {std::unique_ptr<v8::BackingStore>}. The {size_t}
  argument is wrong though, since the pointer actually points
  to a {v8::internal::BackingStore} object.
  The solution is to explicitly provide a {operator delete}, preventing
  an implicitly generated {size_t} operator.

  Bug:v8:11081

  Change-Id: Iee0aa47a67f0e41000bea628942f7e3d70198b83
  Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2506712
  Commit-Queue: Ulan Degenbaev <[email protected]>
  Reviewed-by: Camillo Bruni <[email protected]>
  Cr-Commit-Position: refs/heads/master@{#70916}

PR-URL: #35939
Fixes: #35669
Refs: v8/v8@9a49b22
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
targos pushed a commit to targos/node that referenced this issue Nov 15, 2020
@danbev @targos
deps: cherry-pick 9a49b22 from V8 upstream
59fe6e4 
Original commit message:
  Fix alloc/dealloc size mismatch for v8::BackingStore

  On newer compilers the {operator delete} with explicit {size_t}
  argument would be instantiated for {v8::BackingStore} and used
  in the destructor of {std::unique_ptr<v8::BackingStore>}. The {size_t}
  argument is wrong though, since the pointer actually points
  to a {v8::internal::BackingStore} object.
  The solution is to explicitly provide a {operator delete}, preventing
  an implicitly generated {size_t} operator.

  Bug:v8:11081

  Change-Id: Iee0aa47a67f0e41000bea628942f7e3d70198b83
  Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2506712
  Commit-Queue: Ulan Degenbaev <[email protected]>
  Reviewed-by: Camillo Bruni <[email protected]>
  Cr-Commit-Position: refs/heads/master@{#70916}

PR-URL: nodejs#35939
Fixes: nodejs#35669
Refs: v8/v8@9a49b22
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
targos pushed a commit to targos/node that referenced this issue Nov 19, 2020
@danbev @targos
deps: cherry-pick 9a49b22 from V8 upstream
1772fe1 
Original commit message:
  Fix alloc/dealloc size mismatch for v8::BackingStore

  On newer compilers the {operator delete} with explicit {size_t}
  argument would be instantiated for {v8::BackingStore} and used
  in the destructor of {std::unique_ptr<v8::BackingStore>}. The {size_t}
  argument is wrong though, since the pointer actually points
  to a {v8::internal::BackingStore} object.
  The solution is to explicitly provide a {operator delete}, preventing
  an implicitly generated {size_t} operator.

  Bug:v8:11081

  Change-Id: Iee0aa47a67f0e41000bea628942f7e3d70198b83
  Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2506712
  Commit-Queue: Ulan Degenbaev <[email protected]>
  Reviewed-by: Camillo Bruni <[email protected]>
  Cr-Commit-Position: refs/heads/master@{#70916}

PR-URL: nodejs#35939
Fixes: nodejs#35669
Refs: v8/v8@9a49b22
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
BethGriggs pushed a commit that referenced this issue Dec 9, 2020
@danbev @BethGriggs
deps: cherry-pick 9a49b22 from V8 upstream
78ab197 
Original commit message:
  Fix alloc/dealloc size mismatch for v8::BackingStore

  On newer compilers the {operator delete} with explicit {size_t}
  argument would be instantiated for {v8::BackingStore} and used
  in the destructor of {std::unique_ptr<v8::BackingStore>}. The {size_t}
  argument is wrong though, since the pointer actually points
  to a {v8::internal::BackingStore} object.
  The solution is to explicitly provide a {operator delete}, preventing
  an implicitly generated {size_t} operator.

  Bug:v8:11081

  Change-Id: Iee0aa47a67f0e41000bea628942f7e3d70198b83
  Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2506712
  Commit-Queue: Ulan Degenbaev <[email protected]>
  Reviewed-by: Camillo Bruni <[email protected]>
  Cr-Commit-Position: refs/heads/master@{#70916}

PR-URL: #35939
Fixes: #35669
Refs: v8/v8@9a49b22
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
BethGriggs pushed a commit that referenced this issue Dec 10, 2020
@danbev @BethGriggs
deps: cherry-pick 9a49b22 from V8 upstream
9db4cea 
Original commit message:
  Fix alloc/dealloc size mismatch for v8::BackingStore

  On newer compilers the {operator delete} with explicit {size_t}
  argument would be instantiated for {v8::BackingStore} and used
  in the destructor of {std::unique_ptr<v8::BackingStore>}. The {size_t}
  argument is wrong though, since the pointer actually points
  to a {v8::internal::BackingStore} object.
  The solution is to explicitly provide a {operator delete}, preventing
  an implicitly generated {size_t} operator.

  Bug:v8:11081

  Change-Id: Iee0aa47a67f0e41000bea628942f7e3d70198b83
  Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2506712
  Commit-Queue: Ulan Degenbaev <[email protected]>
  Reviewed-by: Camillo Bruni <[email protected]>
  Cr-Commit-Position: refs/heads/master@{#70916}

PR-URL: #35939
Fixes: #35669
Refs: v8/v8@9a49b22
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
BethGriggs pushed a commit that referenced this issue Dec 15, 2020
@danbev @BethGriggs
deps: cherry-pick 9a49b22 from V8 upstream
d2ee676 
Original commit message:
  Fix alloc/dealloc size mismatch for v8::BackingStore

  On newer compilers the {operator delete} with explicit {size_t}
  argument would be instantiated for {v8::BackingStore} and used
  in the destructor of {std::unique_ptr<v8::BackingStore>}. The {size_t}
  argument is wrong though, since the pointer actually points
  to a {v8::internal::BackingStore} object.
  The solution is to explicitly provide a {operator delete}, preventing
  an implicitly generated {size_t} operator.

  Bug:v8:11081

  Change-Id: Iee0aa47a67f0e41000bea628942f7e3d70198b83
  Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2506712
  Commit-Queue: Ulan Degenbaev <[email protected]>
  Reviewed-by: Camillo Bruni <[email protected]>
  Cr-Commit-Position: refs/heads/master@{#70916}

PR-URL: #35939
Fixes: #35669
Refs: v8/v8@9a49b22
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Milestone
No milestone
3 participants
@danbev @watilde @mmomtchev