1597273 - Hit MOZ_CRASH(internal error: entered unreachable code) at /builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/debug/build/style-3d3b100190f0a3e1/out/properties.rs:49920

Status: VERIFIED FIXED

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

Description

[Bob Clary [:bc] (inactive)]

Attachment: log

1. https://collab.sundance.org/people/Adeola-Hammed-Giwa-1558568874
2. Hit MOZ_CRASH(internal error: entered unreachable code) at /builds/worker/workspace/build/src/obj-firefox/x86_64-unknown-linux-gnu/debug/build/style-3d3b100190f0a3e1/out/properties.rs:49920

Windows/Linux Nightly opt and debug. Also reproduced locally on Fedora 31 with Beta.

bp-13ef4748-2a42-4411-9807-545a50191118

Operating system: Linux
                  0.0.0 Linux 5.3.11-200.fc30.x86_64 #1 SMP Tue Nov 12 19:25:25 UTC 2019 x86_64
CPU: amd64
     family 6 model 45 stepping 2
     2 CPUs

GPU: UNKNOWN

Crash reason:  SIGSEGV /SEGV_MAPERR
Crash address: 0x0
Process uptime: not available

Thread 0 (crashed)
 0  libxul.so!GeckoCrash [nsAppRunner.cpp:caf55914ccddba34d462a1206530d7868b6c4992 : 5103 + 0x15]
    rax = 0x000056216503de28   rdx = 0x00007ff4d06338b0
    rcx = 0x0000000000000c00   rbx = 0x00007fff22f63cea
    rsi = 0x0000000000000000   rdi = 0x00007ff4d06326a0
    rbp = 0x00007fff22f63ad0   rsp = 0x00007fff22f63ac0
     r8 = 0x0000000000000000    r9 = 0x00007fff22f62e70
    r10 = 0x00007fff22f62d24   r11 = 0x0000000000000000
    r12 = 0x000000000000c300   r13 = 0x000000000000007c
    r14 = 0x000000000000c300   r15 = 0x0000000000000028
    rip = 0x00007ff4c8fa20b4
    Found by: given as instruction pointer in context
 1  libxul.so!gkrust_shared::panic_hook [lib.rs:caf55914ccddba34d462a1206530d7868b6c4992 : 250 + 0x9]
    rbx = 0x00007fff22f63fb0   rbp = 0x00007fff22f63f20
    rsp = 0x00007fff22f63ae0   r12 = 0x000000000000c300
    r13 = 0x000000000000007c   r14 = 0x00007ff4cc7a5f9c
    r15 = 0x0000000000000028   rip = 0x00007ff4ca051cba
    Found by: call frame info
 2  libxul.so!core::ops::function::Fn::call [function.rs:625451e376bb2e5283fc4741caa0a3e8a2ca4d54 : 69 + 0xc]
    rbx = 0x00007ff4cdb86e58   rbp = 0x00007fff22f63f30
    rsp = 0x00007fff22f63f30   r12 = 0x00007ff4cdbf2a18
    r13 = 0x00007fff22f64020   r14 = 0x0000000000000001
    r15 = 0x0000000000000001   rip = 0x00007ff4ca050aec
    Found by: call frame info
 3  libxul.so!std::panicking::rust_panic_with_hook [panicking.rs:625451e376bb2e5283fc4741caa0a3e8a2ca4d54 : 481 + 0x6]
    rbx = 0x00007ff4cdb86e58   rbp = 0x00007ff4cc805690
    rsp = 0x00007fff22f63f40   r12 = 0x00007ff4cdbf2a18
    r13 = 0x00007fff22f64020   r14 = 0x0000000000000001
    r15 = 0x0000000000000001   rip = 0x00007ff4caa9fbe6
    Found by: call frame info
 4  libxul.so!std::panicking::begin_panic [panicking.rs:625451e376bb2e5283fc4741caa0a3e8a2ca4d54 : 411 + 0x8]
    rbx = 0x0000000000000001   rbp = 0x00007fff22f64030
    rsp = 0x00007fff22f64020   r12 = 0x00007ff4ae79bb20
    r13 = 0x0000000000000001   r14 = 0x00007ff4ae530400
    r15 = 0x00007fff22f644e0   rip = 0x00007ff4ca6e8ad6
    Found by: call frame info
 5  libxul.so!style::properties::UnparsedValue::substitute_variables::{{closure}} [properties.rs: : 0 + 0xa]
    rbx = 0x0000000000000001   rbp = 0x00007fff22f64440
    rsp = 0x00007fff22f64040   r12 = 0x00007ff4ae79bb20
    r13 = 0x0000000000000001   r14 = 0x00007ff4ae530400
    r15 = 0x00007fff22f644e0   rip = 0x00007ff4ca7ee4b9
    Found by: call frame info
 6  libxul.so!style::properties::UnparsedValue::substitute_variables [parser.rs:caf55914ccddba34d462a1206530d7868b6c4992 : 634 + 0x8]
    rbx = 0x00007fff22f644e0   rbp = 0x00007fff22f64620
    rsp = 0x00007fff22f64450   r12 = 0x00007fff22f64700
    r13 = 0x0000000000000002   r14 = 0x00007ff4ae530400
    r15 = 0x00007fff22f64458   rip = 0x00007ff4ca7e5360
    Found by: call frame info
 7  libxul.so!style::properties::animated_properties::AnimationValue::from_declaration [properties.rs: : 32342 + 0x1b]
    rbx = 0x0000000000000000   rbp = 0x00007fff22f64820
    rsp = 0x00007fff22f64630   r12 = 0x00007fff22f64700
    r13 = 0x00007fff22f64830   r14 = 0x0000000000000000
    r15 = 0x00007ff4b088ce38   rip = 0x00007ff4ca666bcc
    Found by: call frame info
 8  libxul.so!Servo_GetComputedKeyframeValues [declaration_block.rs:caf55914ccddba34d462a1206530d7868b6c4992 : 197 + 0x1c]
    rbx = 0x00007ff4ae530360   rbp = 0x00007fff22f64d40
    rsp = 0x00007fff22f64830   r12 = 0x000000000000003f
    r13 = 0x00007ff4ae530340   r14 = 0x0000000000000000
    r15 = 0x0000000000000000   rip = 0x00007ff4ca5ac64d
    Found by: call frame info
 9  libxul.so!mozilla::ServoStyleSet::GetComputedKeyframeValuesFor(nsTArray<mozilla::Keyframe> const&, mozilla::dom::Element*, mozilla::ComputedStyle const*) [ServoStyleSet.cpp:caf55914ccddba34d462a1206530d7868b6c4992 : 901 + 0x8]
    rbx = 0x00007fff22f64e00   rbp = 0x00007fff22f64d80
    rsp = 0x00007fff22f64d50   r12 = 0x00007fff22f64e58
    r13 = 0x00007ff4b20f9000   r14 = 0x0000000000000003
    r15 = 0x00007ff4af6989d0   rip = 0x00007ff4c7fff9c6
    Found by: call frame info
10  libxul.so!mozilla::KeyframeUtils::GetAnimationPropertiesFromKeyframes(nsTArray<mozilla::Keyframe> const&, mozilla::dom::Element*, mozilla::ComputedStyle const*, mozilla::dom::CompositeOperation) [KeyframeUtils.cpp:caf55914ccddba34d462a1206530d7868b6c4992 : 281 + 0x5a]
    rbx = 0x00007ff4af6989d0   rbp = 0x00007fff22f64e40
    rsp = 0x00007fff22f64d90   r12 = 0x00007ff4cdcd4ca0
    r13 = 0xaaaaaaaaaaaaaaaa   r14 = 0x00007ff4ae52d978
    r15 = 0x00007fff22f64e00   rip = 0x00007ff4c633ecd6
    Found by: call frame info

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Mozregression points to bug 1520236.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: testcase

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1597273 - Handle logical shorthand animations with variable references correctly. r=birtles,hiro,boris

When we physicalize the declarations for @keyframes, we end up having a physical
declaration with an unparsed value with from_shorthand being the logical
shorthand.

Account for this case properly when substituting custom properties, to avoid
panicking.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9109695 [details]
Bug 1597273 - Handle logical shorthand animations with variable references correctly. r=birtles,hiro,boris

Beta/Release Uplift Approval Request

• User impact if declined: Crashes on pages that use @keyframes with logical shorthands.

These don't work in other browsers yet afaict, but seems some pages use them in the wild.

• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Visit the website in comment 0, shouldn't crash.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): handles a condition that was marked as unreachable. Really low-risk fix.
• String changes made/needed: none

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9109695 [details]
Bug 1597273 - Handle logical shorthand animations with variable references correctly. r=birtles,hiro,boris

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: trivial fix for release build crash that has been found in the wild.
• User impact if declined: crashes, see comment above.
• Fix Landed on Version: N/A
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Only adds code to handle a condition that otherwise crashes. Really low risk.
• String or UUID changes made by this patch: none

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6a9a26055f9c
Handle logical shorthand animations with variable references correctly. r=hiro

Comment 7

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/20311 for changes under testing/web-platform/tests

Comment 8

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 9

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/6a9a26055f9c

Comment 10

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 11

[Alexandru Trif, Desktop QA [:atrif]]

Hello! Reproduced the issue using Firefox 72.0a1 (20191118093852) on Windows 10x64 and STR from comment 0 and the test case provided in comment 2.
The issue is verified fixed with Firefox 72.0a1 (20191120215217) on Windows 10x64, macOS 10.14 and Ubuntu 18.04. No crashes encountered while visiting the link from comment 0 and test case from comment 2.

Comment 12

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9109695 [details]
Bug 1597273 - Handle logical shorthand animations with variable references correctly. r=birtles,hiro,boris

Crash fix in beta with significant volume, has tests and fix was verified on nightly by QA, uplift approved for 71 beta 12, thanks.

Comment 13

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/releases/mozilla-beta/rev/f979fad36ca3

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9109695 [details]
Bug 1597273 - Handle logical shorthand animations with variable references correctly. r=birtles,hiro,boris

This needs a rebased patch for ESR68.

Comment 15

[Alexandru Trif, Desktop QA [:atrif]]

Verified the fix with 71.0b12 (20191121155457) on Windows 10x64, macOS 10.14 and Ubuntu 18.04.

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Attachment: ESR68 patch

Comment 17

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9110803 [details] [diff] [review]
ESR68 patch

Crash fix, verified in beta, OK for uplift for 68.3esr.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/ddcd96385569

Comment 19

[Alexandru Trif, Desktop QA [:atrif]]

Verified using Firefox 68.3.0esr (20191122214540) from comment 18 on Windows 10x64, macOS 10.14 and Ubuntu 18.04. No crashes encountered while visiting the link from comment 0 and the test case from comment 2.