1475772 - Crash in static union core::result::Result<T> style::properties::LonghandId::parse_value

Status: RESOLVED WONTFIX

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

Description

[[:philipp]]

This bug was filed from the Socorro interface and is
report bp-8810a7a8-c58b-4ad0-a408-237b30180711.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll static union core::result::Result<style::properties::PropertyDeclaration, cssparser::parser::ParseError<style_traits::StyleParseErrorKind>> style::properties::LonghandId::parse_value toolkit/library/i686-pc-windows-msvc/release/build/style-842031952ce87c63/out/properties.rs:94184
1 xul.dll static union core::result::Result< toolkit/library/i686-pc-windows-msvc/release/build/style-842031952ce87c63/out/properties.rs:104411
2 xul.dll static union core::result::Result<style::properties::declaration_block::Importance, cssparser::parser::ParseError<style_traits::StyleParseErrorKind>> style::properties::declaration_block::{{impl}}::parse_value servo/components/style/properties/declaration_block.rs:1182
3 xul.dll static union core::option::Option<core::result::Result<style::properties::declaration_block::Importance,  third_party/rust/cssparser/src/rules_and_declarations.rs:250
4 xul.dll static struct style::properties::declaration_block::PropertyDeclarationBlock style::properties::declaration_block::parse_property_declaration_list<style::error_reporting::NullReporter> servo/components/style/properties/declaration_block.rs:1225
5 xul.dll static union core::result::Result<style::stylesheets::CssRule, cssparser::parser::ParseError<style_traits::StyleParseErrorKind>> style::stylesheets::rule_parser::{{impl}}::parse_block<style::error_reporting::NullReporter> servo/components/style/stylesheets/rule_parser.rs:585
6 xul.dll static union core::option::Option<core::result::Result<style::stylesheets::CssRule,  third_party/rust/cssparser/src/rules_and_declarations.rs:369
7 xul.dll static struct style::stylesheets::stylesheet::StylesheetContents style::stylesheets::stylesheet::StylesheetContents::from_str<style::error_reporting::NullReporter> servo/components/style/stylesheets/stylesheet.rs:89
8 xul.dll static void geckoservo::stylesheet_loader::AsyncStylesheetParser::parse servo/ports/geckolib/stylesheet_loader.rs:106
9 xul.dll void geckoservo::glue::Servo_StyleSheet_FromUTF8BytesAsync servo/ports/geckolib/glue.rs:1233

=============================================================

this crash signature on windows has been present for a while already, but had no bug report filed yet.

Comment 1

[Cameron McCormack (:heycam)]

They're mostly startup crashes, presumably parsing UA sheets, although the comment 0 crash is later on.  It's concerning that some of the crashes are EXCEPTION_ACCESS_VIOLATION_EXEC.

Comment 2

[Cameron McCormack (:heycam)]

windbg on one of the crash's minidumps gave me a stack trace with some more (inlined function) frames in it:

 # ChildEBP RetAddr  
...
0e 002bbbb4 14a00a90 ntdll!KiUserExceptionDispatcher+0xf
WARNING: Frame IP not in any known module. Following frames may be wrong.
0f 002bbec8 54b7b68e 0x14a00a90
10 (Inline) -------- xul!core::ptr::drop_in_place+0x5 [C:\projects\rust\src\libcore\ptr.rs @ 59] 
11 (Inline) -------- xul!core::ptr::drop_in_place+0xb [C:\projects\rust\src\libcore\ptr.rs @ 59] 
12 002bbf84 54b07276 xul!style::values::specified::text::{{impl}}::parse+0x11e [z:\build\build\src\servo\components\style\values\specified\text.rs @ 82] 
13 (Inline) -------- xul!style::properties::longhands::line_height::parse+0x6 [z:\build\build\src\obj-firefox\toolkit\library\i686-pc-windows-msvc\release\build\style-25f47d697d295d08\out\longhands\inherited_text.rs @ 66] 
14 (Inline) -------- xul!style::properties::longhands::line_height::parse_declared+0x6 [z:\build\build\src\obj-firefox\toolkit\library\i686-pc-windows-msvc\release\build\style-25f47d697d295d08\out\longhands\inherited_text.rs @ 114] 
15 002bc5c8 54af3324 xul!style::properties::LonghandId::parse_value+0x3dc6 [z:\build\build\src\obj-firefox\toolkit\library\i686-pc-windows-msvc\release\build\style-25f47d697d295d08\out\properties.rs @ 38635] 
16 (Inline) -------- xul!style::properties::{{impl}}::parse_into::{{closure}}::{{closure}}+0x17 [z:\build\build\src\obj-firefox\toolkit\library\i686-pc-windows-msvc\release\build\style-25f47d697d295d08\out\properties.rs @ 49905] 
17 (Inline) -------- xul!cssparser::parser::Parser::parse_entirely+0x17 [z:\build\build\src\third_party\rust\cssparser\src\parser.rs @ 594] 
18 (Inline) -------- xul!style::properties::{{impl}}::parse_into::{{closure}}+0x17 [z:\build\build\src\obj-firefox\toolkit\library\i686-pc-windows-msvc\release\build\style-25f47d697d295d08\out\properties.rs @ 49905] 
19 (Inline) -------- xul!core::result::Result<style::properties::PropertyDeclaration, ()>::or_else+0x17 [C:\projects\rust\src\libcore\result.rs @ 691] 
1a 002bca0c 54ba6eb3 xul!style::properties::PropertyDeclaration::parse_into+0x5e4 [z:\build\build\src\obj-firefox\toolkit\library\i686-pc-windows-msvc\release\build\style-25f47d697d295d08\out\properties.rs @ 49942] 
1b (Inline) -------- xul!cssparser::rules_and_declarations::{{impl}}::next+0xb9b [z:\build\build\src\third_party\rust\cssparser\src\rules_and_declarations.rs @ 252]
...

Comment 3

[Cameron McCormack (:heycam)]

The first drop_in_place call, according to the disassembly, is to drop a StyleParseErrorKind.  Since drop_in_place is called recursively here I suppose it could be dropping a CowRcStr in it (which does some unsafe stuff).  But it's not clear to me why we'd end up jumping off to 0x14a00a90.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Hmm, there are no nightly crashes at all? :(

Comment 5

[Cameron McCormack (:heycam)]

For the comment 0 crash:

0:000> .excr
eax=175054ab ebx=00000004 ecx=00000004 edx=00000001 esi=12980588 edi=0028e038
eip=6a1bdd7b esp=0028dc40 ebp=0028e270 iopl=0         nv up ei pl nz ac po nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00010212
xul!core::slice::{{impl}}::equal+0x14 [inlined in xul!style::properties::LonghandId::parse_value+0x38a8b]:
6a1bdd7b 0083f9070f85    add     byte ptr [ebx-7AF0F807h],al ds:0023:850f07fd=??
0:000> !chkimg -d
    6a1bdd5d-6a1bdd60  4 bytes - xul!style::properties::LonghandId::parse_value+38a6d
	[ 86 6c 85 c0:98 12 73 1a ]
    6a1bdd8b-6a1bdd8e  4 bytes - xul!style::properties::LonghandId::parse_value+38a9b (+0x2e)
	[ 86 6c 74 18:98 12 62 72 ]
    6a1bdd94-6a1bdd97  4 bytes - xul!style::properties::LonghandId::parse_value+38aa4 (+0x09)
	[ 86 6c 50 e8:98 12 3e 42 ]
12 errors : @$ip (6a1bdd5d-6a1bdd97)

So we have some corruption shortly before the crashing address.

And picking a few random other crashes out:

* https://crash-stats.mozilla.com/report/index/e2ac8d76-48bf-41f3-a592-609460180719

0:000> .excr
eax=00000005 ebx=64a3033c ecx=00000004 edx=000098c9 esi=002eec58 edi=000098a7
eip=64a4175f esp=002ed908 ebp=002edf40 iopl=0         nv up ei pl nz na po nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00010202
xul!cssparser::parser::Parser::expect_exhausted+0x9f [inlined in xul!style::properties::LonghandId::parse_value+0xe2af]:
64a4175f 228b4424188b    and     cl,byte ptr [ebx-74E7DBBCh] ds:0023:efbb2780=??
0:000> !chkimg -d
    64a4175a - xul!style::properties::LonghandId::parse_value+e2aa
	[ 89:a9 ]
1 error : @$ip (64a4175a)

This is a random bit flip in the code just before the crashing address.

* https://crash-stats.mozilla.com/report/index/7721358d-127e-4c9b-9a02-49a930180719

0:000> .excr
eax=00000000 ebx=0013e294 ecx=40000000 edx=00000000 esi=0013d8c0 edi=0013dc84
eip=0ff43007 esp=0013d230 ebp=0013d860 iopl=0         nv up ei ng nz ac pe cy
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00010297
xul!core::result::Result<style::values::specified::length::Length, cssparser::parser::ParseError<style_traits::StyleParseErrorKind>>::map+0x9 [inlined in xul!style::properties::LonghandId::parse_value+0xdd17]:
0ff43007 895024          mov     dword ptr [eax+24h],edx ds:0023:00000024=????????
0:000> !chkimg -d
    0ff43008 - xul!style::properties::LonghandId::parse_value+dd18
	[ 4c:50 ]
    0ff43048 - xul!style::properties::LonghandId::parse_value+dd58 (+0x40)
	[ 88:90 ]
2 errors : @$ip (0ff43008-0ff43048)

Corruption on the instruction we crashed on.

* https://crash-stats.mozilla.com/report/index/2915ba7c-a877-4bc3-95d6-a4bd10180714

0:000> .excr
eax=00000001 ebx=5c32b8cf ecx=00000000 edx=00000001 esi=010b8b00 edi=010b8b00
eip=5c335601 esp=010b8460 ebp=010b8a98 iopl=0         nv up ei pl zr na pe nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010246
xul!core::result::Result<style::values::specified::length::LengthOrPercentageOrAuto, cssparser::parser::ParseError<style_traits::StyleParseErrorKind>>::map+0x4c [inlined in xul!style::properties::LonghandId::parse_value+0x9d51]:
5c335601 0000            add     byte ptr [eax],al          ds:002b:00000001=??
0:000> !chkimg -d
    5c335401 - xul!style::properties::LonghandId::parse_value+9b51
	[ 01:00 ]
    5c335404-5c33540f  12 bytes - xul!style::properties::LonghandId::parse_value+9b54 (+0x03)
	[ f2 0f 10 46 14 f2 0f 11:00 00 00 00 00 00 00 00 ]
    5c335412-5c33541d  12 bytes - xul!style::properties::LonghandId::parse_value+9b62 (+0x0e)
	[ f2 0f 10 46 04 f2 0f 11:00 00 00 00 00 00 00 00 ]
    5c335420-5c335426  7 bytes - xul!style::properties::LonghandId::parse_value+9b70 (+0x0e)
	[ f2 0f 11 84 24 34 01:00 00 00 00 00 00 00 ]
    5c335429-5c33542f  7 bytes - xul!style::properties::LonghandId::parse_value+9b79 (+0x09)
	[ f2 0f 10 84 24 5c 01:00 00 00 00 00 00 00 ]
    5c335601-5c33560d  13 bytes - xul!style::properties::LonghandId::parse_value+9d51 (+0x1d8)
	[ f2 0f 10 54 24 74 f2 0f:00 00 00 00 00 00 00 00 ]
    5c335610-5c335627  24 bytes - xul!style::properties::LonghandId::parse_value+9d60 (+0x0f)
	[ f2 0f 10 4c 24 54 f2 0f:00 00 00 00 00 00 00 00 ]
    5c33562a-5c33562f  6 bytes - xul!style::properties::LonghandId::parse_value+9d7a (+0x1a)
	[ f2 0f 10 4c 24 6c:00 00 00 00 00 00 ]
    5c335800 - xul!style::properties::LonghandId::parse_value+9f50 (+0x1d6)
	[ 3c:00 ]
83 errors : @$ip (5c335401-5c335800)

A heap of corruption around the crashing address.


Since LonghandId::parse_value is a massive function with lots of stuff inlined in it, it's more likely to show up as a result of random memory corruption problems, so I'm thinking that is what we mostly have here.

I wonder if it's worth sticking #[inline(never)] on the parse_declared functions to force parse_value to get split up.

Comment 6

[Cameron McCormack (:heycam)]

(And thank you to dmajor who suggested the !chkimg command, which I'll be using on every crash report I look at from now on.)

Comment 7

[Bobby Holley (:bholley)]

Great work Cameron!

Comment 8

[Mats Palmgren (inactive)]

Nice work indeed.  Since it's a single-bit error it seems likely
to be due to hardware error, right?  If so, I don't really see
a reason to keep this bug open.

Comment 9

[Jonathan Watt [:jwatt]]

heycam: you said in the Layout standup meeting that you filed a bug to have crash-stats.m.c check for corruption automatically. Can you drop a link to that bug here for reference?

Also, as mats said, can this be closed?

Comment 10

[Will Kahn-Greene [:willkg] ET needinfo? me]

Pretty sure it's this one: https://bugzilla.mozilla.org/show_bug.cgi?id=1274628

Comment 11

[Sean Voisen (:svoisen)]

Going to close this as part of security bug scrub given that it's a hardware corruption error.