New issue
Advanced search Search tips

Issue 847679 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

blink::LargeObjectPage::PayloadEnd returns a wrong value

Reported by leamov...@gmail.com, May 30 2018

Issue description

Just found a possible code defect while browsing the chromium code.  Probably this does not break any functionality for now, but if anyone considers fixing this, it's really appreciated.

Currently LargeObjectPage::PayloadEnd() returns Payload() + PayloadSize() where we count the size of HeapObjectHeader twice.

A correct logic would be Payload() + GetHeapObjectHeader()->PayloadSize() as GetHeapObjectHeader()->PayloadSize() does not count the size of HeapObjectHeader.

https://chromium.googlesource.com/chromium/src.git/+/f7658f9c5022136748b5b6f5bbb9412f562ea36e/third_party/blink/renderer/platform/heap/heap_page.h#593

  // LargeObjectPage has the following memory layout:
  //
  //     | metadata | HeapObjectHeader | payload |
  //
  // LargeObjectPage::PayloadSize returns the size of HeapObjectHeader and the
  // object payload. HeapObjectHeader::PayloadSize returns just the size of the
  // payload.
  Address Payload() { return GetHeapObjectHeader()->Payload(); }
  size_t PayloadSize() { return payload_size_; }
  Address PayloadEnd() { return Payload() + PayloadSize(); } <<<< should use GetHeapObjectHeader()->PayloadSize()?
 

Comment 1 by tkent@chromium.org, May 30 2018

Components: Blink>MemoryAllocator>GarbageCollection
Cc: haraken@chromium.org
Labels: -Pri-3 Pri-2
Owner: mlippautz@chromium.org
Status: Started (was: Unconfirmed)
Correct, the method is not used right now afaics but we should still fix this asap.
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 15 2018

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

commit b0fd211f0a1557130f8c192802122d75597eaeb8
Author: Michael Lippautz <mlippautz@chromium.org>
Date: Fri Jun 15 05:38:43 2018

[oilpan] LargeObject payload fix

No-try: true
Bug:  chromium:853055 ,  chromium:847679 ,  chromium:852980 
Change-Id: I019eea9d101cbd8e368607e6309a99b0e9779cc2
Reviewed-on: https://chromium-review.googlesource.com/1101605
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567561}
[modify] https://crrev.com/b0fd211f0a1557130f8c192802122d75597eaeb8/third_party/blink/renderer/platform/heap/heap_page.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 15 2018

Labels: merge-merged-3461
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/09704f44b190b6b8fbccb5358768fc49647dc056

commit 09704f44b190b6b8fbccb5358768fc49647dc056
Author: Michael Lippautz <mlippautz@chromium.org>
Date: Fri Jun 15 06:16:52 2018

[oilpan] LargeObject payload fix

No-try: true
Bug:  chromium:853055 ,  chromium:847679 ,  chromium:852980 , chromium:853090
Change-Id: I019eea9d101cbd8e368607e6309a99b0e9779cc2
Reviewed-on: https://chromium-review.googlesource.com/1101605
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#567561}(cherry picked from commit b0fd211f0a1557130f8c192802122d75597eaeb8)
Reviewed-on: https://chromium-review.googlesource.com/1102257
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/branch-heads/3461@{#3}
Cr-Branched-From: c8a271c2b9964bd832b32a9a143f7bf28b658e70-refs/heads/master@{#567544}
[modify] https://crrev.com/09704f44b190b6b8fbccb5358768fc49647dc056/third_party/blink/renderer/platform/heap/heap_page.h

Status: Fixed (was: Started)

Sign in to add a comment