[PATCH][GOLD] Fix dangling pointer bug due to premature unlock.

[Doug Kwan (關振德)]

Hi

   This fixes a bug in which an object is released too early, causing
a pointer to point to unmapped memory.  My fix is to move the locking
code to the caller of get_section_contents() and replace the original
locking code with a check.  This has been tested on x86_64.

-Doug


2011-01-25  Doug Kwan  <dougkwan@google.com>

        * gold/icf.cc (get_section_contents): Instead of locking object in
        two places, ask caller to lock it before calling. Add an assert
        to check that object is locked in the first iteration.
        (match_sections): Lock object before calling get_section_contents()
        in the first iteration.
-------------- next part --------------
Index: gold/icf.cc
===================================================================
RCS file: /cvs/src/src/gold/icf.cc,v
retrieving revision 1.17
diff -u -u -p -r1.17 icf.cc
--- gold/icf.cc	14 Dec 2010 19:03:30 -0000	1.17
+++ gold/icf.cc	25 Jan 2011 08:36:15 -0000
@@ -221,6 +221,8 @@ preprocess_for_unique_sections(const std
 // Parameters  :
 // FIRST_ITERATION    : true if it is the first invocation.
 // SECN               : Section for which contents are desired.
+//			For the first iteration, caller needs to
+//			lock the section's object.
 // SECTION_NUM        : Unique section number of this section.
 // NUM_TRACKED_RELOCS : Vector reference to store the number of relocs
 //                      to ICF sections.
@@ -240,13 +242,13 @@ get_section_contents(bool first_iteratio
   section_size_type plen;
   const unsigned char* contents = NULL;
 
+  // We access the section's object in the first iteration and that requires
+  // locking.  Instead of locking and unlocking in two places here, we require
+  // a caller to lock the object before calling this for the first iteration.
+  gold_assert(!first_iteration || secn.first->is_locked());
+
   if (first_iteration)
     {
-      // Lock the object so we can read from it.  This is only called
-      // single-threaded from queue_middle_tasks, so it is OK to lock.
-      // Unfortunately we have no way to pass in a Task token.
-      const Task* dummy_task = reinterpret_cast<const Task*>(-1);
-      Task_lock_obj<Object> tl(dummy_task, secn.first);
       contents = secn.first->section_contents(secn.second,
                                               &plen,
                                               false);
@@ -373,12 +375,6 @@ get_section_contents(bool first_iteratio
               if (!first_iteration)
                 continue;
 
-              // Lock the object so we can read from it.  This is only called
-              // single-threaded from queue_middle_tasks, so it is OK to lock.
-              // Unfortunately we have no way to pass in a Task token.
-              const Task* dummy_task = reinterpret_cast<const Task*>(-1);
-              Task_lock_obj<Object> tl(dummy_task, it_v->first);
-
               uint64_t secn_flags = (it_v->first)->section_flags(it_v->second);
               // This reloc points to a merge section.  Hash the
               // contents of this section.
@@ -591,6 +587,12 @@ match_sections(unsigned int iteration_nu
       if (iteration_num == 1)
         {
           unsigned int num_relocs = 0;
+
+	  // Lock the object so we can read from it.  This is only called
+	  // single-threaded from queue_middle_tasks, so it is OK to lock.
+	  // Unfortunately we have no way to pass in a Task token.
+	  const Task* dummy_task = reinterpret_cast<const Task*>(-1);
+	  Task_lock_obj<Object> tl(dummy_task, secn.first);
           this_secn_contents = get_section_contents(true, secn, i, &num_relocs,
                                                     symtab, (*kept_section_id),
                                                     section_contents);