[GOLD] Fix PR0765 [2.27 Regression] gold internal error in fix_errata on aarch64-linux-gnu

Message ID CAEt-8LAPg+VJKSR3dArbX+=PzE0ECoVeLPuC0Ap9JgJzcMhqJw@mail.gmail.com
State New
Headers show
Series
  • [GOLD] Fix PR0765 [2.27 Regression] gold internal error in fix_errata on aarch64-linux-gnu
Related show

Commit Message

Peter Smith Nov. 30, 2017, 11:18 a.m.
Hello,

I've recently investigated
https://sourceware.org/bugzilla/show_bug.cgi?id=20765 which is an
internal fault in gold when the --fix-cortex-a53-843419 is applied.
The bug is reported against 2.27 but still exists on trunk. The test
case gitit is a very large program that uses two stub tables for the
.text output section.

The full details are in the PR, to summarise:
- The internal fault is caused when there is more than one stub table
and the addresses in the second stub table are dependent on the size
of the first stub table. As the erratum stub addresses are only set
once, an internal fault is triggered at relocation time when the
inconsistency is detected.
- The patch is to update the erratum stub addresses on each relaxation pass.

After fixing the internal fault, I noticed that about 1/3 of the
erratum stubs were being created and written to the file, but were
being incorrectly skipped over by
fix_errata_and_relocate_erratum_stubs(). This is a separate problem
that, while more likely to hit large programs, could in theory hit
smaller programs so I'll send a separate message with a fix for that.

Peter

Patch

diff --git a/gold/aarch64.cc b/gold/aarch64.cc
index c9c1b33..74c411d 100644
--- a/gold/aarch64.cc
+++ b/gold/aarch64.cc
@@ -1031,6 +1031,18 @@  public:
   set_erratum_address(AArch64_address addr)
   { this->erratum_address_ = addr; }
 
+  // Later relaxation passes of may alter the recorded erratum and destination
+  // address. Given an up to date output section address of shidx_ in
+  // relobj_ we can derive the erratum_address and destination address.
+  void
+  update_erratum_address(AArch64_address output_section_addr)
+  {
+    const int BPI = AArch64_insn_utilities<big_endian>::BYTES_PER_INSN;
+    AArch64_address updated_addr = output_section_addr + this->sh_offset_;
+    this->set_erratum_address(updated_addr);
+    this->set_destination_address(updated_addr + BPI);
+  }
+
   // Comparator used to group Erratum_stubs in a set by (obj, shndx,
   // sh_offset). We do not include 'type' in the calculation, because there is
   // at most one stub type at (obj, shndx, sh_offset).
@@ -2305,6 +2317,19 @@  AArch64_relobj<size, big_endian>::scan_errata(
       output_address = poris->address();
     }
 
+  // Update the addresses in previously generated erratum stubs. Unlike when
+  // we scan relocations for stubs, if section addresses have changed due to
+  // other relaxations we are unlikely to scan the same erratum instances
+  // again.
+  The_stub_table* stub_table = this->stub_table(shndx);
+  if (stub_table)
+    {
+      std::pair<Erratum_stub_set_iter, Erratum_stub_set_iter>
+	  ipair(stub_table->find_erratum_stubs_for_input_section(this, shndx));
+      for (Erratum_stub_set_iter p = ipair.first;  p != ipair.second; ++p)
+          (*p)->update_erratum_address(output_address);
+    }
+
   section_size_type input_view_size = 0;
   const unsigned char* input_view =
     this->section_contents(shndx, &input_view_size, false);