diff mbox

bitmap fix for current

Message ID 61212D56-5FE8-4496-BC6E-19920F55236E@comcast.net
State New
Headers show

Commit Message

Mike Stump Nov. 13, 2014, 7:37 p.m. UTC
I was doing a merge, and it failed to even compile the runtime libraries due to checking in bitmap.  bitmap goes to remove set bits from the bitmap (the second hunk in a two hunk set), and it fails to update the current pointer.  That memory is freed and then reallocated and a new index is put into it, and then we fail a consistency check later on due to the mismatch between head->index and head->current->indx, because current was not properly maintained.  This patch removes the old value of current when we remove what it points to from the bitmap.

Ok?

Comments

Jeff Law Nov. 14, 2014, 1:10 a.m. UTC | #1
On 11/13/14 12:37, Mike Stump wrote:
> I was doing a merge, and it failed to even compile the runtime
> libraries due to checking in bitmap.  bitmap goes to remove set bits
> from the bitmap (the second hunk in a two hunk set), and it fails to
> update the current pointer.  That memory is freed and then
> reallocated and a new index is put into it, and then we fail a
> consistency check later on due to the mismatch between head->index
> and head->current->indx, because current was not properly maintained.
> This patch removes the old value of current when we remove what it
> points to from the bitmap.
Was the calling code iterating through the bit with a form like

EXECUTE_IF_SET_IN_BITMAP (something, 0, i, bi)
  {
    bitmap_clear_bit (something, i)
    [ ... whatever code we want to process i, ... ]
  }

If so, that's the real issue and we'd really like to identify & fix any 
code that has that kind of structure.

See:

https://gcc.gnu.org/ml/gcc/2009-06/msg00482.html

jeff
Richard Biener Nov. 14, 2014, 10:26 a.m. UTC | #2
On Fri, Nov 14, 2014 at 2:10 AM, Jeff Law <law@redhat.com> wrote:
> On 11/13/14 12:37, Mike Stump wrote:
>>
>> I was doing a merge, and it failed to even compile the runtime
>> libraries due to checking in bitmap.  bitmap goes to remove set bits
>> from the bitmap (the second hunk in a two hunk set), and it fails to
>> update the current pointer.  That memory is freed and then
>> reallocated and a new index is put into it, and then we fail a
>> consistency check later on due to the mismatch between head->index
>> and head->current->indx, because current was not properly maintained.
>> This patch removes the old value of current when we remove what it
>> points to from the bitmap.
>
> Was the calling code iterating through the bit with a form like
>
> EXECUTE_IF_SET_IN_BITMAP (something, 0, i, bi)
>  {
>    bitmap_clear_bit (something, i)
>    [ ... whatever code we want to process i, ... ]
>  }
>
> If so, that's the real issue and we'd really like to identify & fix any code
> that has that kind of structure.
>
> See:
>
> https://gcc.gnu.org/ml/gcc/2009-06/msg00482.html

Indeed.  I can't see how this can have triggered:

  prev = elt->prev;
  if (prev)
    {
      prev->next = NULL;
      if (head->current->indx > prev->indx)
        {
          head->current = prev;
          head->indx = prev->indx;

so if there was elt->prev then if current == elt current->indx should
better be > prev->indx.

Sth else must be wrong (and I doubt it's the above bogus use of
bitmaps).

Richard.

> jeff
diff mbox

Patch

diff --git a/gcc/bitmap.c b/gcc/bitmap.c
index 8f7f306..844c32e 100644
--- a/gcc/bitmap.c
+++ b/gcc/bitmap.c
@@ -278,7 +296,8 @@  bitmap_elt_clear_from (bitmap head, bitmap_element *elt)
   if (prev)
     {
       prev->next = NULL;
-      if (head->current->indx > prev->indx)
+      if (head->current->indx > prev->indx
+	  || head->current == elt)
 	{
 	  head->current = prev;
 	  head->indx = prev->indx;