Comment 26 for bug 490228

Revision history for this message
John A Meinel (jameinel) wrote : Re: [Bug 490228] Re: Crash in groupcompress during commit operation

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Gareth White wrote:
> After trying unsuccessfully for several days I managed to get it to
> crash again using Alexander's binaries. The stack trace is similar to
> the previous one but the line number for diff-delta.c looks more
> sensible this time.
>
> ** Attachment added: "WinDbg analysis 3"
> http://launchpadlibrarian.net/36759299/crash3.txt
>

That does look like a better traceback.
        if (old_entry->ptr != NULL
            || old_entry >= old_index->hash[hash_offset + 1]) {

I originally thought we might have walked off the end of the hash
buckets, but looking with a bit more context:

    hash_offset = (entry->val & old_index->hash_mask);
    old_entry = old_index->hash[hash_offset + 1];
    old_entry--;
    while (old_entry->ptr == NULL
           && old_entry >= old_index->hash[hash_offset]) {
        old_entry--;
    }
    old_entry++;
    if (old_entry->ptr != NULL
        || old_entry >= old_index->hash[hash_offset + 1]) {

Rewriting it a bit...

    hash_offset = (entry->val & old_index->hash_mask);
    prev_entry = old_index->hash[hash_offset];
    next_entry = old_index->hash[hash_offset + 1];
    cur_entry = next_entry - 1;
    while (cur_entry->ptr == NULL && cur_entry >= prev_entry) {
      cur_entry--;
    }
    cur_entry++;
    if (cur_entry->ptr != NULL
        || cur_entry >= next_entry) {
    }

The structure is such that each bucket from the hash table references a
location in the data table and has some free space. The data table is
always arranged such that hash buckets are incremental. Something like:

hash_table |0 |1|2 |3|4
             | | | | |
entry_table abNNxycdefghijklmNNNN|

So this shows hash_offset 0 has 2 entries and two empty entries,
hash_offset 1 has two entries and no empty. offset 2 has a bunch of
entries, and 2 empties, and offset 3 has 2 empties. The hash array has
N+1 entries, and the last entry points to just past the last entry in
entry_table.

If we had something that fit into page 1, the above should start
pointing at 'c', then back up to y. it should see that cur_entry->ptr is
not NULL, and then increment back to 'c'. And see that this is both not
null, and == next_entry.

If it slotted into 3, we would grab the pointer from offset 4, if 3 were
then completely full, we would back up, see that 3 had no slots, then
increment back to the off-the-end location, and probably the
cur_entry->ptr is then going to fail, as cur_entry is pointing past the
end of entry_table, and thus "cur_entry->ptr" is not valid. Changing the
order of the if statement to:

if (cur_entry >= next_entry || cur_entry->ptr != NULL)

Should probably fix this.

John
=:->

  status inprogress
  assignee jameinel
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAksmWyUACgkQJdeBCYSNAAM2YACgwydWqgPGC1EMZRyju8R5bHus
pOIAoMsyYCIy1TuYXtebmWF6a7bz+B7b
=a5Ec
-----END PGP SIGNATURE-----