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

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

[Matt Armstrong]

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	}

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.

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

[Matt Armstrong]

[-- Attachment #1: Type: text/plain, Size: 411 bytes --]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> 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.

Attached renames the function and tags two related FIXMEs with this bug
number.  The root issue is that pdumping buffers with overlays is not
implemented.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0005-Add-FIXME-comments-for-overlays.patch --]
[-- Type: text/x-diff, Size: 3016 bytes --]

From 82c448f7f6eda810114151b5339d500fd6cf5826 Mon Sep 17 00:00:00 2001
From: Matt Armstrong <matt@rfc20.org>
Date: Sat, 5 Nov 2022 11:03:09 -0700
Subject: [PATCH 5/5] Add FIXME comments for overlays.

* src/pdumper.c (dump_itree_node): Renamed from dump_interval_node.
Add FIXME(Matt): comment for bug#59029.
(dump_buffer): Tag comment with FIXME bug#59029.
---
 src/pdumper.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/pdumper.c b/src/pdumper.c
index 0a5d96dbb7c..10b6f58bbd8 100644
--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -2134,8 +2134,8 @@ dump_marker (struct dump_context *ctx, const struct Lisp_Marker *marker)
 }
 
 static dump_off
-dump_interval_node (struct dump_context *ctx, struct itree_node *node,
-                    dump_off parent_offset)
+dump_itree_node (struct dump_context *ctx, struct itree_node *node,
+		 dump_off parent_offset)
 {
 #if CHECK_STRUCTS && !defined (HASH_itree_node_50DE304F13)
 # error "itree_node changed. See CHECK_STRUCTS comment in config.h."
@@ -2158,21 +2158,25 @@ dump_interval_node (struct dump_context *ctx, struct itree_node *node,
   DUMP_FIELD_COPY (&out, node, rear_advance);
   DUMP_FIELD_COPY (&out, node, front_advance);
   dump_off offset = dump_object_finish (ctx, &out, sizeof (out));
+  /* FIXME: bug#59029 We haven't implemented the code to dump overlays
+     that are part of a buffer.  The code below will recurse forever
+     if any of parent, left or right is non-NULL, but isn't worth
+     changing until we have a test case.  */
   if (node->parent)
       dump_remember_fixup_ptr_raw
 	(ctx,
 	 offset + dump_offsetof (struct itree_node, parent),
-	 dump_interval_node (ctx, node->parent, offset));
+	 dump_itree_node (ctx, node->parent, offset));
   if (node->left)
       dump_remember_fixup_ptr_raw
 	(ctx,
 	 offset + dump_offsetof (struct itree_node, left),
-	 dump_interval_node (ctx, node->left, offset));
+	 dump_itree_node (ctx, node->left, offset));
   if (node->right)
       dump_remember_fixup_ptr_raw
 	(ctx,
 	 offset + dump_offsetof (struct itree_node, right),
-	 dump_interval_node (ctx, node->right, offset));
+	 dump_itree_node (ctx, node->right, offset));
   return offset;
 }
 
@@ -2189,7 +2193,7 @@ dump_overlay (struct dump_context *ctx, const struct Lisp_Overlay *overlay)
   dump_remember_fixup_ptr_raw
     (ctx,
      offset + dump_offsetof (struct Lisp_Overlay, interval),
-     dump_interval_node (ctx, overlay->interval, offset));
+     dump_itree_node (ctx, overlay->interval, offset));
   return offset;
 }
 
@@ -2864,7 +2868,8 @@ dump_buffer (struct dump_context *ctx, const struct buffer *in_buffer)
   DUMP_FIELD_COPY (out, buffer, long_line_optimizations_p);
 
   if (buffer->overlays && buffer->overlays->root != NULL)
-    /* We haven't implemented the code to dump overlays.  */
+    /* FIXME: bug#59029 We haven't implemented the code to dump
+       overlays that are in a buffer.  */
     emacs_abort ();
   else
     out->overlays = NULL;
-- 
2.35.1

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> See the code for dump_interval_node() in pdumper.c below.

This code is indeed incorrect.  Currently we can only dump deleted
overlays (the dump of a buffer fails if there are overlays in it), so
the code has only been tested for overlays that have been deleted (and
whose parent/left/right fields are NULL, which is not actually
guaranteed for deleted overlays: it's the case if the overlay has been
`delete-overlay`ed, but not if it was deleted by `kill-buffer`, IIRC).

And I fully agree with Gerd it should say "itree" rather than
"interval".


        Stefan

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

[Gerd Möllmann]

On 05.11.22 19:09, Matt Armstrong wrote:
> Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> 
>> 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.
> 
> Attached renames the function and tags two related FIXMEs with this bug
> number.  The root issue is that pdumping buffers with overlays is not
> implemented.
> 

LGTM.

bug#59029: Dumping Emacs crashes when buffers have overlays

[Matt Armstrong]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> See the code for dump_interval_node() in pdumper.c below.
>
> This code is indeed incorrect.

I have time to work on this, but I don't have a clear path and would
like to hear opinions.

I see two options.

a) Drop support for dumping overlays in buffers, as if
`delete-all-overlays' were called for all buffers before dumping.  Fix
involves relaxing the abort() calls to merely print warnings.

b) Restore support, and test it.

For (b) I'm not keen on wiring up a single-purpose test under test/*
somewhere just to test pdumping with overlays.  How about hooking in to
loadup.el to populate a dummy buffer with overlays before dumping?
Emacs could delete it early, so it wouldn't be gone before users
noticed.

Or, are there other alternatives I'm missing?

bug#59029: Dumping Emacs crashes when buffers have overlays

[Eli Zaretskii]

> Cc: 59029@debbugs.gnu.org
> From: Matt Armstrong <matt@rfc20.org>
> Date: Tue, 08 Nov 2022 07:59:45 -0800
> 
> a) Drop support for dumping overlays in buffers, as if
> `delete-all-overlays' were called for all buffers before dumping.  Fix
> involves relaxing the abort() calls to merely print warnings.
> 
> b) Restore support, and test it.

a) could be a short-term band-aid, but if we want ever to support
re-dumping, b) is a must.

> For (b) I'm not keen on wiring up a single-purpose test under test/*
> somewhere just to test pdumping with overlays.  How about hooking in to
> loadup.el to populate a dummy buffer with overlays before dumping?

For testing purposes, you could perhaps use eval-after-load to do
something after loading loadup.el?

bug#59029: Dumping Emacs crashes when buffers have overlays

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> a) Drop support for dumping overlays in buffers, as if
>> `delete-all-overlays' were called for all buffers before dumping.  Fix
>> involves relaxing the abort() calls to merely print warnings.
>> 
>> b) Restore support, and test it.
>
> a) could be a short-term band-aid, but if we want ever to support
> re-dumping, b) is a must.

FWIW, I suspect that the vast majority of redumping uses will/would be
to include more preloaded ELisp code rather than to include
pre-populated buffers, so it's quite likely that it would work just fine
without that extra support for dumping overlays-in-buffers.

But I agree that (b) is a must, because I find it sad if we can't get
the pdumping code working.

>> For (b) I'm not keen on wiring up a single-purpose test under test/*
>> somewhere just to test pdumping with overlays.  How about hooking in to
>> loadup.el to populate a dummy buffer with overlays before dumping?
> For testing purposes, you could perhaps use eval-after-load to do
> something after loading loadup.el?

IMO, we should have a proper separate pdump test, instead.


        Stefan

bug#59029: Dumping Emacs crashes when buffers have overlays

[Matt Armstrong]

[-- Attachment #1: Type: text/plain, Size: 818 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> IMO, we should have a proper separate pdump test, instead.

I tend to agree with Stefan about the relatively low importance of
supporting overlays owned by buffers in the dump.  I have deferred that
work, though the tests I have added will be useful if someone wants to
take it up in the future.

See attached patch, which:

a) Adds a test that exercises portable dumping from a batch mode
subprocess, using the Emacs executable hosting the test as a basis.

b) Performs the dump while a buffer with overlays is live in the
subprocess.

c) Verifies the dump by running a second subprocess and checking how
many overlays are in the buffer that had overlays.

The test took most of the time.  I'm quite open to suggestions there; my
elisp knowledge is novice level.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-portable-dumping-when-buffers-have-overlays.patch --]
[-- Type: text/x-diff, Size: 10567 bytes --]

From eee0419f7d702e67671197a5c65c83b77af3c489 Mon Sep 17 00:00:00 2001
From: Matt Armstrong <matt@rfc20.org>
Date: Tue, 8 Nov 2022 17:10:02 -0800
Subject: [PATCH] Fix portable dumping when buffers have overlays.

See bug#59029.  While it is possible to fully support overlays in dump
files, the code involved is non-trivial and we have decided to defer
this work.  Instead, a buffer's overlays are dumped only if they are
reachable from something other than their buffer, and as if they had
been deleted.

* src/itree.c (itree_node_init): Initialize fields in declared order.
Zero-initialize unspecified fields.
* src/pdumper.c (dump_itree_node): Always dump as if the node had
been deleted from its buffer.  Renamed from dump_interval_node.
(dump_buffer): Never dump a buffer's overlays.
* test/src/pdumper-tests.el: New test to exercise the above.
---
 src/itree.c               |  15 +++--
 src/pdumper.c             |  50 ++++----------
 test/src/pdumper-tests.el | 133 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 152 insertions(+), 46 deletions(-)
 create mode 100644 test/src/pdumper-tests.el

diff --git a/src/itree.c b/src/itree.c
index 989173db4e..fecad886e3 100644
--- a/src/itree.c
+++ b/src/itree.c
@@ -534,14 +534,15 @@ itree_node_init (struct itree_node *node,
 		 bool front_advance, bool rear_advance,
 		 Lisp_Object data)
 {
-  node->parent = NULL;
-  node->left = NULL;
-  node->right = NULL;
-  node->begin = -1;
-  node->end = -1;
-  node->front_advance = front_advance;
-  node->rear_advance = rear_advance;
+  node->begin = 0;
+  node->end = 0;
+  node->limit = 0;
+  node->offset = 0;
+  node->otick = 0;
   node->data = data;
+  node->red = 0;
+  node->rear_advance = rear_advance;
+  node->front_advance = front_advance;
 }
 
 /* Return NODE's begin value, computing it if necessary. */
diff --git a/src/pdumper.c b/src/pdumper.c
index 0a5d96dbb7..652dfc2e71 100644
--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -2134,46 +2134,20 @@ dump_marker (struct dump_context *ctx, const struct Lisp_Marker *marker)
 }
 
 static dump_off
-dump_interval_node (struct dump_context *ctx, struct itree_node *node,
-                    dump_off parent_offset)
+dump_itree_node (struct dump_context *ctx, struct itree_node *node,
+		 dump_off parent_offset)
 {
 #if CHECK_STRUCTS && !defined (HASH_itree_node_50DE304F13)
 # error "itree_node changed. See CHECK_STRUCTS comment in config.h."
 #endif
+  /* Dumping of a buffer's overlays is not implemented.  Overlays are
+     dumped as if they had been deleted.  See bug#59029.  */
   struct itree_node out;
   dump_object_start (ctx, &out, sizeof (out));
-  if (node->parent)
-    dump_field_fixup_later (ctx, &out, node, &node->parent);
-  if (node->left)
-    dump_field_fixup_later (ctx, &out, node, &node->parent);
-  if (node->right)
-    dump_field_fixup_later (ctx, &out, node, &node->parent);
-  DUMP_FIELD_COPY (&out, node, begin);
-  DUMP_FIELD_COPY (&out, node, end);
-  DUMP_FIELD_COPY (&out, node, limit);
-  DUMP_FIELD_COPY (&out, node, offset);
-  DUMP_FIELD_COPY (&out, node, otick);
+  itree_node_init (&out, node->front_advance, node->rear_advance,
+		   node->data);
   dump_field_lv (ctx, &out, node, &node->data, WEIGHT_STRONG);
-  DUMP_FIELD_COPY (&out, node, red);
-  DUMP_FIELD_COPY (&out, node, rear_advance);
-  DUMP_FIELD_COPY (&out, node, front_advance);
-  dump_off offset = dump_object_finish (ctx, &out, sizeof (out));
-  if (node->parent)
-      dump_remember_fixup_ptr_raw
-	(ctx,
-	 offset + dump_offsetof (struct itree_node, parent),
-	 dump_interval_node (ctx, node->parent, offset));
-  if (node->left)
-      dump_remember_fixup_ptr_raw
-	(ctx,
-	 offset + dump_offsetof (struct itree_node, left),
-	 dump_interval_node (ctx, node->left, offset));
-  if (node->right)
-      dump_remember_fixup_ptr_raw
-	(ctx,
-	 offset + dump_offsetof (struct itree_node, right),
-	 dump_interval_node (ctx, node->right, offset));
-  return offset;
+  return dump_object_finish (ctx, &out, sizeof (out));
 }
 
 static dump_off
@@ -2189,7 +2163,7 @@ dump_overlay (struct dump_context *ctx, const struct Lisp_Overlay *overlay)
   dump_remember_fixup_ptr_raw
     (ctx,
      offset + dump_offsetof (struct Lisp_Overlay, interval),
-     dump_interval_node (ctx, overlay->interval, offset));
+     dump_itree_node (ctx, overlay->interval, offset));
   return offset;
 }
 
@@ -2863,11 +2837,9 @@ dump_buffer (struct dump_context *ctx, const struct buffer *in_buffer)
   DUMP_FIELD_COPY (out, buffer, inhibit_buffer_hooks);
   DUMP_FIELD_COPY (out, buffer, long_line_optimizations_p);
 
-  if (buffer->overlays && buffer->overlays->root != NULL)
-    /* We haven't implemented the code to dump overlays.  */
-    emacs_abort ();
-  else
-    out->overlays = NULL;
+  /* Dumping of a buffer's overlays is not implemented.  See
+     bug#59029.  */
+  out->overlays = NULL;
 
   dump_field_lv (ctx, out, buffer, &buffer->undo_list_,
                  WEIGHT_STRONG);
diff --git a/test/src/pdumper-tests.el b/test/src/pdumper-tests.el
new file mode 100644
index 0000000000..18b7a7afb5
--- /dev/null
+++ b/test/src/pdumper-tests.el
@@ -0,0 +1,133 @@
+;;; pdumper-tests.el --- pdumper tests               -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2022  Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert-x)
+
+(defun pdumper-tests--readable-arg (form)
+  (let ((print-gensym t)
+        (print-circle t)
+        (print-length nil)
+        (print-level nil)
+        (print-escape-control-characters t)
+        (print-escape-newlines t)
+        (print-escape-multibyte t)
+        (print-escape-nonascii t))
+    (if-let ((str (readablep form)))
+        str
+      (error "unreadable form %S" form))))
+
+(defun pdumper-tests--eval-arg (form)
+  (concat "--eval=" (pdumper-tests--readable-arg form)))
+
+(defun pdumper-tests--batch-eval-command (emacs form)
+  `(,@emacs "--quick" "--batch"
+            ,(pdumper-tests--eval-arg form)))
+
+(defun pdumper-tests--emacs-binary ()
+  (expand-file-name invocation-name invocation-directory))
+
+(defun pdumper-tests--check-output (dir command)
+  "Run COMMAND as a synchronous subprocess.
+
+The process' current directory is set to DIR.  Fail the test if
+the process exits with a non-zero status.  Otherwise, return its
+output as a string."
+  (with-temp-buffer
+    (setq default-directory dir)
+    (let ((result (apply #'call-process
+                         (car command)
+                         nil t nil
+                         (cdr command))))
+      (unless (equal 0 result)
+        (ert-info ((format "In directory %S" dir))
+          (ert-info ((format "Ran command %S" command))
+            (ert-info ((format "With output %S" (buffer-string)))
+              (should (equal 0 result)))))))
+    (buffer-string)))
+
+(defun pdumper-tests--batch-redump (loadup-form redumped-form)
+  "Redump the current Emacs binary and verify it works.
+
+First, run the current Emacs binary in a synchronous subprocess.
+Evaluate LOADUP-FORM in it, then dump it with
+`dump-emacs-portable'.
+
+Second, run the current Emacs binary in a second subprocess, this
+time with \"--dump-file\" set to the dump file just created.
+Evaluate REDUMPED-FORM in this second process.
+
+Returns the output of the second run as a string."
+  (ert-with-temp-directory tmpdir
+    (let* ((dump-file (concat tmpdir "redump.pdmp"))
+           (loadup-file (concat tmpdir "redump.el"))
+           (emacs-binary (pdumper-tests--emacs-binary))
+           (dump-command (pdumper-tests--batch-eval-command
+                          (list emacs-binary)
+                          `(progn
+                             (load ,loadup-file)
+                             (dump-emacs-portable ,dump-file)))))
+      (with-temp-file loadup-file
+        (insert (prin1-to-string loadup-form)))
+      (should (string-search "Dump complete"
+                             (pdumper-tests--check-output
+                              tmpdir dump-command)))
+      (let* ((redumped-emacs-command
+              (list emacs-binary
+                    (concat "--dump-file=" dump-file)))
+             (redumped-emacs-validate
+              (pdumper-tests--batch-eval-command
+               redumped-emacs-command
+               redumped-form)))
+        (pdumper-tests--check-output
+         tmpdir
+         redumped-emacs-validate)))))
+
+(ert-deftest pdumper-tests-redump-overlays ()
+  "Test portable dumping when overlays are in a buffer.
+
+In an Emacs subprocess create a buffer with some overlays, then
+dump it.  Run with the new dump file and verify the buffer exists
+and is in the correct state."
+  (skip-unless (fboundp 'pdumper-stats))
+  (let* ((overlay-buffer " *redump-overlays*")
+         (phrase "Lorem ipsum dolor sit amet, consectetur...")
+         (redump-output
+          (pdumper-tests--batch-redump
+           `(with-current-buffer (get-buffer-create ,overlay-buffer)
+              (insert ,phrase)
+              (dotimes (i 8)
+                (make-overlay (+ (point-min) i)
+                              (+ (point-min) (+ i 10)))))
+           `(with-current-buffer (get-buffer-create ,overlay-buffer)
+              (unless (equal ,phrase (buffer-string))
+                (error "buffer text not as expected"))
+              (let ((overlays (overlays-in (point-min) (point-max))))
+                (princ (format "Redumped overlay count: <%d>"
+                               (length overlays))))))))
+    ;; Dumping of a buffer's overlays is not implemented.  Overlays
+    ;; are dumped as if they had been deleted, so the redumped Emacs
+    ;; should have zero overlays in the *redump-overlays* buffer.  See
+    ;; bug#59029.  */
+    (should (string-search "Redumped overlay count: <0>"
+                           redump-output))))
+
+(provide 'pdumper-tests)
+;;; pdumper-tests.el ends here
-- 
2.35.1