[PATCH][GOLD] Avoid linker crashing when merge sections have uneven sizes.

[Doug Kwan (關振德)]

Hi Ian,

    This patch fixes a problem exposed by bootstrapping gcc.  libjava
contains objects with unevenly sized merge sections, whose sections
sizes are not multiples of their entsizes.
Output_merge_data::do_add_input_section cannot handle these sections
and returns false but the result is ignored by
Output_section::add_merge_input_section, which updates the merge
section map incorrectly.  Unevenly sized merge sections also cause
another problem Output_merge_data::set_final_data_size because the
method assumes the data member Output_merge_data::p_ is always not
NULL.  However, that condition is possible is an Output_merge_data is
empty due to unevenly sized input sections.

-Doug

2010-04-08  Doug Kwan  <dougkwan@google.com>

        * merge.cc (Output_merge_data::set_final_data_size): Handle empty
        Output_merge_data.
        * output.cc (Output_section::add_merge_input_section): Simplify
        code and return status of Output_merge_base::add_input_section.
        Update merge section map only if Output_merge_base::add_input_section
        returns true.
-------------- next part --------------
Index: gold/merge.cc
===================================================================
RCS file: /cvs/src/src/gold/merge.cc,v
retrieving revision 1.33
diff -u -u -p -r1.33 merge.cc
--- gold/merge.cc	14 Dec 2009 19:53:05 -0000	1.33
+++ gold/merge.cc	8 Apr 2010 23:28:20 -0000
@@ -425,7 +425,10 @@ Output_merge_data::set_final_data_size()
 {
   // Release the memory we don't need.
   this->p_ = static_cast<unsigned char*>(realloc(this->p_, this->len_));
-  gold_assert(this->p_ != NULL);
+  // An Output_merge_data object may be empty and realloc is allowed
+  // to return a NULL pointer in this case.  An Output_merge_data is empty
+  // if all its input sections have sizes that are not multiples of entsize.
+  gold_assert(this->p_ != NULL || this->len_ == 0);
   this->set_data_size(this->len_);
 }
 
Index: gold/output.cc
===================================================================
RCS file: /cvs/src/src/gold/output.cc,v
retrieving revision 1.122
diff -u -u -p -r1.122 output.cc
--- gold/output.cc	6 Mar 2010 02:34:13 -0000	1.122
+++ gold/output.cc	8 Apr 2010 23:28:20 -0000
@@ -2154,58 +2154,55 @@ Output_section::add_merge_input_section(
   gold_assert(this->checkpoint_ == NULL);
 
   // Look up merge sections by required properties.
+  Output_merge_base* pomb;
   Merge_section_properties msp(is_string, entsize, addralign);
   Merge_section_by_properties_map::const_iterator p =
     this->merge_section_by_properties_map_.find(msp);
   if (p != this->merge_section_by_properties_map_.end())
     {
-      Output_merge_base* merge_section = p->second;
-      merge_section->add_input_section(object, shndx);
-      gold_assert(merge_section->is_string() == is_string
-		  && merge_section->entsize() == entsize
-		  && merge_section->addralign() == addralign);
-
-      // Link input section to found merge section.
-      Const_section_id csid(object, shndx);
-      this->merge_section_map_[csid] = merge_section;
-      return true;
+      pomb = p->second;
+      gold_assert(pomb->is_string() == is_string
+		  && pomb->entsize() == entsize
+		  && pomb->addralign() == addralign);
     }
-
-  // We handle the actual constant merging in Output_merge_data or
-  // Output_merge_string_data.
-  Output_merge_base* pomb;
-  if (!is_string)
-    pomb = new Output_merge_data(entsize, addralign);
   else
     {
-      switch (entsize)
+      // Create a new Output_merge_data or Output_merge_string_data.
+      if (!is_string)
+	pomb = new Output_merge_data(entsize, addralign);
+      else
 	{
-        case 1:
-	  pomb = new Output_merge_string<char>(addralign);
-	  break;
-        case 2:
-	  pomb = new Output_merge_string<uint16_t>(addralign);
-	  break;
-        case 4:
-	  pomb = new Output_merge_string<uint32_t>(addralign);
-	  break;
-        default:
-	  return false;
+	  switch (entsize)
+	    {
+	    case 1:
+	      pomb = new Output_merge_string<char>(addralign);
+	      break;
+	    case 2:
+	      pomb = new Output_merge_string<uint16_t>(addralign);
+	      break;
+	    case 4:
+	      pomb = new Output_merge_string<uint32_t>(addralign);
+	      break;
+	    default:
+	      return false;
+	    }
 	}
+      // Add new merge section to this output section and link merge
+      // section properties to new merge section in map.
+      this->add_output_merge_section(pomb, is_string, entsize);
+      this->merge_section_by_properties_map_[msp] = pomb;
     }
 
-  // Add new merge section to this output section and link merge section
-  // properties to new merge section in map.
-  this->add_output_merge_section(pomb, is_string, entsize);
-  this->merge_section_by_properties_map_[msp] = pomb;
-
-  // Add input section to new merge section and link input section to new
-  // merge section in map.
-  pomb->add_input_section(object, shndx);
-  Const_section_id csid(object, shndx);
-  this->merge_section_map_[csid] = pomb;
-
-  return true;
+  if (pomb->add_input_section(object, shndx))
+    {
+      // Add input section to new merge section and link input section to new
+      // merge section in map.
+      Const_section_id csid(object, shndx);
+      this->merge_section_map_[csid] = pomb;
+      return true;
+    }
+  else
+    return false;
 }
 
 // Build a relaxation map to speed up relaxation of existing input sections.