[PATCH][GOLD] gold aarch64 erratum stubs are created but not always applied

Cary Coutant ccoutant@gmail.com
Thu Nov 30 22:04:00 GMT 2017


> As a result of looking into fixing
> https://sourceware.org/bugzilla/show_bug.cgi?id=20765 I checked over
> the results of applying --fix-cortex-a53-843419 to a very large
> program (gitit) with two stub tables and thousands of erratum fixes. I
> noticed that all the erratum_stubs were being created but about 1/3 of
> them were being skipped over by
> fix_errata_and_relocate_erratum_stubs(). By skipped over I mean no
> branch relocation or adrp -> adr transformation was applied to the
> erratum address, leaving the erratum_stub unreachable, and with a
> branch with a 0 immediate.
>
> The root cause of the skipped over erratum_stubs is
> Erratum_stub::invalidate_erratum_stub() that is used to set relobj_ to
> NULL when an erratum_stub has been processed. Unfortunately relobj_ is
> used in operator<() so altering relobj makes the results from
> erratum_stubs_.lower_bound() as used in
> find_erratum_stubs_for_input_section() unreliable.
>
> I've proposed a simple fix that adds a new field that can be used in
> Erratum_stub::invalidate_erratum_stub(). This preserves the order
> relied upon by erratum_stubs_.lower_bound().

Thanks for finding this!

It seems the sole purpose of invalidating the stub is nothing more
than a guard against relocating the same stub twice. Rather than
adding a new field, I've modified your patch to invalidate the stub by
setting erratum_insn_ to invalid_insn. (My first thought was simply to
set the stub type to ST_NONE, but that's a const member in the base
class, so it wasn't worth doing that.)

I've committed the patch below.

-cary


2017-11-30  Peter Smith  <peter.smith@linaro.org>
            Cary Coutant  <ccoutant@gmail.com>

gold/
        PR gold/20765
        * aarch64.cc (Erratum_stub::invalidate_erratum_stub): Use erratum_insn_
        instead of relobj_ to invalidate the stub.
        (Erratum_stub::is_invalidated_erratum_stub): Likewise.


diff --git a/gold/aarch64.cc b/gold/aarch64.cc
index 02fabb7749..04da01d51f 100644
--- a/gold/aarch64.cc
+++ b/gold/aarch64.cc
@@ -1052,13 +1052,13 @@ public:
   void
   invalidate_erratum_stub()
   {
-     gold_assert(this->relobj_ != NULL);
-     this->relobj_ = NULL;
+     gold_assert(this->erratum_insn_ != invalid_insn);
+     this->erratum_insn_ = invalid_insn;
   }

   bool
   is_invalidated_erratum_stub()
-  { return this->relobj_ == NULL; }
+  { return this->erratum_insn_ == invalid_insn; }

 protected:
   virtual void



More information about the Binutils mailing list