* dwarf_reader.cc (Sized_dwarf_line_info): Include all lines,
authorIan Lance Taylor <ian@airs.com>
Thu, 10 Mar 2011 01:31:33 +0000 (01:31 +0000)
committerIan Lance Taylor <ian@airs.com>
Thu, 10 Mar 2011 01:31:33 +0000 (01:31 +0000)
but mark earlier ones as non-canonical
(offset_to_iterator): Update search target and example
(do_addr2line): Return extra lines in a vector*
(format_file_lineno): Extract from do_addr2line
(one_addr2line): Add vector* out-param
* dwarf_reader.h (Offset_to_lineno_entry): New field recording
when a lineno entry appeared last for its instruction
(Dwarf_line_info): Add vector* out-param
* object.cc (Relocate_info): Pass NULL for the vector* out-param
* symtab.cc (Odr_violation_compare): Include the lineno in the
comparison again.
(linenos_from_loc): New. Combine the canonical line for an
address with its other lines.
(True_if_intersect): New. Helper functor to make
std::set_intersection a query.
(detect_odr_violations): Compare sets of lines instead of just
one line for each function. This became less deterministic, but
has fewer false positives.
* symtab.h: Declarations.
* testsuite/Makefile.am (odr_violation2.o): Compile with -O2 to
mix an optimized and non-optimized object in the same binary
(odr_violation2.so): Same.
* testsuite/Makefile.in: Regenerate from Makefile.am.
* testsuite/debug_msg.cc (main): Make OdrDerived classes.
* testsuite/debug_msg.sh: Update line numbers and add
assertions.
* testsuite/odr_violation1.cc: Use OdrDerived, in a
non-optimized context.
* testsuite/odr_violation2.cc: Make sure Ordering::operator()
isn't inlined, and use OdrDerived in an optimized context.
* testsuite/odr_header1.h: Defines OdrDerived, where
optimization will change the
first-instruction-in-the-destructor's file and line number.
* testsuite/odr_header2.h: Defines OdrBase.

14 files changed:
gold/ChangeLog
gold/dwarf_reader.cc
gold/dwarf_reader.h
gold/object.cc
gold/symtab.cc
gold/symtab.h
gold/testsuite/Makefile.am
gold/testsuite/Makefile.in
gold/testsuite/debug_msg.cc
gold/testsuite/debug_msg.sh
gold/testsuite/odr_header1.h [new file with mode: 0644]
gold/testsuite/odr_header2.h [new file with mode: 0644]
gold/testsuite/odr_violation1.cc
gold/testsuite/odr_violation2.cc

index bbb8185..77c2870 100644 (file)
@@ -1,3 +1,41 @@
+2011-03-09  Jeffrey Yasskin  <jyasskin@google.com>
+
+       * dwarf_reader.cc (Sized_dwarf_line_info): Include all lines,
+       but mark earlier ones as non-canonical
+       (offset_to_iterator): Update search target and example
+       (do_addr2line): Return extra lines in a vector*
+       (format_file_lineno): Extract from do_addr2line
+       (one_addr2line): Add vector* out-param
+       * dwarf_reader.h (Offset_to_lineno_entry): New field recording
+       when a lineno entry appeared last for its instruction
+       (Dwarf_line_info): Add vector* out-param
+       * object.cc (Relocate_info): Pass NULL for the vector* out-param
+       * symtab.cc (Odr_violation_compare): Include the lineno in the
+       comparison again.
+       (linenos_from_loc): New. Combine the canonical line for an
+       address with its other lines.
+       (True_if_intersect): New. Helper functor to make
+       std::set_intersection a query.
+       (detect_odr_violations): Compare sets of lines instead of just
+       one line for each function. This became less deterministic, but
+       has fewer false positives.
+       * symtab.h: Declarations.
+       * testsuite/Makefile.am (odr_violation2.o): Compile with -O2 to
+       mix an optimized and non-optimized object in the same binary
+       (odr_violation2.so): Same.
+       * testsuite/Makefile.in: Regenerate from Makefile.am.
+       * testsuite/debug_msg.cc (main): Make OdrDerived classes.
+       * testsuite/debug_msg.sh: Update line numbers and add
+       assertions.
+       * testsuite/odr_violation1.cc: Use OdrDerived, in a
+       non-optimized context.
+       * testsuite/odr_violation2.cc: Make sure Ordering::operator()
+       isn't inlined, and use OdrDerived in an optimized context.
+       * testsuite/odr_header1.h: Defines OdrDerived, where
+       optimization will change the
+       first-instruction-in-the-destructor's file and line number.
+       * testsuite/odr_header2.h: Defines OdrBase.
+
 2011-03-09  Ian Lance Taylor  <iant@google.com>
 
        * fileread.cc (File_read::clear_views): Don't delete the whole
index 8110f38..e8fe04e 100644 (file)
@@ -492,19 +492,18 @@ Sized_dwarf_line_info<size, big_endian>::read_lines(unsigned const char* lineptr
             {
               Offset_to_lineno_entry entry
                   = { lsm.address, this->current_header_index_,
-                      lsm.file_num, lsm.line_num };
+                      lsm.file_num, true, lsm.line_num };
              std::vector<Offset_to_lineno_entry>&
                map(this->line_number_map_[lsm.shndx]);
              // If we see two consecutive entries with the same
-             // offset and a real line number, then always use the
-             // second one.
+             // offset and a real line number, then mark the first
+             // one as non-canonical.
              if (!map.empty()
                  && (map.back().offset == static_cast<off_t>(lsm.address))
                  && lsm.line_num != -1
                  && map.back().line_num != -1)
-               map.back() = entry;
-             else
-               map.push_back(entry);
+               map.back().last_line_for_offset = false;
+             map.push_back(entry);
             }
           lineptr += oplength;
         }
@@ -612,7 +611,7 @@ static std::vector<Offset_to_lineno_entry>::const_iterator
 offset_to_iterator(const std::vector<Offset_to_lineno_entry>* offsets,
                    off_t offset)
 {
-  const Offset_to_lineno_entry lookup_key = { offset, 0, 0, 0 };
+  const Offset_to_lineno_entry lookup_key = { offset, 0, 0, true, 0 };
 
   // lower_bound() returns the smallest offset which is >= lookup_key.
   // If no offset in offsets is >= lookup_key, returns end().
@@ -621,25 +620,27 @@ offset_to_iterator(const std::vector<Offset_to_lineno_entry>* offsets,
 
   // This code is easiest to understand with a concrete example.
   // Here's a possible offsets array:
-  // {{offset = 3211, header_num = 0, file_num = 1, line_num = 16},  // 0
-  //  {offset = 3224, header_num = 0, file_num = 1, line_num = 20},  // 1
-  //  {offset = 3226, header_num = 0, file_num = 1, line_num = 22},  // 2
-  //  {offset = 3231, header_num = 0, file_num = 1, line_num = 25},  // 3
-  //  {offset = 3232, header_num = 0, file_num = 1, line_num = -1},  // 4
-  //  {offset = 3232, header_num = 0, file_num = 1, line_num = 65},  // 5
-  //  {offset = 3235, header_num = 0, file_num = 1, line_num = 66},  // 6
-  //  {offset = 3236, header_num = 0, file_num = 1, line_num = -1},  // 7
-  //  {offset = 5764, header_num = 0, file_num = 1, line_num = 47},  // 8
-  //  {offset = 5765, header_num = 0, file_num = 1, line_num = 48},  // 9
-  //  {offset = 5767, header_num = 0, file_num = 1, line_num = 49},  // 10
-  //  {offset = 5768, header_num = 0, file_num = 1, line_num = 50},  // 11
-  //  {offset = 5773, header_num = 0, file_num = 1, line_num = -1},  // 12
-  //  {offset = 5787, header_num = 1, file_num = 1, line_num = 19},  // 13
-  //  {offset = 5790, header_num = 1, file_num = 1, line_num = 20},  // 14
-  //  {offset = 5793, header_num = 1, file_num = 1, line_num = 67},  // 15
-  //  {offset = 5793, header_num = 1, file_num = 1, line_num = -1},  // 16
-  //  {offset = 5795, header_num = 1, file_num = 1, line_num = 68},  // 17
-  //  {offset = 5798, header_num = 1, file_num = 1, line_num = -1},  // 18
+  // {{offset = 3211, header_num = 0, file_num = 1, last, line_num = 16},  // 0
+  //  {offset = 3224, header_num = 0, file_num = 1, last, line_num = 20},  // 1
+  //  {offset = 3226, header_num = 0, file_num = 1, last, line_num = 22},  // 2
+  //  {offset = 3231, header_num = 0, file_num = 1, last, line_num = 25},  // 3
+  //  {offset = 3232, header_num = 0, file_num = 1, last, line_num = -1},  // 4
+  //  {offset = 3232, header_num = 0, file_num = 1, last, line_num = 65},  // 5
+  //  {offset = 3235, header_num = 0, file_num = 1, last, line_num = 66},  // 6
+  //  {offset = 3236, header_num = 0, file_num = 1, last, line_num = -1},  // 7
+  //  {offset = 5764, header_num = 0, file_num = 1, last, line_num = 48},  // 8
+  //  {offset = 5764, header_num = 0, file_num = 1,!last, line_num = 47},  // 9
+  //  {offset = 5765, header_num = 0, file_num = 1, last, line_num = 49},  // 10
+  //  {offset = 5767, header_num = 0, file_num = 1, last, line_num = 50},  // 11
+  //  {offset = 5768, header_num = 0, file_num = 1, last, line_num = 51},  // 12
+  //  {offset = 5773, header_num = 0, file_num = 1, last, line_num = -1},  // 13
+  //  {offset = 5787, header_num = 1, file_num = 1, last, line_num = 19},  // 14
+  //  {offset = 5790, header_num = 1, file_num = 1, last, line_num = 20},  // 15
+  //  {offset = 5793, header_num = 1, file_num = 1, last, line_num = 67},  // 16
+  //  {offset = 5793, header_num = 1, file_num = 1, last, line_num = -1},  // 17
+  //  {offset = 5793, header_num = 1, file_num = 1,!last, line_num = 66},  // 18
+  //  {offset = 5795, header_num = 1, file_num = 1, last, line_num = 68},  // 19
+  //  {offset = 5798, header_num = 1, file_num = 1, last, line_num = -1},  // 20
   // The entries with line_num == -1 mark the end of a function: the
   // associated offset is one past the last instruction in the
   // function.  This can correspond to the beginning of the next
@@ -651,7 +652,7 @@ offset_to_iterator(const std::vector<Offset_to_lineno_entry>* offsets,
   //         offsets[0].  Since it's not an exact match and we're
   //         at the beginning of offsets, we return end() (invalid).
   // Case 2: lookup_key has offset 10000.  lower_bound returns
-  //         offset[19] (end()).  We return end() (invalid).
+  //         offset[21] (end()).  We return end() (invalid).
   // Case 3: lookup_key has offset == 3211.  lower_bound matches
   //         offsets[0] exactly, and that's the entry we return.
   // Case 4: lookup_key has offset == 3232.  lower_bound returns
@@ -670,16 +671,17 @@ offset_to_iterator(const std::vector<Offset_to_lineno_entry>* offsets,
   //         end-of-function, we know lookup_key is between
   //         functions, so we return end() (not a valid offset).
   // Case 7: lookup_key has offset == 5794.  lower_bound returns
-  //         offsets[17].  Since it's not an exact match, we back
-  //         up to offsets[15].  Note we back up to the *first*
-  //         entry with offset 5793, not just offsets[17-1].
-  //         We note offsets[15] is a valid entry, so we return it.
-  //         If offsets[15] had had line_num == -1, we would have
-  //         checked offsets[16].  The reason for this is that
-  //         15 and 16 can be in an arbitrary order, since we sort
-  //         only by offset.  (Note it doesn't help to use line_number
-  //         as a secondary sort key, since sometimes we want the -1
-  //         to be first and sometimes we want it to be last.)
+  //         offsets[19].  Since it's not an exact match, we back
+  //         up to offsets[16].  Note we back up to the *first*
+  //         entry with offset 5793, not just offsets[19-1].
+  //         We note offsets[16] is a valid entry, so we return it.
+  //         If offsets[16] had had line_num == -1, we would have
+  //         checked offsets[17].  The reason for this is that
+  //         16 and 17 can be in an arbitrary order, since we sort
+  //         only by offset and last_line_for_offset.  (Note it
+  //         doesn't help to use line_number as a tertiary sort key,
+  //         since sometimes we want the -1 to be first and sometimes
+  //         we want it to be last.)
 
   // This deals with cases (1) and (2).
   if ((it == offsets->begin() && offset < it->offset)
@@ -710,19 +712,25 @@ offset_to_iterator(const std::vector<Offset_to_lineno_entry>* offsets,
 
   // This handles cases (5), (6), and (7): if any entry in the
   // equal_range [it, range_end) has a line_num != -1, it's a valid
-  // match.  If not, we're not in a function.
+  // match.  If not, we're not in a function.  The line number we saw
+  // last for an offset will be sorted first, so it'll get returned if
+  // it's present.
   for (; it != range_end; ++it)
     if (it->line_num != -1)
       return it;
   return offsets->end();
 }
 
-// Return a string for a file name and line number.
+// Returns the canonical filename:lineno for the address passed in.
+// If other_lines is not NULL, appends the non-canonical lines
+// assigned to the same address.
 
 template<int size, bool big_endian>
 std::string
-Sized_dwarf_line_info<size, big_endian>::do_addr2line(unsigned int shndx,
-                                                      off_t offset)
+Sized_dwarf_line_info<size, big_endian>::do_addr2line(
+    unsigned int shndx,
+    off_t offset,
+    std::vector<std::string>* other_lines)
 {
   if (this->data_valid_ == false)
     return "";
@@ -743,21 +751,38 @@ Sized_dwarf_line_info<size, big_endian>::do_addr2line(unsigned int shndx,
   if (it == offsets->end())
     return "";
 
-  // Convert the file_num + line_num into a string.
+  std::string result = this->format_file_lineno(*it);
+  if (other_lines != NULL)
+    for (++it; it != offsets->end() && it->offset == offset; ++it)
+      {
+        if (it->line_num == -1)
+          continue;  // The end of a previous function.
+        other_lines->push_back(this->format_file_lineno(*it));
+      }
+  return result;
+}
+
+// Convert the file_num + line_num into a string.
+
+template<int size, bool big_endian>
+std::string
+Sized_dwarf_line_info<size, big_endian>::format_file_lineno(
+    const Offset_to_lineno_entry& loc) const
+{
   std::string ret;
 
-  gold_assert(it->header_num < static_cast<int>(this->files_.size()));
-  gold_assert(it->file_num
-             < static_cast<int>(this->files_[it->header_num].size()));
+  gold_assert(loc.header_num < static_cast<int>(this->files_.size()));
+  gold_assert(loc.file_num
+             < static_cast<int>(this->files_[loc.header_num].size()));
   const std::pair<int, std::string>& filename_pair
-      = this->files_[it->header_num][it->file_num];
+      = this->files_[loc.header_num][loc.file_num];
   const std::string& filename = filename_pair.second;
 
-  gold_assert(it->header_num < static_cast<int>(this->directories_.size()));
+  gold_assert(loc.header_num < static_cast<int>(this->directories_.size()));
   gold_assert(filename_pair.first
-              < static_cast<int>(this->directories_[it->header_num].size()));
+              < static_cast<int>(this->directories_[loc.header_num].size()));
   const std::string& dirname
-      = this->directories_[it->header_num][filename_pair.first];
+      = this->directories_[loc.header_num][filename_pair.first];
 
   if (!dirname.empty())
     {
@@ -769,7 +794,7 @@ Sized_dwarf_line_info<size, big_endian>::do_addr2line(unsigned int shndx,
     ret = "(unknown)";
 
   char buffer[64];   // enough to hold a line number
-  snprintf(buffer, sizeof(buffer), "%d", it->line_num);
+  snprintf(buffer, sizeof(buffer), "%d", loc.line_num);
   ret += ":";
   ret += buffer;
 
@@ -803,7 +828,8 @@ static std::vector<Addr2line_cache_entry> addr2line_cache;
 std::string
 Dwarf_line_info::one_addr2line(Object* object,
                                unsigned int shndx, off_t offset,
-                               size_t cache_size)
+                               size_t cache_size,
+                               std::vector<std::string>* other_lines)
 {
   Dwarf_line_info* lineinfo = NULL;
   std::vector<Addr2line_cache_entry>::iterator it;
@@ -854,7 +880,7 @@ Dwarf_line_info::one_addr2line(Object* object,
   }
 
   // Now that we have our object, figure out the answer
-  std::string retval = lineinfo->addr2line(shndx, offset);
+  std::string retval = lineinfo->addr2line(shndx, offset, other_lines);
 
   // Finally, if our cache has grown too big, delete old objects.  We
   // assume the common (probably only) case is deleting only one object.
index 8fe7898..3f92dd3 100644 (file)
@@ -25,6 +25,7 @@
 
 #include <vector>
 #include <map>
+#include <limits.h>
 
 #include "elfcpp.h"
 #include "elfcpp_swap.h"
@@ -44,14 +45,25 @@ struct Offset_to_lineno_entry
 {
   off_t offset;
   int header_num;  // which file-list to use (i.e. which .o file are we in)
-  int file_num;    // a pointer into files_
-  int line_num;    // the line number in the source file
-
-  // When we add entries to the table, we always use the last entry
-  // with a given offset.  Given proper DWARF info, this should ensure
-  // that the offset is a sufficient sort key.
+  // A pointer into files_.
+  unsigned int file_num : sizeof(int) * CHAR_BIT - 1;
+  // True if this was the last entry for the current offset, meaning
+  // it's the line that actually applies.
+  unsigned int last_line_for_offset : 1;
+  // The line number in the source file.  -1 to indicate end-of-function.
+  int line_num;
+
+  // This sorts by offsets first, and then puts the correct line to
+  // report for a given offset at the beginning of the run of equal
+  // offsets (so that asking for 1 line gives the best answer).  This
+  // is not a total ordering.
   bool operator<(const Offset_to_lineno_entry& that) const
-  { return this->offset < that.offset; }
+  {
+    if (this->offset != that.offset)
+      return this->offset < that.offset;
+    // Note the '>' which makes this sort 'true' first.
+    return this->last_line_for_offset > that.last_line_for_offset;
+  }
 };
 
 // This class is used to read the line information from the debugging
@@ -70,10 +82,13 @@ class Dwarf_line_info
   // Given a section number and an offset, returns the associated
   // file and line-number, as a string: "file:lineno".  If unable
   // to do the mapping, returns the empty string.  You must call
-  // read_line_mappings() before calling this function.
+  // read_line_mappings() before calling this function.  If
+  // 'other_lines' is non-NULL, fills that in with other line
+  // numbers assigned to the same offset.
   std::string
-  addr2line(unsigned int shndx, off_t offset)
-  { return do_addr2line(shndx, offset); }
+  addr2line(unsigned int shndx, off_t offset,
+            std::vector<std::string>* other_lines)
+  { return this->do_addr2line(shndx, offset, other_lines); }
 
   // A helper function for a single addr2line lookup.  It also keeps a
   // cache of the last CACHE_SIZE Dwarf_line_info objects it created;
@@ -83,7 +98,7 @@ class Dwarf_line_info
   // NOTE: Not thread-safe, so only call from one thread at a time.
   static std::string
   one_addr2line(Object* object, unsigned int shndx, off_t offset,
-                size_t cache_size);
+                size_t cache_size, std::vector<std::string>* other_lines);
 
   // This reclaims all the memory that one_addr2line may have cached.
   // Use this when you know you will not be calling one_addr2line again.
@@ -92,7 +107,8 @@ class Dwarf_line_info
 
  private:
   virtual std::string
-  do_addr2line(unsigned int shndx, off_t offset) = 0;
+  do_addr2line(unsigned int shndx, off_t offset,
+               std::vector<std::string>* other_lines) = 0;
 };
 
 template<int size, bool big_endian>
@@ -106,7 +122,12 @@ class Sized_dwarf_line_info : public Dwarf_line_info
 
  private:
   std::string
-  do_addr2line(unsigned int shndx, off_t offset);
+  do_addr2line(unsigned int shndx, off_t offset,
+               std::vector<std::string>* other_lines);
+
+  // Formats a file and line number to a string like "dirname/filename:lineno".
+  std::string
+  format_file_lineno(const Offset_to_lineno_entry& lineno) const;
 
   // Start processing line info, and populates the offset_map_.
   // If SHNDX is non-negative, only store debug information that
index b85e9e0..e2c0113 100644 (file)
@@ -2582,7 +2582,7 @@ Relocate_info<size, big_endian>::location(size_t, off_t offset) const
 
   Sized_dwarf_line_info<size, big_endian> line_info(this->object);
   // This will be "" if we failed to parse the debug info for any reason.
-  file_and_lineno = line_info.addr2line(this->data_shndx, offset);
+  file_and_lineno = line_info.addr2line(this->data_shndx, offset, NULL);
 
   std::string ret(this->object->name());
   ret += ':';
index d4ac297..e882dea 100644 (file)
@@ -3025,7 +3025,7 @@ Symbol_table::print_stats() const
 
 // We check for ODR violations by looking for symbols with the same
 // name for which the debugging information reports that they were
-// defined in different source locations.  When comparing the source
+// defined in disjoint source locations.  When comparing the source
 // location, we consider instances with the same base filename to be
 // the same.  This is because different object files/shared libraries
 // can include the same header file using different paths, and
@@ -3035,7 +3035,7 @@ Symbol_table::print_stats() const
 
 // This struct is used to compare line information, as returned by
 // Dwarf_line_info::one_addr2line.  It implements a < comparison
-// operator used with std::set.
+// operator used with std::sort.
 
 struct Odr_violation_compare
 {
@@ -3043,32 +3043,77 @@ struct Odr_violation_compare
   operator()(const std::string& s1, const std::string& s2) const
   {
     // Inputs should be of the form "dirname/filename:linenum" where
-    // "dirname/" is optional.  We want to compare just the filename.
+    // "dirname/" is optional.  We want to compare just the filename:linenum.
 
-    // Find the last '/' and ':' in each string.
+    // Find the last '/' in each string.
     std::string::size_type s1begin = s1.rfind('/');
     std::string::size_type s2begin = s2.rfind('/');
-    std::string::size_type s1end = s1.rfind(':');
-    std::string::size_type s2end = s2.rfind(':');
     // If there was no '/' in a string, start at the beginning.
     if (s1begin == std::string::npos)
       s1begin = 0;
     if (s2begin == std::string::npos)
       s2begin = 0;
-    // If the ':' appeared in the directory name, compare to the end
-    // of the string.
-    if (s1end < s1begin)
-      s1end = s1.size();
-    if (s2end < s2begin)
-      s2end = s2.size();
-    // Compare takes lengths, not end indices.
-    return s1.compare(s1begin, s1end - s1begin,
-                     s2, s2begin, s2end - s2begin) < 0;
+    return s1.compare(s1begin, std::string::npos,
+                     s2, s2begin, std::string::npos) < 0;
   }
 };
 
+// Returns all of the lines attached to LOC, not just the one the
+// instruction actually came from.
+std::vector<std::string>
+Symbol_table::linenos_from_loc(const Task* task,
+                               const Symbol_location& loc)
+{
+  // We need to lock the object in order to read it.  This
+  // means that we have to run in a singleton Task.  If we
+  // want to run this in a general Task for better
+  // performance, we will need one Task for object, plus
+  // appropriate locking to ensure that we don't conflict with
+  // other uses of the object.  Also note, one_addr2line is not
+  // currently thread-safe.
+  Task_lock_obj<Object> tl(task, loc.object);
+
+  std::vector<std::string> result;
+  // 16 is the size of the object-cache that one_addr2line should use.
+  std::string canonical_result = Dwarf_line_info::one_addr2line(
+      loc.object, loc.shndx, loc.offset, 16, &result);
+  if (!canonical_result.empty())
+    result.push_back(canonical_result);
+  return result;
+}
+
+// OutputIterator that records if it was ever assigned to.  This
+// allows it to be used with std::set_intersection() to check for
+// intersection rather than computing the intersection.
+struct Check_intersection
+{
+  Check_intersection()
+    : value_(false)
+  {}
+
+  bool had_intersection() const
+  { return this->value_; }
+
+  Check_intersection& operator++()
+  { return *this; }
+
+  Check_intersection& operator*()
+  { return *this; }
+
+  template<typename T>
+  Check_intersection& operator=(const T&)
+  {
+    this->value_ = true;
+    return *this;
+  }
+
+ private:
+  bool value_;
+};
+
 // Check candidate_odr_violations_ to find symbols with the same name
-// but apparently different definitions (different source-file/line-no).
+// but apparently different definitions (different source-file/line-no
+// for each line assigned to the first instruction).
 
 void
 Symbol_table::detect_odr_violations(const Task* task,
@@ -3078,48 +3123,73 @@ Symbol_table::detect_odr_violations(const Task* task,
        it != candidate_odr_violations_.end();
        ++it)
     {
-      const char* symbol_name = it->first;
-      // Maps from symbol location to a sample object file we found
-      // that location in.  We use a sorted map so the location order
-      // is deterministic, but we only store an arbitrary object file
-      // to avoid copying lots of names.
-      std::map<std::string, std::string, Odr_violation_compare> line_nums;
-
-      for (Unordered_set<Symbol_location, Symbol_location_hash>::const_iterator
-               locs = it->second.begin();
-           locs != it->second.end();
-           ++locs)
+      const char* const symbol_name = it->first;
+
+      std::string first_object_name;
+      std::vector<std::string> first_object_linenos;
+
+      Unordered_set<Symbol_location, Symbol_location_hash>::const_iterator
+          locs = it->second.begin();
+      const Unordered_set<Symbol_location, Symbol_location_hash>::const_iterator
+          locs_end = it->second.end();
+      for (; locs != locs_end && first_object_linenos.empty(); ++locs)
         {
-         // We need to lock the object in order to read it.  This
-         // means that we have to run in a singleton Task.  If we
-         // want to run this in a general Task for better
-         // performance, we will need one Task for object, plus
-         // appropriate locking to ensure that we don't conflict with
-         // other uses of the object.  Also note, one_addr2line is not
-          // currently thread-safe.
-         Task_lock_obj<Object> tl(task, locs->object);
-          // 16 is the size of the object-cache that one_addr2line should use.
-          std::string lineno = Dwarf_line_info::one_addr2line(
-              locs->object, locs->shndx, locs->offset, 16);
-          if (!lineno.empty())
-            {
-              std::string& sample_object = line_nums[lineno];
-              if (sample_object.empty())
-                sample_object = locs->object->name();
-            }
+          // Save the line numbers from the first definition to
+          // compare to the other definitions.  Ideally, we'd compare
+          // every definition to every other, but we don't want to
+          // take O(N^2) time to do this.  This shortcut may cause
+          // false negatives that appear or disappear depending on the
+          // link order, but it won't cause false positives.
+          first_object_name = locs->object->name();
+          first_object_linenos = this->linenos_from_loc(task, *locs);
         }
 
-      if (line_nums.size() > 1)
+      // Sort by Odr_violation_compare to make std::set_intersection work.
+      std::sort(first_object_linenos.begin(), first_object_linenos.end(),
+                Odr_violation_compare());
+
+      for (; locs != locs_end; ++locs)
         {
-          gold_warning(_("while linking %s: symbol '%s' defined in multiple "
-                         "places (possible ODR violation):"),
-                       output_file_name, demangle(symbol_name).c_str());
-          for (std::map<std::string, std::string>::const_iterator it2 =
-                line_nums.begin();
-              it2 != line_nums.end();
-              ++it2)
-            fprintf(stderr, _("  %s from %s\n"),
-                    it2->first.c_str(), it2->second.c_str());
+          std::vector<std::string> linenos =
+              this->linenos_from_loc(task, *locs);
+          // linenos will be empty if we couldn't parse the debug info.
+          if (linenos.empty())
+            continue;
+          // Sort by Odr_violation_compare to make std::set_intersection work.
+          std::sort(linenos.begin(), linenos.end(), Odr_violation_compare());
+
+          Check_intersection intersection_result =
+              std::set_intersection(first_object_linenos.begin(),
+                                    first_object_linenos.end(),
+                                    linenos.begin(),
+                                    linenos.end(),
+                                    Check_intersection(),
+                                    Odr_violation_compare());
+          if (!intersection_result.had_intersection())
+            {
+              gold_warning(_("while linking %s: symbol '%s' defined in "
+                             "multiple places (possible ODR violation):"),
+                           output_file_name, demangle(symbol_name).c_str());
+              // This only prints one location from each definition,
+              // which may not be the location we expect to intersect
+              // with another definition.  We could print the whole
+              // set of locations, but that seems too verbose.
+              gold_assert(!first_object_linenos.empty());
+              gold_assert(!linenos.empty());
+              fprintf(stderr, _("  %s from %s\n"),
+                      first_object_linenos[0].c_str(),
+                      first_object_name.c_str());
+              fprintf(stderr, _("  %s from %s\n"),
+                      linenos[0].c_str(),
+                      locs->object->name().c_str());
+              // Only print one broken pair, to avoid needing to
+              // compare against a list of the disjoint definition
+              // locations we've found so far.  (If we kept comparing
+              // against just the first one, we'd get a lot of
+              // redundant complaints about the second definition
+              // location.)
+              break;
+            }
         }
     }
   // We only call one_addr2line() in this function, so we can clear its cache.
index c0ae358..d350d1d 100644 (file)
@@ -1560,6 +1560,33 @@ class Symbol_table
   typedef Unordered_map<Symbol_table_key, Symbol*, Symbol_table_hash,
                        Symbol_table_eq> Symbol_table_type;
 
+  // A map from symbol name (as a pointer into the namepool) to all
+  // the locations the symbols is (weakly) defined (and certain other
+  // conditions are met).  This map will be used later to detect
+  // possible One Definition Rule (ODR) violations.
+  struct Symbol_location
+  {
+    Object* object;         // Object where the symbol is defined.
+    unsigned int shndx;     // Section-in-object where the symbol is defined.
+    off_t offset;           // Offset-in-section where the symbol is defined.
+    bool operator==(const Symbol_location& that) const
+    {
+      return (this->object == that.object
+              && this->shndx == that.shndx
+              && this->offset == that.offset);
+    }
+  };
+
+  struct Symbol_location_hash
+  {
+    size_t operator()(const Symbol_location& loc) const
+    { return reinterpret_cast<uintptr_t>(loc.object) ^ loc.offset ^ loc.shndx; }
+  };
+
+  typedef Unordered_map<const char*,
+                        Unordered_set<Symbol_location, Symbol_location_hash> >
+  Odr_map;
+
   // Make FROM a forwarder symbol to TO.
   void
   make_forwarder(Symbol* from, Symbol* to);
@@ -1707,6 +1734,12 @@ class Symbol_table
   do_allocate_commons_list(Layout*, Commons_section_type, Commons_type*,
                           Mapfile*, Sort_commons_order);
 
+  // Returns all of the lines attached to LOC, not just the one the
+  // instruction actually came from.  This helps the ODR checker avoid
+  // false positives.
+  static std::vector<std::string>
+  linenos_from_loc(const Task* task, const Symbol_location& loc);
+
   // Implement detect_odr_violations.
   template<int size, bool big_endian>
   void
@@ -1760,33 +1793,6 @@ class Symbol_table
   // they are defined.
   typedef Unordered_map<const Symbol*, Dynobj*> Copied_symbol_dynobjs;
 
-  // A map from symbol name (as a pointer into the namepool) to all
-  // the locations the symbols is (weakly) defined (and certain other
-  // conditions are met).  This map will be used later to detect
-  // possible One Definition Rule (ODR) violations.
-  struct Symbol_location
-  {
-    Object* object;         // Object where the symbol is defined.
-    unsigned int shndx;     // Section-in-object where the symbol is defined.
-    off_t offset;           // Offset-in-section where the symbol is defined.
-    bool operator==(const Symbol_location& that) const
-    {
-      return (this->object == that.object
-              && this->shndx == that.shndx
-              && this->offset == that.offset);
-    }
-  };
-
-  struct Symbol_location_hash
-  {
-    size_t operator()(const Symbol_location& loc) const
-    { return reinterpret_cast<uintptr_t>(loc.object) ^ loc.offset ^ loc.shndx; }
-  };
-
-  typedef Unordered_map<const char*,
-                        Unordered_set<Symbol_location, Symbol_location_hash> >
-  Odr_map;
-
   // We increment this every time we see a new undefined symbol, for
   // use in archive groups.
   size_t saw_undefined_;
index 53e6660..5ccd397 100644 (file)
@@ -849,8 +849,10 @@ debug_msg.o: debug_msg.cc
        $(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/debug_msg.cc
 odr_violation1.o: odr_violation1.cc
        $(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/odr_violation1.cc
+# Compile with different optimization flags to check that rearranged
+# instructions don't cause a false positive.
 odr_violation2.o: odr_violation2.cc
-       $(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/odr_violation2.cc
+       $(CXXCOMPILE) -O2 -g -c -w -o $@ $(srcdir)/odr_violation2.cc
 debug_msg.err: debug_msg.o odr_violation1.o odr_violation2.o gcctestdir/ld
        @echo $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg debug_msg.o odr_violation1.o odr_violation2.o "2>$@"
        @if $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg debug_msg.o odr_violation1.o odr_violation2.o 2>$@; \
@@ -868,7 +870,7 @@ debug_msg.so: debug_msg.cc gcctestdir/ld
 odr_violation1.so: odr_violation1.cc gcctestdir/ld
        $(CXXCOMPILE) -Bgcctestdir/ -O0 -g -shared -fPIC -w -o $@ $(srcdir)/odr_violation1.cc
 odr_violation2.so: odr_violation2.cc gcctestdir/ld
-       $(CXXCOMPILE) -Bgcctestdir/ -O0 -g -shared -fPIC -w -o $@ $(srcdir)/odr_violation2.cc
+       $(CXXCOMPILE) -Bgcctestdir/ -O2 -g -shared -fPIC -w -o $@ $(srcdir)/odr_violation2.cc
 debug_msg_so.err: debug_msg.so odr_violation1.so odr_violation2.so gcctestdir/ld
        @echo $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg_so debug_msg.so odr_violation1.so odr_violation2.so "2>$@"
        @if $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg_so debug_msg.so odr_violation1.so odr_violation2.so 2>$@; \
@@ -887,7 +889,7 @@ debug_msg_ndebug.so: debug_msg.cc gcctestdir/ld
 odr_violation1_ndebug.so: odr_violation1.cc gcctestdir/ld
        $(CXXCOMPILE) -Bgcctestdir/ -O0 -g0 -shared -fPIC -w -o $@ $(srcdir)/odr_violation1.cc
 odr_violation2_ndebug.so: odr_violation2.cc gcctestdir/ld
-       $(CXXCOMPILE) -Bgcctestdir/ -O0 -g0 -shared -fPIC -w -o $@ $(srcdir)/odr_violation2.cc
+       $(CXXCOMPILE) -Bgcctestdir/ -O2 -g0 -shared -fPIC -w -o $@ $(srcdir)/odr_violation2.cc
 debug_msg_ndebug.err: debug_msg_ndebug.so odr_violation1_ndebug.so odr_violation2_ndebug.so gcctestdir/ld
        @echo $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg_ndebug debug_msg_ndebug.so odr_violation1_ndebug.so odr_violation2_ndebug.so "2>$@"
        @if $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg_ndebug debug_msg_ndebug.so odr_violation1_ndebug.so odr_violation2_ndebug.so 2>$@; \
index 6d9bb7a..f7489ea 100644 (file)
@@ -4017,8 +4017,10 @@ uninstall-am:
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/debug_msg.cc
 @GCC_TRUE@@NATIVE_LINKER_TRUE@odr_violation1.o: odr_violation1.cc
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/odr_violation1.cc
+# Compile with different optimization flags to check that rearranged
+# instructions don't cause a false positive.
 @GCC_TRUE@@NATIVE_LINKER_TRUE@odr_violation2.o: odr_violation2.cc
-@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/odr_violation2.cc
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -O2 -g -c -w -o $@ $(srcdir)/odr_violation2.cc
 @GCC_TRUE@@NATIVE_LINKER_TRUE@debug_msg.err: debug_msg.o odr_violation1.o odr_violation2.o gcctestdir/ld
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ @echo $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg debug_msg.o odr_violation1.o odr_violation2.o "2>$@"
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ @if $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg debug_msg.o odr_violation1.o odr_violation2.o 2>$@; \
@@ -4032,7 +4034,7 @@ uninstall-am:
 @GCC_TRUE@@NATIVE_LINKER_TRUE@odr_violation1.so: odr_violation1.cc gcctestdir/ld
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -Bgcctestdir/ -O0 -g -shared -fPIC -w -o $@ $(srcdir)/odr_violation1.cc
 @GCC_TRUE@@NATIVE_LINKER_TRUE@odr_violation2.so: odr_violation2.cc gcctestdir/ld
-@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -Bgcctestdir/ -O0 -g -shared -fPIC -w -o $@ $(srcdir)/odr_violation2.cc
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -Bgcctestdir/ -O2 -g -shared -fPIC -w -o $@ $(srcdir)/odr_violation2.cc
 @GCC_TRUE@@NATIVE_LINKER_TRUE@debug_msg_so.err: debug_msg.so odr_violation1.so odr_violation2.so gcctestdir/ld
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ @echo $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg_so debug_msg.so odr_violation1.so odr_violation2.so "2>$@"
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ @if $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg_so debug_msg.so odr_violation1.so odr_violation2.so 2>$@; \
@@ -4046,7 +4048,7 @@ uninstall-am:
 @GCC_TRUE@@NATIVE_LINKER_TRUE@odr_violation1_ndebug.so: odr_violation1.cc gcctestdir/ld
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -Bgcctestdir/ -O0 -g0 -shared -fPIC -w -o $@ $(srcdir)/odr_violation1.cc
 @GCC_TRUE@@NATIVE_LINKER_TRUE@odr_violation2_ndebug.so: odr_violation2.cc gcctestdir/ld
-@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -Bgcctestdir/ -O0 -g0 -shared -fPIC -w -o $@ $(srcdir)/odr_violation2.cc
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -Bgcctestdir/ -O2 -g0 -shared -fPIC -w -o $@ $(srcdir)/odr_violation2.cc
 @GCC_TRUE@@NATIVE_LINKER_TRUE@debug_msg_ndebug.err: debug_msg_ndebug.so odr_violation1_ndebug.so odr_violation2_ndebug.so gcctestdir/ld
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ @echo $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg_ndebug debug_msg_ndebug.so odr_violation1_ndebug.so odr_violation2_ndebug.so "2>$@"
 @GCC_TRUE@@NATIVE_LINKER_TRUE@ @if $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg_ndebug debug_msg_ndebug.so odr_violation1_ndebug.so odr_violation2_ndebug.so 2>$@; \
index 1d77bc9..0912002 100644 (file)
@@ -58,6 +58,10 @@ class Derived : public Base
 // This tests One Definition Rule (ODR) violations.
 void SortAscending(int array[], int size);   // in odr_violation1.cc
 void SortDescending(int array[], int size);  // in odr_violation2.cc
+// This tests One Definition Rule (ODR) non-violations.
+#include "odr_header2.h"
+OdrBase* CreateOdrDerived1();  // in odr_violation1.cc
+OdrBase* CreateOdrDerived2();  // in odr_violation2.cc
 
 extern "C" int OverriddenCFunction(int i);  // in odr_violation*.cc
 
@@ -85,5 +89,8 @@ int main()
   OverriddenCFunction(3);
   SometimesInlineFunction(3);
 
+  delete CreateOdrDerived1();
+  delete CreateOdrDerived2();
+
   return 0;
 }
index 74b4b05..b8624e7 100755 (executable)
@@ -73,18 +73,22 @@ check debug_msg.err "debug_msg.o: in function int testfn<double>(double):.*/debu
 
 # Check we detected the ODR (One Definition Rule) violation.
 check debug_msg.err ": symbol 'Ordering::operator()(int, int)' defined in multiple places (possible ODR violation):"
-check debug_msg.err "odr_violation1.cc:5"
-check debug_msg.err "odr_violation2.cc:5"
+check debug_msg.err "odr_violation1.cc:6"
+check debug_msg.err "odr_violation2.cc:9"
+
+# Check we don't have ODR false positives:
+check_missing debug_msg.err "OdrDerived::~OdrDerived()"
+check_missing debug_msg.err "__adjust_heap"
 # We block ODR detection for combinations of C weak and strong
 # symbols, to allow people to use the linker to override things.  We
 # still flag it for C++ symbols since those are more likely to be
 # unintentional.
 check_missing debug_msg.err ": symbol 'OverriddenCFunction' defined in multiple places (possible ODR violation):"
-check_missing debug_msg.err "odr_violation1.cc:15"
-check_missing debug_msg.err "odr_violation2.cc:17"
+check_missing debug_msg.err "odr_violation1.cc:16"
+check_missing debug_msg.err "odr_violation2.cc:20"
 check debug_msg.err ": symbol 'SometimesInlineFunction(int)' defined in multiple places (possible ODR violation):"
-check debug_msg.err "debug_msg.cc:64"
-check debug_msg.err "odr_violation2.cc:21"
+check debug_msg.err "debug_msg.cc:68"
+check debug_msg.err "odr_violation2.cc:24"
 
 # When linking together .so's, we don't catch the line numbers, but we
 # still find all the undefined variables, and the ODR violation.
@@ -92,14 +96,16 @@ check debug_msg_so.err "debug_msg.so: error: undefined reference to 'undef_fn1()
 check debug_msg_so.err "debug_msg.so: error: undefined reference to 'undef_fn2()'"
 check debug_msg_so.err "debug_msg.so: error: undefined reference to 'undef_int'"
 check debug_msg_so.err ": symbol 'Ordering::operator()(int, int)' defined in multiple places (possible ODR violation):"
-check debug_msg_so.err "odr_violation1.cc:5"
-check debug_msg_so.err "odr_violation2.cc:5"
+check debug_msg_so.err "odr_violation1.cc:6"
+check debug_msg_so.err "odr_violation2.cc:9"
+check_missing debug_msg.err "OdrDerived::~OdrDerived()"
+check_missing debug_msg.err "__adjust_heap"
 check_missing debug_msg.err ": symbol 'OverriddenCFunction' defined in multiple places (possible ODR violation):"
-check_missing debug_msg.err "odr_violation1.cc:15"
-check_missing debug_msg.err "odr_violation2.cc:17"
+check_missing debug_msg.err "odr_violation1.cc:16"
+check_missing debug_msg.err "odr_violation2.cc:20"
 check debug_msg.err ": symbol 'SometimesInlineFunction(int)' defined in multiple places (possible ODR violation):"
-check debug_msg.err "debug_msg.cc:64"
-check debug_msg.err "odr_violation2.cc:21"
+check debug_msg.err "debug_msg.cc:68"
+check debug_msg.err "odr_violation2.cc:24"
 
 # These messages shouldn't need any debug info to detect:
 check debug_msg_ndebug.err "debug_msg_ndebug.so: error: undefined reference to 'undef_fn1()'"
diff --git a/gold/testsuite/odr_header1.h b/gold/testsuite/odr_header1.h
new file mode 100644 (file)
index 0000000..3d9c488
--- /dev/null
@@ -0,0 +1,6 @@
+#include "odr_header2.h"
+
+struct OdrDerived : OdrBase {
+  ~OdrDerived() {
+  }
+};
diff --git a/gold/testsuite/odr_header2.h b/gold/testsuite/odr_header2.h
new file mode 100644 (file)
index 0000000..0fb9449
--- /dev/null
@@ -0,0 +1,4 @@
+struct OdrBase {
+  virtual ~OdrBase() {
+  }
+};
index 6c40496..8b37cea 100644 (file)
@@ -1,4 +1,5 @@
 #include <algorithm>
+#include "odr_header1.h"
 
 class Ordering {
  public:
@@ -15,3 +16,8 @@ extern "C" int OverriddenCFunction(int i) __attribute__ ((weak));
 extern "C" int OverriddenCFunction(int i) {
   return i;
 }
+
+// Instantiate the Derived vtable, without optimization.
+OdrBase* CreateOdrDerived1() {
+  return new OdrDerived;
+}
index 09940d4..6c57b6f 100644 (file)
@@ -1,13 +1,16 @@
 #include <algorithm>
+#include "odr_header1.h"
 
 class Ordering {
  public:
-  bool operator()(int a, int b) {
-    // We need the "+ 1" here to force this operator() to be a
-    // different size than the one in odr_violation1.cc.
+  bool operator()(int a, int b) __attribute__((never_inline));
+};
+
+bool Ordering::operator()(int a, int b) {
+  // Optimization makes this operator() a different size than the one
+  // in odr_violation1.cc.
     return a + 1 > b + 1;
   }
-};
 
 void SortDescending(int array[], int size) {
   std::sort(array, array + size, Ordering());
@@ -21,3 +24,8 @@ extern "C" int OverriddenCFunction(int i) {
 int SometimesInlineFunction(int i) {
   return i * i;
 }
+
+// Instantiate the Derived vtable, with optimization (see Makefile.am).
+OdrBase* CreateOdrDerived2() {
+  return new OdrDerived;
+}