[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#59029: 29.0.50; noverlay: pdumper.c: dump_interval_node recursion ha
From: |
Gerd Möllmann |
Subject: |
bug#59029: 29.0.50; noverlay: pdumper.c: dump_interval_node recursion has no base case |
Date: |
Sat, 05 Nov 2022 06:41:41 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
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.