[Bug ld/31009] regression: assertion fail ../../bfd/merge.c:243

[matz at suse dot de]

https://sourceware.org/bugzilla/show_bug.cgi?id=31009

--- Comment #10 from Michael Matz <matz at suse dot de> ---
(In reply to Jonny Weir from comment #7)
> I made the following change:

Thanks!

> XXX resize 1: count=1598 added=1086327410 newnb=1048576

Gah, yeah.  So, one of the input string sections (most likely one of the
.debug_str sections) is about 2GB in size.  We overestimate this (quite much)
to add 1G strings, which ...

> XXX resize 2: newnb=2147483648

... seems to work fine, still ...


> XXX resize 1: count=755248 added=68141 newnb=0
> false1: newnb=0

... but then, when we've already allocated 2G buckets (and used 755248 of
those),
we fail to add space for another (estimated) 68141 ones.  (We fail to do so,
because we can't double the number of buckets again, as it's only a unsigned
int).  Why that leads to errors down the pipe I can't quite see yet, these
input sections which can't be added to the hash table for $reason are supposed
to be ignored and just added as is into the output.  But whatever the reason
for _that_ is doesn't matter at first.  The first thing that goes wrong
is already that we go into the resize-me case at all with these numbers.
When we have 2 billion buckets, of which 755k are used, and want to add
(potentially) another 68k, then obviously we don't need to resize anything.
The existing 2G buckets should be enough :-)

The error is in another overflow in the check:

  if (bfdtab->count + added > table->nbuckets * 2 / 3)
    {
      unsigned i;
      unsigned long newnb = table->nbuckets * 2;
      ... rest of resize code ...

as table->nbuckets is 2G at entry to this, the expression table->nbuckets * 2
is going to be zero, so once nbuckets goes to 2G (the max supported size)
the above expression will always be true, we always go into the resize case,
and we always will fail it (with the followup errors then occurring).

Let's rewrite it into "/ 3 * 2" to avoid overflow in the check.  Can you test
this, please? (patch still contains the printf debugs):

diff --git a/bfd/merge.c b/bfd/merge.c
index 722e6659486..f81cd30197f 100644
--- a/bfd/merge.c
+++ b/bfd/merge.c
@@ -167,7 +167,7 @@ static bool
 sec_merge_maybe_resize (struct sec_merge_hash *table, unsigned added)
 {
   struct bfd_hash_table *bfdtab = &table->table;
-  if (bfdtab->count + added > table->nbuckets * 2 / 3)
+  if (bfdtab->count + added > table->nbuckets / 3 * 2)
     {
       unsigned i;
       unsigned long newnb = table->nbuckets * 2;
@@ -175,12 +175,14 @@ sec_merge_maybe_resize (struct sec_merge_hash *table,
unsigned added)
       uint64_t *newl;
       unsigned long alloc;

-      while (bfdtab->count + added > newnb * 2 / 3)
+      printf ("XXX resize 1: count=%u added=%u newnb=%lu\n", bfdtab->count,
added, newnb);
+      while (bfdtab->count + added > newnb / 3 * 2)
        {
          newnb *= 2;
          if (!newnb)
            return false;
        }
+      printf ("XXX resize 2: newnb=%lu\n", newnb);

       alloc = newnb * sizeof (newl[0]);
       if (alloc / sizeof (newl[0]) != newnb)
@@ -240,7 +242,7 @@ sec_merge_hash_insert (struct sec_merge_hash *table,
   hashp->u.suffix = NULL;
   hashp->next = NULL;
   // We must not need resizing, otherwise _index is wrong
-  BFD_ASSERT (bfdtab->count + 1 <= table->nbuckets * 2 / 3);
+  BFD_ASSERT (bfdtab->count + 1 <= table->nbuckets / 3 * 2);
   bfdtab->count++;
   table->key_lens[_index] = (hash << 32) | (uint32_t)len;
   table->values[_index] = hashp;

-- 
You are receiving this mail because:
You are on the CC list for the bug.