Further improve warnings for relocations referring to discarded sections.
authorCary Coutant <ccoutant@gmail.com>
Fri, 6 Apr 2018 23:06:16 +0000 (16:06 -0700)
committerCary Coutant <ccoutant@gmail.com>
Sat, 7 Apr 2018 05:50:14 +0000 (22:50 -0700)
Relocations referring to discarded sections are now treated as errors
instead of warnings.

Also with this patch, we will now print the section group signature and the
object file with the prevailing definition of that group along with the
name of the symbol that the relocation is referring to. This additional
information should be much more useful to anyone trying to track down
the source of such errors.

To do so, we now map each discarded section to the Kept_section info in
the Layout class, and defer the logic that maps a discarded section to
its counterpart in the kept group. This gives us the information we need
to identify the signature symbol given the discarded section, and the
name of the object file that provided the prevailing (i.e., first)
definition of that group.

gold/
* object.cc (Sized_relobj_file::include_section_group): Store
reference to Kept_section info for discarded comdat sections
regardless of size.  Move size checking to map_to_kept_section.
(Sized_relobj_file::include_linkonce_section): Likewise.
(Sized_relobj_file::map_to_kept_section): Add section name parameter.
Insert size checking logic from above functions.
(Sized_relobj_file::find_kept_section_object): New method.
(Sized_relobj_file::get_symbol_name): New method.
* object.h (Sized_relobj_file::map_to_kept_section): Add section_name
parameter.  Adjust all callers.
(Sized_relobj_file::find_kept_section_object): New method.
(Sized_relobj_file::get_symbol_name): New method.
(Sized_relobj_file::Kept_comdat_section): Replace object and shndx
fields with sh_size, kept_section, symndx, and is_comdat fields.
(Sized_relobj_file::set_kept_comdat_section): Replace kept_object
and kept_shndx parameters with is_comdat, symndx, sh_size, and
kept_section.
(Sized_relobj_file::get_kept_comdat_section): Likewise.
* target-reloc.h (enum Comdat_behavior): Change CB_WARNING to CB_ERROR.
Adjust all references.
(issue_undefined_symbol_error): New function template.
(relocate_section): Pass section name to map_to_kept_section.
Move discarded section code to new function above.
* aarch64.cc (Target_aarch64::scan_reloc_section_for_stubs): Move
declaration for gsym out one level.  Call issue_discarded_error.
* arm.cc (Target_arm::scan_reloc_section_for_stubs): Likewise.
* powerpc.cc (Relocate_comdat_behavior): Change CB_WARNING to CB_ERROR.

gold/ChangeLog
gold/aarch64.cc
gold/arm.cc
gold/object.cc
gold/object.h
gold/powerpc.cc
gold/target-reloc.h

index 97777aa..1c92cf1 100644 (file)
@@ -1,3 +1,33 @@
+2018-04-06  Cary Coutant  <ccoutant@gmail.com>
+
+       * object.cc (Sized_relobj_file::include_section_group): Store
+       reference to Kept_section info for discarded comdat sections
+       regardless of size.  Move size checking to map_to_kept_section.
+       (Sized_relobj_file::include_linkonce_section): Likewise.
+       (Sized_relobj_file::map_to_kept_section): Add section name parameter.
+       Insert size checking logic from above functions.
+       (Sized_relobj_file::find_kept_section_object): New method.
+       (Sized_relobj_file::get_symbol_name): New method.
+       * object.h (Sized_relobj_file::map_to_kept_section): Add section_name
+       parameter.  Adjust all callers.
+       (Sized_relobj_file::find_kept_section_object): New method.
+       (Sized_relobj_file::get_symbol_name): New method.
+       (Sized_relobj_file::Kept_comdat_section): Replace object and shndx
+       fields with sh_size, kept_section, symndx, and is_comdat fields.
+       (Sized_relobj_file::set_kept_comdat_section): Replace kept_object
+       and kept_shndx parameters with is_comdat, symndx, sh_size, and
+       kept_section.
+       (Sized_relobj_file::get_kept_comdat_section): Likewise.
+       * target-reloc.h (enum Comdat_behavior): Change CB_WARNING to CB_ERROR.
+       Adjust all references.
+       (issue_undefined_symbol_error): New function template.
+       (relocate_section): Pass section name to map_to_kept_section.
+       Move discarded section code to new function above.
+       * aarch64.cc (Target_aarch64::scan_reloc_section_for_stubs): Move
+       declaration for gsym out one level.  Call issue_discarded_error.
+       * arm.cc (Target_arm::scan_reloc_section_for_stubs): Likewise.
+       * powerpc.cc (Relocate_comdat_behavior): Change CB_WARNING to CB_ERROR.
+
 2018-04-05  Cary Coutant  <ccoutant@gmail.com>
 
        * target-reloc.h (relocate_section): Add local symbol index or global
index 4dd207f..8de9a26 100644 (file)
@@ -3972,6 +3972,7 @@ Target_aarch64<size, big_endian>::scan_reloc_section_for_stubs(
       const Symbol_value<size> *psymval;
       bool is_defined_in_discarded_section;
       unsigned int shndx;
+      const Symbol* gsym = NULL;
       if (r_sym < local_count)
        {
          sym = NULL;
@@ -4024,7 +4025,6 @@ Target_aarch64<size, big_endian>::scan_reloc_section_for_stubs(
        }
       else
        {
-         const Symbol* gsym;
          gsym = object->global_symbol(r_sym);
          gold_assert(gsym != NULL);
          if (gsym->is_forwarder())
@@ -4065,16 +4065,16 @@ Target_aarch64<size, big_endian>::scan_reloc_section_for_stubs(
       Symbol_value<size> symval2;
       if (is_defined_in_discarded_section)
        {
+         std::string name = object->section_name(relinfo->data_shndx);
+
          if (comdat_behavior == CB_UNDETERMINED)
-           {
-             std::string name = object->section_name(relinfo->data_shndx);
              comdat_behavior = default_comdat_behavior.get(name.c_str());
-           }
+
          if (comdat_behavior == CB_PRETEND)
            {
              bool found;
              typename elfcpp::Elf_types<size>::Elf_Addr value =
-               object->map_to_kept_section(shndx, &found);
+               object->map_to_kept_section(shndx, name, &found);
              if (found)
                symval2.set_output_value(value + psymval->input_value());
              else
@@ -4082,10 +4082,8 @@ Target_aarch64<size, big_endian>::scan_reloc_section_for_stubs(
            }
          else
            {
-             if (comdat_behavior == CB_WARNING)
-               gold_warning_at_location(relinfo, i, offset,
-                                        _("relocation refers to discarded "
-                                          "section"));
+             if (comdat_behavior == CB_ERROR)
+               issue_discarded_error(relinfo, i, offset, r_sym, gsym);
              symval2.set_output_value(0);
            }
          symval2.set_no_output_symtab_entry();
index 8489247..9e18b97 100644 (file)
@@ -12140,6 +12140,7 @@ Target_arm<big_endian>::scan_reloc_section_for_stubs(
       const Symbol_value<32> *psymval;
       bool is_defined_in_discarded_section;
       unsigned int shndx;
+      const Symbol* gsym = NULL;
       if (r_sym < local_count)
        {
          sym = NULL;
@@ -12192,7 +12193,6 @@ Target_arm<big_endian>::scan_reloc_section_for_stubs(
        }
       else
        {
-         const Symbol* gsym;
          gsym = arm_object->global_symbol(r_sym);
          gold_assert(gsym != NULL);
          if (gsym->is_forwarder())
@@ -12233,11 +12233,11 @@ Target_arm<big_endian>::scan_reloc_section_for_stubs(
       Symbol_value<32> symval2;
       if (is_defined_in_discarded_section)
        {
+         std::string name = arm_object->section_name(relinfo->data_shndx);
+
          if (comdat_behavior == CB_UNDETERMINED)
-           {
-             std::string name = arm_object->section_name(relinfo->data_shndx);
              comdat_behavior = default_comdat_behavior.get(name.c_str());
-           }
+
          if (comdat_behavior == CB_PRETEND)
            {
              // FIXME: This case does not work for global symbols.
@@ -12247,7 +12247,7 @@ Target_arm<big_endian>::scan_reloc_section_for_stubs(
              // script.
              bool found;
              typename elfcpp::Elf_types<32>::Elf_Addr value =
-               arm_object->map_to_kept_section(shndx, &found);
+               arm_object->map_to_kept_section(shndx, name, &found);
              if (found)
                symval2.set_output_value(value + psymval->input_value());
              else
@@ -12255,10 +12255,8 @@ Target_arm<big_endian>::scan_reloc_section_for_stubs(
            }
          else
            {
-             if (comdat_behavior == CB_WARNING)
-               gold_warning_at_location(relinfo, i, offset,
-                                        _("relocation refers to discarded "
-                                          "section"));
+             if (comdat_behavior == CB_ERROR)
+               issue_discarded_error(relinfo, i, offset, r_sym, gsym);
              symval2.set_output_value(0);
            }
          symval2.set_no_output_symtab_entry();
index 0dbb0cb..2b58f04 100644 (file)
@@ -1112,42 +1112,17 @@ Sized_relobj_file<size, big_endian>::include_section_group(
        {
          (*omit)[shndx] = true;
 
-         if (is_comdat)
-           {
-             Relobj* kept_object = kept_section->object();
-             if (kept_section->is_comdat())
-               {
-                 // Find the corresponding kept section, and store
-                 // that info in the discarded section table.
-                 unsigned int kept_shndx;
-                 uint64_t kept_size;
-                 if (kept_section->find_comdat_section(mname, &kept_shndx,
-                                                       &kept_size))
-                   {
-                     // We don't keep a mapping for this section if
-                     // it has a different size.  The mapping is only
-                     // used for relocation processing, and we don't
-                     // want to treat the sections as similar if the
-                     // sizes are different.  Checking the section
-                     // size is the approach used by the GNU linker.
-                     if (kept_size == member_shdr.get_sh_size())
-                       this->set_kept_comdat_section(shndx, kept_object,
-                                                     kept_shndx);
-                   }
-               }
-             else
-               {
-                 // The existing section is a linkonce section.  Add
-                 // a mapping if there is exactly one section in the
-                 // group (which is true when COUNT == 2) and if it
-                 // is the same size.
-                 if (count == 2
-                     && (kept_section->linkonce_size()
-                         == member_shdr.get_sh_size()))
-                   this->set_kept_comdat_section(shndx, kept_object,
-                                                 kept_section->shndx());
-               }
-           }
+         // Store a mapping from this section to the Kept_section
+         // information for the group.  This mapping is used for
+         // relocation processing and diagnostics.
+         // If the kept section is a linkonce section, we don't
+         // bother with it unless the comdat group contains just
+         // a single section, making it easy to match up.
+         if (is_comdat
+             && (kept_section->is_comdat() || count == 2))
+           this->set_kept_comdat_section(shndx, true, symndx,
+                                         member_shdr.get_sh_size(),
+                                         kept_section);
        }
     }
 
@@ -1212,10 +1187,8 @@ Sized_relobj_file<size, big_endian>::include_linkonce_section(
       // that the kept section is another linkonce section.  If it is
       // the same size, record it as the section which corresponds to
       // this one.
-      if (kept2->object() != NULL
-         && !kept2->is_comdat()
-         && kept2->linkonce_size() == sh_size)
-       this->set_kept_comdat_section(index, kept2->object(), kept2->shndx());
+      if (kept2->object() != NULL && !kept2->is_comdat())
+       this->set_kept_comdat_section(index, false, 0, sh_size, kept2);
     }
   else if (!include1)
     {
@@ -1226,13 +1199,8 @@ Sized_relobj_file<size, big_endian>::include_linkonce_section(
       // this linkonce section.  We'll handle the simple case where
       // the group has only one member section.  Otherwise, it's not
       // worth the effort.
-      unsigned int kept_shndx;
-      uint64_t kept_size;
-      if (kept1->object() != NULL
-         && kept1->is_comdat()
-         && kept1->find_single_comdat_section(&kept_shndx, &kept_size)
-         && kept_size == sh_size)
-       this->set_kept_comdat_section(index, kept1->object(), kept_shndx);
+      if (kept1->object() != NULL && kept1->is_comdat())
+       this->set_kept_comdat_section(index, false, 0, sh_size, kept1);
     }
   else
     {
@@ -2844,26 +2812,117 @@ template<int size, bool big_endian>
 typename Sized_relobj_file<size, big_endian>::Address
 Sized_relobj_file<size, big_endian>::map_to_kept_section(
     unsigned int shndx,
-    bool* found) const
+    std::string& section_name,
+    bool* pfound) const
 {
-  Relobj* kept_object;
-  unsigned int kept_shndx;
-  if (this->get_kept_comdat_section(shndx, &kept_object, &kept_shndx))
-    {
-      Sized_relobj_file<size, big_endian>* kept_relobj =
-       static_cast<Sized_relobj_file<size, big_endian>*>(kept_object);
-      Output_section* os = kept_relobj->output_section(kept_shndx);
-      Address offset = kept_relobj->get_output_section_offset(kept_shndx);
-      if (os != NULL && offset != invalid_address)
+  Kept_section* kept_section;
+  bool is_comdat;
+  uint64_t sh_size;
+  unsigned int symndx;
+  bool found = false;
+
+  if (this->get_kept_comdat_section(shndx, &is_comdat, &symndx, &sh_size,
+                                   &kept_section))
+    {
+      Relobj* kept_object = kept_section->object();
+      unsigned int kept_shndx = 0;
+      if (!kept_section->is_comdat())
+        {
+         // The kept section is a linkonce section.
+         if (sh_size == kept_section->linkonce_size())
+           found = true;
+        }
+      else
        {
-         *found = true;
-         return os->address() + offset;
+         if (is_comdat)
+           {
+             // Find the corresponding kept section.
+             // Since we're using this mapping for relocation processing,
+             // we don't want to match sections unless they have the same
+             // size.
+             uint64_t kept_size;
+             if (kept_section->find_comdat_section(section_name, &kept_shndx,
+                                                   &kept_size))
+               {
+                 if (sh_size == kept_size)
+                   found = true;
+               }
+           }
+         else
+           {
+             uint64_t kept_size;
+             if (kept_section->find_single_comdat_section(&kept_shndx,
+                                                          &kept_size)
+                 && sh_size == kept_size)
+               found = true;
+           }
+       }
+
+      if (found)
+       {
+         Sized_relobj_file<size, big_endian>* kept_relobj =
+           static_cast<Sized_relobj_file<size, big_endian>*>(kept_object);
+         Output_section* os = kept_relobj->output_section(kept_shndx);
+         Address offset = kept_relobj->get_output_section_offset(kept_shndx);
+         if (os != NULL && offset != invalid_address)
+           {
+             *pfound = true;
+             return os->address() + offset;
+           }
        }
     }
-  *found = false;
+  *pfound = false;
   return 0;
 }
 
+// Look for a kept section corresponding to the given discarded section,
+// and return its object file.
+
+template<int size, bool big_endian>
+Relobj*
+Sized_relobj_file<size, big_endian>::find_kept_section_object(
+    unsigned int shndx, unsigned int *symndx_p) const
+{
+  Kept_section* kept_section;
+  bool is_comdat;
+  uint64_t sh_size;
+  if (this->get_kept_comdat_section(shndx, &is_comdat, symndx_p, &sh_size,
+                                   &kept_section))
+    return kept_section->object();
+  return NULL;
+}
+
+// Return the name of symbol SYMNDX.
+
+template<int size, bool big_endian>
+const char*
+Sized_relobj_file<size, big_endian>::get_symbol_name(unsigned int symndx)
+{
+  if (this->symtab_shndx_ == 0)
+    return NULL;
+
+  section_size_type symbols_size;
+  const unsigned char* symbols = this->section_contents(this->symtab_shndx_,
+                                                       &symbols_size,
+                                                       false);
+
+  unsigned int symbol_names_shndx =
+    this->adjust_shndx(this->section_link(this->symtab_shndx_));
+  section_size_type names_size;
+  const unsigned char* symbol_names_u =
+    this->section_contents(symbol_names_shndx, &names_size, false);
+  const char* symbol_names = reinterpret_cast<const char*>(symbol_names_u);
+
+  const unsigned char* p = symbols + symndx * This::sym_size;
+
+  if (p >= symbols + symbols_size)
+    return NULL;
+
+  elfcpp::Sym<size, big_endian> sym(p);
+
+  return symbol_names + sym.get_st_name();
+}
+
 // Get symbol counts.
 
 template<int size, bool big_endian>
index 9fb9b30..08e5888 100644 (file)
@@ -39,6 +39,7 @@ class General_options;
 class Task;
 class Cref;
 class Layout;
+class Kept_section;
 class Output_data;
 class Output_section;
 class Output_section_data;
@@ -2304,7 +2305,17 @@ class Sized_relobj_file : public Sized_relobj<size, big_endian>
   // and return its output address.  This is used only for relocations in
   // debugging sections.
   Address
-  map_to_kept_section(unsigned int shndx, bool* found) const;
+  map_to_kept_section(unsigned int shndx, std::string& section_name,
+                     bool* found) const;
+
+  // Look for a kept section corresponding to the given discarded section,
+  // and return its object file.
+  Relobj*
+  find_kept_section_object(unsigned int shndx, unsigned int* symndx_p) const;
+
+  // Return the name of symbol SYMNDX.
+  const char*
+  get_symbol_name(unsigned int symndx);
 
   // Compute final local symbol value.  R_SYM is the local symbol index.
   // LV_IN points to a local symbol value containing the input value.
@@ -2598,11 +2609,15 @@ class Sized_relobj_file : public Sized_relobj<size, big_endian>
   // kept section.
   struct Kept_comdat_section
   {
-    Kept_comdat_section(Relobj* a_object, unsigned int a_shndx)
-      : object(a_object), shndx(a_shndx)
+    Kept_comdat_section(uint64_t a_sh_size, Kept_section* a_kept_section,
+                       unsigned int a_symndx, bool a_is_comdat)
+      : sh_size(a_sh_size), kept_section(a_kept_section),
+       symndx (a_symndx), is_comdat(a_is_comdat)
     { }
-    Relobj* object;
-    unsigned int shndx;
+    uint64_t sh_size;          // Section size
+    Kept_section* kept_section;        // Kept section info
+    unsigned int symndx;       // Index of key symbol
+    bool is_comdat;            // True if comdat group, false if linkonce
   };
   typedef std::map<unsigned int, Kept_comdat_section>
       Kept_comdat_section_table;
@@ -2747,25 +2762,29 @@ class Sized_relobj_file : public Sized_relobj<size, big_endian>
   // Record a mapping from discarded section SHNDX to the corresponding
   // kept section.
   void
-  set_kept_comdat_section(unsigned int shndx, Relobj* kept_object,
-                         unsigned int kept_shndx)
+  set_kept_comdat_section(unsigned int shndx, bool is_comdat,
+                         unsigned int symndx, uint64_t sh_size,
+                         Kept_section* kept_section)
   {
-    Kept_comdat_section kept(kept_object, kept_shndx);
+    Kept_comdat_section kept(sh_size, kept_section, symndx, is_comdat);
     this->kept_comdat_sections_.insert(std::make_pair(shndx, kept));
   }
 
   // Find the kept section corresponding to the discarded section
   // SHNDX.  Return true if found.
   bool
-  get_kept_comdat_section(unsigned int shndx, Relobj** kept_object,
-                         unsigned int* kept_shndx) const
+  get_kept_comdat_section(unsigned int shndx, bool* is_comdat,
+                         unsigned int *symndx, uint64_t* sh_size,
+                         Kept_section** kept_section) const
   {
     typename Kept_comdat_section_table::const_iterator p =
       this->kept_comdat_sections_.find(shndx);
     if (p == this->kept_comdat_sections_.end())
       return false;
-    *kept_object = p->second.object;
-    *kept_shndx = p->second.shndx;
+    *is_comdat = p->second.is_comdat;
+    *symndx = p->second.symndx;
+    *sh_size = p->second.sh_size;
+    *kept_section = p->second.kept_section;
     return true;
   }
 
index bddc2b8..a2c9698 100644 (file)
@@ -1351,7 +1351,7 @@ class Target_powerpc : public Sized_target<size, big_endian>
     {
       gold::Default_comdat_behavior default_behavior;
       Comdat_behavior ret = default_behavior.get(name);
-      if (ret == CB_WARNING)
+      if (ret == CB_ERROR)
        {
          if (size == 32
              && (strcmp(name, ".fixup") == 0
index 36032fb..6bde51d 100644 (file)
@@ -120,7 +120,7 @@ enum Comdat_behavior
   CB_UNDETERMINED,   // Not yet determined -- need to look at section name.
   CB_PRETEND,        // Attempt to map to the corresponding kept section.
   CB_IGNORE,         // Ignore the relocation.
-  CB_WARNING         // Print a warning.
+  CB_ERROR           // Print an error.
 };
 
 class Default_comdat_behavior
@@ -138,7 +138,7 @@ class Default_comdat_behavior
     if (strcmp(name, ".eh_frame") == 0
        || strcmp(name, ".gcc_except_table") == 0)
       return CB_IGNORE;
-    return CB_WARNING;
+    return CB_ERROR;
   }
 };
 
@@ -224,6 +224,52 @@ issue_undefined_symbol_error(const Symbol* sym)
   return true;
 }
 
+template<int size, bool big_endian>
+inline void
+issue_discarded_error(
+  const Relocate_info<size, big_endian>* relinfo,
+  size_t shndx,
+  section_offset_type offset,
+  unsigned int r_sym,
+  const Symbol* gsym)
+{
+  Sized_relobj_file<size, big_endian>* object = relinfo->object;
+
+  if (gsym == NULL)
+    {
+      gold_error_at_location(
+         relinfo, shndx, offset,
+         _("relocation refers to local symbol \"%s\" [%u], "
+           "which is defined in a discarded section"),
+         object->get_symbol_name(r_sym), r_sym);
+    }
+  else
+    {
+      gold_error_at_location(
+         relinfo, shndx, offset,
+         _("relocation refers to global symbol \"%s\", "
+           "which is defined in a discarded section"),
+         gsym->demangled_name().c_str());
+    }
+
+  bool is_ordinary;
+  typename elfcpp::Elf_types<size>::Elf_Addr value;
+  unsigned int orig_shndx = object->symbol_section_and_value(r_sym, &value,
+                                                            &is_ordinary);
+  if (orig_shndx != elfcpp::SHN_UNDEF)
+    {
+      unsigned int key_symndx;
+      Relobj* kept_obj = object->find_kept_section_object(orig_shndx,
+                                                         &key_symndx);
+      if (key_symndx != 0)
+       gold_info(_("  section group signature: \"%s\""),
+                 object->get_symbol_name(key_symndx));
+      if (kept_obj != NULL)
+       gold_info(_("  prevailing definition is from %s"),
+                 kept_obj->name().c_str());
+    }
+}
+
 // This function implements the generic part of relocation processing.
 // The template parameter Relocate must be a class type which provides
 // a single function, relocate(), which implements the machine
@@ -360,11 +406,11 @@ relocate_section(
       Symbol_value<size> symval2;
       if (is_defined_in_discarded_section)
        {
+         std::string name = object->section_name(relinfo->data_shndx);
+
          if (comdat_behavior == CB_UNDETERMINED)
-           {
-             std::string name = object->section_name(relinfo->data_shndx);
              comdat_behavior = relocate_comdat_behavior.get(name.c_str());
-           }
+
          if (comdat_behavior == CB_PRETEND)
            {
              // FIXME: This case does not work for global symbols.
@@ -374,7 +420,7 @@ relocate_section(
              // script.
              bool found;
              typename elfcpp::Elf_types<size>::Elf_Addr value =
-               object->map_to_kept_section(shndx, &found);
+                 object->map_to_kept_section(shndx, name, &found);
              if (found)
                symval2.set_output_value(value + psymval->input_value());
              else
@@ -382,25 +428,8 @@ relocate_section(
            }
          else
            {
-             if (comdat_behavior == CB_WARNING)
-               {
-                 if (gsym == NULL)
-                   {
-                     gold_warning_at_location(
-                         relinfo, i, offset,
-                         _("relocation refers to local symbol %d "
-                           "defined in discarded section"),
-                         r_sym);
-                   }
-                 else
-                   {
-                     gold_warning_at_location(
-                         relinfo, i, offset,
-                         _("relocation refers to symbol \"%s\" "
-                           "defined in discarded section"),
-                         gsym->demangled_name().c_str());
-                   }
-               }
+             if (comdat_behavior == CB_ERROR)
+               issue_discarded_error(relinfo, i, offset, r_sym, gsym);
              symval2.set_output_value(0);
            }
          symval2.set_no_output_symtab_entry();