[PATCH] D67284: [LLD][ELF][ARM] Implement --fix-cortex-a8 to fix erratum 657417

[Peter Smith via Phabricator via llvm-commits]

peter.smith created this revision.
peter.smith added reviewers: ruiu, MaskRay, grimar.
Herald added subscribers: kristof.beyls, arichardson, mgorny, emaste.
Herald added a reviewer: espindola.

The --fix-cortex-a8 option implements a linker workaround for the coretex-a8 erratum 657417. A summary of the erratum conditions is:

- A 32-bit Thumb-2 branch instruction B.w, Bcc.w, BL, BLX spans two

4KiB regions.

- The destination of the branch is to the first 4KiB region.
- The instruction before the branch is a 32-bit Thumb-2 non-branch

instruction.

The linker fix is to redirect the branch to a patch not in the first 4KiBregion. The patch forwards the branch on to its target.

The cortex-a8, is an old CPU, with the first implementation of this workaround in ld.bfd appearing in 2009. The cortex-a8 has been used in early Android Phones and there are some critical applications that still need to run on a cortex-a8 that have the erratum. Implementing support for --fix-cortex-a8 will remove the final reason to keep ld.gold in the Android build environment. The patch is applied roughly 10 times on LLD and 20 on Clang when they are built with --fix-cortex-a8 on an Arm system. The formal erratum description is avaliable in the ARM Core Cortex-A8 (AT400/AT401) Errata Notice document. This is available from Arm on request but it seems to be findable via a web search.

Implementation notes:

- I appreciate that this will be difficult to review as it is a big lump of Arm specific code. The erratum fix itself is fairly simple, most of the complexity is in dealing with the combinations of relocations and instructions. Happy to answer any questions as best I can.

- I have not attempted to share any code or merge this into the AArch64ErrataFix.cpp file. There are some parts that could be factored out at the expense of some customization points to do something specific to Arm, AArch64 or the patch. I chose not to do this for the initial patch to keep it as simple as possible. I'm happy to factor bits out or combine the source files if preferred.

- The functions that are very similar to AArch64ErrataFix are:
  - init (Arm and Thumb mapping symbols)
  - insertPatches (Account for smaller Thumb branch range)
  - patchInputSectionDescription (Arm and Thumb mapping symbols)
  - createFixes (different patch name)

- The functions that are specific to the Cortex-A8 erratum and contain most of the logic are:
  - Class Patch657417Section and its member functions
  - scanCortexA8Errata657417
  - implementPatch

- ld.bfd has a bug in its mask for recognizing bcc.w which causes it to recognize nop.w as a conditional branch. This is only relevant if comparing the implementations.

- If dynamic linking is used then if the page-size or alignment of the segment containing the patches were < 4KiB then a dynamic loader could undo the fixes. Given the target audience of Android the LLD default page-size of 4KiB for Arm prevents this from happening. There is scope to force Page Alignment to at least 4KiB if --fix-cortex-a8 is enabled but I haven't done it yet on the principle that people using this option will know what they are doing.


https://reviews.llvm.org/D67284

Files:
  ELF/AArch64ErrataFix.cpp
  ELF/ARMErrataFix.cpp
  ELF/ARMErrataFix.h
  ELF/CMakeLists.txt
  ELF/Config.h
  ELF/Driver.cpp
  ELF/Options.td
  ELF/Writer.cpp
  test/ELF/arm-fix-cortex-a8-blx.s
  test/ELF/arm-fix-cortex-a8-nopatch.s
  test/ELF/arm-fix-cortex-a8-plt.s
  test/ELF/arm-fix-cortex-a8-recognize.s
  test/ELF/arm-fix-cortex-a8-thunk.s
  test/ELF/arm-fix-cortex-a8-toolarge.s

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D67284.219121.patch
Type: text/x-patch
Size: 46826 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190906/0e7cf0f4/attachment.bin>