GNU logs - #77315, boring messages

Message sent to bug-gnu-emacs@HIDDEN:

Subject: bug#77315: 31.0.50; Crash in Finsert_file_contents, file size changed
Resent-From: Pip Cet <pipcet@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Thu, 27 Mar 2025 15:36:02 +0000
Resent-Message-ID: <handler.77315.B.174308970310360 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
To: 77315 <at> debbugs.gnu.org
Date: Thu, 27 Mar 2025 15:34:31 +0000
From: Pip Cet <pipcet@HIDDEN>
Message-ID: <8734eyqyho.fsf@HIDDEN>
Feedback-ID: 112775352:user:proton
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Received-SPF: pass client-ip=185.70.43.22; envelope-from=pipcet@HIDDEN;
 helo=mail-4322.protonmail.ch
X-Spam_score_int: -20
X-Spam_score: -2.1
X-Spam_bar: --
X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1,
 DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001,
 RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=0.001,
 RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001,
 SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no
X-Spam_action: no action
X-Spam-Score: 1.0 (+)
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -0.0 (/)

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:

=09  if (overlap > 0)
=09    same_at_end +=3D overlap;
=09  same_at_end_charpos =3D BYTE_TO_CHAR (same_at_end); <<<<<< HERE

=09  /* Arrange to read only the nonmatching middle part of the file.  */
=09  beg_offset +=3D same_at_start - BEGV_BYTE;
=09  end_offset -=3D ZV_BYTE - same_at_end;

At the time:

        same_at_start =3D 38706
        same_at_end =3D 1499444
        overlap =3D 25
        replace_handled =3D false
        giveup_match_end =3D false
        regular =3D true
        coding =3D {

        st =3D {
          st_size =3D 38681,
        }

        current_buffer->text->z_byte =3D 1499420
        end =3D Qnil
        current_buffer->text->gpt =3D 1399068
        current_buffer->text->beg + 38681 =3D "configure:10959: $? =3D 0\nc=
onfigure:10996: result: none needed\n..."

        coding =3D {
          id =3D 1,
          result =3D CODING_RESULT_SUCCESS,
          decoder =3D 0x5555556c7f6c <decode_coding_raw_text>,
          encoder =3D 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:

=09  overlap =3D (same_at_start - BEGV_BYTE
=09=09     - (same_at_end
=09=09=09+ (! 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:

=09  int nread =3D emacs_fd_read (fd, read_buf, sizeof read_buf);
=09  int bufpos =3D 0;
=09  while (bufpos < nread && same_at_start < ZV_BYTE
=09=09 && FETCH_BYTE (same_at_start) =3D=3D read_buf[bufpos])
=09    same_at_start++, bufpos++;

which is called some time after:

  if (emacs_fd_fstat (fd, &st) !=3D 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: $? =3D 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 =3D 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 !=3D 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,
=09 there's no need to replace anything.  */
      if (same_at_start - BEGV_BYTE =3D=3D end_offset - beg_offset)
=09{
=09  emacs_fd_close (fd);
=09  clear_unwind_protect (fd_index);

=09  /* Truncate the buffer to the size of the file.  */
=09  del_range_1 (same_at_start, same_at_end, 0, 0);
=09  goto handled;
=09}

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 =3D=3D same_at_end or same_at_start =3D=3D
BEGV_BYTE =3D=3D BEGV, and del_range_1 is forgiving in these cases.

Backtrace:

#0  terminate_due_to_signal (sig=3D6, backtrace_limit=3D2147483647) at emac=
s.c:425
#1  0x000055555587f71b in die (msg=3D0x555555a597b8 "BUF_BEG_BYTE (b) <=3D =
bytepos && bytepos <=3D BUF_Z_BYTE (b)", file=3D0x555555a5974c "marker.c", =
line=3D330)
    at alloc.c:8064
#2  0x000055555581e011 in buf_bytepos_to_charpos (b=3D0x7ff8ef791a58, bytep=
os=3D1499444) at marker.c:330
#3  0x0000555555829246 in BYTE_TO_CHAR (bytepos=3D1499444) at /home/pip/ema=
cs-20250320/src/buffer.h:1204
#4  0x0000555555833950 in Finsert_file_contents (filename=3D0x7ff8ef797af5,=
 visit=3D0x38, beg=3D0x0, end=3D0x0, replace=3D0xc908) at fileio.c:4547

Message sent:

Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-Mailer: MIME-tools 5.505 (Entity 5.505)
Content-Type: text/plain; charset=utf-8
X-Loop: help-debbugs@HIDDEN
From: help-debbugs@HIDDEN (GNU bug Tracking System)
To: Pip Cet <pipcet@HIDDEN>
Subject: bug#77315: Acknowledgement (31.0.50; Crash in Finsert_file_contents,
 file size changed)
Message-ID: <handler.77315.B.174308970310360.ack <at> debbugs.gnu.org>
References: <8734eyqyho.fsf@HIDDEN>
X-Gnu-PR-Message: ack 77315
X-Gnu-PR-Package: emacs
Reply-To: 77315 <at> debbugs.gnu.org
Date: Thu, 27 Mar 2025 15:36:02 +0000

Thank you for filing a new bug report with debbugs.gnu.org.

This is an automatically generated reply to let you know your message
has been received.

Your message is being forwarded to the package maintainers and other
interested parties for their attention; they will reply in due course.

Your message has been sent to the package maintainer(s):
 bug-gnu-emacs@HIDDEN

If you wish to submit further information on this problem, please
send it to 77315 <at> debbugs.gnu.org.

Please do not send mail to help-debbugs@HIDDEN unless you wish
to report a problem with the Bug-tracking system.

--=20
77315: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=3D77315
GNU Bug Tracking System
Contact help-debbugs@HIDDEN with problems

Message sent to bug-gnu-emacs@HIDDEN:

Subject: bug#77315: 31.0.50; Crash in Finsert_file_contents, file size changed
Resent-From: Eli Zaretskii <eliz@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Thu, 27 Mar 2025 16:22:04 +0000
Resent-Message-ID: <handler.77315.B77315.17430925056377 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
To: Pip Cet <pipcet@HIDDEN>, Paul Eggert <eggert@HIDDEN>
Cc: 77315 <at> debbugs.gnu.org
Date: Thu, 27 Mar 2025 18:21:31 +0200
Message-Id: <86v7rubg1g.fsf@HIDDEN>
From: Eli Zaretskii <eliz@HIDDEN>
In-Reply-To: <8734eyqyho.fsf@HIDDEN> (bug-gnu-emacs@HIDDEN)
References: <8734eyqyho.fsf@HIDDEN>
Precedence: list
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>

> Date: Thu, 27 Mar 2025 15:34:31 +0000
> From:  Pip Cet via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@HIDDEN>
> 
> 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.

Adding Paul to the discussion.

AFAIR, insert-file-contents needs to be completely rewritten to handle
such situations.  If the file is being modified by another program
while we are trying to figure out the overlap (because REPLACE was
no-nil), then I'm not sure I understand how can REPLACE and overlap
work at all, since the data changes under our feet as we go.  So maybe
if we detect that this is happening, we should give up on being smart
and decide that there's no overlap at all.

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

Message sent to bug-gnu-emacs@HIDDEN:

Subject: bug#77315: 31.0.50; Crash in Finsert_file_contents, file size changed
Resent-From: Paul Eggert <eggert@HIDDEN>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@HIDDEN
Resent-Date: Thu, 27 Mar 2025 17:24:02 +0000
Resent-Message-ID: <handler.77315.B77315.174309618432235 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
To: Eli Zaretskii <eliz@HIDDEN>, Pip Cet <pipcet@HIDDEN>
Cc: 77315 <at> debbugs.gnu.org
Message-ID: <324123b5-a4e7-44da-b778-debf71b9efc4@HIDDEN>
Date: Thu, 27 Mar 2025 11:22:48 -0600
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
References: <8734eyqyho.fsf@HIDDEN> <86v7rubg1g.fsf@HIDDEN>
Content-Language: en-US
From: Paul Eggert <eggert@HIDDEN>
In-Reply-To: <86v7rubg1g.fsf@HIDDEN>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: list
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>

On 3/27/25 10:21, Eli Zaretskii wrote:
> AFAIR, insert-file-contents needs to be completely rewritten to handle
> such situations.

I drafted something a while ago along those lines but never got around 
to finish it. I'll try to bump the priority.

Last modified: Thu, 27 Mar 2025 17:30:02 UTC

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997 nCipher Corporation Ltd, 1994-97 Ian Jackson.