Package: emacs;
Reported by: Werner LEMBERG <wl <at> gnu.org>
Date: Sat, 29 Jun 2019 11:19:01 UTC
Severity: normal
Done: Stefan Monnier <monnier <at> iro.umontreal.ca>
Bug is archived. No further changes may be made.
View this message in rfc822 format
From: Stefan Monnier <monnier <at> iro.umontreal.ca> To: Eli Zaretskii <eliz <at> gnu.org> Cc: wl <at> gnu.org, 36431 <at> debbugs.gnu.org Subject: bug#36431: Crash in marker.c:337 Date: Wed, 03 Jul 2019 00:21:54 -0400
> AFAICT, this patch moves the call to move_gap_both from a fragment
> where we must decode the inserted text to a fragment where such a
> decoding might not be necessary. If I'm right, then this makes
> insert-file-contents slower in some cases, because moving the gap
> might be very expensive with large buffers.
Here's an alternative patch which doesn't suffer from this problem but
also eliminates the transiently-inconsistent multibyte buffer situation.
Stefan
diff --git a/src/fileio.c b/src/fileio.c
index 2825c1b54c..9ed1fcf8ca 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -3705,6 +3705,7 @@ because (1) it preserves some marker positions and (2) it puts less data
CHECK_CODING_SYSTEM (Vcoding_system_for_read);
Fset (Qbuffer_file_coding_system, Vcoding_system_for_read);
}
+ eassert (inserted == 0);
goto notfound;
}
@@ -3731,7 +3732,10 @@ because (1) it preserves some marker positions and (2) it puts less data
not_regular = 1;
if (! NILP (visit))
- goto notfound;
+ {
+ eassert (inserted == 0);
+ goto notfound;
+ }
if (! NILP (replace) || ! NILP (beg) || ! NILP (end))
xsignal2 (Qfile_error,
@@ -4399,10 +4403,10 @@ because (1) it preserves some marker positions and (2) it puts less data
if (how_much < 0)
report_file_error ("Read error", orig_filename);
- /* Make the text read part of the buffer. */
- insert_from_gap_1 (inserted, inserted, false);
-
- notfound:
+ notfound: ;
+ Lisp_Object multibyte
+ = BVAR (current_buffer, enable_multibyte_characters);
+ bool ingap = true; /* Bytes are currently in the gap. */
if (NILP (coding_system))
{
@@ -4411,6 +4415,7 @@ because (1) it preserves some marker positions and (2) it puts less data
Note that we can get here only if the buffer was empty
before the insertion. */
+ eassert (Z == BEG);
if (!NILP (Vcoding_system_for_read))
coding_system = Vcoding_system_for_read;
@@ -4421,8 +4426,6 @@ because (1) it preserves some marker positions and (2) it puts less data
enable-multibyte-characters directly here without taking
care of marker adjustment. By this way, we can run Lisp
program safely before decoding the inserted text. */
- Lisp_Object multibyte
- = BVAR (current_buffer, enable_multibyte_characters);
Lisp_Object undo_list = BVAR (current_buffer, undo_list);
ptrdiff_t count1 = SPECPDL_INDEX ();
@@ -4430,6 +4433,10 @@ because (1) it preserves some marker positions and (2) it puts less data
bset_undo_list (current_buffer, Qt);
record_unwind_protect (restore_buffer, Fcurrent_buffer ());
+ /* Make the text read part of the buffer. */
+ insert_from_gap_1 (inserted, inserted, false);
+ ingap = false;
+
if (inserted > 0 && ! NILP (Vset_auto_coding_function))
{
coding_system = call2 (Vset_auto_coding_function,
@@ -4455,15 +4462,10 @@ because (1) it preserves some marker positions and (2) it puts less data
adjust_overlays_for_delete (BEG, Z - BEG);
set_buffer_intervals (current_buffer, NULL);
TEMP_SET_PT_BOTH (BEG, BEG_BYTE);
-
- /* Change the buffer's multibyteness directly. We used to do this
- from within unbind_to, but it was unsafe since the bytes
- may contain invalid sequences for a multibyte buffer (which is OK
- here since we'll decode them before anyone else gets to see
- them, but is dangerous when we're doing a non-local exit). */
- bset_enable_multibyte_characters (current_buffer, multibyte);
bset_undo_list (current_buffer, undo_list);
inserted = Z_BYTE - BEG_BYTE;
+ /* The bytes may be invalid for a multibyte buffer, so we can't
+ restore the multibyteness yet. */
}
if (NILP (coding_system))
@@ -4471,7 +4473,7 @@ because (1) it preserves some marker positions and (2) it puts less data
else
CHECK_CODING_SYSTEM (coding_system);
- if (NILP (BVAR (current_buffer, enable_multibyte_characters)))
+ if (NILP (multibyte))
/* We must suppress all character code conversion except for
end-of-line conversion. */
coding_system = raw_text_coding_system (coding_system);
@@ -4490,33 +4492,51 @@ because (1) it preserves some marker positions and (2) it puts less data
{
/* Visiting a file with these coding system makes the buffer
unibyte. */
- if (inserted > 0)
+ if (!ingap)
+ multibyte = Qnil;
+ else if (inserted > 0)
bset_enable_multibyte_characters (current_buffer, Qnil);
- else
+ else
Fset_buffer_multibyte (Qnil);
}
}
- coding.dst_multibyte = ! NILP (BVAR (current_buffer, enable_multibyte_characters));
+ coding.dst_multibyte = !NILP (multibyte);
if (CODING_MAY_REQUIRE_DECODING (&coding)
&& (inserted > 0 || CODING_REQUIRE_FLUSHING (&coding)))
{
- move_gap_both (PT, PT_BYTE);
- GAP_SIZE += inserted;
- ZV_BYTE -= inserted;
- Z_BYTE -= inserted;
- ZV -= inserted;
- Z -= inserted;
+ if (ingap)
+ { /* Text is at beginning of gap, move it to the end. */
+ memmove (GAP_END_ADDR - inserted, GPT_ADDR, inserted);
+ }
+ else
+ { /* Text is inside the buffer; move it to end of the gap. */
+ move_gap_both (PT, PT_BYTE);
+ eassert (inserted == Z_BYTE - BEG_BYTE);
+ GAP_SIZE += inserted;
+ ZV = Z = GPT = BEG;
+ ZV_BYTE = Z_BYTE = GPT_BYTE = BEG_BYTE;
+ /* Now we are safe to change the buffer's multibyteness directly. */
+ bset_enable_multibyte_characters (current_buffer, multibyte);
+ }
+
decode_coding_gap (&coding, inserted);
inserted = coding.produced_char;
coding_system = CODING_ID_NAME (coding.id);
}
- else if (inserted > 0)
+ else if (inserted > 0 && ingap)
{
+ /* Make the text read part of the buffer. */
+ eassert (NILP (BVAR (current_buffer, enable_multibyte_characters)));
+ insert_from_gap_1 (inserted, inserted, false);
invalidate_buffer_caches (current_buffer, PT, PT + inserted);
adjust_after_insert (PT, PT_BYTE, PT + inserted, PT_BYTE + inserted,
inserted);
}
+ else if (!ingap)
+ { /* Apparently, no decoding needed, so just set the bytenesss. */
+ bset_enable_multibyte_characters (current_buffer, multibyte);
+ }
/* Call after-change hooks for the inserted text, aside from the case
of normal visiting (not with REPLACE), which is done in a new buffer
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.