bug#59029: 29.0.50; noverlay: pdumper.c: dump_interval_node recursion has no base case

[Gerd Möllmann]

Matt Armstrong <matt@rfc20.org> writes:

> X-Debbugs-cc: Stefan Monnier <monnier@iro.umontreal.ca>
>
> This has been in my head for weeks but I haven't had time to dig into
> it.  Best get it in a bug.
>
> See the code for dump_interval_node() in pdumper.c below.
>
> Imagine 'node' has a left child.  It will recurse to that child on line
> 35.  That child will recurse back to its parent on line 30.  That parent
> will recurse back to its left child on line 35.  This will repeat until
> the stack blows.  All you need is two nodes in the tree.
>
> This is not an immediate issue today because apparently Emacs does not
> dump any buffers with overlays present, or at least, never more than one
> overlay.  I suspect the right fix is to delete lines 26-30, or something
> like that, but I can't claim I understand this code.
>
>      1        static dump_off
>      2        dump_interval_node (struct dump_context *ctx, struct itree_node 
> *node,
>      3                            dump_off parent_offset)
>      4        {
>      5        #if CHECK_STRUCTS && !defined (HASH_itree_node_50DE304F13)
>      6        # error "itree_node changed. See CHECK_STRUCTS comment in 
> config.h."
>      7        #endif
>      8          struct itree_node out;
>      9          dump_object_start (ctx, &out, sizeof (out));
>     10          if (node->parent)
>     11            dump_field_fixup_later (ctx, &out, node, &node->parent);
>     12          if (node->left)
>     13            dump_field_fixup_later (ctx, &out, node, &node->parent);
>     14          if (node->right)
>     15            dump_field_fixup_later (ctx, &out, node, &node->parent);
>     16          DUMP_FIELD_COPY (&out, node, begin);
>     17          DUMP_FIELD_COPY (&out, node, end);
>     18          DUMP_FIELD_COPY (&out, node, limit);
>     19          DUMP_FIELD_COPY (&out, node, offset);
>     20          DUMP_FIELD_COPY (&out, node, otick);
>     21          dump_field_lv (ctx, &out, node, &node->data, WEIGHT_STRONG);
>     22          DUMP_FIELD_COPY (&out, node, red);
>     23          DUMP_FIELD_COPY (&out, node, rear_advance);
>     24          DUMP_FIELD_COPY (&out, node, front_advance);
>     25          dump_off offset = dump_object_finish (ctx, &out, sizeof 
> (out));
>     26          if (node->parent)
>     27              dump_remember_fixup_ptr_raw
>     28                (ctx,
>     29                 offset + dump_offsetof (struct itree_node, parent),
>     30                 dump_interval_node (ctx, node->parent, offset));
>     31          if (node->left)
>     32              dump_remember_fixup_ptr_raw
>     33                (ctx,
>     34                 offset + dump_offsetof (struct itree_node, left),
>     35                 dump_interval_node (ctx, node->left, offset));
>     36          if (node->right)
>     37              dump_remember_fixup_ptr_raw
>     38                (ctx,
>     39                 offset + dump_offsetof (struct itree_node, right),
>     40                 dump_interval_node (ctx, node->right, offset));
>     41          return offset;
>     42        }

Yes, I think you are right.

Could we also rename dump_interval_node to dump_itree_node?  There is
another function dump_interval_tree for text properties, which is a bit
confusing.