Re: (known?) Issue with bitmap iterators

[Jeff Law <law at redhat dot com>]

Alexander Monakov wrote:
> On Fri, 26 Jun 2009, Joe Buck wrote:
>
> > On Fri, Jun 26, 2009 at 03:38:31AM -0700, Alexander Monakov wrote:
> > > 1. Add bool field `modified_p' in bitmap structure.
> > > 2. Make iterator setup functions (e.g. bmp_iter_set_init) reset it to
> > > false.
> > > 3. Make functions that modify the bitmap set it to true.
> > > 4. Make iterator increment function (e.g. bmp_iter_next) assert
> > > !modified_p.
> >
> > Sorry, it doesn't work.  Function foo has a loop that iterates
> > over a bitmap.  During the iteration, it calls a function bar.  bar
> > modifies the bitmap, then iterates over the bitmap.  It then returns
> > to foo, which is in the middle of an iteration, which it continues.
> > The bitmap has been modified (by bar), but modified_p was reset to
> > false by the iteration that happened at the end of bar.
>
> Good catch, thanks!  Let me patch that up a bit:
>
> 1. Add int field `generation' in bitmap structure, initialized 
> arbitrarily at bitmap creation time.
> 2. Make iterator setup functions save initial generation count in the 
> iterator (or generation counts for two bitmaps, if needed).
> 3. Modify generation count when bitmap is modified (e.g. increment it).
> 4. Verify that saved and current generations match when incrementing 
> the iterator.
It's not bad -- we don't catch the offending modification when it 
occurs, but I guess if we trigger the check, then it's not terribly 
difficult to use conditional breakpoint under the debugger to get a 
better idea of when/how we tried to modify a bitmap while iterating over 
it.  I can probably break this with some ugly exit code from an iterator 
(*), but it'd be a big step forward.



Jeff

(*) Imagine something like this (and related variants)

EXECUTE_IF_SET_IN_BITMAP (bitmap, 0, i, bi)
{
 blah blah blah

 if (bitmap_empty_p (bitmap))
   {
     modify bitmap
     break;
   }
 more blah blah
}

We exit without iterating BI and thus miss your check.