Re: (known?) Issue with bitmap iterators

[Jeff Law <law at redhat dot com>]

Richard Guenther wrote:
> On Sat, Jun 20, 2009 at 4:54 PM, Jeff Law<law@redhat.com> wrote:
>   
> > Imagine a loop like this
> >
> > EXECUTE_IF_SET_IN_BITMAP (something, 0, i, bi)
> >  {
> >   bitmap_clear_bit (something, i)
> >   [ ... whatever code we want to process i, ... ]
> >  }
> >
> > This code is unsafe.
> >
> > If bit I happens to be the only bit set in the current bitmap word, then
> > bitmap_clear_bit will free the current element and return it to the element
> > free list where it can be recycled.
> >
> > So assume the bitmap element gets recycled for some other bitmap...  We then
> > come around to the top of the loop where EXECUTE_IF_SET_IN_BITMAP will call
> > bmp_iter_set which can reference the recycled element when it tries to
> > advance to the next element via bi->elt1 = bi->elt1->next.  So we start
> > iterating on elements of a completely different bitmap.  You can assume this
> > is not good.
> >
> > Our documentation clearly states that I is to remain unchanged, but ISTM
> > that the bitmap we're iterating over needs to remain unchanged as well.
> > Is this a known issue, or am I just the lucky bastard who tripped over it
> > and now gets to audit all the EXECUTE_IF_SET_IN_BITMAP loops?
> >     
>
> It is known (but maybe not appropriately documented) that deleting
> bits in the bitmap you iterate over is not safe.  If it would be me I would
> see if I could make it safe though.
>   
It's not a huge deal -- what bothers me is that it's not documented.  
Someone thought enough to document that the loop index shouldn't be 
modified in the loop, but didn't bother to mention that the bitmap 
itself shouldn't be modified in the loop.

I'd lean towards the idea of making the bitmap readonly -- hopefully 
everyone already knows they can't set bits in the bitmap and expect the 
iterator to work, so let's keep things consistent with clearing bits.  
If we can back that up with some checking code, then I think we'd be in 
good shape.

jeff