Re: another core dump to be fixed before 1.4.5

[Eric Blake]

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

Good thing I waited - the previous version of the patch crashed when
doing traceoff without arguments, since I modified a callback funtion
to hack_all_symbols to modify the symbol table while traversing it,
ultimately dereferencing freed memory.  This followon patch solves
that issue; I will still wait a couple days before applying both patches
together, unless I get feedback first.

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

        * src/symtab.c (hack_all_symbols): Allow certain modifications of
        the symbol table during traversal.
        * src/builtin.c (set_trace): Replace SYMBOL_DELETE with
        SYMBOL_POPDEF, since only the latter is safe with
        hack_all_symbols.

diff -u src/builtin.c src/builtin.c
--- src/builtin.c       2 Jun 2006 13:19:21 -0000
+++ src/builtin.c       3 Jun 2006 01:31:09 -0000
@@ -1181,7 +1181,7 @@
   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);
+    lookup_symbol (SYMBOL_NAME (sym), SYMBOL_POPDEF);
 }
 
 static void
diff -u src/symtab.c src/symtab.c
--- src/symtab.c        2 Jun 2006 13:19:21 -0000
+++ src/symtab.c        3 Jun 2006 01:31:09 -0000
@@ -233,24 +233,34 @@
     }
 }
 
-/*----------------------------------------------------------------------.
-| The following function is used for the cases, where we want to do     |
-| something to each and every symbol in the table.  The function        |
-| hack_all_symbols () traverses the symbol table, and calls a specified |
-| function FUNC for each symbol in the table.  FUNC is called with a    |
-| pointer to the symbol, and the DATA argument.                                
|
-`----------------------------------------------------------------------*/
+/*------------------------------------------------------------------.
+| The following function is used for the cases, where we want to do |
+| something to each and every symbol in the table.  The function    |
+| hack_all_symbols () traverses the symbol table, and calls a       |
+| specified function FUNC for each symbol in the table.  FUNC is    |
+| called with a pointer to the symbol, and the DATA argument.       |
+|                                                                   |
+| FUNC may safely call lookup_symbol with mode SYMBOL_POPDEF or     |
+| SYMBOL_LOOKUP, but any other mode can break the iteration.        |
+`------------------------------------------------------------------*/
 
 void
 hack_all_symbols (hack_symbol *func, const char *data)
 {
   int h;
   symbol *sym;
+  symbol *next;
 
   for (h = 0; h < hash_table_size; h++)
     {
-      for (sym = symtab[h]; sym != NULL; sym = SYMBOL_NEXT (sym))
-       (*func) (sym, data);
+      /* We allow func to call SYMBOL_POPDEF, which can invalidate
+         sym, so we must grab the next element to traverse before
+         calling func.  */
+      for (sym = symtab[h]; sym != NULL; sym = next)
+        {
+          next = SYMBOL_NEXT (sym);
+          (*func) (sym, data);
+        }
     }
 }