GNOME Bugzilla – Bug 776932
Singular matrices crash librsvg renderers (Rust panicked)
Last modified: 2017-03-16 14:48:00 UTC
Running for example rsvg-convert -b white -w 480 -h 360 coords-trans-09-t.svg -o out.png results in thread '<unnamed>' panicked at 'Cairo error "invalid matrix (not invertible)"', /home/massimo/.cargo/git/checkouts/cairo-e6055a6d391c3fc9/30699593d8949a6306f29a7bf9614da1befad561/cairo-sys-rs/src/enums.rs:70 See https://www.w3.org/Graphics/SVG/Test/20110816/harness/htmlSVGWeb/coords-trans-09-t.html it uses a <g transform="matrix(0 0 0 0 0 0)"> element so I think it should be considered valid input.
I'm working on this. My strategy is to bite the bullet and finally add error tracking to librsvg. A Node will hold a "result" field, which is a Result<(), node::Error>. In turn, the node::Error will be able to hold things like parse errors or invalid value errors for attributes. These will be collected when nodes are being created, and when they set_atts() to parse their attributes. At rendering time, we will ignore elements which hold an error result. I am not decided if this should be done at the node level, or at the code which calls Node::draw() yet. In this specific bug, the crash happens here:
+ Trace 237218
The 0-transform from the group, which comes in the "state" argument to rsvg_state_reinherit_top()... (gdb) p state->affine $73 = {xx = 0, yx = 0, xy = 0, yy = 0, x0 = 0, y0 = 0} The ctx->state has a valid, identity transform at the start of this function: (gdb) p ctx->state->affine $74 = {xx = 1, yx = 0, xy = 0, yy = 1, x0 = 0, y0 = 0} But by the time rsvg_state_reinherit_top() finishes, the ctx->state has inherited the 0-transform: (gdb) finish Run till exit from #0 rsvg_state_reinherit_top (ctx=0x6ffa60, state=0x8d9d40, dominate=0) at rsvg-styles.c:1738 103 rsvg_push_discrete_layer (ctx); (gdb) p ctx->state->affine $75 = {xx = 0, yx = 0, xy = 0, yy = 0, x0 = 0, y0 = 0} The surrounding code, for context: (gdb) l 98 _rsvg_node_draw_children (RsvgNode *node, RsvgDrawingCtx *ctx, int dominate) 99 { 100 if (dominate != -1) { 101 rsvg_state_reinherit_top (ctx, rsvg_node_get_state (node), dominate); 102 103 rsvg_push_discrete_layer (ctx); 104 } 105 106 rsvg_node_foreach_child (node, draw_child, ctx); 107 _rsvg_node_draw_children() gets called for the group in question. It calls rsvg_state_reinherit_top() with the group's state, and the drawing ctx. At that point, ctx->state is valid. After the call to rsvg_state_reinherit_top(), ctx->state has a 0-transform. In the stack trace above, the faulty group node makes its first appearance in frame 7; that's a call to draw_child(node=0x6d4270, ...). This is the call from when the group's parent is drawing its children, i.e. the group itself and its siblings. a) The bug would not happen if Node::draw() would refuse to draw a node that is in error. b) Or, up the call stack, the bug would also not happen if rsvg_node_draw_from_stack() in frame 6 would refuse to render a node if the node is in error. If (a), then the node would not render, the bug would not happen, but the ancestor call to rsvg_node_draw_from_stack() would create an unnecessary inherited state in its stack. If (b), we would catch the node that is in error earlier, and avoid the extra inherited state. Maybe we should have both: (a) for general correctness, and (b) as an optimization?
Eh, Bugzilla ate my second stack trace and comments. After the stack trace in the comment above, and before "The 0-transform from the group...", please mentally insert the following: [talking about the first stack trace where the crash happens] But that is a secondary effect from the all-zeros transform rippling down the code. The transform gets created in RsvgNodeGroup's set_atts() method (the all-zeros transform comes in an SVG group). Later, when the group is about to be drawn, its state gets reinherited into the ctx->state, and the ctx->state ends up with an all-zeros transform. This is rsvg_state_reinherit_top() being called with the state from the group that has the 0-transform: (gdb) where
+ Trace 237219
Fixed with a slew of commits, starting with 356cfdf8dddfd4054c4d8308410ba0c19f848f1a - node.rs: Start defining node::Error type and ending with 3b118396dded327e1b7bce43dfe37ed518a19e65 - bgo#776932: Don't render nodes whose "transform" attribute is invalid This commit series adds infrastructure so that nodes will remember when they are "in error", per the SVG spec. We will refuse to render nodes that are in error. In this specific case, we parse a "transform" attribute and notice that it produced an invalid affine transformation matrix. We remember this error, and at rendering time we don't render the problematic node.