[gold commit] Improve ODR checking in gold

[gold commit] Improve ODR checking in gold

[Cary Coutant]

[-- Attachment #1: Type: text/plain, Size: 836 bytes --]

I've committed the following patch to make ODR checking slightly more
fuzzy, to tolerate slight differences in line numbers that might
result from different compilers or different compile options.

-cary

gold/
        * debug.h (DEBUG_LOCATION): New.
        (DEBUG_ALL): Include DEBUG_LOCATION.
        (debug_string_to_enum): Add DEBUG_LOCATION.
        * dwarf_reader.cc (Sized_dwarf_line_info::read_lines): Fix debug
        output to print correct context.
        (Sized_dwarf_line_info::do_addr2line): Add debug output. Return
        up to 4 more locations at the beginning of the function.
        * symtab.cc (Symbol_table::detect_odr_violations): Get canonical
        result before sorting list of line numbers.
        * testsuite/debug_msg.sh: Allow range of line numbers for
        canonical results on optimized code.

[-- Attachment #2: 0001-Improve-ODR-checking-in-gold.patch --]
[-- Type: application/octet-stream, Size: 9595 bytes --]

From 437ddf0c4cb63fdb68c4bd1cc155144db344d0c5 Mon Sep 17 00:00:00 2001
From: Cary Coutant <ccoutant@google.com>
Date: Thu, 9 Apr 2015 11:52:21 -0700
Subject: [PATCH] Improve ODR checking in gold.

gold/
	* debug.h (DEBUG_LOCATION): New.
	(DEBUG_ALL): Include DEBUG_LOCATION.
	(debug_string_to_enum): Add DEBUG_LOCATION.
	* dwarf_reader.cc (Sized_dwarf_line_info::read_lines): Fix debug
	output to print correct context.
	(Sized_dwarf_line_info::do_addr2line): Add debug output. Return
	up to 4 more locations at the beginning of the function.
	* symtab.cc (Symbol_table::detect_odr_violations): Get canonical
	result before sorting list of line numbers.
	* testsuite/debug_msg.sh: Allow range of line numbers for
	canonical results on optimized code.
---
 gold/ChangeLog              | 14 ++++++++++++++
 gold/debug.h                |  5 ++++-
 gold/dwarf_reader.cc        | 32 ++++++++++++++++++++++++++------
 gold/symtab.cc              | 11 +++++++----
 gold/testsuite/debug_msg.sh | 12 ++++++------
 5 files changed, 57 insertions(+), 17 deletions(-)

diff --git a/gold/ChangeLog b/gold/ChangeLog
index fa82e42..9943cf5 100644
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -1,3 +1,17 @@
+2015-04-09  Cary Coutant  <ccoutant@google.com>
+
+	* debug.h (DEBUG_LOCATION): New.
+	(DEBUG_ALL): Include DEBUG_LOCATION.
+	(debug_string_to_enum): Add DEBUG_LOCATION.
+	* dwarf_reader.cc (Sized_dwarf_line_info::read_lines): Fix debug
+	output to print correct context.
+	(Sized_dwarf_line_info::do_addr2line): Add debug output. Return
+	up to 4 more locations at the beginning of the function.
+	* symtab.cc (Symbol_table::detect_odr_violations): Get canonical
+	result before sorting list of line numbers.
+	* testsuite/debug_msg.sh: Allow range of line numbers for
+	canonical results on optimized code.
+
 2015-04-07  HC Yen <hc.yen@mediatek.com>
 
 	Add AArch32 support for gold linker.
diff --git a/gold/debug.h b/gold/debug.h
index bca55f3..3fecbd8 100644
--- a/gold/debug.h
+++ b/gold/debug.h
@@ -38,9 +38,11 @@ const int DEBUG_SCRIPT = 0x2;
 const int DEBUG_FILES = 0x4;
 const int DEBUG_RELAXATION = 0x8;
 const int DEBUG_INCREMENTAL = 0x10;
+const int DEBUG_LOCATION = 0x20;
 
 const int DEBUG_ALL = (DEBUG_TASK | DEBUG_SCRIPT | DEBUG_FILES
-		       | DEBUG_RELAXATION | DEBUG_INCREMENTAL);
+		       | DEBUG_RELAXATION | DEBUG_INCREMENTAL
+		       | DEBUG_LOCATION);
 
 // Convert a debug string to the appropriate enum.
 inline int
@@ -54,6 +56,7 @@ debug_string_to_enum(const char* arg)
     { "files", DEBUG_FILES },
     { "relaxation", DEBUG_RELAXATION },
     { "incremental", DEBUG_INCREMENTAL },
+    { "location", DEBUG_LOCATION },
     { "all", DEBUG_ALL }
   };
 
diff --git a/gold/dwarf_reader.cc b/gold/dwarf_reader.cc
index e7c95ce..59b85b8 100644
--- a/gold/dwarf_reader.cc
+++ b/gold/dwarf_reader.cc
@@ -2205,13 +2205,33 @@ Sized_dwarf_line_info<size, big_endian>::do_addr2line(
     return "";
 
   std::string result = this->format_file_lineno(*it);
+  gold_debug(DEBUG_LOCATION, "do_addr2line: canonical result: %s",
+	     result.c_str());
   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));
-      }
+    {
+      unsigned int last_file_num = it->file_num;
+      int last_line_num = it->line_num;
+      // Return up to 4 more locations from the beginning of the function
+      // for fuzzy matching.
+      for (++it; it != offsets->end(); ++it)
+	{
+	  if (it->offset == offset && it->line_num == -1)
+	    continue;  // The end of a previous function.
+	  if (it->line_num == -1)
+	    break;  // The end of the current function.
+	  if (it->file_num != last_file_num || it->line_num != last_line_num)
+	    {
+	      other_lines->push_back(this->format_file_lineno(*it));
+	      gold_debug(DEBUG_LOCATION, "do_addr2line: other: %s",
+			 other_lines->back().c_str());
+	      last_file_num = it->file_num;
+	      last_line_num = it->line_num;
+	    }
+	  if (it->offset > offset && other_lines->size() >= 4)
+	    break;
+	}
+    }
+
   return result;
 }
 
diff --git a/gold/symtab.cc b/gold/symtab.cc
index 045327a..88e9322 100644
--- a/gold/symtab.cc
+++ b/gold/symtab.cc
@@ -3336,8 +3336,11 @@ Symbol_table::detect_odr_violations(const Task* task,
           first_object_name = locs->object->name();
           first_object_linenos = this->linenos_from_loc(task, *locs);
         }
+      if (first_object_linenos.empty())
+	continue;
 
       // Sort by Odr_violation_compare to make std::set_intersection work.
+      std::string first_object_canonical_result = first_object_linenos.back();
       std::sort(first_object_linenos.begin(), first_object_linenos.end(),
                 Odr_violation_compare());
 
@@ -3349,6 +3352,8 @@ Symbol_table::detect_odr_violations(const Task* task,
           if (linenos.empty())
             continue;
           // Sort by Odr_violation_compare to make std::set_intersection work.
+          gold_assert(!linenos.empty());
+          std::string second_object_canonical_result = linenos.back();
           std::sort(linenos.begin(), linenos.end(), Odr_violation_compare());
 
           Check_intersection intersection_result =
@@ -3367,13 +3372,11 @@ Symbol_table::detect_odr_violations(const Task* task,
               // 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_canonical_result.c_str(),
                       first_object_name.c_str());
               fprintf(stderr, _("  %s from %s\n"),
-                      linenos[0].c_str(),
+                      second_object_canonical_result.c_str(),
                       locs->object->name().c_str());
               // Only print one broken pair, to avoid needing to
               // compare against a list of the disjoint definition
diff --git a/gold/testsuite/debug_msg.sh b/gold/testsuite/debug_msg.sh
index 785e5c5..54c72f1 100755
--- a/gold/testsuite/debug_msg.sh
+++ b/gold/testsuite/debug_msg.sh
@@ -74,7 +74,7 @@ fi
 # 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:6"
-check debug_msg.err "odr_violation2.cc:12"
+check debug_msg.err "odr_violation2.cc:1[25]"
 
 # Check we don't have ODR false positives:
 check_missing debug_msg.err "OdrDerived::~OdrDerived()"
@@ -88,7 +88,7 @@ check_missing debug_msg.err "odr_violation1.cc:16"
 check_missing debug_msg.err "odr_violation2.cc:23"
 check debug_msg.err ": symbol 'SometimesInlineFunction(int)' defined in multiple places (possible ODR violation):"
 check debug_msg.err "debug_msg.cc:68"
-check debug_msg.err "odr_violation2.cc:27"
+check debug_msg.err "odr_violation2.cc:2[78]"
 
 # Check for the same error messages when using --compressed-debug-sections.
 if test -r debug_msg_cdebug.err
@@ -106,7 +106,7 @@ then
   fi
   check debug_msg_cdebug.err ": symbol 'Ordering::operator()(int, int)' defined in multiple places (possible ODR violation):"
   check debug_msg_cdebug.err "odr_violation1.cc:6"
-  check debug_msg_cdebug.err "odr_violation2.cc:12"
+  check debug_msg_cdebug.err "odr_violation2.cc:1[25]"
   check_missing debug_msg_cdebug.err "OdrDerived::~OdrDerived()"
   check_missing debug_msg_cdebug.err "__adjust_heap"
   check_missing debug_msg_cdebug.err ": symbol 'OverriddenCFunction' defined in multiple places (possible ODR violation):"
@@ -114,7 +114,7 @@ then
   check_missing debug_msg_cdebug.err "odr_violation2.cc:23"
   check debug_msg_cdebug.err ": symbol 'SometimesInlineFunction(int)' defined in multiple places (possible ODR violation):"
   check debug_msg_cdebug.err "debug_msg.cc:68"
-  check debug_msg_cdebug.err "odr_violation2.cc:27"
+  check debug_msg_cdebug.err "odr_violation2.cc:2[78]"
 fi
 
 # When linking together .so's, we don't catch the line numbers, but we
@@ -124,7 +124,7 @@ 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:6"
-check debug_msg_so.err "odr_violation2.cc:12"
+check debug_msg_so.err "odr_violation2.cc:1[25]"
 check_missing debug_msg_so.err "OdrDerived::~OdrDerived()"
 check_missing debug_msg_so.err "__adjust_heap"
 check_missing debug_msg_so.err ": symbol 'OverriddenCFunction' defined in multiple places (possible ODR violation):"
@@ -132,7 +132,7 @@ check_missing debug_msg_so.err "odr_violation1.cc:16"
 check_missing debug_msg_so.err "odr_violation2.cc:23"
 check debug_msg_so.err ": symbol 'SometimesInlineFunction(int)' defined in multiple places (possible ODR violation):"
 check debug_msg_so.err "debug_msg.cc:68"
-check debug_msg_so.err "odr_violation2.cc:27"
+check debug_msg_so.err "odr_violation2.cc:2[78]"
 
 # 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()'"
-- 
2.2.1