915217 - win64 APPCRASH in plugin-container.exe upon right click on flash object

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Dima]

Attachment: App crash entries for importing to Windows Event Viewer (eventvwr.exe)

When right clicking on any flash object, after some hanging (sometimes immediately) an application error window displayed with app crash data. It is possible to close it (on Windows 8), then Firefox will continue running, but with grey canvas with a message "The Adobe Flash plugin has crashed. Reload the page to try again. No reports available." instead of flash objects.
Firefox 26.0a1 (latest Firefox Nightly x64)
Flash plugin 11.8.800.168.
No crash data collected by Firefox (no new entries in about:crashes), some crash data collected by Windows please find attached.
Problem may be similar or related to this one:
https://bugzilla.mozilla.org/show_bug.cgi?id=347895

Comment 1

[Dima]

Probably, this firefox crash is related to the problem:
https://crash-analysis.mozilla.com/hang-reports/2013/09-11/hr-20130911-2dac0dd8-d362-43bb-9d2d-b4abc195fd62.html

Comment 2

[Benjamin Smedberg]

We're here: http://hg.mozilla.org/mozilla-central/annotate/9e9f74116749/dom/plugins/ipc/PluginInstanceChild.cpp#l1683

calling into 0x2a9001a which is not known code.

Is this a new regression in 64-bit windows builds? This could be a bug in the DLL interceptor mechanism, or how we call it in 64-bit builds. In particular us user32.dll the right DLL in 64-bit builds?

This is the line that actually sets up the interceptor: http://hg.mozilla.org/mozilla-central/annotate/9e9f74116749/dom/plugins/ipc/PluginInstanceChild.cpp#l1726

Comment 3

[TinyButStrong]

I have this issue on Nightly build since few month ago.

Comment 4

[(no longer active)]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #2)
> We're here:
> http://hg.mozilla.org/mozilla-central/annotate/9e9f74116749/dom/plugins/ipc/
> PluginInstanceChild.cpp#l1683
> 
> calling into 0x2a9001a which is not known code.
> 
> Is this a new regression in 64-bit windows builds? This could be a bug in
> the DLL interceptor mechanism, or how we call it in 64-bit builds. In
> particular us user32.dll the right DLL in 64-bit builds?

My 64-bit user32.dll does have a TrackPopupMenu entry point.

My best guess is that we should just add support for more opcodes.  Makoto has done a tonne of that work before.  :-)

Comment 5

[Toady]

Same problem with the below code bases:

25.0b1
25.0b2
25.0b3
25.0b4
25.0b5
25.0b6
25.0b7
25.0b8
25.0b9
25.0b10
25.0 - candidate

To reproduce build firefox 25 with x64 bit flags 

Firefox builds perfectly but plugin container crashes instantly or upon right context menu click, Bug present in 26 not in 27 seems some of the changes from 24 to 25-26 in IPC has caused this bug\regression.

On the error console in web tools it shows an xml error after plugin container crashes.

Comment 6

[Makoto Kato [:m_kato]]

Attachment: fix

incorrect anaylzing mov [r64+disp8], imm32

Comment 7

[(no longer active)]

Comment on attachment 832038 [details] [diff] [review]
fix

Review of attachment 832038 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks!  Out of curiosity, do you know why this test has not caught us failing to hook TrackPopupMenu correctly? <http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/test/win/TestDllInterceptor.cpp#123>

Comment 8

[danny gowing]

Comment on attachment 832038 [details] [diff] [review]
fix

Review of attachment 832038 [details] [diff] [review]:
-----------------------------------------------------------------

Tested, fixed flash player crash, still crashs unity player on right click

Comment 9

[danny gowing]

disregard unity info, seems to be a unrelated bug

Comment 10

[Makoto Kato [:m_kato]]

I'm sorry.   I attached old patch, so I will attach new fix soon.


(In reply to :Ehsan Akhgari (needinfo? me!) from comment #7)
> Comment on attachment 832038 [details] [diff] [review]
> fix
> 
> Review of attachment 832038 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good, thanks!  Out of curiosity, do you know why this test has not
> caught us failing to hook TrackPopupMenu correctly?
> <http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/test/win/
> TestDllInterceptor.cpp#123>

TestDllIntercept doesn't have calling test after hooked.  I should file a new bug for this.

Comment 11

[Makoto Kato [:m_kato]]

Attachment: v2

Comment 12

[Makoto Kato [:m_kato]]

Comment on attachment 8333809 [details] [diff] [review]
v2

correct check for 0xc7 /0 io.  modR/W should be 0x44 for [r64+disp8]

Comment 13

[(no longer active)]

(In reply to Makoto Kato (:m_kato) from comment #10)
> TestDllIntercept doesn't have calling test after hooked.  I should file a
> new bug for this.

I see...  Yeah we should definitely do that!

Comment 14

[Makoto Kato [:m_kato]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9eecc7a369fd

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9eecc7a369fd

Comment 16

[Catalin Varga [QA][:VarCat]]

I reproduced the bug using build FF 26.0a1 build id: 20130911030258 and Win 8 x64.

I verified the bug using the following environment:

FF 28.0b6
Build Id: 20140224220227
Os: Win 8 x64