1652750 - Have WR not crash the browser when given deeply nested 3d transformed elements

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

Attachment: Testcase (warning: crashes browser with WR)

In trying to make a test case for bug 1629710 I ended up making a testcase that reliably kills the browser when loaded with WR enabled, possibly due to stack overflow. The testcase builds a 1000-deep set of nested divs, with a transform on each div, and preserve-3d on the body. When I run with lldb the point at which it gets interrupted the stack is 2134 frames deep, with many iterations of this:

    frame #2066: 0x000000010a538c20 XUL`webrender::scene_building::SceneBuilder::build_reference_frame::hd3d9fb77de0b0b4a(self=0x00007000067fdcb8, traversal=<unavailable>, pipeline_id=(__0 = 0, __1 = 14), parent_spatial_node=(__0 = 12), origin=(x = 0, y = 0, _unit = core::marker::PhantomData<webrender_api::units::LayoutPixel> @ 0x00007000067f33a8), reference_frame=0x00007000067f4098) at scene_building.rs:765:9 [opt]
    frame #2067: 0x000000010a538520 XUL`webrender::scene_building::SceneBuilder::build_items::h625e07851c83602b(self=0x00007000067fdcb8, traversal=0x00007000067f4048, pipeline_id=(__0 = 1, __1 = 14)) at scene_building.rs:621:21 [opt]
    frame #2068: 0x000000010a538e15 XUL`webrender::scene_building::SceneBuilder::build_stacking_context::h812ecb28532d112b(self=0x00007000067fdcb8, traversal=0x00007000067f4048, pipeline_id=(__0 = 1, __1 = 14), stacking_context=<unavailable>, spatial_node_index=(__0 = 12), origin=(x = 0, y = 0, _unit = core::marker::PhantomData<webrender_api::units::LayoutPixel> @ 0x00007000067f3c38), filters=ItemRange<webrender_api::display_item::FilterOp> @ 0x00007000067f3c38, filter_datas=&[webrender_api::display_list::TempFilterData] @ 0x00007f8d4f873c60, filter_primitives=ItemRange<webrender_api::display_item::FilterPrimitive> @ 0x00007000067f3c50, prim_flags=(bits = '\x01')) at scene_building.rs:813:9 [opt]
    frame #2069: 0x000000010a5385ea XUL`webrender::scene_building::SceneBuilder::build_items::h625e07851c83602b(self=0x00007000067fdcb8, traversal=0x00007000067f47f8, pipeline_id=(__0 = 1, __1 = 14)) at scene_building.rs:605:21 [opt]
    frame #2070: 0x000000010a538c20 XUL`webrender::scene_building::SceneBuilder::build_reference_frame::hd3d9fb77de0b0b4a(self=0x00007000067fdcb8, traversal=<unavailable>, pipeline_id=(__0 = 0, __1 = 14), parent_spatial_node=(__0 = 11), origin=(x = 0, y = 0, _unit = core::marker::PhantomData<webrender_api::units::LayoutPixel> @ 0x00007000067f4328), reference_frame=0x00007000067f5018) at scene_building.rs:765:9 [opt]

Testcase is attached. Marking this bug as security sensitive just in case there's some sort of stack-smashing vulnerability potential here.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

I loaded the page in a recent nightly and got this crash report: https://crash-stats.mozilla.org/report/index/21aa5808-9d85-481f-875e-044910200714

Comment 2

[Jessie [:jbonisteel] pls NI]

Dzmitry, can you take a look at this?

Comment 3

[Daniel Veditz [:dveditz]]

Not sure this needs to be hidden, recursion is generally one known way to crash us. I thought one point of WR was that the browser could keep running if WR crashed, to help our stability story.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

If there's a GPU process (i.e. running on Windows) then WR crashes won't take down the browser. Kats was running macOS which doesn't have a GPU process

Comment 5

[Dzmitry Malyshau [:kvark]]

Attachment: Unroll WR scene building recursion

instead of using the stack for all the scene building containers,
we are making the stack explicitly, and doing all the work inside the
build_all function. This affects reference frames, iframes, and stacking
contexts.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3d9324ca6510
Unroll WR scene building recursion r=gw

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/3d9324ca6510

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Might be a good idea to add the testcase as a crashtest?

Comment 9

[Dzmitry Malyshau [:kvark]]

Yes, agreed.

Comment 10

[Dzmitry Malyshau [:kvark]]

Attachment: Add WR stack overflow test

adds the gfx crashtest that kats produced

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/262aea7f6809
Add WR stack overflow test r=kats

Comment 12

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/262aea7f6809