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 failures #268

Closed
targos opened this issue Sep 27, 2023 · 13 comments
Closed

ASAN test failures #268

targos opened this issue Sep 27, 2023 · 13 comments

Comments

@targos
Copy link
Member

targos commented Sep 27, 2023 

=== release test-blob-buffer-too-large ===
Path: parallel/test-blob-buffer-too-large
Error: --- stderr ---
=================================================================
==93809==ERROR: AddressSanitizer: requested allocation size 0x1fffffffffffff (0x20000000001000 after adjustments for alignment, red zones etc.) exceeds maximum supported size of 0x10000000000 (thread T0)
    #0 0xe74012 in calloc (/home/runner/work/node-v8/node-v8/out/Release/node+0xe74012)
    #1 0xeb6374 in node::NodeArrayBufferAllocator::Allocate(unsigned long) (/home/runner/work/node-v8/node-v8/out/Release/node+0xeb6374)
    #2 0x1e5e15e in v8::internal::Heap::AllocateExternalBackingStore(std::function<void* (unsigned long)> const&, unsigned long) (/home/runner/work/node-v8/node-v8/out/Release/node+0x1e5e15e)
    #3 0x21fde55 in v8::internal::BackingStore::Allocate(v8::internal::Isolate*, unsigned long, v8::internal::SharedFlag, v8::internal::InitializedFlag) (/home/runner/work/node-v8/node-v8/out/Release/node+0x21fde55)
    #4 0x19bb809 in v8::internal::(anonymous namespace)::ConstructBuffer(v8::internal::Isolate*, v8::internal::Handle<v8::internal::JSFunction>, v8::internal::Handle<v8::internal::JSReceiver>, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, v8::internal::InitializedFlag) (/home/runner/work/node-v8/node-v8/out/Release/node+0x19bb809)
    #5 0x19b9d96 in v8::internal::Builtin_ArrayBufferConstructor(int, unsigned long*, v8::internal::Isolate*) (/home/runner/work/node-v8/node-v8/out/Release/node+0x19b9d96)
    #6 0x7fbed2861f35 in v8::internal::wasm::WasmEngine::SyncCompile(v8::internal::Isolate*, v8::internal::wasm::WasmFeatures, v8::internal::wasm::ErrorThrower*, v8::internal::wasm::ModuleWireBytes) (/home/runner/work/node-v8/node-v8/out/Release/node+0x3357f35)
    #7 0x7fbed27c9ece in v8::internal::wasm::TurboshaftGraphBuildingInterface::StringViewWtf8Encode(v8::internal::wasm::WasmFullDecoder<v8::internal::wasm::Decoder::FullValidationTag, v8::internal::wasm::TurboshaftGraphBuildingInterface, (v8::internal::wasm::DecodingMode)0>*, v8::internal::wasm::MemoryIndexImmediate const&, unibrow::Utf8Variant, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value const&, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value const&, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value const&, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value const&, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value*, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value*) (/home/runner/work/node-v8/node-v8/out/Release/node+0x32bfece)
    #8 0x7fbed2911850 in v8::internal::wasm::(anonymous namespace)::ExternalReferenceList::ExternalReferenceList() (/home/runner/work/node-v8/node-v8/out/Release/node+0x3407850)
    #9 0x7fbed284fefa in v8::internal::wasm::ModuleDisassembler::PrintMemory(v8::internal::wasm::WasmMemory const&) (/home/runner/work/node-v8/node-v8/out/Release/node+0x3345efa)
    #10 0x7fbed27cd97b in v8::internal::wasm::TurboshaftGraphBuildingInterface::ArrayCopy(v8::internal::wasm::WasmFullDecoder<v8::internal::wasm::Decoder::FullValidationTag, v8::internal::wasm::TurboshaftGraphBuildingInterface, (v8::internal::wasm::DecodingMode)0>*, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value const&, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value const&, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value const&, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value const&, v8::internal::wasm::ArrayIndexImmediate const&, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value const&) (/home/runner/work/node-v8/node-v8/out/Release/node+0x32c397b)
    #11 0x7fbed294dd22 in v8::internal::Assembler::movsxwq(v8::internal::Register, v8::internal::Register) (/home/runner/work/node-v8/node-v8/out/Release/node+0x3443d22)
    #12 0x7fbed27ccf1d in v8::internal::wasm::TurboshaftGraphBuildingInterface::ArrayCopy(v8::internal::wasm::WasmFullDecoder<v8::internal::wasm::Decoder::FullValidationTag, v8::internal::wasm::TurboshaftGraphBuildingInterface, (v8::internal::wasm::DecodingMode)0>*, v8::internal::wasm::TurboshaftGraphBuildingInterf
=== release test-buffer-slow ===
Path: parallel/test-buffer-slow
Error: --- stderr ---
=================================================================
==94660==ERROR: AddressSanitizer: requested allocation size 0x1fffffffffffff (0x20000000001000 after adjustments for alignment, red zones etc.) exceeds maximum supported size of 0x10000000000 (thread T0)
    #0 0xe73e9d in malloc (/home/runner/work/node-v8/node-v8/out/Release/node+0xe73e9d)
    #1 0xeb63dd in node::NodeArrayBufferAllocator::Allocate(unsigned long) (/home/runner/work/node-v8/node-v8/out/Release/node+0xeb63dd)
    #2 0x1e5e15e in v8::internal::Heap::AllocateExternalBackingStore(std::function<void* (unsigned long)> const&, unsigned long) (/home/runner/work/node-v8/node-v8/out/Release/node+0x1e5e15e)
    #3 0x21fde55 in v8::internal::BackingStore::Allocate(v8::internal::Isolate*, unsigned long, v8::internal::SharedFlag, v8::internal::InitializedFlag) (/home/runner/work/node-v8/node-v8/out/Release/node+0x21fde55)
    #4 0x19bb809 in v8::internal::(anonymous namespace)::ConstructBuffer(v8::internal::Isolate*, v8::internal::Handle<v8::internal::JSFunction>, v8::internal::Handle<v8::internal::JSReceiver>, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, v8::internal::InitializedFlag) (/home/runner/work/node-v8/node-v8/out/Release/node+0x19bb809)
    #5 0x19b9d96 in v8::internal::Builtin_ArrayBufferConstructor(int, unsigned long*, v8::internal::Isolate*) (/home/runner/work/node-v8/node-v8/out/Release/node+0x19b9d96)
    #6 0x7fecf5061f35 in v8::internal::wasm::WasmEngine::SyncCompile(v8::internal::Isolate*, v8::internal::wasm::WasmFeatures, v8::internal::wasm::ErrorThrower*, v8::internal::wasm::ModuleWireBytes) (/home/runner/work/node-v8/node-v8/out/Release/node+0x3357f35)
    #7 0x7fecf4fc9ece in v8::internal::wasm::TurboshaftGraphBuildingInterface::StringViewWtf8Encode(v8::internal::wasm::WasmFullDecoder<v8::internal::wasm::Decoder::FullValidationTag, v8::internal::wasm::TurboshaftGraphBuildingInterface, (v8::internal::wasm::DecodingMode)0>*, v8::internal::wasm::MemoryIndexImmediate const&, unibrow::Utf8Variant, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value const&, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value const&, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value const&, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value const&, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value*, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value*) (/home/runner/work/node-v8/node-v8/out/Release/node+0x32bfece)
    #8 0x7fecf5111850 in v8::internal::wasm::(anonymous namespace)::ExternalReferenceList::ExternalReferenceList() (/home/runner/work/node-v8/node-v8/out/Release/node+0x3407850)
    #9 0x7fecf504fefa in v8::internal::wasm::ModuleDisassembler::PrintMemory(v8::internal::wasm::WasmMemory const&) (/home/runner/work/node-v8/node-v8/out/Release/node+0x3345efa)
    #10 0x7fecf4fcd97b in v8::internal::wasm::TurboshaftGraphBuildingInterface::ArrayCopy(v8::internal::wasm::WasmFullDecoder<v8::internal::wasm::Decoder::FullValidationTag, v8::internal::wasm::TurboshaftGraphBuildingInterface, (v8::internal::wasm::DecodingMode)0>*, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value const&, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value const&, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value const&, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value const&, v8::internal::wasm::ArrayIndexImmediate const&, v8::internal::wasm::TurboshaftGraphBuildingInterface::Value const&) (/home/runner/work/node-v8/node-v8/out/Release/node+0x32c397b)
    #11 0x7fecf514dd22 in v8::internal::Assembler::movsxwq(v8::internal::Register, v8::internal::Register) (/home/runner/work/node-v8/node-v8/out/Release/node+0x3443d22)
    #12 0x7fecf4fccf1d in v8::internal::wasm::TurboshaftGraphBuildingInterface::ArrayCopy(v8::internal::wasm::WasmFullDecoder<v8::internal::wasm::Decoder::FullValidationTag, v8::internal::wasm::TurboshaftGraphBuildingInterface, (v8::internal::wasm::DecodingMode)0>*, v8::internal::wasm::TurboshaftGraphBuildingInterf

===
=== 2 tests failed
===

Failed tests:
out/Release/node --no-warnings --test-reporter=spec --test-reporter-destination=stdout --test-reporter=./tools/github_reporter/index.js --test-reporter-destination=stdout /home/runner/work/node-v8/node-v8/test/parallel/test-blob-buffer-too-large.js
out/Release/node --test-reporter=spec --test-reporter-destination=stdout --test-reporter=./tools/github_reporter/index.js --test-reporter-destination=stdout /home/runner/work/node-v8/node-v8/test/parallel/test-buffer-slow.js
@targos
Copy link
Member Author

targos commented Sep 27, 2023

The problem happens when we try to create a Buffer with length buffer.kMaxLength, which is now equal to 2 ** 53 - 1.

@targos
Copy link
Member Author

targos commented Sep 27, 2023

0x10000000000 is 2 ** 40 (1 TB if I'm not mistaken). Is this a fixed limit of ASAN? What can we do about it?

@targos
Copy link
Member Author

targos commented Sep 27, 2023

@nodejs/cpp-reviewers

@bnoordhuis
Copy link
Member

I'd disable that second test but the first one only allocates 2^32-1 bytes though.

@targos
Copy link
Member Author

targos commented Sep 28, 2023

The first test is updated to allocate more in nodejs/node#49876 (canary includes that PR)

@bnoordhuis
Copy link
Member

Ah, okay. 2**53-1 is larger than reasonable (most x86_64 cpus only have 48 address lines after all) so I'd tweak the tests to not allocate so much. Maybe we should also consider changing Buffer.kMaxLength to something more reasonable.

@targos
Copy link
Member Author

targos commented Sep 28, 2023

Is it reasonable to use the same limit as ASAN?

@bnoordhuis
Copy link
Member

I'd say so. 1 TB is still pretty big.

@targos
Copy link
Member Author

targos commented Sep 28, 2023

Can you help me with C++? I'd like to change the kMaxLength constant to something equivalent to Math.min(v8::Uint8Array::kMaxLength, 0x10000000000).

node-v8/src/node_buffer.h

Line 32 in fe8cd53

static const size_t kMaxLength = v8::Uint8Array::kMaxLength;

@bnoordhuis
Copy link
Member

I think you should be able to write it like this:

static constexpr size_t kMaxLength =
    v8::Uint8Array::kMaxLength < 0x10000000000ull
        ? v8::Uint8Array::kMaxLength
        : 0x10000000000ull;

@targos targos mentioned this issue Sep 28, 2023
Closed
targos added a commit to targos/node that referenced this issue Sep 28, 2023
@targos
src: limit Buffer::kMaxLength to 1TB
12f841d 
This change has no real effect for now, as the V8 maximum typed array
length is still 2**32. When V8 is updated to version 11.9 or later, the
limit will be 2**53-1 on 64-bit architectures, much larger than any
reasonable amount of RAM. This caps the limit at 1TB, which is already
very large and corresponds to the maximum memory that AddressSanitizer
allows to allocate.

Refs: nodejs#49876
Refs: nodejs/node-v8#268
@targos
Copy link
Member Author

targos commented Sep 28, 2023

Thanks. I opened nodejs/node#49930 with the change.

targos added a commit to nodejs/node that referenced this issue Sep 30, 2023
@targos
src: limit Buffer::kMaxLength to 1TB
22793fd 
This change has no real effect for now, as the V8 maximum typed array
length is still 2**32. When V8 is updated to version 11.9 or later, the
limit will be 2**53-1 on 64-bit architectures, much larger than any
reasonable amount of RAM. This caps the limit at 1TB, which is already
very large and corresponds to the maximum memory that AddressSanitizer
allows to allocate.

Refs: #49876
Refs: nodejs/node-v8#268
nodejs-github-bot pushed a commit that referenced this issue Oct 1, 2023
@targos @nodejs-github-bot
src: limit Buffer::kMaxLength to 1TB
787545e 
This change has no real effect for now, as the V8 maximum typed array
length is still 2**32. When V8 is updated to version 11.9 or later, the
limit will be 2**53-1 on 64-bit architectures, much larger than any
reasonable amount of RAM. This caps the limit at 1TB, which is already
very large and corresponds to the maximum memory that AddressSanitizer
allows to allocate.

Refs: nodejs/node#49876
Refs: #268
@targos
Copy link
Member Author

targos commented Oct 2, 2023

Unfortunately nodejs/node#49930 doesn't really fix it:

ERROR: AddressSanitizer: requested allocation size 0x10000000000 (0x10000001000 after adjustments for alignment, red zones etc.) exceeds maximum supported size of 0x10000000000 (thread T0)

And introduces more issues, because the artificial limit doesn't apply to V8 typed arrays.
For example, on macOS, the system lets us create a huge typed array (larger than the available RAM), and then it crashes later inside V8 when it tries to do a memmove.

See https://github.com/nodejs/node-v8/actions/runs/6369126692/job/17289078649, https://github.com/nodejs/node-v8/actions/runs/6369126697/job/17289078666, https://github.com/nodejs/node-v8/actions/runs/6369126701/job/17289078755

nodejs-github-bot pushed a commit that referenced this issue Oct 2, 2023
@targos @nodejs-github-bot
src: limit Buffer::kMaxLength to 1TB
1ee2b06 
This change has no real effect for now, as the V8 maximum typed array
length is still 2**32. When V8 is updated to version 11.9 or later, the
limit will be 2**53-1 on 64-bit architectures, much larger than any
reasonable amount of RAM. This caps the limit at 1TB, which is already
very large and corresponds to the maximum memory that AddressSanitizer
allows to allocate.

Refs: nodejs/node#49876
Refs: #268
nodejs-github-bot pushed a commit that referenced this issue Oct 3, 2023
@targos @nodejs-github-bot
src: limit Buffer::kMaxLength to 1TB
91ae59a 
This change has no real effect for now, as the V8 maximum typed array
length is still 2**32. When V8 is updated to version 11.9 or later, the
limit will be 2**53-1 on 64-bit architectures, much larger than any
reasonable amount of RAM. This caps the limit at 1TB, which is already
very large and corresponds to the maximum memory that AddressSanitizer
allows to allocate.

Refs: nodejs/node#49876
Refs: #268
nodejs-github-bot pushed a commit that referenced this issue Oct 4, 2023
@targos @nodejs-github-bot
src: limit Buffer::kMaxLength to 1TB
35c9e0a 
This change has no real effect for now, as the V8 maximum typed array
length is still 2**32. When V8 is updated to version 11.9 or later, the
limit will be 2**53-1 on 64-bit architectures, much larger than any
reasonable amount of RAM. This caps the limit at 1TB, which is already
very large and corresponds to the maximum memory that AddressSanitizer
allows to allocate.

Refs: nodejs/node#49876
Refs: #268
nodejs-github-bot pushed a commit that referenced this issue Oct 5, 2023
@targos @nodejs-github-bot
src: limit Buffer::kMaxLength to 1TB
78989d0 
This change has no real effect for now, as the V8 maximum typed array
length is still 2**32. When V8 is updated to version 11.9 or later, the
limit will be 2**53-1 on 64-bit architectures, much larger than any
reasonable amount of RAM. This caps the limit at 1TB, which is already
very large and corresponds to the maximum memory that AddressSanitizer
allows to allocate.

Refs: nodejs/node#49876
Refs: #268
nodejs-github-bot pushed a commit that referenced this issue Oct 6, 2023
@targos @nodejs-github-bot
src: limit Buffer::kMaxLength to 1TB
556a860 
This change has no real effect for now, as the V8 maximum typed array
length is still 2**32. When V8 is updated to version 11.9 or later, the
limit will be 2**53-1 on 64-bit architectures, much larger than any
reasonable amount of RAM. This caps the limit at 1TB, which is already
very large and corresponds to the maximum memory that AddressSanitizer
allows to allocate.

Refs: nodejs/node#49876
Refs: #268
nodejs-github-bot pushed a commit that referenced this issue Oct 7, 2023
@targos @nodejs-github-bot
src: limit Buffer::kMaxLength to 1TB
ea171d5 
This change has no real effect for now, as the V8 maximum typed array
length is still 2**32. When V8 is updated to version 11.9 or later, the
limit will be 2**53-1 on 64-bit architectures, much larger than any
reasonable amount of RAM. This caps the limit at 1TB, which is already
very large and corresponds to the maximum memory that AddressSanitizer
allows to allocate.

Refs: nodejs/node#49876
Refs: #268
nodejs-github-bot pushed a commit that referenced this issue Oct 8, 2023
@targos @nodejs-github-bot
src: limit Buffer::kMaxLength to 1TB
7ab7a2b 
This change has no real effect for now, as the V8 maximum typed array
length is still 2**32. When V8 is updated to version 11.9 or later, the
limit will be 2**53-1 on 64-bit architectures, much larger than any
reasonable amount of RAM. This caps the limit at 1TB, which is already
very large and corresponds to the maximum memory that AddressSanitizer
allows to allocate.

Refs: nodejs/node#49876
Refs: #268
nodejs-github-bot pushed a commit that referenced this issue Oct 9, 2023
@targos @nodejs-github-bot
src: limit Buffer::kMaxLength to 1TB
4576505 
This change has no real effect for now, as the V8 maximum typed array
length is still 2**32. When V8 is updated to version 11.9 or later, the
limit will be 2**53-1 on 64-bit architectures, much larger than any
reasonable amount of RAM. This caps the limit at 1TB, which is already
very large and corresponds to the maximum memory that AddressSanitizer
allows to allocate.

Refs: nodejs/node#49876
Refs: #268
@targos targos mentioned this issue Oct 10, 2023
Closed
nodejs-github-bot pushed a commit that referenced this issue Oct 10, 2023
@targos @nodejs-github-bot
src: limit Buffer::kMaxLength to 1TB
ab28d63 
This change has no real effect for now, as the V8 maximum typed array
length is still 2**32. When V8 is updated to version 11.9 or later, the
limit will be 2**53-1 on 64-bit architectures, much larger than any
reasonable amount of RAM. This caps the limit at 1TB, which is already
very large and corresponds to the maximum memory that AddressSanitizer
allows to allocate.

Refs: nodejs/node#49876
Refs: #268
@targos
Copy link
Member Author

targos commented Jan 4, 2024

Resolved in nodejs/node#50115

@targos targos closed this as completed Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bnoordhuis @targos