dwarflint: ref_record is now template parametrized by locus typeupstream/pmachata/dwarflint

- the theory being that most of the time all you need to track comes from
  one source
- the upshot is that now the locus::clone and related business can go away

13 files changed, 85 insertions, 122 deletions

diff --git a/dwarflint/addr-record.cc b/dwarflint/addr-record.cc
index f4cc87b9..220f4b1d 100644
--- a/dwarflint/addr-record.cc
+++ b/dwarflint/addr-record.cc
@@ -66,36 +66,3 @@ addr_record::add (uint64_t addr)
   if (it == end () || *it != addr)
     insert (it, addr);
 }
-
-ref::ref ()
-  : addr (-1)
-  , who (NULL)
-{}
-
-ref::ref (uint64_t a_addr, locus const &a_who)
-  : addr (a_addr)
-  , who (a_who.clone ())
-{}
-
-ref::ref (ref const &copy)
-  : who (NULL)
-{
-  *this = copy;
-}
-
-ref &
-ref::operator= (ref const &copy)
-{
-  if (&copy != this)
-    {
-      addr = copy.addr;
-      delete who;
-      who = copy.who ? copy.who->clone () : NULL;
-    }
-  return *this;
-}
-
-ref::~ref ()
-{
-  delete who;
-}
diff --git a/dwarflint/addr-record.hh b/dwarflint/addr-record.hh
index 1772c931..4bbd0c51 100644
--- a/dwarflint/addr-record.hh
+++ b/dwarflint/addr-record.hh
@@ -32,11 +32,10 @@
 
 #include "locus.hh"
 
-/* Functions and data structures for address record handling.  We
-   use that to check that all DIE references actually point to an
-   existing die, not somewhere mid-DIE, where it just happens to be
-   interpretable as a DIE.  */
-
+/// Address record is used to check that all DIE references actually
+/// point to an existing die, not somewhere mid-DIE, where it just
+/// happens to be interpretable as a DIE.  This is stored as sorted
+/// array for quick lookup and duplicate removal.
 struct addr_record
   : private std::vector<uint64_t>
 {
@@ -48,27 +47,31 @@ public:
   void add (uint64_t addr);
 };
 
-/* Functions and data structures for reference handling.  Just like
-   the above, we use this to check validity of DIE references.
-   Unlike the above, this is not stored as sorted set, but simply as
-   an array of records, because duplicates are unlikely.  */
-
-struct ref
+/// One reference for use in ref_record, parametrized by locus type.
+template <class L>
+struct ref_T
 {
   uint64_t addr; // Referee address
-  locus *who;    // Referrer
+  L who;         // Referrer
+
+  ref_T ()
+    : addr (-1)
+  {}
 
-  ref ();
-  ref (uint64_t a_addr, locus const &a_who);
-  ref (ref const &copy);
-  ~ref ();
-  ref &operator= (ref const &copy);
+  ref_T (uint64_t a_addr, L const &a_who)
+    : addr (a_addr)
+    , who (a_who)
+  {}
 };
 
-class ref_record
-  : private std::vector<struct ref>
+/// Reference record is used to check validity of DIE references.
+/// Unlike the above, this is not stored as sorted set, but simply as
+/// an array of records, because duplicates are unlikely.
+template <class L>
+class ref_record_T
+  : private std::vector<ref_T<L> >
 {
-  typedef std::vector<struct ref> _super_t;
+  typedef std::vector<ref_T<L> > _super_t;
 public:
   using _super_t::const_iterator;
   using _super_t::begin;
diff --git a/dwarflint/check_debug_abbrev.hh b/dwarflint/check_debug_abbrev.hh
index 9565f885..54d03ce8 100644
--- a/dwarflint/check_debug_abbrev.hh
+++ b/dwarflint/check_debug_abbrev.hh
@@ -41,7 +41,7 @@ typedef fixed_locus<sec_abbrev,
 		    locus_simple_fmt::hex> abbrev_locus;
 
 class abbrev_attrib_locus
-  : public clonable_locus<abbrev_attrib_locus>
+  : public locus
 {
   uint64_t _m_abbr_offset;
   uint64_t _m_attr_offset;
diff --git a/dwarflint/check_debug_aranges.hh b/dwarflint/check_debug_aranges.hh
index 9840245c..109cf0bc 100644
--- a/dwarflint/check_debug_aranges.hh
+++ b/dwarflint/check_debug_aranges.hh
@@ -56,7 +56,7 @@ public:
 };
 
 class arange_locus
-  : public clonable_locus<arange_locus>
+  : public locus
 {
   Dwarf_Off _m_table_offset;
   Dwarf_Off _m_arange_offset;
diff --git a/dwarflint/check_debug_info.cc b/dwarflint/check_debug_info.cc
index 5aff2b99..e415fb18 100644
--- a/dwarflint/check_debug_info.cc
+++ b/dwarflint/check_debug_info.cc
@@ -109,15 +109,14 @@ namespace
   }
 
   bool
-  check_die_references (struct cu *cu,
-			struct ref_record *die_refs)
+  check_die_references (cu *cu, ref_record *die_refs)
   {
     bool retval = true;
     for (ref_record::const_iterator it = die_refs->begin ();
 	 it != die_refs->end (); ++it)
       if (!cu->die_addrs.has_addr (it->addr))
 	{
-	  wr_error (*it->who)
+	  wr_error (it->who)
 	    << "unresolved reference to " << pri::DIE (it->addr)
 	    << '.' << std::endl;
 	  retval = false;
@@ -143,7 +142,7 @@ namespace
 
 	  if (ref_cu == NULL)
 	    {
-	      wr_error (*rt->who)
+	      wr_error (rt->who)
 		<< "unresolved (non-CU-local) reference to "
 		<< pri::hex (rt->addr) << '.' << std::endl;
 	      retval = false;
@@ -153,7 +152,7 @@ namespace
 	       reference is valid, which it is.  But warn about this
 	       anyway, perhaps local reference could be formed on
 	       smaller number of bytes.  */
-	    wr_message (*rt->who, mc_impact_2 | mc_acc_suboptimal | mc_die_rel)
+	    wr_message (rt->who, mc_impact_2 | mc_acc_suboptimal | mc_die_rel)
 	      << "local reference to " << pri::DIE (rt->addr)
 	      << " formed as global." << std::endl;
 	}
@@ -390,9 +389,9 @@ namespace
   struct value_check_cb_ctx
   {
     struct read_ctx *const ctx;
-    locus const *where;
+    die_locus const *where;
     struct cu *const cu;
-    struct ref_record *local_die_refs;
+    ref_record *local_die_refs;
     Elf_Data *strings;
     struct coverage *strings_coverage;
     struct coverage *pc_coverage;
@@ -525,14 +524,14 @@ namespace
   int
   read_die_chain (dwarf_version const *ver,
 		  elf_file const &file,
-		  struct read_ctx *ctx,
-		  struct cu *cu,
-		  struct abbrev_table const *abbrevs,
+		  read_ctx *ctx,
+		  cu *cu,
+		  abbrev_table const *abbrevs,
 		  Elf_Data *strings,
-		  struct ref_record *local_die_refs,
-		  struct coverage *strings_coverage,
-		  struct relocation_data *reloc,
-		  struct coverage *pc_coverage,
+		  ref_record *local_die_refs,
+		  coverage *strings_coverage,
+		  relocation_data *reloc,
+		  coverage *pc_coverage,
 		  bool *need_rangesp,
 		  unsigned level)
   {
@@ -1075,7 +1074,7 @@ check_debug_info::check_debug_info (checkstack &stack, dwarflint &lint)
   sec &sec = _m_sec_info->sect;
   Elf_Data *const strings = _m_sec_str->sect.data;
 
-  struct ref_record die_refs;
+  ref_record die_refs;
 
   bool success = true;
 
@@ -1253,12 +1252,12 @@ check_debug_info_refs::check_debug_info_refs (checkstack &stack,
       if (it->stmt_list.addr == (uint64_t)-1)
 	for (ref_record::const_iterator jt = it->decl_file_refs.begin ();
 	     jt != it->decl_file_refs.end (); ++jt)
-	  wr_error (*jt->who)
+	  wr_error (jt->who)
 	    << "references .debug_line table, but CU DIE lacks DW_AT_stmt_list."
 	    << std::endl;
       else if (_m_line == NULL
 	       || !_m_line->has_line_table (it->stmt_list.addr))
-	wr_error (*it->stmt_list.who)
+	wr_error (it->stmt_list.who)
 	  << "unresolved reference to .debug_line table "
 	  << pri::hex (it->stmt_list.addr) << '.' << std::endl;
 
diff --git a/dwarflint/check_debug_info.hh b/dwarflint/check_debug_info.hh
index ad139c3a..e8dc8506 100644
--- a/dwarflint/check_debug_info.hh
+++ b/dwarflint/check_debug_info.hh
@@ -37,6 +37,9 @@
 #include "sections_i.hh"
 #include "die_locus.hh"
 
+typedef ref_T<die_locus> ref;
+typedef ref_record_T<die_locus> ref_record;
+
 struct cu_head
 {
   Dwarf_Off offset;
@@ -68,15 +71,15 @@ struct cu
 {
   struct cu *next;              // For compatibility with C level.
                                 // xxx will probably go away eventually
-  struct cu_head const *head;
+  cu_head const *head;
   uint64_t cudie_offset;
   uint64_t low_pc;              // DW_AT_low_pc value of CU DIE, -1 if not present.
-  struct ref stmt_list;
-  struct addr_record die_addrs; // Addresses where DIEs begin in this CU.
-  struct ref_record die_refs;   // DIE references into other CUs from this CU.
-  struct ref_record loc_refs;   // references into .debug_loc from this CU.
-  struct ref_record range_refs; // references into .debug_ranges from this CU.
-  struct ref_record decl_file_refs;  // values of DW_AT_decl_file in this CU.
+  ::ref stmt_list;
+  addr_record die_addrs;        // Addresses where DIEs begin in this CU.
+  ref_record die_refs;          // DIE references into other CUs from this CU.
+  ref_record loc_refs;          // references into .debug_loc from this CU.
+  ref_record range_refs;        // references into .debug_ranges from this CU.
+  ref_record decl_file_refs;    // values of DW_AT_decl_file in this CU.
   bool has_arange;              // Whether we saw arange section pointing at this CU.
   bool has_pubnames;            // Likewise for pubnames.
   bool has_pubtypes;            // Likewise for pubtypes.
diff --git a/dwarflint/check_debug_line.cc b/dwarflint/check_debug_line.cc
index 57238bc4..d75aed2b 100644
--- a/dwarflint/check_debug_line.cc
+++ b/dwarflint/check_debug_line.cc
@@ -358,7 +358,7 @@ check_debug_line::check_debug_line (checkstack &stack, dwarflint &lint)
 		for (ref_record::const_iterator
 		       jt = it->decl_file_refs.begin ();
 		     jt != it->decl_file_refs.end (); ++jt)
-		  if (!use_file (files, jt->addr, *jt->who))
+		  if (!use_file (files, jt->addr, jt->who))
 		    success = false;
 	      }
 	  if (!found)
diff --git a/dwarflint/check_debug_loc_range.cc b/dwarflint/check_debug_loc_range.cc
index 9c001744..ac6f789f 100644
--- a/dwarflint/check_debug_loc_range.cc
+++ b/dwarflint/check_debug_loc_range.cc
@@ -579,9 +579,12 @@ namespace
 
   struct ref_cu
   {
-    struct ref ref;
-    struct cu *cu;
-    bool operator < (ref_cu const& other) const {
+    ::ref ref;
+    ::cu *cu;
+
+    bool
+    operator < (ref_cu const& other) const
+    {
       return ref.addr < other.ref.addr;
     }
   };
@@ -627,7 +630,7 @@ namespace
     ref_cu_vect refs;
     for (struct cu *cu = cu_chain; cu != NULL; cu = cu->next)
       {
-	struct ref_record *rec
+	ref_record *rec
 	  = sec->id == sec_loc ? &cu->loc_refs : &cu->range_refs;
 	for (ref_record::const_iterator it = rec->begin ();
 	     it != rec->end (); ++it)
@@ -664,7 +667,7 @@ namespace
 	   Perhaps that's undesirable.  */
 	if (!check_loc_or_range_ref (ver, file, &ctx, it->cu, sec,
 				     &coverage, coverage_map, pc_coverage,
-				     off, *it->ref.who, cat))
+				     off, it->ref.who, cat))
 	  retval = false;
 	last_off = off;
       }
@@ -858,10 +861,10 @@ namespace
 }
 
 class locexpr_locus
-  : public clonable_locus<locexpr_locus>
+  : public locus
 {
-  uint64_t const _m_offset;
-  locus const *const _m_context;
+  uint64_t _m_offset;
+  locus const *_m_context;
 
 public:
   explicit locexpr_locus (uint64_t offset, locus const *context)
@@ -897,7 +900,9 @@ check_location_expression (dwarf_version const *ver,
       return false;
     }
 
-  ref_record oprefs;
+  typedef ref_T<locexpr_locus> locexpr_ref;
+  typedef ref_record_T<locexpr_locus> locexpr_ref_record;
+  locexpr_ref_record oprefs;
   addr_record opaddrs;
 
   while (!read_ctx_eof (&ctx))
@@ -954,7 +959,7 @@ check_location_expression (dwarf_version const *ver,
 	    else
 	      {
 		uint64_t off_after = read_ctx_get_offset (&ctx) + init_off;
-		oprefs.push_back (ref (off_after + skip, where));
+		oprefs.push_back (locexpr_ref (off_after + skip, where));
 	      }
 
 	    break;
@@ -983,11 +988,11 @@ check_location_expression (dwarf_version const *ver,
     }
 
  out:
-  for (ref_record::const_iterator it = oprefs.begin ();
+  for (locexpr_ref_record::const_iterator it = oprefs.begin ();
        it != oprefs.end (); ++it)
     if (!opaddrs.has_addr (it->addr))
-      wr_error (*it->who) << "unresolved reference to opcode at "
-			  << pri::hex (it->addr) << ".\n";
+      wr_error (it->who) << "unresolved reference to opcode at "
+			 << pri::hex (it->addr) << ".\n";
 
   return true;
 }
diff --git a/dwarflint/check_debug_loc_range.hh b/dwarflint/check_debug_loc_range.hh
index 882b529b..cbc1e7cb 100644
--- a/dwarflint/check_debug_loc_range.hh
+++ b/dwarflint/check_debug_loc_range.hh
@@ -31,7 +31,7 @@
 #include "dwarf_version_i.hh"
 
 class loc_range_locus
-  : public clonable_locus<loc_range_locus>
+  : public locus
 {
   locus const &_m_parent;
   Dwarf_Off _m_offset;
diff --git a/dwarflint/die_locus.hh b/dwarflint/die_locus.hh
index 9363bc8d..0c57d4f5 100644
--- a/dwarflint/die_locus.hh
+++ b/dwarflint/die_locus.hh
@@ -39,13 +39,13 @@ typedef fixed_locus<sec_info,
 		    locus_simple_fmt::dec> cu_locus;
 
 class die_locus
-  : public clonable_locus<die_locus>
+  : public locus
 {
   Dwarf_Off _m_offset;
   int _m_attrib_name;
 
 public:
-  explicit die_locus (Dwarf_Off offset, int attrib_name = -1)
+  explicit die_locus (Dwarf_Off offset = -1, int attrib_name = -1)
     : _m_offset (offset)
     , _m_attrib_name (attrib_name)
   {}
diff --git a/dwarflint/locus.cc b/dwarflint/locus.cc
index 19efd1a6..553e7d8b 100644
--- a/dwarflint/locus.cc
+++ b/dwarflint/locus.cc
@@ -26,6 +26,14 @@
 #include "locus.hh"
 #include "section_id.hh"
 #include <sstream>
+#include <iostream>
+
+std::ostream &
+operator << (std::ostream &os, locus const &loc)
+{
+  os << loc.format ();
+  return os;
+}
 
 char const *
 locus_simple_fmt::offset_n ()
diff --git a/dwarflint/locus.hh b/dwarflint/locus.hh
index 3a61b9a8..4557c2a7 100644
--- a/dwarflint/locus.hh
+++ b/dwarflint/locus.hh
@@ -29,10 +29,8 @@
 #include "section_id.hh"
 
 #include <stdint.h>
-#include <stdlib.h>
 #include <iosfwd>
-#include <iostream>
-#include <cassert>
+#include <string>
 
 /// Instances of the locus subclasses are used as pointers into
 /// debuginfo for documentation purposes (messages and errors).  They
@@ -43,23 +41,10 @@ class locus
 {
 public:
   virtual std::string format (bool brief = false) const = 0;
-  virtual locus *clone () const = 0;
   virtual ~locus () {}
 };
 
-/// This is to simplify creation of subclasses.  Most locus subclasses
-/// should in fact inherit from this using CRTP:
-///   class X: public clonable_locus<X>
-template <class T>
-class clonable_locus
-  : public locus
-{
-public:
-  virtual locus *clone () const
-  {
-    return new T (*static_cast<T const*> (this));
-  }
-};
+std::ostream &operator << (std::ostream &os, locus const &loc);
 
 /// Helper class for simple_locus to reduce the template bloat.
 class simple_locus_aux
@@ -79,7 +64,7 @@ protected:
 template<char const *(*N) (),
 	 void (*F) (std::ostream &, uint64_t)>
 class simple_locus
-  : public clonable_locus<simple_locus<N, F> >
+  : public locus
   , private simple_locus_aux
 {
   section_id _m_sec;
@@ -124,11 +109,4 @@ namespace locus_simple_fmt
 typedef simple_locus<locus_simple_fmt::offset_n,
 		     locus_simple_fmt::hex> section_locus;
 
-inline std::ostream &
-operator << (std::ostream &os, locus const &loc)
-{
-  os << loc.format ();
-  return os;
-}
-
 #endif//DWARFLINT_WHERE_HH
diff --git a/dwarflint/reloc.cc b/dwarflint/reloc.cc
index dce458a1..ff1aaa2d 100644
--- a/dwarflint/reloc.cc
+++ b/dwarflint/reloc.cc
@@ -41,7 +41,7 @@
 namespace
 {
   class reloc_locus
-    : public clonable_locus<reloc_locus>
+    : public locus
   {
     locus const &_m_ref;
     size_t _m_index;