1411008 - stylo: thread '<unnamed>' panicked at 'called `Option::unwrap()` on a `None` value', /checkout/src/libcore/option.rs:335:20

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Jason Kratzer [:jkratzer]]

Attachment: trigger.html

Testcase found while fuzzing mozilla-central rev 0bd9b61304e2.

thread '<unnamed>' panicked at 'called `Option::unwrap()` on a `None` value', /checkout/src/libcore/option.rs:335:20
stack backtrace:
   0:     0x7fccd9d487b3 - std::sys::imp::backtrace::tracing::imp::unwind_backtrace::hcdf51e4c9dc54357
   1:     0x7fccd9d42fc4 - std::sys_common::backtrace::_print::h9da91fd31a37d0f1
   2:     0x7fccd9d55dd3 - std::panicking::default_hook::{{closure}}::h46820a72bf0cb624
   3:     0x7fccd9d55b42 - std::panicking::default_hook::h4c1ef1cc83189c8e
   4:     0x7fccd9d562d6 - std::panicking::rust_panic_with_hook::h99016f44bdcb8544
   5:     0x7fccd9d56164 - std::panicking::begin_panic_new::hae931f4b9fe56a90
   6:     0x7fccd9d56069 - std::panicking::begin_panic_fmt::h3b8db4524c33692e
   7:     0x7fccd9d55ffa - rust_begin_unwind
   8:     0x7fccd9d6c9e0 - core::panicking::panic_fmt::h955f7c5ec61a82d4
   9:     0x7fccd9d6c916 - core::panicking::panic::hec1812dcc135e139
  10:     0x7fccd9e3efa8 - <core::option::Option<T>>::unwrap::hb9f07895f734ba18
  11:     0x7fccd9f22b2e - style::stylist::Stylist::push_applicable_declarations::ha762c15084c74898
  12:     0x7fccd9e12f9f - <style::style_resolver::StyleResolverForElement<'a, 'ctx, 'le, E>>::match_primary::h0bc210fa556ab5c7
  13:     0x7fccd9e13719 - <style::style_resolver::StyleResolverForElement<'a, 'ctx, 'le, E>>::resolve_primary_style::h5717fe7b77f4edf0
  14:     0x7fccd9e132f1 - <style::style_resolver::StyleResolverForElement<'a, 'ctx, 'le, E>>::resolve_style::he08d00be51ad621e
  15:     0x7fccd9e72938 - style::traversal::resolve_style::h428c89ed1953a71d
  16:     0x7fccd955d7ee - Servo_ResolveStyleLazily
  17:     0x7fccd7b3d1c2 - _ZN7mozilla13ServoStyleSet26ResolveStyleLazilyInternalEPNS_3dom7ElementENS_20CSSPseudoElementTypeENS_18StyleRuleInclusionEb
  18:     0x7fccd7b3d341 - _ZN7mozilla13ServoStyleSet18ResolveStyleLazilyEPNS_3dom7ElementENS_20CSSPseudoElementTypeENS_18StyleRuleInclusionE
  19:     0x7fccd7b69e6d - _ZN18nsComputedDOMStyle24DoGetStyleContextNoFlushEPN7mozilla3dom7ElementEP6nsAtomP12nsIPresShellNS_9StyleTypeENS_13AnimationFlagE
  20:     0x7fccd7a48c89 - _ZN7mozilla10EditorBase14IsPreformattedEP10nsIDOMNodePb
  21:     0x7fccd7abae22 - _ZN7mozilla11WSRunObject7GetRunsEv
  22:     0x7fccd7aa2ee5 - _ZN7mozilla13HTMLEditRules14AfterEditInnerE10EditActions
  23:     0x7fccd7aa338d - _ZN7mozilla13HTMLEditRules9AfterEditE10EditActions
  24:     0x7fccd7ab3050 - _ZN7mozilla10HTMLEditor12EndOperationEv
  25:     0x7fccd7a3dccc - _ZN7mozilla9AutoRulesD2Ev
  26:     0x7fccd7ac4da8 - _ZN7mozilla10TextEditor10InsertTextERK12nsTSubstringIDsE.part.180
  27:     0x7fccd7a41f1d - _ZN7mozilla22InsertPlaintextCommand9DoCommandEPKcP11nsISupports
  28:     0x7fccd7267b10 - _ZN24nsControllerCommandTable9DoCommandEPKcP11nsISupports
  29:     0x7fccd726590d - _ZN23nsBaseCommandController9DoCommandEPKc
  30:     0x7fccd726758d - _ZN16nsCommandManager9DoCommandEPKcP16nsICommandParamsP18mozIDOMWindowProxy
  31:     0x7fccd73cb15c - _ZN14nsHTMLDocument11ExecCommandERK12nsTSubstringIDsEbS3_R12nsIPrincipalRN7mozilla11ErrorResultE
  32:     0x7fccd70ff2fc - _ZN7mozilla3dom19HTMLDocumentBindingL11execCommandEP9JSContextN2JS6HandleIP8JSObjectEEP14nsHTMLDocumentRK19JSJitMethodCallArgs
  33:     0x7fccd71a4e6f - _ZN7mozilla3dom20GenericBindingMethodEP9JSContextjPN2JS5ValueE
  34:     0x7fccd8aa7c30 - _ZN2js12CallJSNativeEP9JSContextPFbS1_jPN2JS5ValueEERKNS2_8CallArgsE
  35:     0x7fccd8abc59e - _ZN2js23InternalCallOrConstructEP9JSContextRKN2JS8CallArgsENS_14MaybeConstructE
  36:     0x7fccd8abc95d - _ZL12InternalCallP9JSContextRKN2js13AnyInvokeArgsE
  37:     0x7fccd8aaffa2 - _ZL9InterpretP9JSContextRN2js8RunStateE
  38:     0x7fccd8abc185 - _ZN2js9RunScriptEP9JSContextRNS_8RunStateE
  39:     0x7fccd8abf04c - _ZN2js13ExecuteKernelEP9JSContextN2JS6HandleIP8JSScriptEER8JSObjectRKNS2_5ValueENS_16AbstractFramePtrEPS9_
  40:     0x7fccd8abf3e8 - _ZN2js7ExecuteEP9JSContextN2JS6HandleIP8JSScriptEER8JSObjectPNS2_5ValueE
  41:     0x7fccd8eb3731 - _ZL13ExecuteScriptP9JSContextN2JS6HandleIP8JSObjectEENS2_IP8JSScriptEEPNS1_5ValueE
  42:     0x7fccd8eb3b7d - _ZL13ExecuteScriptP9JSContextRN2JS16AutoObjectVectorENS1_6HandleIP8JSScriptEEPNS1_5ValueE
  43:     0x7fccd6ae06f3 - _ZN9nsJSUtils16ExecutionContext14CompileAndExecERN2JS14CompileOptionsERNS1_18SourceBufferHolderENS1_13MutableHandleIP8JSScriptEE
  44:     0x7fccd797ac1f - _ZN7mozilla3dom12ScriptLoader14EvaluateScriptEPNS0_17ScriptLoadRequestE.part.308
  45:     0x7fccd797b1c8 - _ZN7mozilla3dom12ScriptLoader14ProcessRequestEPNS0_17ScriptLoadRequestE
  46:     0x7fccd7982ca1 - _ZN7mozilla3dom12ScriptLoader20ProcessScriptElementEP16nsIScriptElement
  47:     0x7fccd7982f1f - _ZN7mozilla3dom13ScriptElement18MaybeProcessScriptEv
  48:     0x7fccd66abb7f - _ZN16nsIScriptElement16AttemptToExecuteEv
  49:     0x7fccd66b6a1f - _ZN21nsHtml5TreeOpExecutor9RunScriptEP10nsIContent
  50:     0x7fccd66c8ce3 - _ZN21nsHtml5TreeOpExecutor12RunFlushLoopEv
  51:     0x7fccd66c8e0e - _ZN22nsHtml5ExecutorFlusher3RunEv
  52:     0x7fccd5e247e9 - _ZN8nsThread16ProcessNextEventEbPb.part.276
  53:     0x7fccd5e2555e - _Z19NS_ProcessNextEventP9nsIThreadb
  54:     0x7fccd61e4600 - _ZN7mozilla3ipc11MessagePump3RunEPN4base11MessagePump8DelegateE
  55:     0x7fccd61b9586 - _ZN11MessageLoop11RunInternalEv
  56:     0x7fccd61b95b2 - _ZN11MessageLoop3RunEv
  57:     0x7fccd79e7d22 - _ZN14nsBaseAppShell3RunEv
  58:     0x7fccd8935456 - _ZN12nsAppStartup3RunEv
  59:     0x7fccd89bb436 - _ZN7XREMain11XRE_mainRunEv
  60:     0x7fccd89bbc33 - _ZN7XREMain8XRE_mainEiPPcRKN7mozilla15BootstrapConfigE
  61:     0x7fccd89bbf18 - _Z8XRE_mainiPPcRKN7mozilla15BootstrapConfigE
  62:           0x4073ac - _ZL7do_mainiPPcS0_
  63:           0x406c59 - main
  64:     0x7fcce772b82f - __libc_start_main
  65:           0x406eb8 - <unknown>
Redirecting call to abort() to mozalloc_abort

Comment 1

[Jason Kratzer [:jkratzer]]

Attachment: log_minidump.txt

Comment 2

[Jason Kratzer [:jkratzer]]

Attachment: log_stderr.txt

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Not sure yet if it's a bug in stylo or the editing code that calls into it, but can look at it tomorrow if nobody is faster.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Actually, just took a quick look... So this is passing a destroyed NAC element to nsComputedDOMStyle::GetStyleContextNoFlush from:

  http://searchfox.org/mozilla-central/rev/d30462037ffea383e74c42542c820cf65b2b144e/editor/libeditor/EditorBase.cpp#3933

When we try to compute its style, we go and crash, since we can't find its nearest element in order to match it as a pseudo-element.

The node is an already-unbound ::-moz-progress-bar. This code looks pretty old, but I suspect we're not supposed to be dealing with out-of-doc content or NAC here.

Masayuki, you've been one of the recent people that have fiddled with this file. I have various ideas, more or less wallpaper-ish, but... Are you aware of a sane case where the node in:

  mRangeItem->mStartContainer

  http://searchfox.org/mozilla-central/rev/d30462037ffea383e74c42542c820cf65b2b144e/editor/libeditor/HTMLEditRules.cpp#521

Would be a NAC pseudo-element outside of the doc? (What about any element outside of the doc anyway?)

Happy to fix this if you tell me what needs to happen here, roughly :)

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Crashes plain opt builds for me too. Reproduces all the way back to when we started building Stylo by default in automation.

Comment 6

[Chris Peterson [:cpeterson]]

status-firefox57=wontfix unless someone thinks this bug should block 57

Comment 9

[Marcia Knous [:marcia]]

Adding a signature I saw in crash stats that mapped to the attachment in this bug.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

I don't think this is particularly prioritary, and I'm still waiting on Masayuki.

If this blocks fuzzing or something like that, or we have URLs that repro it that aren't this test-case, I can dig a bit more into it.

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick)]

Oh, sorry for the delay to reply. I skipped the needinfo request.

(In reply to Emilio Cobos Álvarez [:emilio] from comment #4)
> Actually, just took a quick look... So this is passing a destroyed NAC
> element to nsComputedDOMStyle::GetStyleContextNoFlush from:
> 
>  
> http://searchfox.org/mozilla-central/rev/d30462037ffea383e74c42542c820cf65b2b144e/editor/libeditor/EditorBase.cpp#3933
> 
> When we try to compute its style, we go and crash, since we can't find its
> nearest element in order to match it as a pseudo-element.
> 
> The node is an already-unbound ::-moz-progress-bar. This code looks pretty
> old, but I suspect we're not supposed to be dealing with out-of-doc content
> or NAC here.
> 
> Masayuki, you've been one of the recent people that have fiddled with this
> file. I have various ideas, more or less wallpaper-ish, but... Are you aware
> of a sane case where the node in:
> 
>   mRangeItem->mStartContainer
> 
>  
> http://searchfox.org/mozilla-central/rev/d30462037ffea383e74c42542c820cf65b2b144e/editor/libeditor/HTMLEditRules.cpp#521
> 
> Would be a NAC pseudo-element outside of the doc? (What about any element
> outside of the doc anyway?)

Hmm, mRangeItem is set to first range of Selection before editing.  Then, it's manually modified after running each edit transaction.  So, if there are some bugs in the code modifying mRangeItem, it's possible.  Unfortunately, this could happen in any cases if mutation observer runs when editor modifies the DOM tree.  So, wallpaper-ish patch might be better than trying to fix this bug completely.

Comment 13

[James Teh [:Jamie]]

How do I reproduce this crash with this test case? I was trying to test whether the patch in bug 1454572 fixes this too. However, I can't reproduce these on Nightly nor Firefox 60. I was able to repro in 57. Has this perhaps been fixed somewhere else for this case?

Comment 14

[Emilio Cobos Álvarez (:emilio)]

I think it's been fixed in this time... Finding a regression range should be helpful, but otherwise we should land the crashtest. I'll do that tomorrow morning if nobody wins me to it.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f113bac293a
Add crashtest. r=me

Comment 16

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/0f113bac293a

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Knowing what fixed this would be useful. We do still see some crashes on recent Fx60 beta builds.

Comment 18

[James Teh [:Jamie]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #17)
> Knowing what fixed this would be useful. We do still see some crashes on
> recent Fx60 beta builds.

The specific test case in this bug isn't reproduceable on Firefox 60. However, other crashes with the same underlying cause (and signatures) are fixed by bug 1454572. I've requested uplift to 60 for that bug.

Comment 19

[Emilio Cobos Álvarez (:emilio)]

(In reply to James Teh [:Jamie] from comment #18)
> (In reply to Ryan VanderMeulen [:RyanVM] from comment #17)
> > Knowing what fixed this would be useful. We do still see some crashes on
> > recent Fx60 beta builds.
> 
> The specific test case in this bug isn't reproduceable on Firefox 60.
> However, other crashes with the same underlying cause (and signatures) are
> fixed by bug 1454572. I've requested uplift to 60 for that bug.

That's exactly right. Thanks Jamie!