From: Matt Armstrong <matt@rfc20.org>
To: Stefan Monnier <monnier@iro.umontreal.ca>, Eli Zaretskii <eliz@gnu.org>
Cc: 59029@debbugs.gnu.org
Subject: bug#59029: Dumping Emacs crashes when buffers have overlays
Date: Wed, 09 Nov 2022 12:04:03 -0800 [thread overview]
Message-ID: <87y1sjlwss.fsf@rfc20.org> (raw)
In-Reply-To: <jwv35attlj0.fsf-monnier+emacs@gnu.org>
[-- 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
prev parent reply other threads:[~2022-11-09 20:04 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-04 23:09 bug#59029: 29.0.50; noverlay: pdumper.c: dump_interval_node recursion has no base case Matt Armstrong
2022-11-05 5:41 ` Gerd Möllmann
2022-11-05 18:09 ` Matt Armstrong
2022-11-06 5:21 ` Gerd Möllmann
2022-11-05 20:38 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-11-08 15:59 ` bug#59029: Dumping Emacs crashes when buffers have overlays Matt Armstrong
2022-11-08 16:59 ` Eli Zaretskii
2022-11-08 17:21 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-11-09 20:04 ` Matt Armstrong [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87y1sjlwss.fsf@rfc20.org \
--to=matt@rfc20.org \
--cc=59029@debbugs.gnu.org \
--cc=eliz@gnu.org \
--cc=monnier@iro.umontreal.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.