Closed Bug 1411008 Opened 3 years ago Closed 2 years ago

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

Categories

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

57 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox56 --- disabled
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- wontfix
firefox60 --- fixed
firefox61 --- fixed

People

(Reporter: jkratzer, Assigned: Jamie)

References

(Blocks 2 open bugs)

Details

(Keywords: assertion, crash, testcase)

Crash Data

Attachments

(3 files)

Attached file 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
Flags: in-testsuite?
Attached file log_minidump.txt
Attached file log_stderr.txt
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.
Flags: needinfo?(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 :)
Flags: needinfo?(masayuki)
Crashes plain opt builds for me too. Reproduces all the way back to when we started building Stylo by default in automation.
Has Regression Range: --- → no
Keywords: crash
Version: unspecified → 57 Branch
status-firefox57=wontfix unless someone thinks this bug should block 57
See Also: → 1418856
Severity: normal → critical
Duplicate of this bug: 1406601
Duplicate of this bug: 1418856
Crash Signature: [@ mozalloc_abort | abort | style::style_resolver::StyleResolverForElement<T>::match_primary<T> ] [@ style::style_resolver::StyleResolverForElement<T>::match_primary<T> ]
Adding a signature I saw in crash stats that mapped to the attachment in this bug.
Crash Signature: [@ mozalloc_abort | abort | style::style_resolver::StyleResolverForElement<T>::match_primary<T> ] [@ style::style_resolver::StyleResolverForElement<T>::match_primary<T> ] → [@ mozalloc_abort | abort | style::style_resolver::StyleResolverForElement<T>::match_primary<T> ] [@ style::style_resolver::StyleResolverForElement<T>::match_primary<T> ] [@ style::dom::TElement::rule_hash_target<T>]
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.
Flags: needinfo?(emilio)
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.
Flags: needinfo?(masayuki)
Duplicate of this bug: 1432633
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?
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.
Flags: needinfo?(emilio)
Flags: needinfo?(emilio)
https://hg.mozilla.org/mozilla-central/rev/0f113bac293a
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Knowing what fixed this would be useful. We do still see some crashes on recent Fx60 beta builds.
Flags: needinfo?(emilio)
Flags: in-testsuite?
Flags: in-testsuite+
(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.
(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!
Flags: needinfo?(emilio)
Assignee: nobody → jteh
You need to log in before you can comment on or make changes to this bug.