This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [gold][aarch64] Patch for erratum-843419 (1 of 2 - report erratum occurrences)
- From: Han Shen <shenhan at google dot com>
- To: ramrad01 at arm dot com
- Cc: Cary Coutant <ccoutant at gmail dot com>, binutils <binutils at sourceware dot org>, Jing Yu <jingyu at google dot com>, Doug Kwan <dougkwan at google dot com>, Luis Lozano <llozano at google dot com>, Bhaskar <bjanakiraman at google dot com>, Andrew Hsieh <andrewhsieh at google dot com>
- Date: Mon, 6 Jul 2015 15:01:33 -0700
- Subject: Re: [gold][aarch64] Patch for erratum-843419 (1 of 2 - report erratum occurrences)
- Authentication-results: sourceware.org; auth=none
- References: <CACkGtrhuUi7DtTQ89khOARvV3SMgyHdSea8EAMqioWT5Kh4R9g at mail dot gmail dot com> <CAJA7tRbOe10=t=o-47yEViY4C1Juv6SwG8_yYNH0mwGHv+-=5A at mail dot gmail dot com>
Hi Ramana, thanks for the review and point this out. I'll prepare a CL soon.
Thanks,
Han
On Mon, Jul 6, 2015 at 2:00 PM, Ramana Radhakrishnan
<ramana.gcc@googlemail.com> wrote:
> On Tue, Apr 14, 2015 at 8:49 PM, HÃn ShÄn (ææ) <shenhan@google.com> wrote:
>> Hi, here is the first patch to address cortex-a53 erratum-843419. It implemented
>> scanning the binary and reporting occurrences to users when '--fix-cortex-a53'
>> is turned on. With this, gold users will be able to see if or not there are such
>> erratum occurrences in the output binary. Also included in the CL is
>> reading/recording mapping symbols, which is needed during scan.
>
> <quite a lot snipped>
>
>> +// Override to record mapping symbol information.
>> +template<int size, bool big_endian>
>> +void
>> +AArch64_relobj<size, big_endian>::do_count_local_symbols(
>> + Stringpool_template<char>* pool, Stringpool_template<char>* dynpool)
>> +{
>
> <more snipped>
>
>> + // Skip the first dummy symbol.
>> + psyms += sym_size;
>> + typename Sized_relobj_file<size, big_endian>::Local_values*
>> + plocal_values = this->local_values();
>> + for (unsigned int i = 1; i < loccount; ++i, psyms += sym_size)
>> + {
>> + elfcpp::Sym<size, big_endian> sym(psyms);
>> + Symbol_value<size>& lv((*plocal_values)[i]);
>> + AArch64_address input_value = lv.input_value();
>> +
>> + // Check to see if this is a mapping symbol.
>> + const char* sym_name = pnames + sym.get_st_name();
>> + if (sym_name[0] == '$' && (sym_name[1] == 'x' || sym_name[1] == 'd')
>> + && sym_name[2] == '\0')
>> + {
>
> It is worth noting that the AArch64 ELF ABI allows 2 kinds of mapping
> symbols. See section 4.5.4 of
> http://infocenter.arm.com/help/topic/com.arm.doc.ihi0056b/IHI0056B_aaelf64.pdf
> which describes the longer form of mapping symbols.
>
> While GAS does not produce such mapping symbols, IIRC llvm's
> integrated assembler produces the longer form of such mapping symbols.
> Aha ! look in http://llvm.org/docs/doxygen/html/AArch64ELFStreamer_8cpp_source.html
> : EmitMappingSymbol for more.
>
> Thus I would drop the check for sym_name[2] == '\0'
>
> regards
> Ramana
>
>
>> + bool is_ordinary;
>> + unsigned int input_shndx =
>> + this->adjust_sym_shndx(i, sym.get_st_shndx(), &is_ordinary);
>> + gold_assert(is_ordinary);
>> +
>> + Mapping_symbol_position msp(input_shndx, input_value);
>> + // Insert mapping_symbol_info into map whose ordering is defined by
>> + // (shndx, offset_within_section).
>> + this->mapping_symbol_info_[msp] = sym_name[1];
>> + }
>> + }
>> +}
>> +
>> +
>> // Relocate sections.
>>
>> template<int size, bool big_endian>
>> void
>> AArch64_relobj<size, big_endian>::do_relocate_sections(
>> @@ -1103,10 +1491,75 @@ AArch64_relobj<size,
>> big_endian>::section_needs_reloc_stub_scanning(
>> return this->text_section_is_scannable(text_shdr, text_shndx,
>> out_sections[text_shndx], symtab);
>> }
>>
>>
>> +// Scan section SHNDX for erratum 843419.
>> +
>> +template<int size, bool big_endian>
>> +void
>> +AArch64_relobj<size, big_endian>::scan_erratum_843419(
>> + unsigned int shndx, const elfcpp::Shdr<size, big_endian>& shdr,
>> + Output_section* os, const Symbol_table* symtab,
>> + The_target_aarch64* target)
>> +{
>> + if (shdr.get_sh_size() == 0
>> + || (shdr.get_sh_flags() &
>> + (elfcpp::SHF_ALLOC | elfcpp::SHF_EXECINSTR)) == 0
>> + || shdr.get_sh_type() != elfcpp::SHT_PROGBITS)
>> + return;
>> +
>> + if (!os || symtab->is_section_folded(this, shndx)) return;
>> +
>> + AArch64_address output_offset = this->get_output_section_offset(shndx);
>> + AArch64_address output_address;
>> + if (output_offset != invalid_address)
>> + output_address = os->address() + output_offset;
>> + else
>> + {
>> + const Output_relaxed_input_section* poris =
>> + os->find_relaxed_input_section(this, shndx);
>> + if (!poris) return;
>> + output_address = poris->address();
>> + }
>> +
>> + section_size_type input_view_size = 0;
>> + const unsigned char* input_view =
>> + this->section_contents(shndx, &input_view_size, false);
>> +
>> + Mapping_symbol_position section_start(shndx, 0);
>> + // Find the first mapping symbol record within section shndx.
>> + typename Mapping_symbol_info::const_iterator p =
>> + this->mapping_symbol_info_.lower_bound(section_start);
>> + if (p == this->mapping_symbol_info_.end() || p->first.shndx_ != shndx)
>> + gold_warning(_("cannot scan executable section %u of %s for Cortex-A53 "
>> + "erratum because it has no mapping symbols."),
>> + shndx, this->name().c_str());
>> + while (p != this->mapping_symbol_info_.end() &&
>> + p->first.shndx_ == shndx)
>> + {
>> + typename Mapping_symbol_info::const_iterator next =
>> + this->mapping_symbol_info_.upper_bound(p->first);
>> + if (p->second == 'x')
>> + {
>> + section_size_type span_start =
>> + convert_to_section_size_type(p->first.offset_);
>> + section_size_type span_end;
>> + if (next != this->mapping_symbol_info_.end()
>> + && next->first.shndx_ == shndx)
>> + span_end = convert_to_section_size_type(next->first.offset_);
>> + else
>> + span_end = convert_to_section_size_type(shdr.get_sh_size());
>> + target->scan_erratum_843419_span(
>> + this, shndx, span_start, span_end,
>> + const_cast<unsigned char*>(input_view), output_address);
>> + }
>> + p = next;
>> + }
>> +}
>> +
>> +
>> // Scan relocations for stub generation.
>>
>> template<int size, bool big_endian>
>> void
>> AArch64_relobj<size, big_endian>::scan_sections_for_stubs(
>> @@ -1136,10 +1589,12 @@ AArch64_relobj<size,
>> big_endian>::scan_sections_for_stubs(
>> // Do relocation stubs scanning.
>> const unsigned char* p = pshdrs + shdr_size;
>> for (unsigned int i = 1; i < shnum; ++i, p += shdr_size)
>> {
>> const elfcpp::Shdr<size, big_endian> shdr(p);
>> + if (parameters->options().fix_cortex_a53())
>> + scan_erratum_843419(i, shdr, out_sections[i], symtab, target);
>> if (this->section_needs_reloc_stub_scanning(shdr, out_sections, symtab,
>> pshdrs))
>> {
>> unsigned int index = this->adjust_shndx(shdr.get_sh_info());
>> AArch64_address output_offset =
>> @@ -1642,10 +2097,11 @@ class Target_aarch64 : public
>> Sized_target<size, big_endian>
>> typedef AArch64_input_section<size, big_endian> The_aarch64_input_section;
>> typedef AArch64_output_section<size, big_endian> The_aarch64_output_section;
>> typedef Unordered_map<Section_id,
>> AArch64_input_section<size, big_endian>*,
>> Section_id_hash> AArch64_input_section_map;
>> + typedef AArch64_insn_utilities<big_endian> Insn_utilities;
>> const static int TCB_SIZE = size / 8 * 2;
>>
>> Target_aarch64(const Target::Target_info* info = &aarch64_info)
>> : Sized_target<size, big_endian>(info),
>> got_(NULL), plt_(NULL), got_plt_(NULL), got_irelative_(NULL),
>> @@ -1833,10 +2289,21 @@ class Target_aarch64 : public
>> Sized_target<size, big_endian>
>> && parameters->target().get_size() == size
>> && parameters->target().is_big_endian() == big_endian);
>> return static_cast<This*>(parameters->sized_target<size, big_endian>());
>> }
>>
>> +
>> + // Scan erratum for a part of a section.
>> + void
>> + scan_erratum_843419_span(
>> + AArch64_relobj<size, big_endian>*,
>> + unsigned int shndx,
>> + const section_size_type,
>> + const section_size_type,
>> + unsigned char*,
>> + Address);
>> +
>> protected:
>> void
>> do_select_as_default_target()
>> {
>> gold_assert(aarch64_reloc_property_table == NULL);
>> @@ -2126,10 +2593,16 @@ class Target_aarch64 : public
>> Sized_target<size, big_endian>
>> {
>> gold_assert(this->plt_ != NULL);
>> return this->plt_;
>> }
>>
>> + // Return whether this is a 3-insn erratum sequence.
>> + bool is_erratum_843419_sequence(
>> + typename elfcpp::Swap<32,big_endian>::Valtype insn1,
>> + typename elfcpp::Swap<32,big_endian>::Valtype insn2,
>> + typename elfcpp::Swap<32,big_endian>::Valtype insn3);
>> +
>> // Get the dynamic reloc section, creating it if necessary.
>> Reloc_section*
>> rela_dyn_section(Layout*);
>>
>> // Get the section to use for TLSDESC relocations.
>> @@ -6717,10 +7190,110 @@ Target_aarch64<size, big_endian>::relocate_relocs(
>> reloc_view,
>> reloc_view_size);
>> }
>>
>>
>> +// Return whether this is a 3-insn erratum sequence.
>> +
>> +template<int size, bool big_endian>
>> +bool
>> +Target_aarch64<size, big_endian>::is_erratum_843419_sequence(
>> + typename elfcpp::Swap<32,big_endian>::Valtype insn1,
>> + typename elfcpp::Swap<32,big_endian>::Valtype insn2,
>> + typename elfcpp::Swap<32,big_endian>::Valtype insn3)
>> +{
>> + unsigned rt1, rt2;
>> + bool load, pair;
>> +
>> + // The 2nd insn is a single register load or store; or register pair
>> + // store.
>> + if (Insn_utilities::aarch64_mem_op_p(insn2, &rt1, &rt2, &pair, &load)
>> + && (!pair || (pair && !load)))
>> + {
>> + // The 3rd insn is a load or store instruction from the "Load/store
>> + // register (unsigned immediate)" encoding class, using Rn as the
>> + // base address register.
>> + if (Insn_utilities::aarch64_ldst_uimm(insn3) &&
>> + Insn_utilities::aarch64_rn(insn3)
>> + == Insn_utilities::aarch64_rd(insn1))
>> + return true;
>> + }
>> + return false;
>> +}
>> +
>> +
>> +// Scan erratum for section SHNDX range
>> +// [output_address + span_start, output_address + span_end).
>> +
>> +template<int size, bool big_endian>
>> +void
>> +Target_aarch64<size, big_endian>::scan_erratum_843419_span(
>> + AArch64_relobj<size, big_endian>* relobj,
>> + unsigned int shndx,
>> + const section_size_type span_start,
>> + const section_size_type span_end,
>> + unsigned char* input_view,
>> + Address output_address)
>> +{
>> + typedef typename Insn_utilities::Insntype Insntype;
>> +
>> + // Erratum sequence is at least 3 insns.
>> + if (span_end - span_start < 3 * Insn_utilities::BYTES_PER_INSN)
>> + return ;
>> +
>> + Insntype* ip = reinterpret_cast<Insntype*>(input_view + span_start);
>> + unsigned int insn_index = 0;
>> + for (section_size_type i = span_start; i < span_end;
>> + i += Insn_utilities::BYTES_PER_INSN, ++insn_index)
>> + {
>> + if (span_end - i < 3 * Insn_utilities::BYTES_PER_INSN)
>> + break;
>> + Address address = output_address + i;
>> + // The first instruction must be ending at 0xFF8 or 0xFFC and it must
>> + // be an ADRP.
>> + Address a3 = (address & 0xFFF);
>> + if (a3 != 0xFF8 && a3 != 0xFFC)
>> + continue;
>> + Insntype insn1 = ip[insn_index];
>> + if (!Insn_utilities::is_adrp(insn1))
>> + continue;
>> +
>> + Insntype insn2 = ip[insn_index + 1];
>> + Insntype insn3 = ip[insn_index + 2];
>> + bool do_report = false;
>> + if (is_erratum_843419_sequence(insn1, insn2, insn3))
>> + do_report = true;
>> + else if (span_end - i > 16)
>> + {
>> + // Optionally we can have an insn between ins2 and ins3
>> + Insntype insn_opt = ip[insn_index + 2];
>> + // And insn_opt must not be a branch.
>> + if (Insn_utilities::aarch64_b(insn_opt)
>> + || Insn_utilities::aarch64_bl(insn_opt)
>> + || Insn_utilities::aarch64_blr(insn_opt)
>> + || Insn_utilities::aarch64_br(insn_opt))
>> + continue;
>> +
>> + // And insn_opt must not write to dest reg in insn1. However we do a
>> + // conservative scan, which means we may fix/report more than
>> + // necessary, but it doesn't hurt.
>> +
>> + Insntype insn4 = ip[insn_index + 3];
>> + if (is_erratum_843419_sequence(insn1, insn2, insn4))
>> + do_report = true;
>> + }
>> + if (do_report)
>> + {
>> + gold_error(_("Erratum 943419 found at \"%s\", section %d, "
>> + "offset 0x%08x."),
>> + relobj->name().c_str(), shndx, (unsigned int)i);
>> + }
>> + }
>> + return ;
>> +}
>> +
>> +
>> // The selector for aarch64 object files.
>>
>> template<int size, bool big_endian>
>> class Target_selector_aarch64 : public Target_selector
>> {
>> diff --git a/gold/options.h b/gold/options.h
>> index 8c9c934..6bf989c 100644
>> --- a/gold/options.h
>> +++ b/gold/options.h
>> @@ -800,10 +800,14 @@ class General_options
>>
>> DEFINE_bool(fix_cortex_a8, options::TWO_DASHES, '\0', false,
>> N_("(ARM only) Fix binaries for Cortex-A8 erratum."),
>> N_("(ARM only) Do not fix binaries for Cortex-A8 erratum."));
>>
>> + DEFINE_bool(fix_cortex_a53, options::TWO_DASHES, '\0', false,
>> + N_("(AArch64 only) Scan and fix binaries for Cortex-A53 erratums."),
>> + N_("(AArch64 only) Do not scan for Cortex-A53 erratums."));
>> +
>> DEFINE_bool(fix_arm1176, options::TWO_DASHES, '\0', true,
>> N_("(ARM only) Fix binaries for ARM1176 erratum."),
>> N_("(ARM only) Do not fix binaries for ARM1176 erratum."));
>>
>> DEFINE_bool(merge_exidx_entries, options::TWO_DASHES, '\0', true,
>>
>>
>> --
>> Han Shen
--
Han Shen | Software Engineer | shenhan@google.com | +1-650-440-3330