New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 838043 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Wasm memory accounting problem

Project Member Reported by clemensh@chromium.org, Apr 30 2018

Issue description

Running the wasm_compile_fuzzer locally on x64 *sometimes* hits this error:


#
# Fatal error in ../../v8/src/wasm/wasm-memory.cc, line 124
# Debug check failed: allocated_address_space_ + allocation_length <= reserved_address_space_ (96796672 vs. 93650944).
#
#
#
AddressSanitizer:DEADLYSIGNAL
=================================================================
==149497==ERROR: AddressSanitizer: ILL on unknown address 0x7fbc0721ef08 (pc 0x7fbc0721ef08 bp 0x7ffd10967f70 sp 0x7ffd10967f70 T0)
SCARINESS: 10 (signal)
    #0 0x7fbc0721ef07 in v8::base::OS::Abort() v8/src/base/platform/platform-posix.cc:381:5
    #1 0x7fbc072100a1 in V8_Fatal(char const*, int, char const*, ...) v8/src/base/logging.cc:170:3
    #2 0x7fbc0720f7ce in v8::base::(anonymous namespace)::DefaultDcheckHandler(char const*, int, char const*) v8/src/base/logging.cc:56:3
    #3 0x7fbc0666005d in v8::internal::wasm::WasmMemoryTracker::RegisterAllocation(void*, unsigned long, void*, unsigned long) v8/src/wasm/wasm-memory.cc:123:3
    #4 0x7fbc06660ec2 in v8::internal::wasm::(anonymous namespace)::TryAllocateBackingStore(v8::internal::wasm::WasmMemoryTracker*, v8::internal::Heap*, unsigned long, bool, void**, unsigned long*) v8/src/wasm/wasm-memory.cc:72:19
    #5 0x7fbc06661fab in v8::internal::wasm::NewArrayBuffer(v8::internal::Isolate*, unsigned long, bool, v8::internal::SharedFlag) v8/src/wasm/wasm-memory.cc:255:13
    #6 0x7fbc06690094 in GrowMemoryBuffer v8/src/wasm/wasm-objects.cc:464:10
    #7 0x7fbc06690094 in v8::internal::WasmMemoryObject::Grow(v8::internal::Isolate*, v8::internal::Handle<v8::internal::WasmMemoryObject>, unsigned int) v8/src/wasm/wasm-objects.cc:571
    #8 0x7fbc06342468 in v8::internal::__RT_impl_Runtime_WasmGrowMemory(v8::internal::Arguments, v8::internal::Isolate*) v8/src/runtime/runtime-wasm.cc:85:48
    #9 0x7fbc06340ec9 in v8::internal::Runtime_WasmGrowMemory(int, v8::internal::Object**, v8::internal::Isolate*) v8/src/runtime/runtime-wasm.cc:71:1
    #10 0x7ee8d0d84223  (<unknown module>)
[...]

It is hard to reproduce, since just running the crashing input does not produce the error. It's probably the interplay with all the previously allocated wasm memories.

This might also happen in the wild without us noticing, since it's only DCHECKs. It might be worth temporarily turning them into CHECKs (should not be performance critical), and see if we get crash reports.
 

Comment 1 by eholk@chromium.org, Apr 30 2018

I bet this is just a standard data race, and probably benign. Here's what I think is happening. Let's say T1 is the main V8 thread and T2 is the thread running a concurrent GC.

T2: collect Wasm Memory, running ReleaseAllocation. Let old_reserved = reserved_address_space_ - num_bytes.
T1: Allocate a new Wasm memory. Increment reserved_address_space_ and allocated_address_space_.
T2: DCHECK(old_reserved - num_bytes >= allocated_address_space_)

I think this means the DCHECK is just invalid. There are probably some other invalid DCHECKs then too, such as the one on line 134.

It might actually be better just to take the memory tracker mutex in all these functions though.
Project Member

Comment 2 by bugdroid1@chromium.org, May 2 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/07ef612fbfbbabaf29f4aaf267f4a65730cb06a4

commit 07ef612fbfbbabaf29f4aaf267f4a65730cb06a4
Author: Eric Holk <eholk@chromium.org>
Date: Wed May 02 16:05:28 2018

[wasm] Remove racy DCHECKs

These DCHECKs involve reading and comparing two variables that may be modified
on a separate thread. Thus, there is no way to ensure these comparisons happen
atomically. This leads to runtime failures that are otherwise benign.

The other option would be to take the memory tracker mutex, but this seems
unnecessary given that two atomic counters is sufficient and these checks are
only used during debug builds.

Bug:  chromium:838043 
Change-Id: I1b87698c46c550bd2d58bfef956b5a07cb2ec52c
Reviewed-on: https://chromium-review.googlesource.com/1038886
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Eric Holk <eholk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52920}
[modify] https://crrev.com/07ef612fbfbbabaf29f4aaf267f4a65730cb06a4/src/wasm/wasm-memory.cc

Comment 3 by glider@chromium.org, Jun 13 2018

Is this fixed?
Status: Fixed (was: Assigned)
Looks like it's fixed.

Sign in to add a comment