From: Julian Seward Date: Sat, 30 Jan 2010 13:07:08 +0000 (+0000) Subject: Make the FPO reader much more robust against apparently nonsensical X-Git-Tag: svn/VALGRIND_3_6_0~390 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=09e143240db239f42bb8bf712e22fa88645b97c3;p=thirdparty%2Fvalgrind.git 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 90d2442da0..7a6f3c7cc3 100644 --- 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 ) {