From Craig Silverstein: Rework debug info code a bit, add option for

ODR violations, add test case.

gold/dwarf_reader.cc

@@ -25,6 +25,7 @@
 #include "elfcpp_swap.h"
 #include "dwarf.h"
 #include "object.h"
+#include "parameters.h"
 #include "reloc.h"
 #include "dwarf_reader.h"
 
@@ -117,7 +118,7 @@ ResetLineStateMachine(struct LineStateMachine* lsm, bool default_is_stmt)
 }
 
 template<int size, bool big_endian>
-Dwarf_line_info<size, big_endian>::Dwarf_line_info(Object* object)
+Sized_dwarf_line_info<size, big_endian>::Sized_dwarf_line_info(Object* object)
   : data_valid_(false), buffer_(NULL), symtab_buffer_(NULL),
     directories_(), files_(), current_header_index_(-1)
 {
@@ -177,7 +178,7 @@ Dwarf_line_info<size, big_endian>::Dwarf_line_info(Object* object)
 
 template<int size, bool big_endian>
 const unsigned char*
-Dwarf_line_info<size, big_endian>::read_header_prolog(
+Sized_dwarf_line_info<size, big_endian>::read_header_prolog(
     const unsigned char* lineptr)
 {
   uint32_t initial_length = elfcpp::Swap<32, big_endian>::readval(lineptr);
@@ -238,7 +239,7 @@ Dwarf_line_info<size, big_endian>::read_header_prolog(
 
 template<int size, bool big_endian>
 const unsigned char*
-Dwarf_line_info<size, big_endian>::read_header_tables(
+Sized_dwarf_line_info<size, big_endian>::read_header_tables(
     const unsigned char* lineptr)
 {
   ++this->current_header_index_;
@@ -311,7 +312,7 @@ Dwarf_line_info<size, big_endian>::read_header_tables(
 
 template<int size, bool big_endian>
 bool
-Dwarf_line_info<size, big_endian>::process_one_opcode(
+Sized_dwarf_line_info<size, big_endian>::process_one_opcode(
     const unsigned char* start, struct LineStateMachine* lsm, size_t* len)
 {
   size_t oplen = 0;
@@ -490,7 +491,7 @@ Dwarf_line_info<size, big_endian>::process_one_opcode(
 
 template<int size, bool big_endian>
 unsigned const char*
-Dwarf_line_info<size, big_endian>::read_lines(unsigned const char* lineptr)
+Sized_dwarf_line_info<size, big_endian>::read_lines(unsigned const char* lineptr)
 {
   struct LineStateMachine lsm;
 
@@ -530,7 +531,7 @@ Dwarf_line_info<size, big_endian>::read_lines(unsigned const char* lineptr)
 
 template<int size, bool big_endian>
 unsigned int
-Dwarf_line_info<size, big_endian>::symbol_section(
+Sized_dwarf_line_info<size, big_endian>::symbol_section(
     unsigned int sym,
     typename elfcpp::Elf_types<size>::Elf_Addr* value)
 {
@@ -545,7 +546,7 @@ Dwarf_line_info<size, big_endian>::symbol_section(
 
 template<int size, bool big_endian>
 void
-Dwarf_line_info<size, big_endian>::read_relocs()
+Sized_dwarf_line_info<size, big_endian>::read_relocs()
 {
   if (this->symtab_buffer_ == NULL)
     return;
@@ -565,7 +566,7 @@ Dwarf_line_info<size, big_endian>::read_relocs()
 
 template<int size, bool big_endian>
 void
-Dwarf_line_info<size, big_endian>::read_line_mappings()
+Sized_dwarf_line_info<size, big_endian>::read_line_mappings()
 {
   gold_assert(this->data_valid_ == true);
 
@@ -595,7 +596,7 @@ Dwarf_line_info<size, big_endian>::read_line_mappings()
 
 template<int size, bool big_endian>
 bool
-Dwarf_line_info<size, big_endian>::input_is_relobj()
+Sized_dwarf_line_info<size, big_endian>::input_is_relobj()
 {
   // Only .o files have relocs and the symtab buffer that goes with them.
   return this->symtab_buffer_ != NULL;
@@ -606,7 +607,8 @@ Dwarf_line_info<size, big_endian>::input_is_relobj()
 
 template<int size, bool big_endian>
 std::string
-Dwarf_line_info<size, big_endian>::addr2line(unsigned int shndx, off_t offset)
+Sized_dwarf_line_info<size, big_endian>::do_addr2line(unsigned int shndx,
+                                                      off_t offset)
 {
   if (this->data_valid_ == false)
     return "";
@@ -669,24 +671,62 @@ Dwarf_line_info<size, big_endian>::addr2line(unsigned int shndx, off_t offset)
   return ret;
 }
 
+// Dwarf_line_info routines.
+
+// Note: this routine instantiates the appropriate
+// Sized_dwarf_line_info templates for this config, so we don't have
+// to have a separte instantiation section for them.
+
+std::string
+Dwarf_line_info::one_addr2line(Object* object,
+                               unsigned int shndx, off_t offset)
+{
+  if (parameters->get_size() == 32 && !parameters->is_big_endian())
+#ifdef HAVE_TARGET_32_LITTLE
+    return Sized_dwarf_line_info<32, false>(object).addr2line(shndx, offset);
+#else
+    gold_unreachable();
+#endif
+  else if (parameters->get_size() == 32 && parameters->is_big_endian())
+#ifdef HAVE_TARGET_32_BIG
+    return Sized_dwarf_line_info<32, true>(object).addr2line(shndx, offset);
+#else
+    gold_unreachable();
+#endif
+  else if (parameters->get_size() == 64 && !parameters->is_big_endian())
+#ifdef HAVE_TARGET_64_LITTLE
+    return Sized_dwarf_line_info<64, false>(object).addr2line(shndx, offset);
+#else
+    gold_unreachable();
+#endif
+  else if (parameters->get_size() == 64 && parameters->is_big_endian())
+#ifdef HAVE_TARGET_64_BIT
+    return Sized_dwarf_line_info<64, true>(object).addr2line(shndx, offset);
+#else
+    gold_unreachable();
+#endif
+  else
+    gold_unreachable();
+}
+
 #ifdef HAVE_TARGET_32_LITTLE
 template
-class Dwarf_line_info<32, false>;
+class Sized_dwarf_line_info<32, false>;
 #endif
 
 #ifdef HAVE_TARGET_32_BIG
 template
-class Dwarf_line_info<32, true>;
+class Sized_dwarf_line_info<32, true>;
 #endif
 
 #ifdef HAVE_TARGET_64_LITTLE
 template
-class Dwarf_line_info<64, false>;
+class Sized_dwarf_line_info<64, false>;
 #endif
 
 #ifdef HAVE_TARGET_64_BIG
 template
-class Dwarf_line_info<64, true>;
+class Sized_dwarf_line_info<64, true>;
 #endif
 
 } // End namespace gold.

gold/dwarf_reader.h

@@ -41,21 +41,51 @@ struct LineStateMachine;
 // This class is used to read the line information from the debugging
 // section of an object file.
 
-template<int size, bool big_endian>
 class Dwarf_line_info
 {
  public:
-  // Initializes a .debug_line reader for a given object file.
-  Dwarf_line_info(Object* object);
+  Dwarf_line_info()
+  { }
+
+  virtual
+  ~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.
   std::string
-  addr2line(unsigned int shndx, off_t offset);
+  addr2line(unsigned int shndx, off_t offset)
+  { return do_addr2line(shndx, offset); }
+
+  // A helper function for a single addr2line lookup.  It uses
+  // parameters() to figure out the size and endianness.  This is less
+  // efficient than using the templatized size and endianness, so only
+  // call this from an un-templatized context.
+  static std::string
+  one_addr2line(Object* object, unsigned int shndx, off_t offset);
 
  private:
+  virtual std::string
+  do_addr2line(unsigned int shndx, off_t offset) = 0;
+};
+
+template<int size, bool big_endian>
+class Sized_dwarf_line_info
+{
+ public:
+  // Initializes a .debug_line reader for a given object file.
+  Sized_dwarf_line_info(Object* object);
+
+  std::string
+  addr2line(unsigned int shndx, off_t offset)
+  { return do_addr2line(shndx, offset); }
+
+ private:
+  std::string
+  do_addr2line(unsigned int shndx, off_t offset);
+
   // Start processing line info, and populates the offset_map_.
   void
   read_line_mappings();

gold/object.cc

@@ -1093,7 +1093,7 @@ Relocate_info<size, big_endian>::location(size_t, off_t offset) const
   std::string filename;
   std::string file_and_lineno;   // Better than filename-only, if available.
 
-  Dwarf_line_info<size, big_endian> line_info(this->object);
+  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);
 

gold/options.cc

@@ -353,6 +353,9 @@ options::Command_line_options::options[] =
 	       &Position_dependent_options::set_static_search),
   GENERAL_NOARG('\0', "Bsymbolic", N_("Bind defined symbols locally"),
 		NULL, ONE_DASH, &General_options::set_symbolic),
+  GENERAL_NOARG('\0', "detect-odr-violations",
+                N_("Try to detect violations of the One Definition Rule"),
+                NULL, TWO_DASHES, &General_options::set_detect_odr_violations),
   GENERAL_NOARG('E', "export-dynamic", N_("Export all dynamic symbols"),
                 NULL, TWO_DASHES, &General_options::set_export_dynamic),
   GENERAL_NOARG('\0', "eh-frame-hdr", N_("Create exception frame header"),
@@ -473,6 +476,7 @@ General_options::General_options()
     is_relocatable_(false),
     strip_(STRIP_NONE),
     symbolic_(false),
+    detect_odr_violations_(false),
     create_eh_frame_hdr_(false),
     rpath_(),
     rpath_link_(),

gold/options.h

@@ -153,6 +153,11 @@ class General_options
   symbolic() const
   { return this->symbolic_; }
 
+  // --detect-odr-violations: Whether to search for One Defn Rule violations.
+  bool
+  detect_odr_violations() const
+  { return this->detect_odr_violations_; }
+
   // --eh-frame-hdr: Whether to generate an exception frame header.
   bool
   create_eh_frame_hdr() const
@@ -299,6 +304,10 @@ class General_options
   set_symbolic()
   { this->symbolic_ = true; }
 
+  void
+  set_detect_odr_violations()
+  { this->detect_odr_violations_ = true; }
+
   void
   set_create_eh_frame_hdr()
   { this->create_eh_frame_hdr_ = true; }
@@ -412,6 +421,7 @@ class General_options
   bool is_relocatable_;
   Strip strip_;
   bool symbolic_;
+  bool detect_odr_violations_;
   bool create_eh_frame_hdr_;
   Dir_list rpath_;
   Dir_list rpath_link_;

gold/parameters.cc

@@ -33,7 +33,7 @@ namespace gold
 Parameters::Parameters(Errors* errors)
   : errors_(errors), output_file_name_(NULL),
     output_file_type_(OUTPUT_INVALID), sysroot_(),
-    strip_(STRIP_INVALID), symbolic_(false),
+    strip_(STRIP_INVALID), symbolic_(false), detect_odr_violations_(false),
     optimization_level_(0), export_dynamic_(false),
     is_doing_static_link_valid_(false), doing_static_link_(false),
     is_size_and_endian_valid_(false), size_(0), is_big_endian_(false)
@@ -48,6 +48,7 @@ Parameters::set_from_options(const General_options* options)
   this->output_file_name_ = options->output_file_name();
   this->sysroot_ = options->sysroot();
   this->symbolic_ = options->symbolic();
+  this->detect_odr_violations_ = options->detect_odr_violations();
   this->optimization_level_ = options->optimization_level();
   this->export_dynamic_ = options->export_dynamic();
 

gold/parameters.h

@@ -121,6 +121,14 @@ class Parameters
     return this->symbolic_;
   }
 
+  // Whether we should try to detect violations of the One Definition Rule.
+  bool
+  detect_odr_violations() const
+  {
+    gold_assert(this->options_valid_);
+    return this->detect_odr_violations_;
+  }
+
   // The general linker optimization level.
   int
   optimization_level() const
@@ -218,6 +226,8 @@ class Parameters
   Strip strip_;
   // Whether we are doing a symbolic link.
   bool symbolic_;
+  // Whether we try to detect One Definition Rule violations.
+  bool detect_odr_violations_;
   // The optimization level.
   int optimization_level_;
   // Whether the -E/--export-dynamic flag is set.

gold/resolve.cc

@@ -243,7 +243,8 @@ Symbol_table::resolve(Sized_symbol<size>* to,
   // is an ODR violation.  But it's helpful to warn about.)
   // We use orig_sym here because we want the symbol exactly as it
   // appears in the object file, not munged via our future processing.
-  if (orig_sym.get_st_bind() == elfcpp::STB_WEAK
+  if (parameters->detect_odr_violations()
+      && orig_sym.get_st_bind() == elfcpp::STB_WEAK
       && to->binding() == elfcpp::STB_WEAK
       && orig_sym.get_st_shndx() != elfcpp::SHN_UNDEF
       && to->shndx() != elfcpp::SHN_UNDEF

gold/symtab.cc

@@ -1807,54 +1807,6 @@ Symbol_table::sized_write_section_symbol(const Output_section* os,
 
 void
 Symbol_table::detect_odr_violations() const
-{
-  if (parameters->get_size() == 32)
-    {
-      if (!parameters->is_big_endian())
-	{
-#ifdef HAVE_TARGET_32_LITTLE
-	  this->sized_detect_odr_violations<32, false>();
-#else
-	  gold_unreachable();
-#endif
-	}
-      else
-	{
-#ifdef HAVE_TARGET_32_BIG
-	  this->sized_detect_odr_violations<32, true>();
-#else
-	  gold_unreachable();
-#endif
-	}
-    }
-  else if (parameters->get_size() == 64)
-    {
-      if (!parameters->is_big_endian())
-	{
-#ifdef HAVE_TARGET_64_LITTLE
-	  this->sized_detect_odr_violations<64, false>();
-#else
-	  gold_unreachable();
-#endif
-	}
-      else
-	{
-#ifdef HAVE_TARGET_64_BIG
-	  this->sized_detect_odr_violations<64, true>();
-#else
-	  gold_unreachable();
-#endif
-	}
-    }
-  else
-    gold_unreachable();
-}
-
-// Implement detect_odr_violations.
-
-template<int size, bool big_endian>
-void
-Symbol_table::sized_detect_odr_violations() const
 {
   for (Odr_map::const_iterator it = candidate_odr_violations_.begin();
        it != candidate_odr_violations_.end();
@@ -1874,9 +1826,9 @@ Symbol_table::sized_detect_odr_violations() const
 	  // one Task for object, plus appropriate locking to ensure
 	  // that we don't conflict with other uses of the object.
           locs->object->lock();
-          Dwarf_line_info<size, big_endian> line_info(locs->object);
+          std::string lineno = Dwarf_line_info::one_addr2line(
+              locs->object, locs->shndx, locs->offset);
           locs->object->unlock();
-          std::string lineno = line_info.addr2line(locs->shndx, locs->offset);
           if (!lineno.empty())
             line_nums.insert(lineno);
         }

gold/testsuite/Makefile.am

@@ -118,8 +118,12 @@ if GCC
 
 debug_msg.o: debug_msg.cc
 	$(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/debug_msg.cc
-debug_msg.err: debug_msg.o
-	if $(CXXLINK) -Bgcctestdir/ -o debug_msg debug_msg.o 2>$@; \
+odr_violation1.o: odr_violation1.cc
+	$(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/odr_violation1.cc
+odr_violation2.o: odr_violation2.cc
+	$(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/odr_violation2.cc
+debug_msg.err: debug_msg.o odr_violation1.o odr_violation2.o
+	if $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg debug_msg.o odr_violation1.o odr_violation2.o 2>$@; \
 	then \
 	  echo 2>&1 "Link of debug_msg.o should have failed"; \
 	  exit 1; \

gold/testsuite/Makefile.in

@@ -1206,8 +1206,12 @@ uninstall-am: uninstall-info-am
 
 @GCC_TRUE@debug_msg.o: debug_msg.cc
 @GCC_TRUE@	$(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/debug_msg.cc
-@GCC_TRUE@debug_msg.err: debug_msg.o
-@GCC_TRUE@	if $(CXXLINK) -Bgcctestdir/ -o debug_msg debug_msg.o 2>$@; \
+@GCC_TRUE@odr_violation1.o: odr_violation1.cc
+@GCC_TRUE@	$(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/odr_violation1.cc
+@GCC_TRUE@odr_violation2.o: odr_violation2.cc
+@GCC_TRUE@	$(CXXCOMPILE) -O0 -g -c -w -o $@ $(srcdir)/odr_violation2.cc
+@GCC_TRUE@debug_msg.err: debug_msg.o odr_violation1.o odr_violation2.o
+@GCC_TRUE@	if $(CXXLINK) -Bgcctestdir/ -Wl,--detect-odr-violations -o debug_msg debug_msg.o odr_violation1.o odr_violation2.o 2>$@; \
 @GCC_TRUE@	then \
 @GCC_TRUE@	  echo 2>&1 "Link of debug_msg.o should have failed"; \
 @GCC_TRUE@	  exit 1; \

gold/testsuite/debug_msg.cc

@@ -55,6 +55,10 @@ class Derived : public Base
   virtual void virtfn() { undef_fn2(); }
 };
 
+// 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
+
 int main()
 {
   testfn(5);
@@ -63,5 +67,13 @@ int main()
   Base b;
   Derived d;
 
+  int kInput1[] = {1, 6, 9, 7, 3, 4, 2, 10, 5, 8};
+  int kSize1 = sizeof(kInput1) / sizeof(int);
+  SortAscending(kInput1, kSize1);
+
+  int kInput2[] = {1, 6, 9, 7, 3, 4, 2, 10, 5, 8};
+  int kSize2 = sizeof(kInput2) / sizeof(int);
+  SortDescending(kInput2, kSize2);
+
   return 0;
 }

gold/testsuite/debug_msg.sh

@@ -55,4 +55,9 @@ check "debug_msg.o: in function int testfn<double>(double):${srcdir}/debug_msg.c
 check "debug_msg.o: in function int testfn<double>(double):${srcdir}/debug_msg.cc:44: undefined reference to 'undef_fn2()'"
 check "debug_msg.o: in function int testfn<double>(double):${srcdir}/debug_msg.cc:45: undefined reference to 'undef_int'"
 
+# Check we detected the ODR (One Definition Rule) violation.
+check "warning: symbol Ordering::operator()(int, int) *defined in multiple places (possible ODR violation):"
+check "odr_violation1.cc:5"
+check "odr_violation2.cc:5"
+
 exit 0

gold/testsuite/odr_violation1.cc

@@ -0,0 +1,12 @@
+#include <algorithm>
+
+class Ordering {
+ public:
+  bool operator()(int a, int b) {
+    return a < b;
+  }
+};
+
+void SortAscending(int array[], int size) {
+  std::sort(array, array + size, Ordering());
+}

gold/testsuite/odr_violation2.cc

@@ -0,0 +1,14 @@
+#include <algorithm>
+
+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.
+    return a + 1 > b + 1;
+  }
+};
+
+void SortDescending(int array[], int size) {
+  std::sort(array, array + size, Ordering());
+}