Re: bug report and patch: handling of chained wrapups

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi John,

According to john on 1/24/2006 12:31 PM:
> Hello,
> 
> There is a bug in how GNU m4 processes text saved with m4wrap()
> once it is already using such text as input.
> 
> The behaviour is exposed by:
> 
>    $ m4
>    m4wrap(`m4wrap(a)m4wrap(b)')
>    ^D

Thanks again for the report.  I decided that for the 1.4.x release branch,
minimal code changes was better than fixing the POSIX semantic bug at the
same time.

> Gory details and analysis are in the attached PDF.
> A simple patch against m4-1.4.4 to fix this bug is also attached.

I went with the less invasive patch here (54 lines instead of 84,
according to diffstat), and added the missing ChangeLog.  Also in
branch-1_4 of CVS, I have added testsuite exposure, so that we can make
sure we don't break this in the future when we fix m4wrap to obey POSIX
semantics of FIFO ordering instead of LIFO ordering.  I will (eventually)
work on porting this to CVS head.

I would appreciate a second set of eyes validating that my patch is
correct, but went ahead and checked it in.

Still one more coredump to solve before 1.4.5 is ready for release (your
analysis of undefining a macro during its expansion).

2006-05-31  Eric Blake  <address@hidden>
            John Brzustowski  <address@hidden>

        * src/input.c (input_stack): Delete; use current_input instead.
        (wrapup_stack): Dynamically allocate, so that recursion is handled
        properly.
        (push_wrapup): Use current wrapup stack.
        (pop_wrapup): Rotate wrapup stack to current, and create new
        wrapup stack.
        (input_init): Dynamically allocate stacks.
        * NEWS: Update, now that recursive m4wrap can no longer cause
        core dump.

- --
Life is short - so eat dessert first!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFEfi/884KuGfSFAYARAr8QAJ9C5xGg/do0XoXGFMTkE5+uswHdrACaA4pX
XNxstcOBWhAMW/11gs9mIMo=
=Phyy
-----END PGP SIGNATURE-----

Index: src/input.c
===================================================================
RCS file: /sources/m4/m4/src/Attic/input.c,v
retrieving revision 1.1.1.1.2.4
diff -u -p -r1.1.1.1.2.4 input.c
--- src/input.c 4 Dec 2005 20:17:36 -0000       1.1.1.1.2.4
+++ src/input.c 31 May 2006 23:56:30 -0000
@@ -1,6 +1,6 @@
 /* GNU m4 -- A simple macro processor
 
-   Copyright (C) 1989, 1990, 1991, 1992, 1993, 1994, 2004, 2005
+   Copyright (C) 1989, 1990, 1991, 1992, 1993, 1994, 2004, 2005, 2006
    Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify
@@ -28,15 +28,17 @@
    or quoted macro definitions (as returned by the builtin "defn").
    Unread input are organised in a stack, implemented with an obstack.
    Each input source is described by a "struct input_block".  The obstack
-   is "input_stack".  The top of the input stack is "isp".
+   is "current_input".  The top of the input stack is "isp".
 
-   The macro "m4wrap" places the text to be saved on another input stack,
-   on the obstack "wrapup_stack", whose top is "wsp".  When EOF is seen
-   on normal input (eg, when "input_stack" is empty), input is switched
-   over to "wrapup_stack".  To make this easier, all references to the
-   current input stack, whether it be "input_stack" or "wrapup_stack",
-   are done through a pointer "current_input", which points to either
-   "input_stack" or "wrapup_stack".
+   The macro "m4wrap" places the text to be saved on another input
+   stack, on the obstack "wrapup_stack", whose top is "wsp".  When EOF
+   is seen on normal input (eg, when "current_input" is empty), input is
+   switched over to "wrapup_stack", and the original "current_input" is
+   freed.  A new stack is allocated for "wrapup_stack", which will
+   accept any text produced by calls to "m4wrap" from within the
+   wrapped text.  This process of shuffling "wrapup_stack" to
+   "current_input" can continue indefinitely, even generating infinite
+   loops (e.g. "define(`f',`m4wrap(`f')')f"), without memory leaks.
 
    Pushing new input on the input stack is done by push_file (),
    push_string (), push_wrapup () (for wrapup text), and push_macro ()
@@ -109,13 +111,10 @@ int current_line;
 /* Obstack for storing individual tokens.  */
 static struct obstack token_stack;
 
-/* Normal input stack.  */
-static struct obstack input_stack;
-
 /* Wrapup input stack.  */
-static struct obstack wrapup_stack;
+static struct obstack *wrapup_stack;
 
-/* Input or wrapup.  */
+/* Current stack, from input or wrapup.  */
 static struct obstack *current_input;
 
 /* Bottom of token_stack, for obstack_free.  */
@@ -283,11 +282,11 @@ push_string_finish (void)
 void
 push_wrapup (const char *s)
 {
-  input_block *i = (input_block *) obstack_alloc (&wrapup_stack,
+  input_block *i = (input_block *) obstack_alloc (wrapup_stack,
                                                  sizeof (struct input_block));
   i->prev = wsp;
   i->type = INPUT_STRING;
-  i->u.u_s.string = obstack_copy0 (&wrapup_stack, s, strlen (s));
+  i->u.u_s.string = obstack_copy0 (wrapup_stack, s, strlen (s));
   wsp = i;
 }
 
@@ -343,10 +342,20 @@ pop_input (void)
 boolean
 pop_wrapup (void)
 {
+  obstack_free (current_input, NULL);
+  xfree (current_input);
+
   if (wsp == NULL)
-    return FALSE;
+    {
+      obstack_free (wrapup_stack, NULL);
+      xfree (wrapup_stack);
+      return FALSE;
+    }
+
+  current_input = wrapup_stack;
+  wrapup_stack = (struct obstack *) xmalloc (sizeof (struct obstack));
+  obstack_init (wrapup_stack);
 
-  current_input = &wrapup_stack;
   isp = wsp;
   wsp = NULL;
 
@@ -566,10 +575,11 @@ input_init (void)
   current_line = 0;
 
   obstack_init (&token_stack);
-  obstack_init (&input_stack);
-  obstack_init (&wrapup_stack);
 
-  current_input = &input_stack;
+  current_input = (struct obstack *) xmalloc (sizeof (struct obstack));
+  obstack_init (current_input);
+  wrapup_stack = (struct obstack *) xmalloc (sizeof (struct obstack));
+  obstack_init (wrapup_stack);
 
   obstack_1grow (&token_stack, '\0');
   token_bottom = obstack_finish (&token_stack);
@@ -699,7 +709,7 @@ set_word_regexp (const char *regexp)
 | for a quoted string; TOKEN_WORD for something that is a potential macro  |
 | name; and TOKEN_SIMPLE for any single character that is not a part of        
   |
 | any of the previous types.                                              |
-|                                                                         |
+|                                                                         |
 | Next_token () return the token type, and passes back a pointer to the        
   |
 | token data through TD.  The token text is collected on the obstack      |
 | token_stack, which never contains more than one token text at a time.        
   |