New issue
Advanced search Search tips

Issue 683848 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug

Blocking:
issue 501975



Sign in to add a comment

Chrome: Crash Report - ChromeMain when AppContainer is enabled

Project Member Reported by grt@chromium.org, Jan 23 2017

Issue description

Product name: Chrome
Magic Signature: ChromeMain

Current link:
https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.ptype%3D'renderer'%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D'ChromeMain'%20AND%20product.name%3D'Chrome'%20AND%20product.Version%3D'57.0.2986.0'%20AND%20ReportID%3D'a1aa49e580000000'&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D#3


Search properties:
custom_data.chromecrashproto.ptype: renderer
custom_data.chromecrashproto.magic_signature_1.name: ChromeMain
product.name: Chrome
product.version: 57.0.2986.0
reportid: a1aa49e580000000

Metadata :
Product Name: Chrome
Product Version: 57.0.2986.0
Report ID: a1aa49e580000000
Report Time: Mon, 23 Jan 2017 08:11:48 GMT
Uptime: 2000 ms
Cumulative Uptime: 0 ms
User Email: 
OS Name: Windows NT
OS Version: 10.0.14393 0
CPU Architecture: amd64
CPU Info: family 6 model 55 stepping 8

Avast is somehow breaking runtime lookup of symbols exported from chrome_elf. I fixed one in r444547 (issue 674541) by switching to loader-based resolution for GetInstallDetailsPayload. Can we do the same for DumpProcessWithoutCrash and any other symbols exported from chrome_elf?

Here's the crashing code now:

  // SetDumpWithoutCrashingFunction must be passed the DumpProcess function
  // from chrome_elf and not from the DLL in order for DumpWithoutCrashing to
  // function correctly.
  typedef void (__cdecl *DumpProcessFunction)();
  DumpProcessFunction DumpProcess = reinterpret_cast<DumpProcessFunction>(
      ::GetProcAddress(::GetModuleHandle(chrome::kChromeElfDllName),
                       "DumpProcessWithoutCrash"));
  CHECK(DumpProcess);   <-- KABLEWIE!
 
Project Member

Comment 1 by sheriffbot@chromium.org, Jan 23 2017

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

Win Canary 58.0.2989.0 -  3.05 CPM, 36 reports, 21 clients (signature ChromeMain)

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

- Go/Fracas
Project Member

Comment 2 by sheriffbot@chromium.org, Jan 31 2017

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

Win Beta 56.0.2924.76 -  1.84 CPM, 1352 reports, 318 clients (signature ChromeMain)

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

- Go/Fracas

Comment 3 by wfh@chromium.org, Feb 1 2017

Blocking: 501975
Cc: forshaw@chromium.org scottmg@chromium.org
Components: -Internals>CrashReporting Internals>Sandbox
Labels: -Restrict-View-Google
Owner: wfh@chromium.org
Status: Started (was: Assigned)
Summary: Chrome: Crash Report - ChromeMain when AppContainer is enabled (was: Chrome: Crash Report - ChromeMain)
I performed manual analysis (cdb pipeline ftw!) on 301 crashes on beta channel matching this signature and found that out of these crashes, 264 had the AppContainer/Enabled experiment enabled, and 0 had the AppContainer/Control experiment. The experiment is 50/50 on beta.

This means there is potentially a tight correlation between this crash, and App Container being enabled.

Comment 4 by wfh@chromium.org, Feb 1 2017

Cc: brucedaw...@chromium.org
interestingly the other 37 files, all had issues reading the command line from the PEB - !peb failed because of missing symbol

error 3 InitTypeRead( nt!_PEB at <addr>)

example crash exhibiting this is crash/f0a2b52880000000.

All of these seem to be Windows 10 - ntdll.dll 10.0.10586.672 (th2_release_sec.161024-1825) with symbol hash EBACFB2C299D49E1BFDC3EB3F9E6B2191 and 580EE3211c1000 shrug.
wfh@ I think you just need to delete the existing cached copy of ntdll.pdb and WinDBG will redownload a fixed on. I had a look at that crash and I can dump the PEB fine.

Comment 6 by siggi@chromium.org, Feb 1 2017

It would be helpful for diagnosis of this and like issues if minidumps contained the afflicted IATs, I filed a Crashpad feature request <https://bugs.chromium.org/p/crashpad/issues/detail?id=155>.

Comment 7 by wfh@chromium.org, Feb 1 2017

re: #5 I tried that and it didn't help :(

Comment 8 by grt@chromium.org, Feb 11 2017

Labels: Stability-ThirdParty
Project Member

Comment 9 by sheriffbot@chromium.org, Feb 20 2017

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

Win Beta 57.0.2987.54 -  0.60 CPM, 441 reports, 187 clients (signature ChromeMain)

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

- Go/Fracas

Comment 10 by wfh@chromium.org, Feb 23 2017

Cc: robertshield@chromium.org
I am now working on this bug!

I looked at exports of chrome_elf

   1    0 00002C90 AddDllToBlacklist = _AddDllToBlacklist
          2    1 00027700 BlockUntilHandlerStartedImpl = _BlockUntilHandlerStartedImpl
          3    2 00027660 ClearCrashKeyValueImpl = _ClearCrashKeyValueImpl
          4    3 00029450 CrashForException = _CrashForException
          5    4 00029390 DumpProcessWithoutCrash = _DumpProcessWithoutCrash
          6    5 00002C50 GetBlacklistIndex = _GetBlacklistIndex
          7    6 000039A0 GetCrashReportsImpl = _GetCrashReportsImpl
          8    7 00016410 GetHandleVerifier = _GetHandleVerifier
          9    8 000011B0 GetInstallDetailsPayload = _GetInstallDetailsPayload
         10    9 00001010 GetUserDataDirectoryThunk = _GetUserDataDirectoryThunk
         11    A 000296B0 InjectDumpForHangDebugging = _InjectDumpForHangDebugging
         12    B 000294D0 InjectDumpForHungInput = _InjectDumpForHungInput
         13    C 00029600 InjectDumpForHungInputNoCrashKeys = _InjectDumpForHungInputNoCrashKeys
         14    D 00029470 InjectDumpProcessWithoutCrash = _InjectDumpProcessWithoutCrash
         15    E 00002C40 IsBlacklistInitialized = _IsBlacklistInitialized
         16    F 000276F0 RequestSingleCrashUploadImpl = _RequestSingleCrashUploadImpl
         17   10 00027590 SetCrashKeyValueImpl = _SetCrashKeyValueImpl
         18   11 000039E0 SetMetricsClientId = _SetMetricsClientId
         19   12 00027580 SetUploadConsentImpl = _SetUploadConsentImpl
         20   13 00001000 SignalChromeElf = _SignalChromeElf
         21   14 00002D20 SuccessfullyBlocked = _SuccessfullyBlocked

robert -> was/is there a reason why these were exported and resolved dynamically using getprocaddress rather than using loader based resolution - if no compelling reason it's probably worth thinking about just moving these all into a chrome_elf header file that anyone can use if they want access, and let the runtime linker do this resolution...
Ugh.. memory, wracking brain.. we *might* have been worried once upon a time about things that were trying to load early replacing the elf dll, missing a needed export (quite plausible across updates) and then breaking loader resolution which results in a pretty icky system error message and Chrome not starting anymore. 

This was before elf started growing to contain all manner of things and probably not all (any?) of the cases where we do dynamic resolution will actually fail gracefully.

Comment 12 by siggi@chromium.org, Feb 23 2017

Cc: grt@chromium.org
+grt who's been changing use of elf exports to explicit imports for similar reasons.

Comment 13 by grt@chromium.org, Feb 23 2017

I filed this bug, dude! :-)

Comment 14 by siggi@chromium.org, Feb 23 2017

Ugh, sorry. +grt to pre-read all the bugs and recite them to me.

Comment 15 by grt@chromium.org, Feb 23 2017

I'll start sending all bugs to the printer in the hall. robertshield will hand deliver them.
If you write them in limerick form, I will also recite them loudly in Siggi's presence.

There once was a crash from a pad
That Siggi did think was bad
He wrote a design doc
And debugged a deadlock
And crashes stopped making him sad
Project Member

Comment 17 by sheriffbot@chromium.org, Mar 4 2017

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

Win Canary 59.0.3030.0 -  5.23 CPM, 24 reports, 14 clients (signature ChromeMain)

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

- Go/Fracas
Project Member

Comment 18 by sheriffbot@chromium.org, Apr 23 2017

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

Win Canary 60.0.3078.0 -  0.20 CPM, 7 reports, 4 clients (signature ChromeMain)

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

- Go/Fracas
Just to update the latest behavior, Still crashes observed on latest beta channel. Currently this crash is ranked as number #20 for Windows platform under renderer process. Below information provides the comparison between previous and latest channels including total number of instances.
+--------------------------------------------------+      
|Latest Channel        |    Previous Channel       |
+--------------------------------------------------+  
|59.0.3071.36   22     |  59.0.3071.29	  150	   |--> Beta
+--------------------------------------------------+
	
Link to the list of the builds getting crash:
---------------------------------------------
https://crash.corp.google.com/browse?q=product.name%3D%27Chrome%27%20AND%20custom_data.ChromeCrashProto.channel%3D%27beta%27%20AND%20custom_data.ChromeCrashProto.ptype%3D%27renderer%27%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D%27ChromeMain%27&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D#samplereports:5,productversion:1000
Project Member

Comment 20 by bugdroid1@chromium.org, Jun 28 2017

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

commit 93846275952fa28f83a4ea318e56c4f494bf7005
Author: wfh <wfh@chromium.org>
Date: Wed Jun 28 11:51:06 2017

Change DumpProcessWithoutCrash to use load-time dynamic linking.

Instead of exporting DumpProcessWithoutCrash from chrome_elf directly
pass the correct function to call from crashpad, and use load-time
dynamic linking. This avoids an explicit call to LoadLibrary early in
Chrome startup which can cause issues with certain third party code.

See also bug 674541 where this was done for code in install_static.

BUG=683848
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Review-Url: https://codereview.chromium.org/2909623002
Cr-Commit-Position: refs/heads/master@{#482949}

[modify] https://crrev.com/93846275952fa28f83a4ea318e56c4f494bf7005/base/debug/dump_without_crashing.cc
[modify] https://crrev.com/93846275952fa28f83a4ea318e56c4f494bf7005/base/debug/dump_without_crashing.h
[modify] https://crrev.com/93846275952fa28f83a4ea318e56c4f494bf7005/chrome/app/chrome_main.cc
[modify] https://crrev.com/93846275952fa28f83a4ea318e56c4f494bf7005/chrome_elf/chrome_elf.def
[modify] https://crrev.com/93846275952fa28f83a4ea318e56c4f494bf7005/chrome_elf/chrome_elf_main.cc
[modify] https://crrev.com/93846275952fa28f83a4ea318e56c4f494bf7005/chrome_elf/chrome_elf_main.h
[modify] https://crrev.com/93846275952fa28f83a4ea318e56c4f494bf7005/chrome_elf/crash/crash_helper.cc
[modify] https://crrev.com/93846275952fa28f83a4ea318e56c4f494bf7005/chrome_elf/crash/crash_helper.h
[modify] https://crrev.com/93846275952fa28f83a4ea318e56c4f494bf7005/components/crash/content/app/crashpad.cc
[modify] https://crrev.com/93846275952fa28f83a4ea318e56c4f494bf7005/components/crash/content/app/crashpad.h
[modify] https://crrev.com/93846275952fa28f83a4ea318e56c4f494bf7005/components/crash/content/app/crashpad_win.cc

Project Member

Comment 21 by sheriffbot@chromium.org, Jun 30 2017

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

Win Dev 61.0.3141.7 -  0.22 CPM, 17 reports, 7 clients (signature ChromeMain)

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

- Go/Fracas
Just to update, latest crash rates on all channels are as below:
This crash not seen on latest Canary & Dev

60.0.3112.72	5.58%	624	-Beta
59.0.3071.115	1.62%	181	-Stable

Link to the list of builds:
---------------------------
https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.ptype%3D%27renderer%27%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D%27ChromeMain%27&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D#-property-selector,samplereports:5,+productversion:1000

Thank You!

Sign in to add a comment