From: Roland McGrath Date: Thu, 2 Jul 2009 03:06:17 +0000 (-0700) Subject: Major revamp of ref tracker for efficiency and to handle circular reference chains. X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0dd9f27d6c68a147de29ade2eade4b4c29fe7bc3;p=thirdparty%2Felfutils.git Major revamp of ref tracker for efficiency and to handle circular reference chains. --- diff --git a/libdw/ChangeLog b/libdw/ChangeLog index 9d185441b..28cc39a5b 100644 --- a/libdw/ChangeLog +++ b/libdw/ChangeLog @@ -1,3 +1,9 @@ +2009-07-01 Roland McGrath + + * c++/dwarf_tracker: Major revamp for efficiency and to handle + circular reference chains. + * c++/dwarf_comparator: Tracker interface changes. + 2009-06-19 Roland McGrath * c++/dwarf_comparator: New file. diff --git a/libdw/c++/dwarf_comparator b/libdw/c++/dwarf_comparator index 56bcd81dc..55a417170 100644 --- a/libdw/c++/dwarf_comparator +++ b/libdw/c++/dwarf_comparator @@ -57,9 +57,8 @@ namespace elfutils // Prototypical stub for reference tracker object. // This keeps no state, and no two contexts ever match. template - class dwarf_tracker_base + struct dwarf_tracker_base { - protected: typedef typename dwarf1::compile_units::const_iterator cu1; typedef typename dwarf2::compile_units::const_iterator cu2; typedef typename dwarf1::debug_info_entry dwarf1_die; @@ -69,19 +68,27 @@ namespace elfutils typedef typename dwarf1_die::attributes_type::const_iterator attr1; typedef typename dwarf2_die::attributes_type::const_iterator attr2; - public: - inline void start_walk (const cu1 &a, const cu2 &b) - { - } - inline void finish_walk (const cu1 &a, const cu2 &b) - { - } - inline void pre_order (const die1 &a, const die2 &b) + // This object is created to start a walk and destroyed to finish one. + struct walk { - } - inline void post_order (const die1 &a, const die2 &b) + inline walk (dwarf_tracker_base *, const cu1 &a, const cu2 &b) + { + } + inline ~walk () + { + } + }; + + // This object is created in pre-order and destroyed in post-order. + struct step { - } + inline step (dwarf_tracker_base *, const die1 &a, const die2 &b) + { + } + inline ~step () + { + } + }; inline void visit (const typename dwarf1::debug_info_entry &a, const typename dwarf2::debug_info_entry &b) @@ -104,12 +111,15 @@ namespace elfutils } struct left_context_type {}; + struct right_context_type {}; + + // Return the lhs context of an arbitrary DIE. inline const left_context_type left_context (const die1 &die) { return left_context_type (); } - struct right_context_type {}; + // Return the rhs context of an arbitrary DIE. inline const right_context_type right_context (const die2 &die) { return right_context_type (); @@ -128,22 +138,31 @@ namespace elfutils return false; } + struct reference_match + { + inline bool cannot_match () const + { + return false; + } + inline void notice_match (const die2 &, bool) const + { + } + }; + + inline bool reference_matched (reference_match &, + const die1 &, const die2 &) + { + return false; + } + inline dwarf_tracker_base () {} - inline dwarf_tracker_base (const dwarf_tracker_base &, + inline dwarf_tracker_base (const dwarf_tracker_base &, reference_match &, const left_context_type &, const die1 &, const right_context_type &, const die2 &) {} - inline bool quick_reference_match (const die1 &, const die2 &) - { - return false; - } - - inline void notice_reference_match (const die1 &, const die2 &) - { - } }; template + matcher (*this) inline bool match (const dwarf1 &a, const dwarf2 &b) { @@ -188,8 +208,7 @@ namespace elfutils const cu1_it end1 = a.end (); const cu2_it end2 = b.end (); if (subr::container_equal - (it1, end1, it2, end2, - MATCHER (compile_units::const_iterator) (*this))) + (it1, end1, it2, end2, MATCHER (compile_units))) return true; _m_tracker.mismatch (it1, end1, it2, end2); return false; @@ -199,19 +218,8 @@ namespace elfutils typedef typename dwarf2::debug_info_entry die2; inline bool match (const cu1_it &a, const cu2_it &b) { - bool result; - _m_tracker.start_walk (a, b); - try - { - result = match (static_cast (*a), static_cast (*b)); - } - catch (...) - { - _m_tracker.finish_walk (a, b); - throw; - } - _m_tracker.finish_walk (a, b); - return result; + typename tracker::walk in (&_m_tracker, a, b); + return match (static_cast (*a), static_cast (*b)); } inline bool match (const die1 &a, const die2 &b) @@ -329,38 +337,39 @@ namespace elfutils typedef typename dwarf2::debug_info_entry::children_type children2; typedef typename children1::const_iterator cit1; typedef typename children2::const_iterator cit2; - struct die_matcher - : public std::binary_function + + inline bool match_child (const cit1 &a, const cit2 &b) { - dwarf_comparator &_m_cmp; - die_matcher (dwarf_comparator &cmp) - : _m_cmp (cmp) - {} + typename tracker::step into (&_m_tracker, a, b); + return match (*a, *b); + } + + inline bool match (const cit1 &a, const cit2 &b) + { + // Maybe the tracker has already cached a correspondence of DIEs. + typename tracker::reference_match matched; + if (_m_tracker.reference_matched (matched, a, b)) + return true; + + if (matched.cannot_match ()) + return false; + + bool result = match_child (a, b); + + // Let the tracker cache a result for its reference_matched. + matched.notice_match (b, result); + + return result; + } - inline bool operator () (const cit1 &a, const cit2 &b) - { - bool result; - _m_cmp._m_tracker.pre_order (a, b); - try - { - result = _m_cmp.match (*a, *b); - } - catch (...) - { - _m_cmp._m_tracker.post_order (a, b); - throw; - } - _m_cmp._m_tracker.post_order (a, b); - return result; - } - }; inline bool match (const children1 &a, const children2 &b) { cit1 it1 = a.begin (); cit2 it2 = b.begin (); const cit1 end1 = a.end (); const cit2 end2 = b.end (); - if (subr::container_equal (it1, end1, it2, end2, die_matcher (*this))) + if (subr::container_equal (it1, end1, it2, end2, + MATCHER (debug_info_entry::children_type))) return true; _m_tracker.mismatch (it1, end1, it2, end2); return false; @@ -454,20 +463,24 @@ namespace elfutils if (a.tag () != b.tag ()) return false; - const bool any_children = a.has_children (); - if (any_children != b.has_children ()) + const bool has_children = a.has_children (); + if (has_children != b.has_children ()) return false; - // Now we have to get the tracker involved. + // Maybe the tracker has already cached a correspondence of references. + typename tracker::reference_match matched; + if (_m_tracker.reference_matched (matched, ref1, ref2)) + return true; + + if (matched.cannot_match ()) + return false; + + // Now we really have to get the tracker involved. const typename tracker::left_context_type &lhs = _m_tracker.left_context (ref1); const typename tracker::right_context_type &rhs = _m_tracker.right_context (ref2); - // Maybe the tracker has already cached a correspondence of references. - if (_m_tracker.quick_reference_match (ref1, ref2)) - return true; - /* First do the cheap mismatch check on the contexts, then check the contents and contexts in ascending order of costliness of a check. */ if (_m_tracker.context_quick_mismatch (lhs, rhs) @@ -476,16 +489,17 @@ namespace elfutils return false; /* To compare the children, we have to clone the tracker and use a - new one. The new tracker jump-starts its walk to the referenced - DIE from the root of the CU. */ - bool result = !any_children || (dwarf_comparator (tracker (_m_tracker, + new one, in case of any reference attributes in their subtrees. + The new tracker jump-starts its walk to the referenced DIE from + the root of the CU. */ + bool result = !has_children || (dwarf_comparator (tracker (_m_tracker, + matched, lhs, ref1, rhs, ref2)) .match (a.children (), b.children ())); - // Let the tracker cache a known match for its quick_reference_match. - if (result) - _m_tracker.notice_reference_match (ref1, ref2); + // Let the tracker cache a result for its reference_matched. + matched.notice_match (ref2, result); return result; } diff --git a/libdw/c++/dwarf_tracker b/libdw/c++/dwarf_tracker index 70782258d..c27a737f8 100644 --- a/libdw/c++/dwarf_tracker +++ b/libdw/c++/dwarf_tracker @@ -52,6 +52,8 @@ #include "dwarf" #include "dwarf_comparator" +#include +#include namespace elfutils { @@ -62,34 +64,26 @@ namespace elfutils private: typedef dwarf_tracker_base _base; + public: typedef typename _base::cu1 cu1; typedef typename _base::cu2 cu2; typedef typename _base::die1 die1; typedef typename _base::die2 die2; - /* We maintain the current path down the logical DIE tree from the CU - as a stack of iterators pointing to the DIE at each level. */ + private: template struct tracker { + /* We maintain the current path down the logical DIE tree from the CU + as a stack of iterators pointing to the DIE at each level. */ typedef std::list die_path; - /* We record every DIE we have seen here, mapping its .identity () - to the die_path of parent DIEs taken to reach it. */ - typedef std::tr1::unordered_map< ::Dwarf_Off, const die_path> die_map; - die_map *_m_seen; - bool _m_delete_seen; - - cu _m_root; - - die_path _m_path; - - static const die_path bad_die_path () + // We use a singleton list of a default-constructed iterator as a marker. + static inline const die_path bad_die_path () { return die_path (1); } - - static bool bad_die_path (const die_path &path) + static inline bool bad_die_path (const die_path &path) { typename die_path::const_iterator it = path.begin (); if (it == path.end ()) @@ -98,99 +92,234 @@ namespace elfutils return ++it == path.end () && elt == die (); } + /* We record every DIE we have seen here, mapping its .identity () + to the die_path of parent DIEs taken to reach it. */ + typedef std::tr1::unordered_map< ::Dwarf_Off, const die_path> die_map; + die_map *_m_seen; + bool _m_delete_seen; + + cu _m_root; + + die_path _m_path; + + // Default constructor: an original tracker. inline tracker () : _m_seen (new die_map), _m_delete_seen (true) {} - inline tracker (const tracker &proto, const die_path &context) + // Construct a derived tracker: does its own walk, but sharing caches. + inline tracker (const tracker &proto, + const die_path &context, const die &there) : _m_seen (proto._m_seen), _m_delete_seen (false), _m_root (proto._m_root), _m_path (context) { + _m_path.push_back (there); } - ~tracker () + inline ~tracker () { - // We should never be left with a partial walk on the books. - assert (_m_path.empty ()); if (_m_delete_seen) - delete _m_seen; + { + delete _m_seen; + // We should never be left with a partial walk on the books. + assert (_m_path.empty ()); + } } + // Main hooks for a normal walk. + + /* A walk object does set-up work when constructed and tear-down + work when destroyed, so tear-down is done even for exceptions. */ + struct walk + { + tracker *_m_tracker; + inline walk (tracker *w, const cu &root) + : _m_tracker (w) + { + assert (_m_tracker->_m_path.empty ()); + _m_tracker->_m_root = root; + } + inline ~walk () + { + assert (_m_tracker->_m_path.empty ()); + _m_tracker->_m_root = cu (); + } + }; + + /* A step object does pre-order work when constructed and post-order + work when destroyed, so post-order is done even for exceptions. + While this object lives, HERE is on the _m_path stack. */ + struct step + { + tracker *_m_walker; + inline step (tracker *w, const die &here) + : _m_walker (w) + { + // Record the path down from the CU to see this DIE. + _m_walker->_m_seen->insert (std::make_pair (here->identity (), + _m_walker->_m_path)); + + // Append this DIE to the path we'll record for its children. + _m_walker->_m_path.push_back (here); + } + inline ~step () + { + _m_walker->_m_path.pop_back (); + } + }; + + // Random access to a DIE, find the path of the walk that gets there. inline const die_path &path_to (const die &a) { ::Dwarf_Off id = a->identity (); std::pair found = _m_seen->insert (std::make_pair (id, bad_die_path ())); - if (found.second) - { - /* We have a forward reference. That is, we didn't - already hit this DIE in our top-level walk and so - it is not in _m_seen yet. We must do a separate - walk to find it. */ - - die_path path; - if (!walk_to (*_m_root, id, path)) - throw std::runtime_error ("DIE not reachable from CU!"); - _m_seen->erase (found.first); - found.first = _m_seen->insert (found.first, - std::make_pair (id, path)); - } - - if (bad_die_path (found.first->second)) - abort (); // XXX - + if (found.second + /* It's not in our _m_seen map. Our main walk recording + into _m_seen is exhaustive, so this can only be a forward + reference. That is, we didn't already hit this DIE in + our top-level walk and so it is not in _m_seen yet. + + We must do a separate walk to find it. Since we know + this is a forward reference, we don't have to start a + fresh walk from the root, just momentarily wind forward + from where we are. */ + && !walk_down_to (a, found.first) + && !walk_over_to (a, found.first) + && !walk_up_to (a, found.first)) + throw std::runtime_error ("DIE not reachable from CU!"); + assert (&found.first->second != NULL); + assert (!bad_die_path (found.first->second)); return found.first->second; } - // Return true if this is the droid we're looking for, - // or recurse on its children. - bool walk_to (const typename die::value_type from, ::Dwarf_Off to, - die_path &path) + inline bool walk_to (const typename die::value_type &here, + const die &there, typename die_map::iterator &cache) { - if (from.identity () == to) - return true; - for (die it = from.children ().begin (); - it != from.children ().end (); - ++it) - if (walk_to (it, to, path)) - return true; - return false; + return walk_to (here.children ().begin (), + here.children ().end (), + there, cache); } - // Recursing on a child, include FROM in the path if the child matches. - bool walk_to (const die from, ::Dwarf_Off to, die_path &path) + bool walk_to (die it, const die &end, + const die &there, typename die_map::iterator &cache) { - if (walk_to (*from, to, path)) + for (; it != end; ++it) { - path.push_front (from); - return true; + if (it == there) + { + /* We can't keep the old CACHE iterator and avoid this + find (hash lookup), because there could have been + other insertions in the map since it was taken. + Those can invalidate old iterators. */ + cache = _m_seen->find (there->identity ()); + _m_seen->erase (cache); + cache = _m_seen->insert (cache, + std::make_pair (there->identity (), + _m_path)); + return true; + } + else + { + /* Do "step into" even for !has_children () + because it records this child in _m_seen, + which we will rely on later. */ + step into (this, it); + const typename die::value_type &child = *it; + if (child.has_children () && walk_to (child, there, cache)) + return true; + } } return false; } - inline void start_walk (const cu &a) + /* First descend into the current DIE's children. + _m_path already has the current DIE, so it is ready to go. */ + // XXX is a reference to an owned DIE really possible?? + inline bool walk_down_to (const die &there, + typename die_map::iterator &cache) { - assert (_m_path.empty ()); - _m_root = a; - } + const die &start = _m_path.back (); + const typename die::value_type &here = *start; - inline void finish_walk (const cu &a) - { - assert (_m_path.empty ()); - _m_root = cu (); + /* It's common to have a reference to the next sibling DIE. + So bypass the descent to HERE's children if THERE is + HERE's immediate next sibling. */ + if (!here.has_children () || there == ++die (start)) + return false; + + return walk_to (here, there, cache); } - inline void pre_order (const die &a) + /* A step_back object pops the current DIE off _m_path when + constructed, and pushes it back when destroyed. */ + struct step_back { - // Record the path down from the CU to see this DIE. - _m_seen->insert (std::make_pair (a->identity (), _m_path)); - // Append this DIE to the path we'll record for its children. - _m_path.push_back (a); + tracker *_m_walker; + const die _m_here; + inline step_back (tracker *w, die ©) + : _m_walker (w), _m_here (w->_m_path.back ()) + { + w->_m_path.pop_back (); + copy = _m_here; + } + inline ~step_back () + { + _m_walker->_m_path.push_back (_m_here); + } + }; + + /* Now wind the walk forward starting from the current DIE's + immediate sibling. */ + inline bool walk_over_to (const die &there, + typename die_map::iterator &cache) + { + die next; + step_back from (this, next); + ++next; + + return walk_to (next, (_m_path.empty () + ? (*_m_root).children ().end () + : _m_path.back ()->children ().end ()), + there, cache); } - inline void post_order (const die &a) + /* A step_up object saves _m_path when constructed + and restores it when destroyed. */ + struct step_up + { + tracker *_m_walker; + die_path _m_save; + inline step_up (tracker *w) + : _m_walker (w), _m_save (w->_m_path) + { + } + inline ~step_up () + { + _m_walker->_m_path.swap (_m_save); + } + }; + + /* Now wind the walk forward starting from the current DIE's + parent's immediate sibling. */ + inline bool walk_up_to (const die &there, + typename die_map::iterator &cache) { - _m_path.pop_back (); + if (_m_path.empty ()) + return false; + + step_up from (this); + + do + { + _m_path.pop_back (); + assert (!_m_path.empty ()); + if (walk_over_to (there, cache)) + return true; + } + while (!_m_path.empty ()); + + return false; } }; @@ -200,6 +329,13 @@ namespace elfutils tracker1 _m_left; tracker2 _m_right; + typedef std::tr1::unordered_map< + ::Dwarf_Off, std::pair > + > equiv_map; + equiv_map *_m_equiv; + bool _m_delete_equiv; + /* Predicate for DIEs "equal enough" to match as context for a subtree. The definition we use is that the DIE has the same tag and all its attributes are equal, excepting that references in attribute values @@ -216,39 +352,43 @@ namespace elfutils public: inline dwarf_ref_tracker () + : _m_equiv (new equiv_map), _m_delete_equiv (true) {} - inline void start_walk (const cu1 &a, const cu2 &b) + inline void reset () { - _m_left.start_walk (a); - _m_right.start_walk (b); + _m_equiv->clear (); + assert (!_m_right->_m_delete_seen); + _m_right._m_seen->clear (); } - inline void finish_walk (const cu1 &a, const cu2 &b) + struct walk { - _m_left.finish_walk (a); - _m_right.finish_walk (b); - } + typename tracker1::walk _m_left; + typename tracker2::walk _m_right; - inline void pre_order (const die1 &a, const die2 &b) - { - _m_left.pre_order (a); - _m_right.pre_order (b); - } + inline walk (dwarf_ref_tracker *w, const cu1 &a, const cu2 &b) + : _m_left (&w->_m_left, a), _m_right (&w->_m_right, b) + {} + }; - inline void post_order (const die1 &a, const die2 &b) + struct step { - _m_left.post_order (a); - _m_right.post_order (b); - } + typename tracker1::step _m_left; + typename tracker2::step _m_right; - typedef std::list left_context_type; + inline step (dwarf_ref_tracker *w, const die1 &a, const die2 &b) + : _m_left (&w->_m_left, a), _m_right (&w->_m_right, b) + {} + }; + + typedef typename tracker1::die_path left_context_type; inline const left_context_type &left_context (const die1 &die) { return _m_left.path_to (die); } - typedef std::list right_context_type; + typedef typename tracker2::die_path right_context_type; inline const right_context_type &right_context (const die2 &die) { return _m_right.path_to (die); @@ -269,14 +409,79 @@ namespace elfutils return std::equal (a.begin (), a.end (), b.begin (), equal_enough ()); } + class reference_match + { + friend class dwarf_ref_tracker; + private: + typename equiv_map::mapped_type *_m_elt; + + public: + + inline reference_match () + : _m_elt (NULL) + {} + + inline ~reference_match () + { + if (_m_elt != NULL) + _m_elt->first = NULL; + } + + inline bool cannot_match () const + { + return _m_elt == NULL; + } + + inline void notice_match (const die2 &b, bool matches) const + { + if (matches && _m_elt != NULL) + _m_elt->second.insert (b->identity ()); + } + }; + + inline bool + reference_matched (reference_match &matched, const die1 &a, const die2 &b) + { + typename equiv_map::mapped_type *elt = &(*_m_equiv)[a->identity ()]; + if (elt->first == NULL) + { + /* Record that we have a walk in progress crossing A. + When MATCHED goes out of scope in our caller, its + destructor will reset ELT->first to clear this record. */ + elt->first = &b; + matched._m_elt = elt; + + // Short-circuit if we have already matched B to A. + return elt->second.find (b->identity ()) != elt->second.end (); + } + + /* We have a circularity. We can tell because ELT->first remains + set from an outer recursion still in progress. + + The circular chain of references rooted at A matches B if B is + also the root of its own circularity and everything along those + parallel chains matches. If the chains hadn't matched so far, + we would not have kept following them to get here. + + We recorded the B that arrived at the first comparison with A. + We actually record the pointer on the caller's stack rather + than a copy of B, just because the iterator might be larger. */ + + return *elt->first == b; + } + // Share the _m_seen maps with the prototype tracker, // but start a fresh walk from the given starting point. inline dwarf_ref_tracker (const dwarf_ref_tracker &proto, - const left_context_type &lhs, const die1 &, - const right_context_type &rhs, const die2 &) - : _m_left (tracker1 (proto._m_left, lhs)), - _m_right (tracker2 (proto._m_right, rhs)) - {} + reference_match &matched, + const left_context_type &lhs, const die1 &a, + const right_context_type &rhs, const die2 &b) + : _m_left (tracker1 (proto._m_left, lhs, a)), + _m_right (tracker2 (proto._m_right, rhs, b)), + _m_equiv (proto._m_equiv), _m_delete_equiv (false) + { + // We are starting a recursive consideration of a vs b. + } }; }; diff --git a/src/ChangeLog b/src/ChangeLog index 65dff1bcf..190a33d7c 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,7 @@ +2009-07-01 Roland McGrath + + * dwarfcmp.cc (talker): Update constructor parameters. + 2009-06-19 Roland McGrath * dwarfcmp.cc: Revamp using dwarf_comparator. diff --git a/src/dwarfcmp.cc b/src/dwarfcmp.cc index c779fc49b..4deff59dc 100644 --- a/src/dwarfcmp.cc +++ b/src/dwarfcmp.cc @@ -143,10 +143,10 @@ struct talker : public dwarf_ref_tracker : a_ (NULL), b_ (NULL) {} - inline talker (const talker &proto, + inline talker (const talker &proto, typename _tracker::reference_match &m, const typename _tracker::left_context_type &l, const die1 &a, const typename _tracker::right_context_type &r, const die2 &b) - : _tracker (static_cast (proto), l, a, r, b), + : _tracker (static_cast (proto), m, l, a, r, b), a_ (NULL), b_ (NULL) { }