Re: another core dump to be fixed before 1.4.5

[Eric Blake]

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

According to Eric Blake on 5/31/2006 10:48 PM:
> Just found this one:
> 
> $ m4 -tfoo
> indir(`foo')
> stdin:1: m4: INTERNAL ERROR: Bad symbol type in call_macro ()
> Aborted (core dumped)
> 
> It turns out the hash table tracks macro names that must be traced once 
> defined, but indir does not filter on whether the macro is already defined.  
> I'll add a test case and patch this soon.

And here's the proposed patch.  I will wait a couple days for comments
before applying this, since it changes the semantics of traceon and
traceoff (although it changes them to match how CVS head treats them).  In
the process, I also noticed that ifdef(`foo') was incorrectly treating foo
as defined in the same situations that indir(`foo') was crashing.

2006-06-02  Eric Blake  <address@hidden>

        Solve crash when passing "indir(`foo')" to "m4 -tfoo".
        * src/symtab.c (lookup_symbol) <SYMBOL_DELETE, SYMBOL_POPDEF>:
        Preserve placeholder when macro is being traced.
        * src/builtin.c (m4_ifdef, m4_indir): A traced but undefined
        symbol is not defined.
        (set_trace): Remove placeholder when no longer traced.
        (m4_traceon): On named traces, always reserve a slot in the
        symbol table.
        (m4_traceoff): Don't warn about untracing a nonexistent symbol.
        * NEWS: Document new trace behavior.
        * doc/m4.texinfo (Trace): Tracing by name now consistently works
        no matter whether that macro is currently defined.
        (Incompatibilities): Document differences between traditional and
        GNU trace.

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

iD8DBQFEgD2c84KuGfSFAYARAgRVAJ0e3hfv3rtWPuykH68/zZ6KV9GyzwCggeQ5
XPkmNYRmE8zJ9tHRHWAF/ng=
=8og0
-----END PGP SIGNATURE-----

Index: NEWS
===================================================================
RCS file: /sources/m4/m4/NEWS,v
retrieving revision 1.1.1.1.2.13
diff -u -p -r1.1.1.1.2.13 NEWS
--- NEWS        1 Jun 2006 00:09:06 -0000       1.1.1.1.2.13
+++ NEWS        2 Jun 2006 13:19:20 -0000
@@ -7,6 +7,11 @@ Version 1.4.5 - ??? 2006, by ???
 * Fix a recursive push_string crashing bug.
 * Use automake to fix build portability issues.
 * Fix a recursive m4wrap crashing bug.
+* Tracing a macro by name is now persistent, even if the macro is
+  subsequently undefined or redefined.  The traceon and traceoff macros no
+  longer warn about undefined symbols.  This solves a crash when using
+  indir on an undefined macro traced with the -t option, as well as an
+  incorrect result of ifdef.
 
 Version 1.4.4 - October 2005, by Gary V. Vaughan
 
Index: doc/m4.texinfo
===================================================================
RCS file: /sources/m4/m4/doc/m4.texinfo,v
retrieving revision 1.1.1.1.2.15
diff -u -p -r1.1.1.1.2.15 m4.texinfo
--- doc/m4.texinfo      30 May 2006 13:52:29 -0000      1.1.1.1.2.15
+++ doc/m4.texinfo      2 Jun 2006 13:19:21 -0000
@@ -1620,6 +1620,36 @@ The number between dashes is the depth o
 of the time, signifying an expansion at the outermost level, but it
 increases when macro arguments contain unquoted macro calls.
 
+Tracing by name is an attribute that is preserved whether the macro is
+defined or not.  This allows the @samp{-t} option to select macros to
+trace before those macros are defined.
+
address@hidden
+traceoff(`foo')
address@hidden
+traceon(`foo')
address@hidden
+foo
address@hidden
+define(`foo', `bar')
address@hidden
+foo
address@hidden: -1- foo -> `bar'
address@hidden
+undefine(`foo')
address@hidden
+ifdef(`foo', `yes', `no')
address@hidden
+indir(`foo')
address@hidden:17: m4: Undefined macro `foo'
address@hidden
+define(`foo', `blah')
address@hidden
+foo
address@hidden: -1- foo -> `blah'
address@hidden
address@hidden example
+
 @xref{Debug Levels}, for information on controlling the details of the
 display.
 
@@ -3462,6 +3492,18 @@ leaves the door open for a data race in 
 a file by the same name.  GNU @code{m4} actually creates a temporary
 file for each invocation of @code{maketemp}, which means that the output
 of the macro is different even if the input is identical.
+
address@hidden
+Traditional @code{m4} treats @code{traceon} (@pxref{Trace}) without
+arguments as a global variable, independent of named macro tracing.
+Also, once a macro is undefined, named tracing of that macro is lost.
+On the other hand, when GNU @code{m4} encounters @code{traceon} without
+arguments, it turns tracing on for all existing definitions at the time,
+but does not trace future definitions; @code{traceoff} without arguments
+turns tracing off for all definitions regardless of whether they were
+also traced by name; and tracing by name, such as with @samp{-tfoo} at
+the command line or @code{traceon(`foo')} in the input, is an attribute
+that is preserved even if the macro is currently undefined.
 @end itemize
 
 @node Other Incompatibilities,  , Incompatibilities, Compatibility
Index: src/builtin.c
===================================================================
RCS file: /sources/m4/m4/src/Attic/builtin.c,v
retrieving revision 1.1.1.1.2.5
diff -u -p -r1.1.1.1.2.5 builtin.c
--- src/builtin.c       31 May 2006 22:54:13 -0000      1.1.1.1.2.5
+++ src/builtin.c       2 Jun 2006 13:19:21 -0000
@@ -488,7 +488,7 @@ m4_ifdef (struct obstack *obs, int argc,
     return;
   s = lookup_symbol (ARG (1), SYMBOL_LOOKUP);
 
-  if (s != NULL)
+  if (s != NULL && SYMBOL_TYPE (s) != TOKEN_VOID)
     result = ARG (2);
   else if (argc == 4)
     result = ARG (3);
@@ -695,7 +695,7 @@ m4_indir (struct obstack *obs, int argc,
     return;
 
   s = lookup_symbol (name, SYMBOL_LOOKUP);
-  if (s == NULL)
+  if (s == NULL || SYMBOL_TYPE (s) == TOKEN_VOID)
     M4ERROR ((warning_status, 0,
              "Undefined macro `%s'", name));
   else
@@ -1179,6 +1179,9 @@ static void
 set_trace (symbol *sym, const char *data)
 {
   SYMBOL_TRACED (sym) = (boolean) (data != NULL);
+  /* Remove placeholder from table if macro is undefined and untraced.  */
+  if (SYMBOL_TYPE (sym) == TOKEN_VOID && data == NULL)
+    lookup_symbol (SYMBOL_NAME (sym), SYMBOL_DELETE);
 }
 
 static void
@@ -1192,12 +1195,8 @@ m4_traceon (struct obstack *obs, int arg
   else
     for (i = 1; i < argc; i++)
       {
-       s = lookup_symbol (TOKEN_DATA_TEXT (argv[i]), SYMBOL_LOOKUP);
-       if (s != NULL)
-         set_trace (s, (char *) obs);
-       else
-         M4ERROR ((warning_status, 0,
-                   "Undefined name %s", TOKEN_DATA_TEXT (argv[i])));
+       s = lookup_symbol (TOKEN_DATA_TEXT (argv[i]), SYMBOL_INSERT);
+       set_trace (s, (char *) obs);
       }
 }
 
@@ -1219,9 +1218,6 @@ m4_traceoff (struct obstack *obs, int ar
        s = lookup_symbol (TOKEN_DATA_TEXT (argv[i]), SYMBOL_LOOKUP);
        if (s != NULL)
          set_trace (s, NULL);
-       else
-         M4ERROR ((warning_status, 0,
-                   "Undefined name %s", TOKEN_DATA_TEXT (argv[i])));
       }
 }
 
Index: src/symtab.c
===================================================================
RCS file: /sources/m4/m4/src/Attic/symtab.c,v
retrieving revision 1.1.1.1.2.2
diff -u -p -r1.1.1.1.2.2 symtab.c
--- src/symtab.c        1 May 2005 11:54:12 -0000       1.1.1.1.2.2
+++ src/symtab.c        2 Jun 2006 13:19:21 -0000
@@ -1,6 +1,6 @@
 /* GNU m4 -- A simple macro processor
 
-   Copyright (C) 1989, 1990, 1991, 1992, 1993, 1994, 2003 Free
+   Copyright (C) 1989, 1990, 1991, 1992, 1993, 1994, 2003, 2006 Free
    Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify
@@ -89,18 +89,18 @@ free_symbol (symbol *sym)
   xfree ((voidstar) sym);
 }
 
-/*------------------------------------------------------------------------.
-| Search in, and manipulation of the symbol table, are all done by       |
-| lookup_symbol ().  It basically hashes NAME to a list in the symbol    |
-| table, and searched this list for the first occurence of a symbol with  |
-| the name.                                                              |
-|                                                                        |
-| The MODE parameter determines what lookup_symbol () will do.  It can   |
-| either just do a lookup, do a lookup and insert if not present, do an        
  |
-| insertion even if the name is already in the list, delete the first    |
-| occurrence of the name on the list or delete all occurences of the name |
-| on the list.                                                           |
-`------------------------------------------------------------------------*/
+/*-------------------------------------------------------------------.
+| Search in, and manipulation of the symbol table, are all done by   |
+| lookup_symbol ().  It basically hashes NAME to a list in the      |
+| symbol table, and searches this list for the first occurrence of a |
+| symbol with the name.                                                     |
+|                                                                   |
+| The MODE parameter determines what lookup_symbol () will do.  It   |
+| can either just do a lookup, do a lookup and insert if not        |
+| present, do an insertion even if the name is already in the list,  |
+| delete the first occurrence of the name on the list, or delete all |
+| occurrences of the name on the list.                              |
+`-------------------------------------------------------------------*/
 
 symbol *
 lookup_symbol (const char *name, symbol_lookup mode)
@@ -166,29 +166,64 @@ lookup_symbol (const char *name, symbol_
 
     case SYMBOL_DELETE:
 
-      /* Delete all occurences of symbols with NAME.  */
+      /* Delete all occurrences of symbols with NAME.  However, if symbol
+        is marked for tracing, leave a placeholder in the table.  */
 
       if (cmp != 0 || sym == NULL)
        return NULL;
-      do
-       {
-         *spp = SYMBOL_NEXT (sym);
-         free_symbol (sym);
-         sym = *spp;
-       }
-      while (sym != NULL && strcmp (name, SYMBOL_NAME (sym)) == 0);
+      {
+       boolean traced = SYMBOL_TRACED (sym);
+       while (SYMBOL_NEXT (sym) != NULL
+              && SYMBOL_SHADOWED (SYMBOL_NEXT (sym)))
+         {
+           *spp = SYMBOL_NEXT (sym);
+           free_symbol (sym);
+           sym = *spp;
+         }
+       if (traced)
+         {
+           if (SYMBOL_TYPE (sym) == TOKEN_TEXT)
+             xfree (SYMBOL_TEXT (sym));
+           SYMBOL_TYPE (sym) = TOKEN_VOID;
+           SYMBOL_TRACED (sym) = TRUE;
+           SYMBOL_SHADOWED (sym) = FALSE;
+         }
+       else
+         {
+           *spp = SYMBOL_NEXT (sym);
+           free_symbol (sym);
+           sym = *spp;
+         }
+      }
       return NULL;
 
     case SYMBOL_POPDEF:
 
-       /* Delete the first occurence of a symbol with NAME.  */
+      /* Delete the first occurrence of a symbol with NAME.  However, if
+        symbol is marked for tracing, and this is the last copy, leave a
+        placeholder in the table.  */
 
       if (cmp != 0 || sym == NULL)
        return NULL;
-      if (SYMBOL_NEXT (sym) != NULL && cmp == 0)
-       SYMBOL_SHADOWED (SYMBOL_NEXT (sym)) = FALSE;
-      *spp = SYMBOL_NEXT (sym);
-      free_symbol (sym);
+      if (SYMBOL_NEXT (sym) != NULL
+         && SYMBOL_SHADOWED (SYMBOL_NEXT (sym)))
+       {
+         SYMBOL_SHADOWED (SYMBOL_NEXT (sym)) = FALSE;
+         SYMBOL_TRACED (SYMBOL_NEXT (sym)) = SYMBOL_TRACED (sym);
+         *spp = SYMBOL_NEXT (sym);
+         free_symbol (sym);
+       }
+      else if (SYMBOL_TRACED (sym))
+       {
+         if (SYMBOL_TYPE (sym) == TOKEN_TEXT)
+           xfree (SYMBOL_TEXT (sym));
+         SYMBOL_TYPE (sym) = TOKEN_VOID;
+       }
+      else
+       {
+         *spp = SYMBOL_NEXT (sym);
+         free_symbol (sym);
+       }
       return NULL;
 
     default: