Issue metadata
Sign in to add a comment
|
Incorrect instance size for derived class with embedder fields
Reported by
davidrob...@gmail.com,
Feb 28 2018
|
||||||||||||||||||||||
Issue descriptionUserAgent: 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 ```
,
Feb 28 2018
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
,
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
,
Mar 1 2018
,
Mar 1 2018
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.
,
Mar 1 2018
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.
,
Mar 1 2018
,
Mar 6 2018
,
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
,
Mar 7 2018
Issue 819646 has been merged into this issue.
,
Mar 7 2018
This is happening on M65 latest Chrome Stable.
,
Mar 7 2018
+cmasso@ FYI M65 stable blocker.
,
Mar 7 2018
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?
,
Mar 7 2018
,
Mar 7 2018
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 :/
,
Mar 7 2018
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).
,
Mar 7 2018
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.
,
Mar 8 2018
Issue 819797 has been merged into this issue.
,
Mar 8 2018
,
Mar 8 2018
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
,
Mar 8 2018
The issue is not security critical, but it might affect a lot of CustomElements depending on inheritance depth and number of properties.
,
Mar 8 2018
We should for sure merge this to 65. I don't expect sufficient Canary coverage until Monday though.
,
Mar 8 2018
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).
,
Mar 8 2018
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.
,
Mar 9 2018
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
,
Mar 9 2018
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.
,
Mar 9 2018
,
Mar 11 2018
Pls request a merge to M66 and M65 if canary result looks good on Monday morning.
,
Mar 12 2018
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!
,
Mar 12 2018
,
Mar 12 2018
,
Mar 12 2018
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
,
Mar 12 2018
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
,
Mar 12 2018
The NextAction date has arrived: 2018-03-12
,
Mar 12 2018
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.
,
Mar 12 2018
,
Mar 12 2018
Can this be marked as fixed if nothing is pending?
,
Mar 12 2018
I think so... given that we already backmerged there is nothing to be waiting for :)
,
Mar 13 2018
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!
,
Mar 13 2018
Issue 820922 has been merged into this issue.
,
Jun 12 2018
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by davidrob...@gmail.com
, Feb 28 2018333 KB
333 KB Download