Package: emacs;
Reported by: Pip Cet <pipcet <at> protonmail.com>
Date: Thu, 27 Mar 2025 15:36:02 UTC
Severity: normal
Found in version 31.0.50
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Pip Cet <pipcet <at> protonmail.com> To: bug-gnu-emacs <at> gnu.org Subject: 31.0.50; Crash in Finsert_file_contents, file size changed Date: Thu, 27 Mar 2025 15:34:31 +0000
I experienced a crash on the feature/igc branch (unfortunately, with
local modifications) in a long-running session. I still have the (very
large) coredump file so I can do some limited post-mortem debugging of
the session, but I think I've identified the bug and we can just fix it
instead.
Note that line numbers will probably be off in the backtrace because of
the local modifications.
The crash happened when a buffer showing config.log in an Emacs
directory auto-reverted while I was running ./configure in that
directory in another shell buffer.
After looking at this more closely, I think it's a bug in
Finsert_file_contents, which currently assumes that a call to
emacs_fd_fstat establishes st_size as an upper limit of how many
characters read from the beginning of the file can match the current
buffer.
Most likely, the race window is so small that this happens only very
rarely on the master branch, but feature/igc introduces the occasional
unexpected delay so is more vulnerable to such bugs (see also
bug#74590, which also is most likely explained by a delay introduced by
MPS exposing a previously-latent bug). However, I believe this isn't
likely to be an MPS-specific bug.
The crash happened in this line/call in fileio.c:
if (overlap > 0)
same_at_end += overlap;
same_at_end_charpos = BYTE_TO_CHAR (same_at_end); <<<<<< HERE
/* Arrange to read only the nonmatching middle part of the file. */
beg_offset += same_at_start - BEGV_BYTE;
end_offset -= ZV_BYTE - same_at_end;
At the time:
same_at_start = 38706
same_at_end = 1499444
overlap = 25
replace_handled = false
giveup_match_end = false
regular = true
coding = {
st = {
st_size = 38681,
}
current_buffer->text->z_byte = 1499420
end = Qnil
current_buffer->text->gpt = 1399068
current_buffer->text->beg + 38681 = "configure:10959: $? = 0\nconfigure:10996: result: none needed\n..."
coding = {
id = 1,
result = CODING_RESULT_SUCCESS,
decoder = 0x5555556c7f6c <decode_coding_raw_text>,
encoder = 0x5555556c80c1 <encode_coding_raw_text>
}
So the BYTE_TO_CHAR failed because same_at_end was out of range after
overlap was added to it. Given the calculation of overlap as:
overlap = (same_at_start - BEGV_BYTE
- (same_at_end
+ (! NILP (end) ? end_offset : st.st_size) - ZV_BYTE));
I believe the intention was for same_at_start - BEGV_BYTE never to
exceed st.st_size or end_offset (the two are the same here). See below
for why I think that the overlap calculation should also subtract
beg_offset from this value.
same_at_start is set here:
int nread = emacs_fd_read (fd, read_buf, sizeof read_buf);
int bufpos = 0;
while (bufpos < nread && same_at_start < ZV_BYTE
&& FETCH_BYTE (same_at_start) == read_buf[bufpos])
same_at_start++, bufpos++;
which is called some time after:
if (emacs_fd_fstat (fd, &st) != 0)
report_file_error ("Input file status", orig_filename);
so it seems entirely possible to me that the file grew by the time we
set same_at_start, leading to the backtrace.
The extra text written appears to have been
"configure:10959: $? = 0\n"
My impression is that overlap should have been calculated as 0 (or a
negative number), not 25, because only one byte matches at the end of
the buffer (which ends in "s\n") and same_at_end was thus, rightfully,
1494419, one less than Z_BYTE = ZV_BYTE.
This code is very hard to understand; I believe that's because it
doesn't currently always do the right thing, particularly not if st_size
changes. However, a preliminary suggestion for fixing this follows:
Let's add an additional condition to the loop counting bytes for
same_at_start, so it never exceeds end_offset - beg_offset + BEGV_BYTE,
and a symmetric condition in the loop counting bytes for same_at_end, so
it never becomes less than ZV_BYTE - (end_offset - beg_offset -
same_at_start + BEGV_BYTE).
This would fix both this bug and what seems to me to be a faulty
calculation, failing to detect an overlap, in the case where beg_offset
is != 0.
IOW, we would fix the overlap calculation and make it so no overlap can
ever happen, after which the overlap code can be removed.
In addition, this code:
/* If the file matches the buffer completely,
there's no need to replace anything. */
if (same_at_start - BEGV_BYTE == end_offset - beg_offset)
{
emacs_fd_close (fd);
clear_unwind_protect (fd_index);
/* Truncate the buffer to the size of the file. */
del_range_1 (same_at_start, same_at_end, 0, 0);
goto handled;
}
confuses me, because same_at_start and same_at_end are byte positions
and del_range_1 takes character positions. I expect it's hit very
rarely except when same_at_start == same_at_end or same_at_start ==
BEGV_BYTE == BEGV, and del_range_1 is forgiving in these cases.
Backtrace:
#0 terminate_due_to_signal (sig=6, backtrace_limit=2147483647) at emacs.c:425
#1 0x000055555587f71b in die (msg=0x555555a597b8 "BUF_BEG_BYTE (b) <= bytepos && bytepos <= BUF_Z_BYTE (b)", file=0x555555a5974c "marker.c", line=330)
at alloc.c:8064
#2 0x000055555581e011 in buf_bytepos_to_charpos (b=0x7ff8ef791a58, bytepos=1499444) at marker.c:330
#3 0x0000555555829246 in BYTE_TO_CHAR (bytepos=1499444) at /home/pip/emacs-20250320/src/buffer.h:1204
#4 0x0000555555833950 in Finsert_file_contents (filename=0x7ff8ef797af5, visit=0x38, beg=0x0, end=0x0, replace=0xc908) at fileio.c:4547
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.