[gold][aarch64]: Fixing gold pr/21491 - Errata workaround can produce broken images.

["Han Shen via binutils" <binutils at sourceware dot org>]

Hi Cary, this patch fixes gold pr/21491 - Errata workaround can
produce broken images.

The root cause of the bug is that in the origin implementation,
relocate_sections and relocate_stubs phases are interleaved, in some
cases, relocate_stubs copy instructions before they are relocated in
relocate_sections phase, resulting stale and invalid insns in the
final stub table.

This patch fixes the problem by adding a new task "Relocstub_task"
that runs after "Relocate_task", so relocate_stubs always pick the
relocated insn.

Tested: test case in bug passed and from Android toolchain team -
"built all platform source tree for all Android architectures; pushed
Angler build onto device & tested device."

gold/ChangeLog:

2017-07-06  Han Shen  <shenhan@google.com>

* aarch64.cc (do_relocate_stub_tables): New method.
(do_relocate_sections): Removed relocate stub code.
* gold.cc (queue_final_tasks): Queue Relocstub_tasks.
* object.h (Relobj::relocate_stub_tables): New method.
(Relobj::clear_views): New method.
(Relobj::do_relocate_stub_tables): New method.
(Sized_relobj_file::do_relocate_stub_tables): New method.
(Sized_relobj_file::create_views): New method.
(Sized_relobj_file::get_views): New method.
(Sized_relobj_file::clear_views): New method.
* reloc.h (Relocstub_task): New task class.
(Relocate_task::symbol_table): New method.
(Relocate_task::layout): New method.
(Relocate_task::object): New method.
* reloc.cc (Relocstub_task): New task class definition.
(Sized_relobj_file::do_relocate): Removed relocate_stub code.
(Sized_relobj_file::do_relocate_stub_tables): New specialized functions.
(Sized_relobj_file::do_clear_views): New specialized functions.

Ok for trunk?

diff --git a/gold/aarch64.cc b/gold/aarch64.cc
index 33f7abe6b7..8a79c29d3e 100644
--- a/gold/aarch64.cc
+++ b/gold/aarch64.cc
@@ -1781,6 +1781,56 @@ class AArch64_relobj : public Sized_relobj_file<size, big_endian>
     this->set_relocs_must_follow_section_writes();
   }
 
+  // This action is executed during Relocstub_task phase.
+  void do_relocate_stub_tables(const Symbol_table* symtab, const Layout* layout)
+  {
+    unsigned int shnum = this->shnum();
+    if (shnum == 0)
+      return;
+    const unsigned char* pshdrs =
+        this->get_view(this->elf_file()->shoff(),
+                       shnum * elfcpp::Elf_sizes<size>::shdr_size,
+                       true,
+                       true);
+    typename Sized_relobj_file<size, big_endian>::Views* pviews = this->get_views();
+
+    Relocate_info<size, big_endian> relinfo;
+    relinfo.symtab = symtab;
+    relinfo.layout = layout;
+    relinfo.object = this;
+
+    // Relocate stub tables.
+    The_target_aarch64* target = The_target_aarch64::current_target();
+
+    for (unsigned int i = 1; i < shnum; ++i) {
+      The_aarch64_input_section* aarch64_input_section =
+          target->find_aarch64_input_section(this, i);
+      if (aarch64_input_section != NULL &&
+          aarch64_input_section->is_stub_table_owner() &&
+          !aarch64_input_section->stub_table()->empty()) {
+        Output_section* os = this->output_section(i);
+        gold_assert(os != NULL);
+
+        relinfo.reloc_shndx = elfcpp::SHN_UNDEF;
+        relinfo.reloc_shdr = NULL;
+        relinfo.data_shndx = i;
+        relinfo.data_shdr = pshdrs + i * elfcpp::Elf_sizes<size>::shdr_size;
+
+        typename Sized_relobj_file<size, big_endian>::View_size& view_struct =
+            (*pviews)[i];
+        gold_assert(view_struct.view != NULL);
+
+        The_stub_table* stub_table = aarch64_input_section->stub_table();
+        off_t offset = stub_table->address() - view_struct.address;
+        unsigned char* view = view_struct.view + offset;
+        AArch64_address address = stub_table->address();
+        section_size_type view_size = stub_table->data_size();
+        stub_table->relocate_stubs(&relinfo, target, os, view, address,
+                                   view_size);
+      }
+    }
+  }
+
   // Structure for mapping symbol position.
   struct Mapping_symbol_position
   {
@@ -2089,45 +2139,6 @@ AArch64_relobj<size, big_endian>::do_relocate_sections(
   if (parameters->options().fix_cortex_a53_843419()
       || parameters->options().fix_cortex_a53_835769())
     this->fix_errata(pviews);
-
-  Relocate_info<size, big_endian> relinfo;
-  relinfo.symtab = symtab;
-  relinfo.layout = layout;
-  relinfo.object = this;
-
-  // Relocate stub tables.
-  unsigned int shnum = this->shnum();
-  The_target_aarch64* target = The_target_aarch64::current_target();
-
-  for (unsigned int i = 1; i < shnum; ++i)
-    {
-      The_aarch64_input_section* aarch64_input_section =
-	  target->find_aarch64_input_section(this, i);
-      if (aarch64_input_section != NULL
-	  && aarch64_input_section->is_stub_table_owner()
-	  && !aarch64_input_section->stub_table()->empty())
-	{
-	  Output_section* os = this->output_section(i);
-	  gold_assert(os != NULL);
-
-	  relinfo.reloc_shndx = elfcpp::SHN_UNDEF;
-	  relinfo.reloc_shdr = NULL;
-	  relinfo.data_shndx = i;
-	  relinfo.data_shdr = pshdrs + i * elfcpp::Elf_sizes<size>::shdr_size;
-
-	  typename Sized_relobj_file<size, big_endian>::View_size&
-	      view_struct = (*pviews)[i];
-	  gold_assert(view_struct.view != NULL);
-
-	  The_stub_table* stub_table = aarch64_input_section->stub_table();
-	  off_t offset = stub_table->address() - view_struct.address;
-	  unsigned char* view = view_struct.view + offset;
-	  AArch64_address address = stub_table->address();
-	  section_size_type view_size = stub_table->data_size();
-	  stub_table->relocate_stubs(&relinfo, target, os, view, address,
-				     view_size);
-	}
-    }
 }
 
 
diff --git a/gold/gold.cc b/gold/gold.cc
index a76d155f94..8f5a61f3c0 100644
--- a/gold/gold.cc
+++ b/gold/gold.cc
@@ -805,6 +805,10 @@ queue_final_tasks(const General_options& options,
 
   bool any_postprocessing_sections = layout->any_postprocessing_sections();
 
+  bool need_relocstub_tasks = !parameters->options().relocatable() &&
+      (parameters->options().fix_cortex_a53_843419()
+       || parameters->options().fix_cortex_a53_835769());
+
   // Use a blocker to wait until all the input sections have been
   // written out.
   Task_token* input_sections_blocker = NULL;
@@ -814,6 +818,9 @@ queue_final_tasks(const General_options& options,
       // Write_symbols_task, Relocate_tasks.
       input_sections_blocker->add_blocker();
       input_sections_blocker->add_blockers(input_objects->number_of_relobjs());
+      // Blockers for n Relocstub_tasks.
+      if (need_relocstub_tasks)
+        input_sections_blocker->add_blockers(input_objects->number_of_relobjs());
     }
 
   // Use a blocker to block any objects which have to wait for the
@@ -827,6 +834,9 @@ queue_final_tasks(const General_options& options,
   // Relocate_tasks.
   final_blocker->add_blockers(3);
   final_blocker->add_blockers(input_objects->number_of_relobjs());
+  // Blockers for n Relocstub_tasks.
+  if (need_relocstub_tasks)
+    final_blocker->add_blockers(input_objects->number_of_relobjs());
   if (!any_postprocessing_sections)
     final_blocker->add_blocker();
 
@@ -855,7 +865,17 @@ queue_final_tasks(const General_options& options,
     workqueue->queue(new Relocate_task(symtab, layout, *p, of,
 				       input_sections_blocker,
 				       output_sections_blocker,
-				       final_blocker));
+				       final_blocker,
+                                       need_relocstub_tasks));
+
+  // Queue a task for each input object to relocate stub tables.
+  if (need_relocstub_tasks)
+    for (Input_objects::Relobj_iterator p = input_objects->relobj_begin();
+         p != input_objects->relobj_end(); ++p)
+      workqueue->queue(new Relocstub_task(symtab, layout, *p, of,
+                                          input_sections_blocker,
+                                          output_sections_blocker,
+                                          final_blocker));
 
   // Queue a task to write out the output sections which depend on
   // input sections.  If there are any sections which require
diff --git a/gold/object.h b/gold/object.h
index 508e79cb3c..6a091f17d4 100644
--- a/gold/object.h
+++ b/gold/object.h
@@ -1279,6 +1279,10 @@ class Relobj : public Object
   relocate(const Symbol_table* symtab, const Layout* layout, Output_file* of)
   { return this->do_relocate(symtab, layout, of); }
 
+  void
+  relocate_stub_tables(const Symbol_table* symtab, const Layout* layout)
+  { this->do_relocate_stub_tables(symtab, layout); }
+
   // Return whether an input section is being included in the link.
   bool
   is_section_included(unsigned int shndx) const
@@ -1375,6 +1379,10 @@ class Relobj : public Object
   is_big_endian() const
   { return this->do_is_big_endian(); }
 
+  virtual void
+  clear_views()
+  {}
+
  protected:
   // The output section to be used for each input section, indexed by
   // the input section number.  The output section is NULL if the
@@ -1457,6 +1465,10 @@ class Relobj : public Object
   virtual void
   do_relocate(const Symbol_table* symtab, const Layout*, Output_file* of) = 0;
 
+  virtual void
+  do_relocate_stub_tables(const Symbol_table*, const Layout*)
+  {}
+
   // Set the offset of a section--implemented by child class.
   virtual void
   do_set_section_offset(unsigned int shndx, uint64_t off) = 0;
@@ -2326,6 +2338,15 @@ class Sized_relobj_file : public Sized_relobj<size, big_endian>
   bool is_deferred_layout() const
   { return this->is_deferred_layout_; }
 
+  // Discard output_views_ created in create_views().
+  void
+  clear_views()
+  {
+    gold_assert(this->output_views_);
+    delete this->output_views_;
+    this->output_views_ = NULL;
+  }
+
  protected:
   typedef typename Sized_relobj<size, big_endian>::Output_sections
       Output_sections;
@@ -2432,6 +2453,10 @@ class Sized_relobj_file : public Sized_relobj<size, big_endian>
   void
   do_relocate(const Symbol_table* symtab, const Layout*, Output_file* of);
 
+  virtual void
+  do_relocate_stub_tables(const Symbol_table*, const Layout*)
+  {}
+
   // Get the size of a section.
   uint64_t
   do_section_size(unsigned int shndx)
@@ -2531,7 +2556,7 @@ class Sized_relobj_file : public Sized_relobj<size, big_endian>
   elfcpp::Elf_file<size, big_endian, Object>*
   elf_file()
   { return &this->elf_file_; }
-  
+
   // Allow a child class to access the local values.
   Local_values*
   local_values()
@@ -2584,6 +2609,22 @@ class Sized_relobj_file : public Sized_relobj<size, big_endian>
   unsigned char*
   do_get_output_view(unsigned int, section_size_type*) const;
 
+  Views*
+  create_views()
+  {
+    gold_assert(this->output_views_ == NULL);
+    this->output_views_ = new Views();
+    return this->output_views_;
+  }
+
+  // Allow child access to output_views_.
+  Views*
+  get_views() const
+  {
+    gold_assert(this->output_views_);
+    return this->output_views_;
+  }
+
  private:
   // For convenience.
   typedef Sized_relobj_file<size, big_endian> This;
@@ -2853,7 +2894,7 @@ class Sized_relobj_file : public Sized_relobj<size, big_endian>
   // The list of relocation sections whose layout was deferred.
   std::vector<Deferred_layout> deferred_layout_relocs_;
   // Pointer to the list of output views; valid only during do_relocate().
-  const Views* output_views_;
+  Views* output_views_;
 };
 
 // A class to manage the list of all objects.
diff --git a/gold/reloc.cc b/gold/reloc.cc
index 26d84d0aa8..fcf665b976 100644
--- a/gold/reloc.cc
+++ b/gold/reloc.cc
@@ -238,11 +238,15 @@ Relocate_task::run(Workqueue*)
 {
   this->object_->relocate(this->symtab_, this->layout_, this->of_);
 
-  // This is normally the last thing we will do with an object, so
-  // uncache all views.
-  this->object_->clear_view_cache_marks();
-
-  this->object_->release();
+  // When defer_object_cleanup_ is true, we do cleanup work at the end of
+  // Relocstub_task.
+  if (!this->defer_object_cleanup_) {
+    // This is normally the last thing we will do with an object, so
+    // uncache all views.
+    this->object_->clear_views();
+    this->object_->clear_view_cache_marks();
+    this->object_->release();
+  }
 }
 
 // Return a debugging name for the task.
@@ -253,6 +257,25 @@ Relocate_task::get_name() const
   return "Relocate_task " + this->object_->name();
 }
 
+std::string
+Relocstub_task::get_name() const
+{
+  return "Relocstub_task " + this->object()->name();
+}
+
+void
+Relocstub_task::run(Workqueue*)
+{
+  this->object()->relocate_stub_tables(this->symbol_table(), this->layout());
+
+  // This is normally the last thing we will do with an object, so
+  // uncache all views.
+  this->object()->clear_views();
+  this->object()->clear_view_cache_marks();
+  this->object()->release();
+
+}
+
 // Read the relocs and local symbols from the object file and store
 // the information in RD.
 
@@ -595,7 +618,9 @@ Sized_relobj_file<size, big_endian>::do_relocate(const Symbol_table* symtab,
 					       shnum * This::shdr_size,
 					       true, true);
 
-  Views views;
+  // "Views" created here is either discarded at the end of "Relocate_task" or
+  // "Relocstub_task", depending whether the latter ones are needed.
+  Views& views = *(this->create_views());
   views.resize(shnum);
 
   // Make two passes over the sections.  The first one copies the
@@ -608,24 +633,6 @@ Sized_relobj_file<size, big_endian>::do_relocate(const Symbol_table* symtab,
   // input offsets to output addresses.
   this->initialize_input_to_output_maps();
 
-  // Make the views available through get_output_view() for the duration
-  // of this routine.  This RAII class will reset output_views_ to NULL
-  // when the views go out of scope.
-  struct Set_output_views
-  {
-    Set_output_views(const Views** ppviews, const Views* pviews)
-    {
-      ppviews_ = ppviews;
-      *ppviews = pviews;
-    }
-
-    ~Set_output_views()
-    { *ppviews_ = NULL; }
-
-    const Views** ppviews_;
-  };
-  Set_output_views set_output_views(&this->output_views_, &views);
-
   // Apply relocations.
 
   this->relocate_sections(symtab, layout, pshdrs, of, &views);
@@ -1704,6 +1711,15 @@ void
 Sized_relobj_file<32, false>::do_relocate(const Symbol_table* symtab,
 					  const Layout* layout,
 					  Output_file* of);
+
+template
+void
+Sized_relobj_file<32, false>::do_relocate_stub_tables(const Symbol_table* symtab,
+                                                      const Layout* layout);
+
+template
+void
+Sized_relobj_file<32, false>::clear_views();
 #endif
 
 #ifdef HAVE_TARGET_32_BIG
@@ -1712,6 +1728,15 @@ void
 Sized_relobj_file<32, true>::do_relocate(const Symbol_table* symtab,
 					 const Layout* layout,
 					 Output_file* of);
+
+template
+void
+Sized_relobj_file<32, true>::do_relocate_stub_tables(const Symbol_table* symtab,
+                                                     const Layout* layout);
+
+template
+void
+Sized_relobj_file<32, true>::clear_views();
 #endif
 
 #ifdef HAVE_TARGET_64_LITTLE
@@ -1720,6 +1745,15 @@ void
 Sized_relobj_file<64, false>::do_relocate(const Symbol_table* symtab,
 					  const Layout* layout,
 					  Output_file* of);
+
+template
+void
+Sized_relobj_file<64, false>::do_relocate_stub_tables(const Symbol_table* symtab,
+                                                      const Layout* layout);
+
+template
+void
+Sized_relobj_file<64, false>::clear_views();
 #endif
 
 #ifdef HAVE_TARGET_64_BIG
@@ -1728,6 +1762,15 @@ void
 Sized_relobj_file<64, true>::do_relocate(const Symbol_table* symtab,
 					 const Layout* layout,
 					 Output_file* of);
+
+template
+void
+Sized_relobj_file<64, true>::do_relocate_stub_tables(const Symbol_table* symtab,
+                                                     const Layout* layout);
+
+template
+void
+Sized_relobj_file<64, true>::clear_views();
 #endif
 
 #ifdef HAVE_TARGET_32_LITTLE
diff --git a/gold/reloc.h b/gold/reloc.h
index 7ec247411d..60991b0299 100644
--- a/gold/reloc.h
+++ b/gold/reloc.h
@@ -183,27 +183,38 @@ class Relocate_task : public Task
   Relocate_task(const Symbol_table* symtab, const Layout* layout,
 		Relobj* object, Output_file* of,
 		Task_token* input_sections_blocker,
-		Task_token* output_sections_blocker, Task_token* final_blocker)
+		Task_token* output_sections_blocker, Task_token* final_blocker,
+                bool defer_object_cleanup)
     : symtab_(symtab), layout_(layout), object_(object), of_(of),
       input_sections_blocker_(input_sections_blocker),
       output_sections_blocker_(output_sections_blocker),
-      final_blocker_(final_blocker)
+      final_blocker_(final_blocker),
+      defer_object_cleanup_(defer_object_cleanup)
   { }
 
   // The standard Task methods.
 
-  Task_token*
+  virtual Task_token*
   is_runnable();
 
-  void
+  virtual void
   locks(Task_locker*);
 
-  void
+  virtual void
   run(Workqueue*);
 
-  std::string
+  virtual std::string
   get_name() const;
 
+  const Symbol_table* symbol_table() const
+  { return this->symtab_; }
+
+  const Layout* layout() const
+  { return this->layout(); }
+
+  Relobj* object() const
+  { return this->object_; }
+
  private:
   const Symbol_table* symtab_;
   const Layout* layout_;
@@ -212,6 +223,39 @@ class Relocate_task : public Task
   Task_token* input_sections_blocker_;
   Task_token* output_sections_blocker_;
   Task_token* final_blocker_;
+  // When this is true, do not do object cleanup, because later, Relocstub_task
+  // will do the chores. Only use this in Relocate_task.  Not accessible in
+  // subclasses.
+  bool defer_object_cleanup_;
+};
+
+// This task relocates stub_tables. It does similar things as a
+// Relocate_task. The reason why we have this is that we have to wait for all
+// instructions to be relocated before we can copy the instruction into
+// stub_tables. More details here - pr/21491.
+
+class Relocstub_task : public Relocate_task
+{
+ public:
+
+  Relocstub_task(const Symbol_table* symtab,
+                 const Layout* layout,
+                 Relobj* object,
+                 Output_file* of,
+                 Task_token* input_sections_blocker,
+                 Task_token* output_sections_blocker,
+                 Task_token* final_blocker)
+      : Relocate_task(symtab, layout, object, of,
+                      input_sections_blocker,
+                      output_sections_blocker,
+                      final_blocker, true)
+  {}
+
+  virtual void
+  run(Workqueue*);
+
+  virtual std::string
+  get_name() const;
 };
 
 // During a relocatable link, this class records how relocations