From df2f63a6a0fc19c54e58aab8afe262baf3cb1a3c Mon Sep 17 00:00:00 2001 From: Han Shen Date: Mon, 10 Jul 2017 15:23:05 -0700 Subject: [PATCH] Fixing for PR gold/21491 - Errata workaround can produce broken images. The problem is caused by the fact that gold is relocating the stubs for an entire output section when it processes the relocations for a particular input section that happened to be designated as the stub table "owner". The Relocate_task for that input section may or may not run before the Relocate_task for another input section that contains the code that needs the erratum fix, but doesn't "own" the stub table. If it runs before (or might even race with) that other task, it ends up with a copy of the unrelocated original instruction. In other words - when calling fix_errata() from do_relocate_sections(), gold is going through the list of errata stubs that are associated only with that object. This routine updates the stored original instruction and replaces it in the output view with a branch to the stub. Later, as gold is going through the object file's input sections, it then checks for stub tables "owned" by each input section, and writes out all the stubs from that stub table, regardless of what object file each stub is associated with. Fixed by relocating the erratum stub only after the corresponding errata spot is fixed. That is to have fix_errata() call Stub_table::relocate_erratum_stub() for each stub. gold/ChangeLog 2017-07-06 Han Shen PR gold/21491 * aarch64.cc (Erratum_stub::invalidate_erratum_stub): New method. (Erratum_stub::is_invalidated_erratum_stub): New method. (Stub_table::relocate_reloc_stub): Renamed from "relocate_stub". (Stub_table::relocate_reloc_stubs): Renamed from "relocate_stubs". (Stub_table::relocate_erratum_stub): New method. (AArch64_relobj::fix_errata_and_relocate_erratum_stubs): Renamed from "fix_errata". (Target_aarch64::relocate_reloc_stub): Renamed from "relocate_stub". --- gold/aarch64.cc | 222 ++++++++++++++++++++++++++++-------------------- 1 file changed, 132 insertions(+), 90 deletions(-) diff --git a/gold/aarch64.cc b/gold/aarch64.cc index 11bb48e3b00..696df19dca1 100644 --- a/gold/aarch64.cc +++ b/gold/aarch64.cc @@ -1049,6 +1049,17 @@ public: return this->sh_offset_ < k.sh_offset_; } + void + invalidate_erratum_stub() + { + gold_assert(this->relobj_ != NULL); + this->relobj_ = NULL; + } + + bool + is_invalidated_erratum_stub() + { return this->relobj_ == NULL; } + protected: virtual void do_write(unsigned char*, section_size_type); @@ -1346,7 +1357,8 @@ Reloc_stub::stub_type_for_reloc( return ST_LONG_BRANCH_ABS; } -// A class to hold stubs for the ARM target. +// A class to hold stubs for the ARM target. This contains 2 different types of +// stubs - reloc stubs and erratum stubs. template class Stub_table : public Output_data @@ -1438,14 +1450,18 @@ class Stub_table : public Output_data return (p != this->reloc_stubs_.end()) ? p->second : NULL; } - // Relocate stubs in this stub table. + // Relocate reloc stubs in this stub table. This does not relocate erratum stubs. void - relocate_stubs(const The_relocate_info*, - The_target_aarch64*, - Output_section*, - unsigned char*, - AArch64_address, - section_size_type); + relocate_reloc_stubs(const The_relocate_info*, + The_target_aarch64*, + Output_section*, + unsigned char*, + AArch64_address, + section_size_type); + + // Relocate an erratum stub. + void + relocate_erratum_stub(The_erratum_stub*, unsigned char*); // Update data size at the end of a relaxation pass. Return true if data size // is different from that of the previous relaxation pass. @@ -1485,15 +1501,15 @@ class Stub_table : public Output_data { this->set_data_size(this->current_data_size()); } private: - // Relocate one stub. + // Relocate one reloc stub. void - relocate_stub(The_reloc_stub*, - const The_relocate_info*, - The_target_aarch64*, - Output_section*, - unsigned char*, - AArch64_address, - section_size_type); + relocate_reloc_stub(The_reloc_stub*, + const The_relocate_info*, + The_target_aarch64*, + Output_section*, + unsigned char*, + AArch64_address, + section_size_type); private: // Owner of this stub table. @@ -1593,76 +1609,85 @@ Stub_table::add_reloc_stub( } -// Relocate all stubs in this stub table. +// Relocate an erratum stub. template void Stub_table:: -relocate_stubs(const The_relocate_info* relinfo, - The_target_aarch64* target_aarch64, - Output_section* output_section, - unsigned char* view, - AArch64_address address, - section_size_type view_size) +relocate_erratum_stub(The_erratum_stub* estub, + unsigned char* view) { - // "view_size" is the total size of the stub_table. - gold_assert(address == this->address() && - view_size == static_cast(this->data_size())); - for(Reloc_stub_map_const_iter p = this->reloc_stubs_.begin(); - p != this->reloc_stubs_.end(); ++p) - relocate_stub(p->second, relinfo, target_aarch64, output_section, - view, address, view_size); - // Just for convenience. const int BPI = AArch64_insn_utilities::BYTES_PER_INSN; - // Now 'relocate' erratum stubs. - for(Erratum_stub_set_iter i = this->erratum_stubs_.begin(); - i != this->erratum_stubs_.end(); ++i) + gold_assert(!estub->is_invalidated_erratum_stub()); + AArch64_address stub_address = this->erratum_stub_address(estub); + // The address of "b" in the stub that is to be "relocated". + AArch64_address stub_b_insn_address; + // Branch offset that is to be filled in "b" insn. + int b_offset = 0; + switch (estub->type()) { - AArch64_address stub_address = this->erratum_stub_address(*i); - // The address of "b" in the stub that is to be "relocated". - AArch64_address stub_b_insn_address; - // Branch offset that is to be filled in "b" insn. - int b_offset = 0; - switch ((*i)->type()) - { - case ST_E_843419: - case ST_E_835769: - // The 1st insn of the erratum could be a relocation spot, - // in this case we need to fix it with - // "(*i)->erratum_insn()". - elfcpp::Swap<32, big_endian>::writeval( - view + (stub_address - this->address()), - (*i)->erratum_insn()); - // For the erratum, the 2nd insn is a b-insn to be patched - // (relocated). - stub_b_insn_address = stub_address + 1 * BPI; - b_offset = (*i)->destination_address() - stub_b_insn_address; - AArch64_relocate_functions::construct_b( - view + (stub_b_insn_address - this->address()), - ((unsigned int)(b_offset)) & 0xfffffff); - break; - default: - gold_unreachable(); - break; - } + case ST_E_843419: + case ST_E_835769: + // The 1st insn of the erratum could be a relocation spot, + // in this case we need to fix it with + // "(*i)->erratum_insn()". + elfcpp::Swap<32, big_endian>::writeval( + view + (stub_address - this->address()), + estub->erratum_insn()); + // For the erratum, the 2nd insn is a b-insn to be patched + // (relocated). + stub_b_insn_address = stub_address + 1 * BPI; + b_offset = estub->destination_address() - stub_b_insn_address; + AArch64_relocate_functions::construct_b( + view + (stub_b_insn_address - this->address()), + ((unsigned int)(b_offset)) & 0xfffffff); + break; + default: + gold_unreachable(); + break; } + estub->invalidate_erratum_stub(); } -// Relocate one stub. This is a helper for Stub_table::relocate_stubs(). +// Relocate only reloc stubs in this stub table. This does not relocate erratum +// stubs. template void Stub_table:: -relocate_stub(The_reloc_stub* stub, - const The_relocate_info* relinfo, - The_target_aarch64* target_aarch64, - Output_section* output_section, - unsigned char* view, - AArch64_address address, - section_size_type view_size) +relocate_reloc_stubs(const The_relocate_info* relinfo, + The_target_aarch64* target_aarch64, + Output_section* output_section, + unsigned char* view, + AArch64_address address, + section_size_type view_size) +{ + // "view_size" is the total size of the stub_table. + gold_assert(address == this->address() && + view_size == static_cast(this->data_size())); + for(Reloc_stub_map_const_iter p = this->reloc_stubs_.begin(); + p != this->reloc_stubs_.end(); ++p) + relocate_reloc_stub(p->second, relinfo, target_aarch64, output_section, + view, address, view_size); +} + + +// Relocate one reloc stub. This is a helper for +// Stub_table::relocate_reloc_stubs(). + +template +void +Stub_table:: +relocate_reloc_stub(The_reloc_stub* stub, + const The_relocate_info* relinfo, + The_target_aarch64* target_aarch64, + Output_section* output_section, + unsigned char* view, + AArch64_address address, + section_size_type view_size) { // "offset" is the offset from the beginning of the stub_table. section_size_type offset = stub->offset(); @@ -1670,8 +1695,8 @@ relocate_stub(The_reloc_stub* stub, // "view_size" is the total size of the stub_table. gold_assert(offset + stub_size <= view_size); - target_aarch64->relocate_stub(stub, relinfo, output_section, - view + offset, address + offset, view_size); + target_aarch64->relocate_reloc_stub(stub, relinfo, output_section, + view + offset, address + offset, view_size); } @@ -1829,9 +1854,11 @@ class AArch64_relobj : public Sized_relobj_file Stringpool_template*); private: - // Fix all errata in the object. + // Fix all errata in the object, and for each erratum, relocate corresponding + // erratum stub. void - fix_errata(typename Sized_relobj_file::Views* pviews); + fix_errata_and_relocate_erratum_stubs( + typename Sized_relobj_file::Views* pviews); // Try to fix erratum 843419 in an optimized way. Return true if patch is // applied. @@ -1943,11 +1970,12 @@ AArch64_relobj::do_count_local_symbols( } -// Fix all errata in the object. +// Fix all errata in the object and for each erratum, we relocate the +// corresponding erratum stub (by calling Stub_table::relocate_erratum_stub). template void -AArch64_relobj::fix_errata( +AArch64_relobj::fix_errata_and_relocate_erratum_stubs( typename Sized_relobj_file::Views* pviews) { typedef typename elfcpp::Swap<32,big_endian>::Valtype Insntype; @@ -1988,6 +2016,17 @@ AArch64_relobj::fix_errata( AArch64_relocate_functions::construct_b( pview.view + stub->sh_offset(), b_offset & 0xfffffff); } + + // Erratum fix is done (or skipped), continue to relocate erratum + // stub. Note, when erratum fix is skipped (either because we + // proactively change the code sequence or the code sequence is + // changed by relaxation, etc), we can still safely relocate the + // erratum stub, ignoring the fact the erratum could never be + // executed. + stub_table->relocate_erratum_stub( + stub, pview.view + (stub_table->address() - pview.address)); + + // Next erratum stub. ++p; } } @@ -2086,16 +2125,19 @@ AArch64_relobj::do_relocate_sections( if (parameters->options().relocatable()) return; + // This part only relocates erratum stubs that belong to input sections of this + // object file. if (parameters->options().fix_cortex_a53_843419() || parameters->options().fix_cortex_a53_835769()) - this->fix_errata(pviews); + this->fix_errata_and_relocate_erratum_stubs(pviews); Relocate_info relinfo; relinfo.symtab = symtab; relinfo.layout = layout; relinfo.object = this; - // Relocate stub tables. + // This part relocates all reloc stubs that are contained in stub_tables of + // this object file. unsigned int shnum = this->shnum(); The_target_aarch64* target = The_target_aarch64::current_target(); @@ -2124,8 +2166,8 @@ AArch64_relobj::do_relocate_sections( 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); + stub_table->relocate_reloc_stubs(&relinfo, target, os, view, address, + view_size); } } } @@ -3016,11 +3058,11 @@ class Target_aarch64 : public Sized_target Address view_address, section_size_type); - // Relocate a single stub. + // Relocate a single reloc stub. void - relocate_stub(The_reloc_stub*, const Relocate_info*, - Output_section*, unsigned char*, Address, - section_size_type); + relocate_reloc_stub(The_reloc_stub*, const Relocate_info*, + Output_section*, unsigned char*, Address, + section_size_type); // Get the default AArch64 target. static This* @@ -4035,16 +4077,16 @@ Target_aarch64::scan_section_for_stubs( } -// Relocate a single stub. +// Relocate a single reloc stub. template void Target_aarch64:: -relocate_stub(The_reloc_stub* stub, - const The_relocate_info*, - Output_section*, - unsigned char* view, - Address address, - section_size_type) +relocate_reloc_stub(The_reloc_stub* stub, + const The_relocate_info*, + Output_section*, + unsigned char* view, + Address address, + section_size_type) { typedef AArch64_relocate_functions The_reloc_functions; typedef typename The_reloc_functions::Status The_reloc_functions_status; -- 2.30.2