Subject: bug#78898: Make read/readevalloop reentrant

From: Lynn Winebarger <[email-masked]>
Date: Tue, 1 Jul 2025 19:47:45 -0400

On Sun, Jun 29, 2025 at 11:55 PM Stefan Monnier
<[email-masked]> wrote:
>
> That was my assumption.  Since then, compilers have gotten much better
> at inlining functions, so you can do
>
>     init_inbuffer (&x);
>
> and they'll generate the same code as it does with your
>
>     struct inbuffer x = INIT_INBUFFER ();
>
> [ Well, admittedly, not with `-O0`.  ]
>
> Note also that
>
>     struct inbuffer x = { .buffer = buf };
>
> should work just as well, so maybe "nothing" is an even better option
> than either.
>

I've replaced the READCHAR_* macros with inline functions.  GDB just
cannot do anything with function-like macros.  I also noticed and
fixed the bug that was causing Fload to fail trying to load
rx-tests.elc.
None of the other test failures went away, but there were no new ones either.
From 3622ad561dfc24406ec3f0e57131dddd02a444e6 Mon Sep 17 00:00:00 2001
From: Onnie Lynn Winebarger <[email-masked]>
Date: Tue, 1 Jul 2025 19:39:37 -0400
Subject: [PATCH 2/2] Transform READCHAR_* macros into inline functions

---
 src/lread.c | 272 ++++++++++++++++++++++++++++------------------------
 1 file changed, 145 insertions(+), 127 deletions(-)

diff --git a/src/lread.c b/src/lread.c
index 57ca1732983..91d181c604a 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -1167,41 +1167,6 @@ get_emacs_mule_char_length (struct charset *cs, int c)
 }

-#define READCHAR_BEGIN(frame) \
-  int c; \
-  do \
-    { \
-      if (frame->unread_char >= 0) \
-	{ \
-	  c = frame->unread_char; \
-	  frame->unread_char = -1; \
-	  frame->char_offset++; \
-	  frame->byte_offset += frame->unread_bytes; \
-	  frame->unread_bytes = 0; \
-	  return c; \
-	} \
-    } \
-  while (0)
-
-#define READCHAR_FROM_FILE_BEGIN(frame) \
-  unsigned char buf[4]; \
-  READCHAR_BEGIN(frame); \
-  c = readbyte_from_file (frame); \
-  if (c < 0) \
-    { \
-      frame->read_eof = true; \
-      return c; \
-    } \
-  do \
-    { \
-      if (ASCII_CHAR_P (c)) \
-	{ \
-	  frame->char_offset++; \
-	  frame->byte_offset++; \
-	  return c; \
-       } \
-  } while (0)
-
 /* internal form of /dev/null */
 static int
 readchar_from_null (register lread_frame *frame)
@@ -1210,12 +1175,45 @@ readchar_from_null (register lread_frame *frame)
   return -1;
 }
 
+INLINE int
+handle_buffered_unread_character (register lread_frame *frame)
+{
+  int c = frame->unread_char;
+  frame->unread_char = -1;
+  frame->char_offset++;
+  frame->byte_offset += frame->unread_bytes;
+  frame->unread_bytes = 0;
+  return c;
+}
+
+INLINE int
+handle_readchar_eof (register lread_frame *frame)
+{
+  frame->read_eof = true;
+  return -1;
+}
+
+INLINE int
+handle_readchar_unibyte (register lread_frame *frame, int c)
+{
+  frame->char_offset++;
+  frame->byte_offset++;
+  return c;
+}
+
 /* Handle emacs-internal coding */
 static int
 readchar_from_multibyte_file (register lread_frame *frame)
 {
-  READCHAR_FROM_FILE_BEGIN(frame);
+  if (frame->unread_char >= 0)
+    return handle_buffered_unread_character (frame);
+  int c = readbyte_from_file (frame);
+  if (c < 0)
+    return handle_readchar_eof (frame);
+  if (ASCII_CHAR_P (c))
+    return handle_readchar_unibyte (frame, c);
   int i = 0;
+  unsigned char buf[MAX_MULTIBYTE_LENGTH] = { 0 };
   buf[i++] = c;
   int len = BYTES_BY_CHAR_HEAD (c);
   while (i < len)
@@ -1224,7 +1222,7 @@ readchar_from_multibyte_file (register lread_frame *frame)
       if (c < 0 || ! TRAILING_CODE_P (c))
 	{
 	  for (i -= c < 0; 0 < --i; )
-	    frame->input.f.buf[frame->input.f.lookahead++] = c;
+	    frame->input.f.buf[frame->input.f.lookahead++] = buf[i];
 	  return BYTE8_TO_CHAR (buf[0]);
 	}
     }
@@ -1237,16 +1235,12 @@ readchar_from_multibyte_file (register lread_frame *frame)
 static int
 readchar_from_unibyte_file (register lread_frame *frame)
 {
-  READCHAR_BEGIN(frame);
-  c = readbyte_from_file (frame);
+  if (frame->unread_char >= 0)
+    return handle_buffered_unread_character (frame);
+  int c = readbyte_from_file (frame);
   if (c < 0)
-    {
-      frame->read_eof = true;
-      return c;
-    }
-  frame->char_offset++;
-  frame->byte_offset++;
-  return c;
+    return handle_readchar_eof (frame);
+  return handle_readchar_unibyte (frame, c);
 }
 
 /* Read one non-ASCII character from INFILE.  The character is
@@ -1256,7 +1250,13 @@ readchar_from_unibyte_file (register lread_frame *frame)
 static int
 readchar_from_mule_file (register lread_frame *frame)
 {
-  READCHAR_FROM_FILE_BEGIN(frame);
+  if (frame->unread_char >= 0)
+    return handle_buffered_unread_character (frame);
+  int c = readbyte_from_file (frame);
+  if (c < 0)
+    return handle_readchar_eof (frame);
+  if (ASCII_CHAR_P (c))
+    return handle_readchar_unibyte (frame, c);
   /* Emacs-mule coding uses at most 4-byte for one character.  */
   int len = emacs_mule_bytes[c];
   struct charset *charset;
@@ -1267,6 +1267,7 @@ readchar_from_mule_file (register lread_frame *frame)
     return BYTE8_TO_CHAR (c);
 
   int i = 0;
+  unsigned char buf[4] = { 0 };
   buf[i++] = c;
   while (i < len)
     {
@@ -1274,7 +1275,7 @@ readchar_from_mule_file (register lread_frame *frame)
       if (c < 0xA0)
 	{
 	  for (i -= c < 0; 0 < --i; )
-	    frame->input.f.buf[frame->input.f.lookahead++] = c;
+	    frame->input.f.buf[frame->input.f.lookahead++] = buf[i];
 	  return BYTE8_TO_CHAR (buf[0]);
 	}
     }
@@ -1310,128 +1311,145 @@ readchar_from_mule_file (register lread_frame *frame)
   return c;
 }
 
-#define READCHAR_STRING_BEGIN(frame) \
-  register int c; \
-  do \
-    { \
-      if (frame->char_offset >= frame->input.s.limit) \
-	{ \
-	  c = -1; \
-	  frame->read_eof = true; \
-	  return c; \
-	} \
-    } \
-  while (0);
-
-
 static int
 readchar_from_multibyte_string (register lread_frame *frame)
 {
-  READCHAR_STRING_BEGIN (frame);
-  c = fetch_string_char_advance_no_check (frame->input.s.string,
-					  &(frame->char_offset),
-					  &(frame->byte_offset));
+  if (frame->char_offset >= frame->input.s.limit)
+    return handle_readchar_eof (frame);
+  int c = fetch_string_char_advance_no_check (frame->input.s.string,
+					      &(frame->char_offset),
+					      &(frame->byte_offset));
   return c;
 }
 
 static int
 readchar_from_unibyte_string (register lread_frame *frame)
 {
-  READCHAR_STRING_BEGIN (frame);
-  c = SREF (frame->input.s.string, frame->byte_offset);
-  frame->char_offset++;
-  frame->byte_offset++;
+  if (frame->char_offset >= frame->input.s.limit)
+    return handle_readchar_eof (frame);
+  int c = SREF (frame->input.s.string, frame->byte_offset);
+  return handle_readchar_unibyte (frame, c);
+}
+
+
+INLINE struct buffer *
+readchar_from_buffer_start (register lread_frame *frame)
+{
+  struct buffer *inbuffer = XBUFFER (frame->input.b.buffer);
+  if (! BUFFER_LIVE_P (inbuffer))
+    return NULL;
+  frame->byte_offset = BUF_PT_BYTE (inbuffer);
+  if (frame->byte_offset >= BUF_ZV_BYTE (inbuffer))
+    return NULL;
+  return inbuffer;
+}
+
+INLINE int
+readchar_buffer_fetch_multibyte (register lread_frame *frame,
+				 struct buffer *inbuffer)
+{
+  /* Fetch the character code from the buffer.  */
+  unsigned char *p = BUF_BYTE_ADDRESS (inbuffer, frame->byte_offset);
+  int clen;
+  int c = string_char_and_length (p, &clen);
+  frame->byte_offset += clen;
   return c;
 }
 
+INLINE int
+readchar_buffer_fetch_unibyte (register lread_frame *frame,
+			       struct buffer *inbuffer)
+{
+  int c = BUF_FETCH_BYTE (inbuffer, frame->byte_offset);
+  c = UNIBYTE_TO_CHAR (c);
+  frame->byte_offset++;
+  return c;
+}
 
-#define READCHAR_BUFFER_SETUP(frame,src,pos)	\
-  int c;					\
-  struct buffer *inbuffer = XBUFFER ( (src) );	\
-  if (! BUFFER_LIVE_P (inbuffer))		\
-    return -1;					\
-  ptrdiff_t bytepos = ( pos );			\
-  do						\
-    {						\
-      if (bytepos >= BUF_ZV_BYTE (inbuffer))	\
-	{					\
-	  frame->read_eof = true;		\
-	  return -1;				\
-	}					\
-    } while (0)
-
-#define READCHAR_BUFFER_BEGIN(frame,src)			\
-  READCHAR_BUFFER_SETUP (frame, src, BUF_PT_BYTE (inbuffer))
-
-#define READCHAR_BUFFER_FETCH_MULTIBYTE(frame) \
-  /* Fetch the character code from the buffer.  */ \
-  unsigned char *p = BUF_BYTE_ADDRESS (inbuffer, bytepos); \
-  int clen; \
-  c = string_char_and_length (p, &clen); \
-  bytepos += clen
-
-#define READCHAR_BUFFER_FETCH_UNIBYTE(frame) \
-  c = BUF_FETCH_BYTE (inbuffer, bytepos); \
-  c = UNIBYTE_TO_CHAR (c); \
-  bytepos++
-
-#define READCHAR_BUFFER_END(frame) \
-  SET_BUF_PT_BOTH (inbuffer, BUF_PT (inbuffer) + 1, bytepos); \
-  frame->char_offset = BUF_PT(inbuffer); \
-  frame->byte_offset = bytepos
-
-#define READCHAR_MARKER_BEGIN(frame) \
-  READCHAR_BUFFER_SETUP (frame, frame->input.m.b.buffer, \
-			 marker_byte_position (frame->input.m.marker))
-
-#define READCHAR_MARKER_END(frame) \
-  frame->byte_offset = XMARKER (frame->input.m.marker)->bytepos = bytepos; \
-  frame->char_offset = ++(XMARKER (frame->input.m.marker)->charpos)
+INLINE void
+readchar_buffer_end (register lread_frame *frame, struct buffer *inbuffer)
+{
+  frame->char_offset = BUF_PT (inbuffer) + 1;
+  SET_BUF_PT_BOTH (inbuffer, frame->char_offset, frame->byte_offset);
+}
 
 static int
 readchar_from_multibyte_buffer (register lread_frame *frame)
 {
-  READCHAR_BUFFER_BEGIN (frame, frame->input.b.buffer);
-  READCHAR_BUFFER_FETCH_MULTIBYTE(frame);
-  READCHAR_BUFFER_END(frame);
+  struct buffer *inbuffer = readchar_from_buffer_start (frame);
+  if (inbuffer == NULL)
+    return handle_readchar_eof (frame);
+  int c = readchar_buffer_fetch_multibyte (frame, inbuffer);
+  readchar_buffer_end (frame, inbuffer);
   return c;
 }
 
 static int
 readchar_from_unibyte_buffer (register lread_frame *frame)
 {
-  READCHAR_BUFFER_BEGIN (frame,frame->input.b.buffer);
-  READCHAR_BUFFER_FETCH_UNIBYTE (frame);
-  READCHAR_BUFFER_END (frame);
+  struct buffer *inbuffer = readchar_from_buffer_start (frame);
+  if (inbuffer == NULL)
+    return handle_readchar_eof (frame);
+  int c = readchar_buffer_fetch_unibyte (frame, inbuffer);
+  readchar_buffer_end (frame, inbuffer);
   return c;
 }
 
+INLINE struct buffer *
+readchar_from_marker_start (register lread_frame *frame)
+{
+  struct buffer *inbuffer = XBUFFER ( (frame->input.m.b.buffer) );
+  if (! BUFFER_LIVE_P (inbuffer))
+    return NULL;
+  frame->byte_offset = marker_byte_position (frame->input.m.marker);
+  if (frame->byte_offset >= BUF_ZV_BYTE (inbuffer))
+    return NULL;
+  return inbuffer;
+}
+
+
+INLINE void
+readchar_marker_end (register lread_frame *frame)
+{
+  XMARKER (frame->input.m.marker)->bytepos = frame->byte_offset;
+  XMARKER (frame->input.m.marker)->charpos = ++(frame->char_offset);
+}
+
+
 static int
 readchar_from_multibyte_marker (register lread_frame *frame)
 {
-  READCHAR_MARKER_BEGIN (frame);
-  READCHAR_BUFFER_FETCH_MULTIBYTE (frame);
-  READCHAR_MARKER_END (frame);
+  struct buffer *inbuffer = readchar_from_marker_start (frame);
+  if (inbuffer == NULL)
+    return handle_readchar_eof (frame);
+  int c = readchar_buffer_fetch_multibyte (frame, inbuffer);
+  readchar_marker_end (frame);
   return c;
 }
 
 static int
 readchar_from_unibyte_marker (register lread_frame *frame)
 {
-  READCHAR_MARKER_BEGIN (frame);
-  READCHAR_BUFFER_FETCH_UNIBYTE (frame);
-  READCHAR_MARKER_END (frame);
+  struct buffer *inbuffer = readchar_from_marker_start (frame);
+  if (inbuffer == NULL)
+    return handle_readchar_eof (frame);
+  int c = readchar_buffer_fetch_unibyte (frame, inbuffer);
+  readchar_marker_end (frame);
   return c;
 }
 
 static int
 readchar_from_multibyte_procedure (register lread_frame *frame)
 {
-  READCHAR_BEGIN(frame);
+  if (frame->unread_char >= 0)
+    return handle_buffered_unread_character (frame);
   Lisp_Object tem = call0 (frame->input.p.proc);
-  c = (!FIXNUMP (tem)) ? -1 : XFIXNUM(tem);
+  int c = (!FIXNUMP (tem)) ? -1 : XFIXNUM(tem);
   if (c == -1)
-      frame->read_eof = true;
+    return handle_readchar_eof (frame);
+  frame->char_offset++;
+  /* Just approximate byte count as if it were coded in emacs-internal */
+  frame->byte_offset += CHAR_BYTES(c);
   return c;
 }
 
-- 
2.49.0