Author: sewardj Date: 2008-02-17 20:54:12 +0000 (Sun, 17 Feb 2008) New Revision: 7421
Log: Misc tidying, including better comments, and better checking of info passed to ML_(addVar). Modified: branches/DATASYMS/coregrind/m_debuginfo/d3basics.c branches/DATASYMS/coregrind/m_debuginfo/debuginfo.c branches/DATASYMS/coregrind/m_debuginfo/priv_storage.h branches/DATASYMS/coregrind/m_debuginfo/readdwarf3.c branches/DATASYMS/coregrind/m_debuginfo/readelf.c branches/DATASYMS/coregrind/m_debuginfo/storage.c branches/DATASYMS/coregrind/m_debuginfo/tytypes.c Modified: branches/DATASYMS/coregrind/m_debuginfo/d3basics.c =================================================================== --- branches/DATASYMS/coregrind/m_debuginfo/d3basics.c 2008-02-17 18:51:06 UTC (rev 7420) +++ branches/DATASYMS/coregrind/m_debuginfo/d3basics.c 2008-02-17 20:54:12 UTC (rev 7421) @@ -472,6 +472,14 @@ sw1 = (Word)read_leb128S( &expr ); PUSH( fbval.res + sw1 ); break; + /* DW_OP_breg* denotes 'contents of specified register, plus + constant offset'. So provided we know what the register's + value is, we can evaluate this. Contrast DW_OP_reg*, + which indicates that denoted location is in a register + itself. For DW_OP_reg* we must always fail, since this + function is intended to compute a memory address of some + kind. See D3 Spec sec 2.6.1 ("Register Name Operations") + for details. */ case DW_OP_breg0 ... DW_OP_breg31: if (!regs) FAIL("evaluate_Dwarf3_Expr: DW_OP_breg* but no reg info"); @@ -493,6 +501,7 @@ "Warning: DWARF3 CFI reader: unhandled DW_OP_ " "opcode 0x%x", (Int)opcode); FAIL("evaluate_Dwarf3_Expr: unhandled DW_OP_"); + /*NOTREACHED*/ } } Modified: branches/DATASYMS/coregrind/m_debuginfo/debuginfo.c =================================================================== --- branches/DATASYMS/coregrind/m_debuginfo/debuginfo.c 2008-02-17 18:51:06 UTC (rev 7420) +++ branches/DATASYMS/coregrind/m_debuginfo/debuginfo.c 2008-02-17 20:54:12 UTC (rev 7421) @@ -1444,7 +1444,7 @@ inapplicable DebugInfos quickly. */ if (si->cfsi_used == 0) continue; - if (*ipP < si->cfsi_minaddr || *ipP > si->cfsi_maxaddr) + if (*ipP < si->cfsi_minavma || *ipP > si->cfsi_maxavma) continue; i = ML_(search_one_cfitab)( si, *ipP ); Modified: branches/DATASYMS/coregrind/m_debuginfo/priv_storage.h =================================================================== --- branches/DATASYMS/coregrind/m_debuginfo/priv_storage.h 2008-02-17 18:51:06 UTC (rev 7420) +++ branches/DATASYMS/coregrind/m_debuginfo/priv_storage.h 2008-02-17 20:54:12 UTC (rev 7421) @@ -350,8 +350,8 @@ DiCfSI* cfsi; UInt cfsi_used; UInt cfsi_size; - Addr cfsi_minaddr; - Addr cfsi_maxaddr; + Addr cfsi_minavma; + Addr cfsi_maxavma; XArray* cfsi_exprs; /* XArray of CfiExpr */ /* Expandable arrays of characters -- the string table. Pointers Modified: branches/DATASYMS/coregrind/m_debuginfo/readdwarf3.c =================================================================== --- branches/DATASYMS/coregrind/m_debuginfo/readdwarf3.c 2008-02-17 18:51:06 UTC (rev 7420) +++ branches/DATASYMS/coregrind/m_debuginfo/readdwarf3.c 2008-02-17 20:54:12 UTC (rev 7421) @@ -34,19 +34,55 @@ without prior written permission. */ -/* Current hacks: - DW_TAG_{const,volatile}_type no DW_AT_type is allowed; it is - assumed to mean "const void" or "volatile void" respectively. - GDB appears to interpret them like this, anyway. +/* REFERENCE (without which this code will not make much sense): + DWARF Debugging Information Format, Version 3, + dated 20 December 2005 (the "D3 spec"). + + Available at http://www.dwarfstd.org/Dwarf3.pdf. There's also a + .doc (MS Word) version, but for some reason the section numbers + between the Word and PDF versions differ by 1 in the first digit. + All section references in this code are to the PDF version. + + CURRENT HACKS: + + DW_TAG_{const,volatile}_type no DW_AT_type is allowed; it is + assumed to mean "const void" or "volatile void" respectively. + GDB appears to interpret them like this, anyway. + + In many cases it is important to know the svma of a CU (the "base + address of the CU", as the D3 spec calls it). There are some + situations in which the spec implies this value is unknown, but the + Dwarf3 produced by gcc-4.1 seems to assume is not unknown but + merely zero when not explicitly stated. So we too have to make + that assumption. + + TODO, 2008 Feb 17: + get rid of cu_svma_known and document the assumed-zero svma hack. - (text)-bias the code ranges handed to ML_(addVar); add check that - they actually fall into the text segment + ML_(sizeOfType): differentiate between zero sized types and types + for which the size is unknown. Is this important? I don't know. - parse all the types first, then resolve, then parse all the vars, - so that when we come to add vars, we know what their types are. - This is important, else we cannot know their sizes. + DW_AT_array_types: deal with explicit sizes (currently we compute + the size from the bounds and the element size, although that's + fragile, if the bounds incompletely specified, or completely + absent) + + Document reason for difference (by 1) of stack preening depth in + parse_var_DIE vs parse_type_DIE. + + Don't hand to ML_(addVars), vars whose locations are entirely in + registers (DW_OP_reg*). This is merely a space-saving + optimisation, as ML_(evaluate_Dwarf3_Expr) should handle these + expressions correctly, by failing to evaluate them and hence + effectively ignoring the variable with which they are associated. + + Deal with DW_AT_array_types which have element size != stride + + In some cases, the info for a variable is split between two + different DIEs (generally a declarer and a definer). We punt on + these. Could do better here. */ #include "pub_core_basics.h" @@ -118,11 +154,11 @@ return c->region_next >= c->region_szB; } -static Word get_position_of_Cursor ( Cursor* c ) { +static inline UWord get_position_of_Cursor ( Cursor* c ) { vg_assert(is_sane_Cursor(c)); return c->region_next; } -static void set_position_of_Cursor ( Cursor* c, Word pos ) { +static inline void set_position_of_Cursor ( Cursor* c, UWord pos ) { c->region_next = pos; vg_assert(is_sane_Cursor(c)); } @@ -466,7 +502,7 @@ VG_(memcpy)(p, block, nbytes); p += nbytes; * ((UChar*)p) = 1; /*isEnd*/ p += sizeof(UChar); - vg_assert(p - pstart == bytesReqd); + vg_assert( (SizeT)(p - pstart) == bytesReqd); vg_assert( &gx->payload[bytesReqd] == ((UChar*)gx) + sizeof(GExpr) + bytesReqd ); @@ -493,7 +529,7 @@ init_Cursor( &loc, cc->debug_loc_img, cc->debug_loc_sz, 0, cc->barf, "Overrun whilst reading .debug_loc section(2)" ); - set_position_of_Cursor( &loc, (Word)debug_loc_offset ); + set_position_of_Cursor( &loc, debug_loc_offset ); /* Who frees this xa? It is freed before this fn exits. */ xa = VG_(newXA)( ML_(dinfo_zalloc), ML_(dinfo_free), @@ -610,7 +646,7 @@ init_Cursor( &ranges, cc->debug_ranges_img, cc->debug_ranges_sz, 0, cc->barf, "Overrun whilst reading .debug_ranges section(2)" ); - set_position_of_Cursor( &ranges, (Word)debug_ranges_offset ); + set_position_of_Cursor( &ranges, debug_ranges_offset ); /* Who frees this xa? varstack_preen() does. */ xa = VG_(newXA)( ML_(dinfo_zalloc), ML_(dinfo_free), @@ -1177,8 +1213,8 @@ Int ctsSzB; UWord ctsMemSzB; - Word saved_die_c_offset = get_position_of_Cursor( c_die ); - Word saved_abbv_c_offset = get_position_of_Cursor( c_abbv ); + UWord saved_die_c_offset = get_position_of_Cursor( c_die ); + UWord saved_abbv_c_offset = get_position_of_Cursor( c_abbv ); varstack_preen( parser, td3, level ); @@ -1238,6 +1274,8 @@ invalid, and we can legitimately stop and complain. */ } #else + /* .. whereas The Reality is, simply assume the SVMA is zero + if it isn't specified. */ if (level == 0) { vg_assert(!cc->cu_svma_known); cc->cu_svma_known = True; @@ -1415,7 +1453,7 @@ ? "<anon_variable>" : "<anon_formal>", -1 ); - /* If this is a local variable (non-external), try to find + /* If this is a local variable (non-external), try to find the GExpr for the DW_AT_frame_base of the containing function. It should have been pushed on the stack at the time we encountered its DW_TAG_subprogram DIE, so the way @@ -1437,10 +1475,12 @@ } } if (!found) { - VG_(printf)( - "parse_var_DIE: found non-external variable " - "outside DW_TAG_subprogram\n"); - // FIXME goto bad_DIE; + if (VG_(clo_verbosity) >= 0) { + VG_(message)(Vg_DebugMsg, + "warning: parse_var_DIE: non-external variable " + "outside DW_TAG_subprogram"); + } + // FIXME goto bad_DIE; } } @@ -1507,7 +1547,7 @@ DW_AT_type : <13e> <2><2c3>: Abbrev Number: 13 (DW_TAG_formal_parameter) DW_AT_type : <133> - */ + */ /* ignore */ } else @@ -1601,7 +1641,7 @@ typedef struct { /* What source language? 'C'=C/C++, 'F'=Fortran, '?'=other - Established once per compilation unit. */ + Established once per compilation unit. */ UChar language; /* A stack of types which are currently under construction */ Int sp; /* [sp] is innermost active entry; sp==-1 for empty @@ -1710,8 +1750,8 @@ D3Expr* expr = NULL; TyBounds* bounds = NULL; - Word saved_die_c_offset = get_position_of_Cursor( c_die ); - Word saved_abbv_c_offset = get_position_of_Cursor( c_abbv ); + UWord saved_die_c_offset = get_position_of_Cursor( c_die ); + UWord saved_abbv_c_offset = get_position_of_Cursor( c_abbv ); /* If we've returned to a level at or above any previously noted parent, un-note it, so we don't believe we're still collecting @@ -2354,9 +2394,6 @@ } case TyA_Type: { UChar enc; -#if 0 - Word i; -#endif XArray* xa; Type* ty = (Type*)adp->payload; switch (ty->tag) { @@ -2399,30 +2436,10 @@ || ty->Ty.Enum.szB < 1 || ty->Ty.Enum.szB > 8) goto baaad; xa = ty->Ty.Enum.atomRs; -#if 0 - for (i = 0; i < VG_(sizeXA)(xa); i++) { - void** ppAtom = VG_(indexXA)(xa,i); - ok = resolve_binding( &payload, map, - *ppAtom, D3TyA_Atom, - False/*!allow_invalid*/ ); - if (!ok) goto baaad; - *ppAtom = payload; - } -#endif break; case Ty_StOrUn: xa = ty->Ty.StOrUn.fields; if (!xa) goto baaad; -#if 0 - for (i = 0; i < VG_(sizeXA)(xa); i++) { - void** ppField = VG_(indexXA)(xa,i); - ok = resolve_binding( &payload, map, - *ppField, D3TyA_Field, - False/*!allow_invalid*/ ); - if (!ok) goto baaad; - *ppField = payload; - } -#endif break; case Ty_Fn: break; @@ -2458,8 +2475,11 @@ payload = NULL; ok = resolve_binding( &payload, map, vars->typeR, TyA_Type, True/*allow_invalid*/ ); -//if (!ok) VG_(printf)("Can't resolve type reference 0x%lx\n", (UWord)vars->typeR); -//vg_assert(ok); + + if (0 && !ok) + VG_(printf)("Can't resolve type reference 0x%lx\n", + (UWord)vars->typeR); + //vg_assert(ok); vars->typeR = payload; } @@ -2481,8 +2501,8 @@ ULong atag, abbv_code; UWord posn; UInt has_children; - Word start_die_c_offset, start_abbv_c_offset; - Word after_die_c_offset, after_abbv_c_offset; + UWord start_die_c_offset, start_abbv_c_offset; + UWord after_die_c_offset, after_abbv_c_offset; /* --- Deal with this DIE --- */ posn = get_position_of_Cursor( c ); @@ -2779,7 +2799,7 @@ TRACE_D3("\n------ Parsing .debug_info section ------\n"); while (True) { - Word cu_start_offset, cu_offset_now; + UWord cu_start_offset, cu_offset_now; CUConst cc; if (is_at_end_Cursor( &info )) break; Modified: branches/DATASYMS/coregrind/m_debuginfo/readelf.c =================================================================== --- branches/DATASYMS/coregrind/m_debuginfo/readelf.c 2008-02-17 18:51:06 UTC (rev 7420) +++ branches/DATASYMS/coregrind/m_debuginfo/readelf.c 2008-02-17 20:54:12 UTC (rev 7421) @@ -1152,7 +1152,8 @@ /* Make sure the PT_LOADable entries are in order */ if (phdr->p_type == PT_LOAD) { - TRACE_SYMTAB("PT_LOAD in order?: %p %p\n", prev_svma, phdr->p_vaddr); + TRACE_SYMTAB("PT_LOAD in order?: %p %p\n", + prev_svma, phdr->p_vaddr); if (phdr->p_vaddr < prev_svma) { ML_(symerr)(di, True, "ELF Program Headers are not in ascending order"); Modified: branches/DATASYMS/coregrind/m_debuginfo/storage.c =================================================================== --- branches/DATASYMS/coregrind/m_debuginfo/storage.c 2008-02-17 18:51:06 UTC (rev 7420) +++ branches/DATASYMS/coregrind/m_debuginfo/storage.c 2008-02-17 20:54:12 UTC (rev 7421) @@ -708,6 +708,39 @@ vg_assert(type); vg_assert(gexpr); + /* Ignore any variables whose aMin .. aMax (that is, range of text + addresses for which they actually exist) falls outside the text + segment. Is this indicative of a bug in the reader? Maybe. */ + if (di->text_size > 0 + && level > 0 + && (aMax < di->text_avma + || aMin >= di->text_avma + di->text_size)) { + if (VG_(clo_verbosity) >= 0) { + VG_(message)(Vg_DebugMsg, + "warning: addVar: in range %p .. %p outside " + "segment %p .. %p (%s)", + aMin, aMax, + di->text_avma, di->text_avma + di->text_size -1, + name + ); + } + return; + } + + /* If the type's size is zero (which can mean unknown size), ignore + it. We will never be able to actually relate a data address to + a data object with zero size, so there's no point in storing + info on it. */ + if (ML_(sizeOfType)(type) == 0) { + if (VG_(clo_verbosity) >= 0) { + VG_(message)(Vg_DebugMsg, + "warning: addVar: zero or unknown size (%s)", + name + ); + } + return; + } + if (!di->varinfo) { di->varinfo = VG_(newXA)( ML_(dinfo_zalloc), ML_(dinfo_free), sizeof(OSet*) ); @@ -726,7 +759,9 @@ VG_(addToXA)( di->varinfo, &scope ); /* Add a single range covering the entire address space. At level 0 we require this doesn't get split. At levels above 0 - we require that any additions to it cause it to get split. */ + we require that any additions to it cause it to get split. + All of these invariants get checked both add_var_to_arange + and after reading is complete, in canonicaliseVarInfo. */ nyu = VG_(OSetGen_AllocNode)( scope, sizeof(DiAddrRange) ); vg_assert(nyu); nyu->aMin = (Addr)0; @@ -1162,8 +1197,8 @@ static void canonicaliseCFI ( struct _DebugInfo* di ) { Int i, j; - const Addr minAddr = 0; - const Addr maxAddr = ~minAddr; + const Addr minAvma = 0; + const Addr maxAvma = ~minAvma; /* Note: take care in here. di->cfsi can be NULL, in which case _used and _size fields will be zero. */ @@ -1172,23 +1207,23 @@ vg_assert(di->cfsi_size == 0); } - /* Set cfsi_minaddr and cfsi_maxaddr to summarise the entire + /* Set cfsi_minavma and cfsi_maxavma to summarise the entire address range contained in cfsi[0 .. cfsi_used-1]. */ - di->cfsi_minaddr = maxAddr; - di->cfsi_maxaddr = minAddr; + di->cfsi_minavma = maxAvma; + di->cfsi_maxavma = minAvma; for (i = 0; i < (Int)di->cfsi_used; i++) { Addr here_min = di->cfsi[i].base; Addr here_max = di->cfsi[i].base + di->cfsi[i].len - 1; - if (here_min < di->cfsi_minaddr) - di->cfsi_minaddr = here_min; - if (here_max > di->cfsi_maxaddr) - di->cfsi_maxaddr = here_max; + if (here_min < di->cfsi_minavma) + di->cfsi_minavma = here_min; + if (here_max > di->cfsi_maxavma) + di->cfsi_maxavma = here_max; } if (di->trace_cfi) VG_(printf)("canonicaliseCfiSI: %d entries, %p .. %p\n", di->cfsi_used, - di->cfsi_minaddr, di->cfsi_maxaddr); + di->cfsi_minavma, di->cfsi_maxavma); /* Sort the cfsi array by base address. */ VG_(ssort)(di->cfsi, di->cfsi_used, sizeof(*di->cfsi), compare_DiCfSI); @@ -1223,9 +1258,9 @@ /* No zero-length ranges. */ vg_assert(di->cfsi[i].len > 0); /* Makes sense w.r.t. summary address range */ - vg_assert(di->cfsi[i].base >= di->cfsi_minaddr); + vg_assert(di->cfsi[i].base >= di->cfsi_minavma); vg_assert(di->cfsi[i].base + di->cfsi[i].len - 1 - <= di->cfsi_maxaddr); + <= di->cfsi_maxavma); if (i < di->cfsi_used - 1) { /* Modified: branches/DATASYMS/coregrind/m_debuginfo/tytypes.c =================================================================== --- branches/DATASYMS/coregrind/m_debuginfo/tytypes.c 2008-02-17 18:51:06 UTC (rev 7420) +++ branches/DATASYMS/coregrind/m_debuginfo/tytypes.c 2008-02-17 20:54:12 UTC (rev 7421) @@ -360,6 +360,7 @@ Word i; switch (ty->tag) { case Ty_Base: + vg_assert(ty->Ty.Base.szB > 0); return ty->Ty.Base.szB; case Ty_Qual: return ML_(sizeOfType)( ty->Ty.Qual.typeR ); @@ -425,7 +426,7 @@ Word i; GXResult res; TyField *field = NULL, *fields; - SizeT offMin = 0, offMax1 = 0; + OffT offMin = 0, offMax1 = 0; if (!ty->Ty.StOrUn.isStruct) goto done; fields = ty->Ty.StOrUn.fields; if ((!fields) || VG_(sizeXA)(fields) == 0) goto done; ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ _______________________________________________ Valgrind-developers mailing list Valgrind-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/valgrind-developers