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

Incorrect instance size for derived class with embedder fields

Reported by davidrob...@gmail.com, Feb 28 2018

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12) AppleWebKit/602.1.50 (KHTML, like Gecko) Version/10.0 Safari/602.1.50

Steps to reproduce the problem:
1. Launch Chrome
2. Load https://chrome-bug-objectscc.glitch.me/examples/custom-columns.html
3. An “Aw, Snap!” message appears and the page fails to load

What is the expected behavior?
The page should render and show an empty data table.

What went wrong?
An “Aw, Snap!” message appears and the page fails to load

Screenshots attached:

1. Expected result (Firefox)
2. Actual result (Chrome)

Did this work before? Yes Last version before the beta I am using (unsure of the version number)

Chrome version: 65.0.3325.88  Channel: beta
OS Version: OS X 10.12
Flash Version: 

Chrome Version: 65.0.3325.88 (Official Build) beta (64-bit)

URLs: https://chrome-bug-objectscc.glitch.me/examples/custom-columns.html

Browsers tested:

* Safari 10.0 (12602.1.50.0.10): OK
* Firefox 58.0.2 (64-bit): OK
* Chrome : FAIL

Console Output:

```
#
# Fatal error in ../../v8/src/objects.cc, line 13791
# Check failed: requested_embedder_fields <= max_nof_fields - *in_object_properties.
#
0   Google Chrome Framework             0x00000001052d000c ChromeMain + 31111260
1   Google Chrome Framework             0x0000000108185c3b ChromeMain + 80090251
2   Google Chrome Framework             0x0000000107fb5e4c ChromeMain + 78190236
3   Google Chrome Framework             0x0000000104a14691 ChromeMain + 21954273
4   Google Chrome Framework             0x00000001049e3405 ChromeMain + 21752917
5   Google Chrome Framework             0x00000001049e2b84 ChromeMain + 21750740
6   Google Chrome Framework             0x0000000104571ad3 ChromeMain + 17093411
7   Google Chrome Framework             0x000000010457186e ChromeMain + 17092798
8   Google Chrome Framework             0x00000001045f145c ChromeMain + 17616044
9   Google Chrome Framework             0x00000001045f0f89 ChromeMain + 17614809
10  ???                                 0x000025136618431d 0x0 + 40765247472413
```
 
expected-ff.png
122 KB View Download
actual-chrome.png
165 KB View Download
Relevant crash dump attached.
Uploaded Crash Report ID: a7cae72412d0bf23
aba9a353-f573-4bff-8d47-6260b3ade6d1.dmp
333 KB Download
Confirmed with a colleague that this page loads and works as expected on the following Chrome build and system configuration:

Chrome version: 64.0.3282.167 (Official Build) (64-bit)
OS Version: OS X 10.11.6

Comment 3 by woxxom@gmail.com, Feb 28 2018

FWIW in Windows the first bad build is 66.0.3347.0, not 65.0.3325.88: 536293 (good) - 536300 (bad)
https://chromium.googlesource.com/chromium/src/+log/3c85dbce..153eb022?pretty=fuller

In r536295 "Update V8 to version 6.6.233." suspecting 7b27040e66c8a83006aebc90fe97d21bb42156a7
"[runtime] Harden JSFunction::CalculateInstanceSizeHelper(...)"
because the crash stacktrace contains this path:

 	chrome_child.dll!V8_Fatal(char const *,int,char const *,...)
 	chrome_child.dll!v8::internal::JSFunction::CalculateInstanceSizeHelper(enum v8::internal::InstanceType,bool,int,int,int *,int *)

and the suspected CL contains the code mentioned in #c0:
https://crrev.com/c/v8/v8/+/902103/8/src/objects.cc#13797
Labels: Needs-Bisect Needs-Triage-M65
Cc: vamshi.kommuri@chromium.org cbruni@chromium.org
Labels: -Needs-Bisect Triaged-ET M-66 FoundIn-66 Target-66 OS-Linux OS-Windows
Status: Untriaged (was: Unconfirmed)
Able to reproduce the issue on reported chrome version 65.0.3325.88 and on the latest canary 66.0.3357.0 using Windows 10, Ubuntu 14.04 and mac 10.13.1

We have checked few other chrome versions in order to triage the issue further(Bisecting), following are our observaions
*M60(60.03072.0)      - Good behaviour 
*M65(Reported version)- Bad behaviour
*M66(66.0.3345.0)     - Good behaviour 
*M66(66.0.3347.0)     - Bad behaviour(Including latest canary). 
Could not provide the Normal/reverse Bisect info in the above case. Hence removing the label Needs-Bisect, Please feel free to add it back if required. and marking it as Untriaged.

From Comment#3 requesting cbruni@ for further inputs on this issue.
Components: -Blink Blink>JavaScript>Runtime
Labels: -Pri-2 FoundIn-65 OS-Android OS-Chrome OS-Fuchsia Pri-1
Owner: cbruni@chromium.org
Status: Assigned (was: Untriaged)
The culprits are the following CLs:

65.0.3325.109 Merged: [runtime] Fix overzealous check for derived constructor instance size
66.0.3352.3   [runtime] Fix overzealous check for derived constructor instance size

I will probably have to soften the chech.
Labels: ReleaseBlock-Stable
Status: Started (was: Assigned)

Comment 9 by plato...@gmail.com, Mar 7 2018

The issue might be caused by too many “mixins” (class extends superClass expressions) applied on top of each other.

Here is an isolated reproduction in the attached HTML file. Inside, the script generates a custom element class by applying 100 dummy “mixins” over HTMLElement, then registers that as a custom element and uses it on the page.

You can try different numbers of mixins applied by varying the querystring. For me, the page works with `crash.html?25`, but crashes with `crash.html?26` and above.

Google Chrome	67.0.3362.0 (Official Build) canary (64-bit)
Revision	bc0fc7083e5466f5227bc868c2543957672def9a-refs/heads/master@{#540777}
OS	Mac OS X
JavaScript	V8 6.7.8

Crash id: 24e1895c-9c74-4aea-ad12-ae2ac0d5cb7a


crash.html
307 bytes View Download
 Issue 819646  has been merged into this issue.
Cc: gov...@chromium.org
Labels: M-65
This is happening on M65 latest Chrome Stable.
Cc: cma...@chromium.org
+cmasso@ FYI M65 stable blocker.
Cc: pbomm...@chromium.org hablich@chromium.org
cbruni@, M65 went out to stable yesterday. Is this M65 stable blocker? Also may I pls know why M-65 label wasn't applied at  #5, #6 and #7?
Cc: jkummerow@chromium.org adamk@chromium.org
Sorry, there was an offsite today, I have the CL almost ready: https://crrev.com/c/950724

There was an offsite today and didn't have time to add the required test yet.

govind@ I must have mistaken FoundIn-65 for M-65, sorry :/
Thank you cbruni@ and no worries. It was first missed by  vamshi.kommuri@ at #5.

Can you please help me determine how critical this is for M65 as it already went out to stable yesterday at small percentage and we're planning to ramp up AU? If it isn't too critical, I'm planning to move forward with M65 stable au ramp up and we can take the fix/merge in for next M65 respin (if any). 



This was reported by one of our partners that it's affecting a custom element that they vend (https://github.com/vaadin/vaadin-grid). Based on our internal data, vaadin-grid is the most popular custom element out there, excluding elements vended by our team.

We therefore have reason to believe this will affect a relatively large number of sites and would like to request this is fixed in M65. Looking at the crash numbers doesn't show it being too high right now, but I do expect that number to increase if M65 was ramped up.
 Issue 819797  has been merged into this issue.
Cc: bustamante@chromium.org
I am not sure if it's relevant to your debugging, but vaadin-grid have fixed this on their end. Take a look at their ticket:
https://github.com/vaadin/vaadin-grid/pull/1245

The issue is not security critical, but it might affect a lot of CustomElements depending on inheritance depth and number of properties.
We should for sure merge this to 65. I don't expect sufficient Canary coverage until Monday though.
To be clear, cbruni's fix has landed in v8 as 0d26307046625251bc9b5f568265854f35ba7630.

@hablich, why would you not expect Canary coverage until Monday? I'd expect we'll have it tomorrow (Friday).
Well, it first needs to pass lkgr, then get rolled and then get picked up by the canary builder. The last time I checked (noon CET) the fix hadn't landed yet.

So, there is a possibility it is on tomorrow's canary, but that would be the best case.

Comment 25 Deleted

Project Member

Comment 26 by sheriffbot@chromium.org, Mar 9 2018

Labels: FoundIn-M-67 Fracas
Users experienced this crash on the following builds:

Mac Canary 67.0.3365.0 -  0.53 CPM, 1 reports, 1 clients (signature v8::internal::JSFunction::CalculateInstanceSizeForDerivedClass)

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas
This bug is currently marked as M65 stable blocker and we're planning M65 Stable release next week. Pls try to land the fix ASAP to trunk and ready for merge by EOD Monday, 03/13/18. Thank you.
Cc: ishell@chromium.org
NextAction: 2018-03-12
Pls request a merge to M66 and M65 if canary result looks good on Monday morning.
Labels: TE-Verified-M67 TE-Verified-67.0.3368.0
Able to reproduce the issue on reported chrome version. Crash IDs from chrome version with out fix.
Local Crash ID ff2c07c1-7e00-4fa6-a9ae-3a12385c29ad
Local Crash ID 84d24df5-034a-4a10-90cf-20d2fee276d3.


Verified the fix on Mac 10.13.1, Windows-10 and Ubuntu 14.04 using Chrome version #67.0.3368.0 as per the comment #0.
Attaching screen shot for reference.
Observed that the page loaded with out any crash.
Hence, the fix is working as expected. 
Adding the verified labels. 

Thanks!
CL Verif 817499.png
116 KB View Download
Summary: Incorrect instance size for derived class with embedder fields (was: Get "Aw, Snap!" error coming from v8/src/objects.cc)
Labels: Merge-Approved-66 Merge-Approved-65
Project Member

Comment 33 by bugdroid1@chromium.org, Mar 12 2018

Labels: merge-merged-6.6
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/7295dbf9fd904b3491ff118f5f05bebb63e79292

commit 7295dbf9fd904b3491ff118f5f05bebb63e79292
Author: Camillo Bruni <cbruni@chromium.org>
Date: Mon Mar 12 10:05:59 2018

Merged: [runtime] Properly calculate upper bound for NOF in_object_properties

Revision: 0d26307046625251bc9b5f568265854f35ba7630

BUG= chromium:817499 
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=ishell@chromium.org

Change-Id: I6e3222443d66c8adad154896bde9f22569a8455e
Reviewed-on: https://chromium-review.googlesource.com/958469
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.6@{#15}
Cr-Branched-From: d500271571b92cb18dcd7b15885b51e8f437d640-refs/heads/6.6.346@{#1}
Cr-Branched-From: 265ef0b635f8761df7c89eb4e8ec9c1a6ebee184-refs/heads/master@{#51624}
[modify] https://crrev.com/7295dbf9fd904b3491ff118f5f05bebb63e79292/src/field-index.h
[modify] https://crrev.com/7295dbf9fd904b3491ff118f5f05bebb63e79292/src/objects-inl.h
[modify] https://crrev.com/7295dbf9fd904b3491ff118f5f05bebb63e79292/src/objects.cc
[modify] https://crrev.com/7295dbf9fd904b3491ff118f5f05bebb63e79292/src/objects.h
[modify] https://crrev.com/7295dbf9fd904b3491ff118f5f05bebb63e79292/src/property-details.h
[modify] https://crrev.com/7295dbf9fd904b3491ff118f5f05bebb63e79292/test/cctest/test-api.cc

Project Member

Comment 34 by bugdroid1@chromium.org, Mar 12 2018

Labels: merge-merged-6.5
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/de1b8ea026bf25f6cd8fe522b5558bb5fab7f61e

commit de1b8ea026bf25f6cd8fe522b5558bb5fab7f61e
Author: Camillo Bruni <cbruni@chromium.org>
Date: Mon Mar 12 10:23:36 2018

Merged: [runtime] Properly calculate upper bound for NOF in_object_properties

Revision: 0d26307046625251bc9b5f568265854f35ba7630

BUG= chromium:817499 
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=ishell@chromium.org

Change-Id: I5ce7203243c0ac84d654a106131f20725833766a
Reviewed-on: https://chromium-review.googlesource.com/958471
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/branch-heads/6.5@{#65}
Cr-Branched-From: 73c55f57fe8506011ff854b15026ca765b669700-refs/heads/6.5.254@{#1}
Cr-Branched-From: 594a1a0b6e551397cfdf50870f6230da34db2dc8-refs/heads/master@{#50664}
[modify] https://crrev.com/de1b8ea026bf25f6cd8fe522b5558bb5fab7f61e/src/field-index.h
[modify] https://crrev.com/de1b8ea026bf25f6cd8fe522b5558bb5fab7f61e/src/objects-inl.h
[modify] https://crrev.com/de1b8ea026bf25f6cd8fe522b5558bb5fab7f61e/src/objects.cc
[modify] https://crrev.com/de1b8ea026bf25f6cd8fe522b5558bb5fab7f61e/src/objects.h
[modify] https://crrev.com/de1b8ea026bf25f6cd8fe522b5558bb5fab7f61e/src/property-details.h
[modify] https://crrev.com/de1b8ea026bf25f6cd8fe522b5558bb5fab7f61e/test/cctest/test-api.cc

The NextAction date has arrived: 2018-03-12
Seems like this is already merged to M66 and M65 at #33 and #34. If nothing is pending, pls remove "Merge-Approved-66" and "Merge-Approved-65" label. Thank you.
Labels: -Merge-Approved-65 -Merge-Approved-66
Can this be marked as fixed if nothing is pending?
Status: Fixed (was: Started)
I think so... given that we already backmerged there is nothing to be waiting for :)
Labels: TE-Verified-M65 TE-Verified-65.0.3325.162
Verified the fix on Mac 10.13.1, Windows-10 and Ubuntu 14.04 using Chrome version #65.0.3325.162 as per the comment #0.
Attaching screen shot for reference.
Observed that the page loaded with out any crash.
Hence, the fix is working as expected. 
Adding the verified labels. 
Note: Able to reproduce the issue on reported chrome version(Ref:Comment#30)

Thanks!
CL stable verif.mp4
512 KB View Download
Issue 820922 has been merged into this issue.
Labels: NodeJS-Backport-Done

Sign in to add a comment