Make the FPO reader much more robust against apparently nonsensical

FPO tables.  Also, improve debug printing for FPO reading.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@11034

diff --git a/coregrind/m_debuginfo/readpdb.c b/coregrind/m_debuginfo/readpdb.c
index 90d2442da06957c47a4b0fc4c229a4321069d51e..7a6f3c7cc3a0495b5ff0f889e88a7c8e3d9eac83 100644 (file)
--- a/coregrind/m_debuginfo/readpdb.c
+++ b/coregrind/m_debuginfo/readpdb.c
@@ -1791,6 +1791,21 @@ static ULong codeview_dump_linetab2(
 /*---                                                      ---*/
 /*------------------------------------------------------------*/
 
+static Int cmp_FPO_DATA_for_canonicalisation ( void* f1V, void* f2V )
+{
+   /* Cause FPO data to be sorted first in ascending order of range
+      starts, and for entries with the same range start, with the
+      shorter range (length) first. */
+   FPO_DATA* f1 = (FPO_DATA*)f1V;
+   FPO_DATA* f2 = (FPO_DATA*)f2V;
+   if (f1->ulOffStart < f2->ulOffStart) return -1;
+   if (f1->ulOffStart > f2->ulOffStart) return  1;
+   if (f1->cbProcSize < f2->cbProcSize) return -1;
+   if (f1->cbProcSize > f2->cbProcSize) return  1;
+   return 0; /* identical in both start and length */
+}
+
+
 /* JRS fixme: compare with version in current Wine sources */
 static void pdb_dump( struct pdb_reader* pdb,
                       DebugInfo* di,
@@ -1862,76 +1877,162 @@ static void pdb_dump( struct pdb_reader* pdb,
          meaningless?) */
       unsigned sz = 0;
       di->fpo = pdb->read_file( pdb, 5, &sz );
+
+      // FIXME: seems like the size can be a non-integral number
+      // of FPO_DATAs.  Force-align it (moronically).  Perhaps this
+      // signifies that we're not looking at a valid FPO table ..
+      // who knows.  Needs investigation.
+      while (sz > 0 && (sz % sizeof(FPO_DATA)) != 0)
+         sz--;
+
       di->fpo_size = sz;
+      if (0) VG_(printf)("FPO: got fpo_size %lu\n", (UWord)sz);
+      vg_assert(0 == (di->fpo_size % sizeof(FPO_DATA)));
    } else {
       vg_assert(di->fpo == NULL);
       vg_assert(di->fpo_size == 0);
    }
 
-   if (di->fpo) {
-      Word i;
-      Addr min_svma = ~(Addr)0;
-      Addr max_svma = (Addr)0;
+   // BEGIN clean up FPO data
+   if (di->fpo && di->fpo_size > 0) {
+      Word i, j;
+      Bool anyChanges;
+      Int itersAvail = 10;
+
       vg_assert(sizeof(di->fpo[0]) == 16);
       di->fpo_size /= sizeof(di->fpo[0]);
 
-      /* Sanity-check the table, and find the min and max avmas. */
+      // BEGIN FPO-data tidying-up loop
+      do {
+
+         vg_assert(itersAvail >= 0); /* safety check -- don't loop forever */
+         itersAvail--;
+
+         anyChanges = False;
+
+         /* First get them in ascending order of start point */
+         VG_(ssort)( di->fpo, (SizeT)di->fpo_size, (SizeT)sizeof(FPO_DATA),
+                              cmp_FPO_DATA_for_canonicalisation );
+         /* Get rid of any zero length entries */
+         j = 0;
+         for (i = 0; i < di->fpo_size; i++) {
+            if (di->fpo[i].cbProcSize == 0) {
+               anyChanges = True;
+               continue;
+            }
+            di->fpo[j++] = di->fpo[i];
+         }
+         vg_assert(j >= 0 && j <= di->fpo_size);
+         di->fpo_size = j;
+
+         /* Get rid of any dups */
+         if (di->fpo_size > 1) {
+            j = 1;
+            for (i = 1; i < di->fpo_size; i++) {
+               Bool dup
+                  = di->fpo[j-1].ulOffStart == di->fpo[i].ulOffStart
+                    && di->fpo[j-1].cbProcSize == di->fpo[i].cbProcSize;
+               if (dup) {
+                 anyChanges = True;
+                 continue;
+               }
+               di->fpo[j++] = di->fpo[i];
+            }
+            vg_assert(j >= 0 && j <= di->fpo_size);
+            di->fpo_size = j;
+         }
+
+         /* Truncate any overlapping ranges */
+         for (i = 1; i < di->fpo_size; i++) {
+            vg_assert(di->fpo[i-1].ulOffStart <= di->fpo[i].ulOffStart);
+            if (di->fpo[i-1].ulOffStart + di->fpo[i-1].cbProcSize 
+                > di->fpo[i].ulOffStart) {
+               anyChanges = True;
+               di->fpo[i-1].cbProcSize
+                  = di->fpo[i].ulOffStart - di->fpo[i-1].ulOffStart;
+            }
+         }
+
+      } while (anyChanges);
+      // END FPO-data tidying-up loop
+
+      /* Should now be in ascending order, non overlapping, no zero ranges.
+         Check this, get the min and max avmas, and bias the entries. */
       for (i = 0; i < di->fpo_size; i++) {
-         /* If any of the following assertions fail, we'll need to add
-            an extra pass to tidy up the FPO info -- make them be in
-            order and non-overlapping, since in-orderness and
-            non-overlappingness are required for safe use of
-            ML_(search_one_fpotab). */
          vg_assert(di->fpo[i].cbProcSize > 0);
-         if (i > 0) {
-            Bool ok;
-            Bool dup
-               = di->fpo[i-1].ulOffStart == di->fpo[i].ulOffStart
-                 && di->fpo[i-1].cbProcSize == di->fpo[i].cbProcSize;
-            /* tolerate exact duplicates -- I think they are harmless
-               w.r.t. termination properties of the binary search in
-               ML_(search_one_fpotab). */
-            if (dup)
-               continue;
 
-            ok = di->fpo[i-1].ulOffStart + di->fpo[i-1].cbProcSize 
-                 <= di->fpo[i].ulOffStart;
-            if (1 && !ok)
-               VG_(printf)("%#x +%d  then  %#x +%d\n",
-                           di->fpo[i-1].ulOffStart, di->fpo[i-1].cbProcSize,
-                           di->fpo[i-0].ulOffStart, di->fpo[i-0].cbProcSize );
-            vg_assert(ok);
+         if (i > 0) {
+            vg_assert(di->fpo[i-1].ulOffStart < di->fpo[i].ulOffStart);
+            vg_assert(di->fpo[i-1].ulOffStart + di->fpo[i-1].cbProcSize
+                      <= di->fpo[i].ulOffStart);
          }
-         /* Update min/max limits as we go along. */
-         if (di->fpo[i].ulOffStart < min_svma)
-            min_svma = di->fpo[i].ulOffStart;
-         if (di->fpo[i].ulOffStart + di->fpo[i].cbProcSize - 1 > max_svma)
-            max_svma = di->fpo[i].ulOffStart + di->fpo[i].cbProcSize - 1;
       }
+
       /* Now bias the table.  This can't be done in the same pass as
          the sanity check, hence a second loop. */
       for (i = 0; i < di->fpo_size; i++) {
          di->fpo[i].ulOffStart += bias_for_fpo;
+         // make sure the biasing didn't royally screw up, by wrapping
+         // the range around the end of the address space
+         vg_assert(0xFFFFFFFF - di->fpo[i].ulOffStart /* "remaining space" */
+                   >= di->fpo[i].cbProcSize);
       }
 
-      /* And record min/max */
-      vg_assert(min_svma <= max_svma); /* should always hold */
-
-      di->fpo_minavma = min_svma + bias_for_fpo;
-      di->fpo_maxavma = max_svma + bias_for_fpo;
+      /* Dump any entries which point outside the text segment and
+         compute the min/max avma "hint" addresses. */
+      Addr min_avma = ~(Addr)0;
+      Addr max_avma = (Addr)0;
+      vg_assert(di->text_present);
+      j = 0;
+      for (i = 0; i < di->fpo_size; i++) {
+         if ((Addr)(di->fpo[i].ulOffStart) >= di->text_avma
+             && (Addr)(di->fpo[i].ulOffStart + di->fpo[i].cbProcSize)
+                <= di->text_avma + di->text_size) {
+            /* Update min/max limits as we go along. */
+            if (di->fpo[i].ulOffStart < min_avma)
+               min_avma = di->fpo[i].ulOffStart;
+            if (di->fpo[i].ulOffStart + di->fpo[i].cbProcSize - 1 > max_avma)
+               max_avma = di->fpo[i].ulOffStart + di->fpo[i].cbProcSize - 1;
+            /* Keep */
+            di->fpo[j++] = di->fpo[i];
+            if (0)
+            VG_(printf)("FPO: keep text=[0x%lx,0x%lx) 0x%lx 0x%lx\n",
+                        di->text_avma, di->text_avma + di->text_size,
+                        (Addr)di->fpo[i].ulOffStart,
+                        (Addr)di->fpo[i].ulOffStart 
+                        + (Addr)di->fpo[i].cbProcSize - 1);
+         } else {
+            if (0)
+            VG_(printf)("FPO: SKIP text=[0x%lx,0x%lx) 0x%lx 0x%lx\n",
+                        di->text_avma, di->text_avma + di->text_size,
+                        (Addr)di->fpo[i].ulOffStart,
+                        (Addr)di->fpo[i].ulOffStart 
+                        + (Addr)di->fpo[i].cbProcSize - 1);
+            /* out of range; ignore */
+         }
+      }
+      vg_assert(j >= 0 && j < di->fpo_size); // should be <=
+      di->fpo_size = j;
 
+      /* And record min/max */
       /* biasing shouldn't cause wraparound (?!) */
-      vg_assert(di->fpo_minavma <= di->fpo_maxavma);
+      if (di->fpo_size > 0) {
+         vg_assert(min_avma <= max_avma); /* should always hold */
+         di->fpo_minavma = min_avma;
+         di->fpo_maxavma = max_avma;
+      } else {
+         di->fpo_minavma = 0;
+         di->fpo_maxavma = 0;
+      }
 
       if (0) {
-         VG_(printf)("XXXXXXXXX min/max svma %#lx %#lx\n",
-                     min_svma, max_svma);
-         VG_(printf)("XXXXXXXXX min/max avma %#lx %#lx\n",
+         VG_(printf)("FPO: min/max avma %#lx %#lx\n",
                      di->fpo_minavma, di->fpo_maxavma);
       }
 
       n_fpos_read += (ULong)di->fpo_size;
    }
+   // END clean up FPO data
 
    pdb_convert_types_header( &types, types_image );
    switch ( types.version ) {