[PATCH] D36749: [LLD][ELF][AArch64] Complete implementation of -fix-cortex-a53-843419

[Peter Smith via Phabricator via llvm-commits]

peter.smith updated this revision to Diff 114321.
peter.smith added a comment.

I've updated the diff for the inline comments. With respect to the overall approach:

> I have an impression that this patch can be simplified.
> 
> Does it make sense to separate Patch and PatchSection? Naively, I was thinking that patching can be done this way.
> 
> 1. Fix the output layout
> 2. Scan all input sections in address-increasing order while creating and adding patch sections
> 
>   In step 2, we create a synthetic patch section for each problematic instruction and add it after the current input section. When you add a synthetic patch section, you already know the distance from the problematic instruction and the synthetic patch section, so you don't need to create Relocation but can just fix instruction operands.

The main reason to use the PatchSection is that it can be rounded up to a multiple of the page-size so that adding it doesn't affect the address calculations modulo page size of other InputSections. This allows us to get away with a single pass, and to only patch instructions that we need to patch.

With the approach outlined above the patch(es) that are added after each InputSection will affect the address calculations of all subsequent sections. To account for this we have to do one of the following:

- Make the patch(es) that we add after an InputSection  have a size that is 0 modulo page-size, this is potentially wasteful as we make each patch a minimum of 4k
- Recalculate addresses after patching and like range-thunks, iterate until no more patches added. This may result in over-patching of InputSections which is not desirable but ok for correctness. We also have to handle the special case of skipping over a sequence we've already patch.
- Attempt to keep a running total of the size of patches we've added in between sections. I think that this can be done for all the InputSections within an InputSectionDescription, but outside of that a linker script can make this very complicated to get right ( we can't know for sure whether the address of the next InputSectionDescription is relative to the previous one or absolute without reading the linker script).

One thing I don't quite understand is how we would directly modify the opcodes in the InputSection. My naive understanding is that at the point we do patching the contents of the InputSection are read-only, it is only when the contents has been copied to the output memory buffer that we can modify them. At this point describing the modifications in terms of relocations seemed the simplest way of modifying the instructions. Have I got this wrong?

I'm happy to try an alternative approach, although my current thinking right now would be that it wouldn't significantly simplify the solution, just a different set of trade-offs. If you have a preference for any of the alternatives let me know and I'll send out an alternative review to see if it is any better than this approach?


https://reviews.llvm.org/D36749

Files:
  ELF/SectionPatcher.cpp
  ELF/SectionPatcher.h
  ELF/SyntheticSections.cpp
  ELF/SyntheticSections.h
  test/ELF/aarch64-cortex-a53-843419-address.s
  test/ELF/aarch64-cortex-a53-843419-recognize.s

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D36749.114321.patch
Type: text/x-patch
Size: 40640 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170908/4948caba/attachment.bin>