Crash on recursive mask which refers to itself

Bug #190130 reported by Lubomir Rintel
4
Affects Status Importance Assigned to Milestone
Inkscape
Fix Released
High
Unassigned

Bug Description

Inkscape dumps core when attempting to open 384637-1.svg from Firefox crash tests. Traces back somewhere to boehm-gc.

Tags: crash

Related branches

Revision history for this message
Lubomir Rintel (lkundrak) wrote :

(gdb) bt
#0 0x0000003d10c178b6 in GC_clear_stack_inner (arg=0x4ebfc80, limit=0x7fffc1347d20 <Address 0x7fffc1347d20 out of bounds>) at misc.c:243
#1 0x0000003d10c178cb in GC_clear_stack_inner (arg=0x4ebfc80, limit=0x7fffc1347d20 <Address 0x7fffc1347d20 out of bounds>) at misc.c:245
#2 0x0000003d10c1794b in GC_clear_stack (arg=0x4ebfc80) at misc.c:291
#3 0x00000000007d93c1 in NRObject::alloc (type=7) at ./gc-core.h:74
#4 0x00000000004b307d in sp_shape_show (item=0x1800060, arena=0x1186f00) at libnr/nr-object.h:115
#5 0x00000000004976b8 in sp_item_invoke_show (item=0x1800060, arena=0x1186f00, key=84190, flags=2) at sp-item.cpp:964
#6 0x000000000049bda8 in sp_mask_show (mask=0x17fd840, arena=0x1186f00, key=84190) at sp-mask.cpp:318
#7 0x0000000000497852 in sp_item_invoke_show (item=0x1800060, arena=0x1186f00, key=<value optimized out>, flags=<value optimized out>) at sp-item.cpp:1001
#8 0x000000000049bda8 in sp_mask_show (mask=0x17fd840, arena=0x1186f00, key=84187) at sp-mask.cpp:318
#9 0x0000000000497852 in sp_item_invoke_show (item=0x1800060, arena=0x1186f00, key=<value optimized out>, flags=<value optimized out>) at sp-item.cpp:1001
#10 0x000000000049bda8 in sp_mask_show (mask=0x17fd840, arena=0x1186f00, key=84184) at sp-mask.cpp:318
#11 0x0000000000497852 in sp_item_invoke_show (item=0x1800060, arena=0x1186f00, key=<value optimized out>, flags=<value optimized out>) at sp-item.cpp:1001
#12 0x000000000049bda8 in sp_mask_show (mask=0x17fd840, arena=0x1186f00, key=84181) at sp-mask.cpp:318
#13 0x0000000000497852 in sp_item_invoke_show (item=0x1800060, arena=0x1186f00, key=<value optimized out>, flags=<value optimized out>) at sp-item.cpp:1001
#14 0x000000000049bda8 in sp_mask_show (mask=0x17fd840, arena=0x1186f00, key=84178) at sp-mask.cpp:318
#15 0x0000000000497852 in sp_item_invoke_show (item=0x1800060, arena=0x1186f00, key=<value optimized out>, flags=<value optimized out>) at sp-item.cpp:1001
#16 0x000000000049bda8 in sp_mask_show (mask=0x17fd840, arena=0x1186f00, key=84175) at sp-mask.cpp:318
...
(gdb) l
238 /*ARGSUSED*/
239 void * GC_clear_stack_inner(void *arg, ptr_t limit)
240 {
241 word dummy[CLEAR_SIZE];
242
243 BZERO(dummy, CLEAR_SIZE*sizeof(word));
244 if ((ptr_t)(dummy) COOLER_THAN limit) {
245 (void) GC_clear_stack_inner(arg, limit);
246 }
247 /* Make sure the recursive call is not a tail call, and the bzero */
(gdb)

Revision history for this message
Lubomir Rintel (lkundrak) wrote :

It seems it's just stack memory consumption due to infinite recursion. It just hangs when I unlimit stack size.

bbyak (buliabyak)
Changed in inkscape:
importance: Undecided → High
status: New → Confirmed
tags: added: crash
Revision history for this message
Alex Leone (acleone) wrote :

This patch adds a warning and removes the mask reference.

When run with the reproducer file above:

$ ~/opt/local/bin/inkscape ~/Desktop/384637-1.svg

** (inkscape:29588): WARNING **: Recursive mask reference '#mask1' removed

Revision history for this message
Jon A. Cruz (jon-joncruz) wrote :

That we are changing data the user has in his contents is bad. However, code in that specific area does the same thing.

We probably can commit this patch as long as we open a new bug to address the data loss. Inkscape should stop processing there, but leave the reference in place (just stop following it). We should warn in the status area, and probably highlight in the XML Editor. However... that is a refinement for later.

Revision history for this message
Alex Leone (acleone) wrote :

There was a bug in the old patch: editing the mask attribute (adding a space to the end, eg "url(#mask1) ") in the XML editor would cause infinite recursion at sp-item.cpp:497 item->mask_ref->attach(Inkscape::URI(uri));

...
#46772 0x08106d79 in sp_mask_show (mask=0x9293008, arena=0x8eedf18, key=29)
    at sp-mask.cpp:316
#46773 0x080f6631 in sp_item_invoke_show (item=0x9296090, arena=0x8eedf18, key=26,
    flags=2) at sp-item.cpp:1151
#46774 0x08106d79 in sp_mask_show (mask=0x9293008, arena=0x8eedf18, key=26)
    at sp-mask.cpp:316
#46775 0x080f460e in mask_ref_changed (old_mask=0x0, mask=0x9293008, item=0x9296090)
    at sp-item.cpp:610
#46776 0x08156897 in sigc::internal::signal_emit2<void, SPObject*, SPObject*, sigc::nil>::emit (this=0x9295f70, obj=0x9293008) at /usr/include/sigc++-2.0/sigc++/signal.h:836
#46777 sigc::signal2<void, SPObject*, SPObject*, sigc::nil>::emit (this=0x9295f70,
    obj=0x9293008) at /usr/include/sigc++-2.0/sigc++/signal.h:1928
#46778 Inkscape::URIReference::_setObject (this=0x9295f70, obj=0x9293008)
    at uri-references.cpp:124
#46779 0x08156e28 in Inkscape::URIReference::attach (this=0x9295f70, uri=...)
    at uri-references.cpp:94
#46780 0x080f511b in sp_item_set (object=0x9296090, key=313,
    value=0x9d57110 "url(#mask1) ") at sp-item.cpp:497
#46781 0x08105d14 in sp_lpe_item_set (object=0x9296090, key=141788168,
    value=0x9d57110 "url(#mask1) ") at sp-lpe-item.cpp:247
#46782 0x0810b374 in sp_object_read_attr (object=0x9296090, key=0x8bbe250 "mask")
    at sp-object.cpp:1072
#46783 0x0810b458 in sp_object_repr_attr_changed (key=0x8bbe250 "mask",
    is_interactive=false, data=0x9296090) at sp-object.cpp:1084
#46784 0x085ceadd in notifyAttributeChanged (this=0x8eeafe0, node=..., name=2088,
    old_value=..., new_value=...) at xml/composite-node-observer.cpp:144
#46785 0x085cf128 in Inkscape::XML::CompositeNodeObserver::notifyAttributeChanged (
    this=0x8ee5530, node=..., name=2088, old_value=..., new_value=...)
    at xml/composite-node-observer.cpp:94
#46786 0x08365dd0 in Inkscape::XML::SimpleNode::setAttribute (this=0x8ee5500,
    name=0xb0ed000 "mask", value=0xaada730 "url(#mask1) ") at xml/simple-node.cpp:356
#46787 0x081987f3 in cmd_set_attr () at dialogs/xml-tree.cpp:1461
#46788 0x08198984 in sp_xml_tree_key_press (event=0xb08bb20)
    at dialogs/xml-tree.cpp:624

I moved the recursive check into sp-mask.h:54 SPMaskReference::_acceptObject(SPObject *obj). Essentially the reference is set to NULL (in uri-references.cpp:108 URIReference::_setObject(SPObject *obj)), but the attribute is still set to "url(#mask1) ".

$ ~/opt/local/bin/inkscape ~/Desktop/384637-1.svg

** (inkscape:8632): WARNING **: Ignoring recursive mask reference <svg:rect mask="url(#mask1)"> in <svg:mask id="mask1">

Revision history for this message
ScislaC (scislac) wrote :

Tested and committed in bzr r8959. Thank you Alex!

Changed in inkscape:
status: Confirmed → Fix Committed
milestone: none → 0.48
Revision history for this message
bbyak (buliabyak) wrote :

Thanks for the patch, but I think we have exactly this problem with clipboxes as well. Can you please look into it? The fix might be similar.

Revision history for this message
bbyak (buliabyak) wrote :

err, by clipboxes I of course meant "clipping paths"

Revision history for this message
ScislaC (scislac) wrote :

bbyak, I adapted this patch to protect from crashing on clipping paths too, committed in r8969

jazzynico (jazzynico)
Changed in inkscape:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Bug attachments

Remote bug watches

Bug watches keep track of this bug in other bug trackers.