Re: possible gawk bug

[Aharon Robbins]

Greetings.  Re this:

> From: address@hidden
> Date: Mon, 5 Feb 2001 09:30:51 -0500 (EST)
> To: address@hidden
> cc: address@hidden
> Subject: possible gawk bug
>
> I'm having a hell of a time with what should be a fairly simple awk
> program.  I'm about to conclude there is a bug in gawk.  I've tried nawk
> and mawk on various operating systems and those give the result I
> expect. 
>
> the bug is in handling of an associative array.  My program reads data
> from a file into an array.  Then some processing is done by 
> for (toppkg in topdepends){do stuff .... }
> but it seems that sometimes elements in the array get missed.
> [......]

The way gawk currently handles associative arrays makes it possible for
weird things to happen if the array is updated while the for (... in ...)
loop is running.  This is what's happening in your program, although I'm
not sure how you're managing to do this.

The following unofficial patch revamps the loop handling to simply loop
through all the elements that exist when the loop starts.  It also makes
your program work like you expect, and should be, in general, an improvement.

A fix similar to this will be in the upcoming 3.1 release.

Thanks for the bug report,

Arnold

*** ../gawk-3.0.6/awk.h Sun Jul 16 16:52:37 2000
--- awk.h       Tue Feb  6 11:00:33 2001
***************
*** 471,484 ****
        NODE *incr;
  } FOR_LOOP_HEADER;
  
- /* for "for(iggy in foo) {" */
- struct search {
-       NODE *sym;
-       size_t idx;
-       NODE *bucket;
-       NODE *retval;
- };
- 
  /* for faster input, bypass stdio */
  typedef struct iobuf {
        const char *name;
--- 471,476 ----
***************
*** 720,727 ****
  extern NODE **assoc_lookup P((NODE *symbol, NODE *subs));
  extern void do_delete P((NODE *symbol, NODE *tree));
  extern void do_delete_loop P((NODE *symbol, NODE *tree));
- extern void assoc_scan P((NODE *symbol, struct search *lookat));
- extern void assoc_next P((struct search *lookat));
  extern NODE *assoc_dump P((NODE *symbol));
  extern NODE *do_adump P((NODE *tree));
  /* awktab.c */
--- 712,717 ----
*** ../gawk-3.0.6/array.c       Thu Aug  3 20:08:01 2000
--- array.c     Tue Feb  6 11:00:46 2001
***************
*** 455,520 ****
        assoc_clear(symbol);
  }
  
- /* assoc_scan --- start a ``for (iggy in foo)'' loop */
- 
- void
- assoc_scan(symbol, lookat)
- NODE *symbol;
- struct search *lookat;
- {
-       lookat->sym = symbol;
-       lookat->idx = 0;
-       lookat->bucket = NULL;
-       lookat->retval = NULL;
-       if (symbol->var_array != NULL)
-               assoc_next(lookat);
- }
- 
- /* assoc_next --- actually find the next element in array */
- 
- void
- assoc_next(lookat)
- struct search *lookat;
- {
-       register NODE *symbol = lookat->sym;
-       
-       if (symbol == NULL)
-               fatal("null symbol in assoc_next");
-       if (symbol->var_array == NULL || lookat->idx > symbol->array_size) {
-               lookat->retval = NULL;
-               return;
-       }
-       /*
-        * This is theoretically unsafe.  The element bucket might have
-        * been freed if the body of the scan did a delete on the next
-        * element of the bucket.  The only way to do that is by array
-        * reference, which is unlikely.  Basically, if the user is doing
-        * anything other than an operation on the current element of an
-        * assoc array while walking through it sequentially, all bets are
-        * off.  (The safe way is to register all search structs on an
-        * array with the array, and update all of them on a delete or
-        * insert)
-        */
-       if (lookat->bucket != NULL) {
-               lookat->retval = lookat->bucket->ahname;
-               lookat->bucket = lookat->bucket->ahnext;
-               return;
-       }
-       for (; lookat->idx < symbol->array_size; lookat->idx++) {
-               NODE *bucket;
- 
-               if ((bucket = symbol->var_array[lookat->idx]) != NULL) {
-                       lookat->retval = bucket->ahname;
-                       lookat->bucket = bucket->ahnext;
-                       lookat->idx++;
-                       return;
-               }
-       }
-       lookat->retval = NULL;
-       lookat->bucket = NULL;
-       return;
- }
- 
  /* grow_table --- grow a hash table */
  
  static void
--- 455,460 ----
*** ../gawk-3.0.6/eval.c        Sun Jul 16 16:54:12 2000
--- eval.c      Tue Feb  6 11:04:33 2001
***************
*** 479,504 ****
  
        case Node_K_arrayfor:
                {
-               volatile struct search l;       /* For array_for */
                Func_ptr after_assign = NULL;
  
  #define hakvar forloop->init
  #define arrvar forloop->incr
                PUSH_BINDING(loop_tag_stack, loop_tag, loop_tag_valid);
                lhs = get_lhs(tree->hakvar, &after_assign);
-               t = tree->arrvar;
-               if (t->type == Node_param_list)
-                       t = stack_ptr[t->param_cnt];
-               if (t->type == Node_array_ref)
-                       t = t->orig_array;
                stable_tree = tree;
!               if ((t->flags & SCALAR) != 0)
!                       fatal("attempt to use scalar as array");
!               for (assoc_scan(t, (struct search *)&l);
!                    l.retval;
!                    assoc_next((struct search *)&l)) {
                        unref(*((NODE **) lhs));
!                       *lhs = dupnode(l.retval);
                        if (after_assign)
                                (*after_assign)();
                        switch (setjmp(loop_tag)) {
--- 479,532 ----
  
        case Node_K_arrayfor:
                {
                Func_ptr after_assign = NULL;
+               NODE **list = 0;
+               NODE **lhs;
+               NODE *array;
+               size_t i, j, num_elems;
+               int retval = 0;
  
  #define hakvar forloop->init
  #define arrvar forloop->incr
+               /* get the array */
+               array = tree->arrvar;
+               if (array->type == Node_param_list)
+                       array = stack_ptr[array->param_cnt];
+               if (array->type == Node_array_ref)
+                       array = array->orig_array;
+               if ((array->flags & SCALAR) != 0)
+                       fatal("attempt to use scalar as array");
+ 
+               /* sanity: do nothing if empty */
+               if (array->type == Node_var || array->var_array == NULL
+                   || array->table_size == 0) {
+                       break;  /* from switch */
+               }
+ 
+               /* allocate space for array */
+               num_elems = array->table_size;
+               emalloc(list, NODE **, num_elems * sizeof(NODE *), "for_loop");
+ 
+               /* populate it */
+               for (i = j = 0; i < array->array_size; i++) {
+                       NODE *t = array->var_array[i];
+ 
+                       if (t == NULL)
+                               continue;
+ 
+                       for (; t != NULL; t = t->ahnext) {
+                               list[j++] = dupnode(t->ahname);
+                       }
+               }
+ 
+               /* now we can run the loop */
                PUSH_BINDING(loop_tag_stack, loop_tag, loop_tag_valid);
+ 
                lhs = get_lhs(tree->hakvar, &after_assign);
                stable_tree = tree;
!               for (i = 0; i < num_elems; i++) {
                        unref(*((NODE **) lhs));
!                       *lhs = dupnode(list[i]);
                        if (after_assign)
                                (*after_assign)();
                        switch (setjmp(loop_tag)) {
***************
*** 508,520 ****
                                break;
  
                        case TAG_BREAK:
!                               RESTORE_BINDING(loop_tag_stack, loop_tag, 
loop_tag_valid);
!                               return 1;
                        default:
                                cant_happen();
                        }
                }
                RESTORE_BINDING(loop_tag_stack, loop_tag, loop_tag_valid);
                break;
                }
  
--- 536,563 ----
                                break;
  
                        case TAG_BREAK:
!                               retval = 1;
!                               goto done;
! 
                        default:
                                cant_happen();
                        }
                }
+ 
+       done:
                RESTORE_BINDING(loop_tag_stack, loop_tag, loop_tag_valid);
+ 
+               if (do_lint && num_elems != array->table_size)
+                       warning("for loop: array `%s' changed size from %d to 
%d during loop execution",
+                                       array->vname, num_elems, 
array->table_size);
+               
+               for (i = 0; i < num_elems; i++)
+                       unref(list[i]);
+ 
+               free(list);
+ 
+               if (retval == 1)
+                       return 1;
                break;
                }