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

Reply via email to