New issue
Advanced search Search tips

Issue 754080 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

HeapAllocator.h isn't compatible with vsan

Project Member Reported by tsepez@chromium.org, Aug 10 2017

Issue description

There isn't any functional issue in the code itself, but the cast at
or about line 332:

    T* buffer = reinterpret_cast<T*>(pointer);

can't happen until checking length is not zero.  Also, given the
scary comment:

  // As commented above, HeapVectorBacking calls finalizers for unused slots                                                                                                            
  // (which are already zeroed out).                                                                                                                                                    

we can't make this cast for non-zero length for sparse arrays lacking an initialized element at index 0.

See https://bugs.chromium.org/p/chromium/issues/detail?id=753415 for details.

 

Comment 1 by tsepez@chromium.org, Aug 10 2017

Additionally, the function argument |pointer| is shadowed by a local

      char* pointer = reinterpret_cast<char*>(buffer);

which is kinda icky.

Comment 2 by tsepez@chromium.org, Aug 10 2017

Owner: tsepez@chromium.org
Status: Assigned (was: Untriaged)
I'm going to see if I can cobble up a quick fix for this.

Comment 4 by lfg@chromium.org, Aug 10 2017

Cc: keishi@chromium.org haraken@chromium.org
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8af3e28c117320fe274cda6f35bde9a7f148b9b5

commit 8af3e28c117320fe274cda6f35bde9a7f148b9b5
Author: Tom Sepez <tsepez@chromium.org>
Date: Fri Aug 11 18:31:59 2017

Prevent VSAN from flagging cast in HeapAllocator.h

HeapAllocator is first casting random memory to polymorphic object pointers,
and then checking to see if they can be de-referenced. This is fine in
practice, but VSAN will flag the cast as it happens. Code this using the
stronger pattern where the cast isn't made until we're sure its pointing
at a legitimate object.

Fixes shadowing of |pointer| argument by a local while we're at it.

Bug:  754080 , 753415
Change-Id: Ie0206740f907dac98a06674b29c9c20f897e50de
Reviewed-on: https://chromium-review.googlesource.com/610505
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493821}
[modify] https://crrev.com/8af3e28c117320fe274cda6f35bde9a7f148b9b5/third_party/WebKit/Source/platform/heap/HeapAllocator.h

Comment 6 by tsepez@chromium.org, Aug 11 2017

Status: Fixed (was: Assigned)

Sign in to add a comment