[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 126368.
peter.smith added a comment.

Comments from Rafael

> These comment updates are fine. Please commit them first.

Ok, committed in r320372

>> +class SectionPatcher {
> 
> Maybe call it AArch843419Patcher?

I've gone for AArch64ErrPatcher.

>> +  // Apply any relocation transferred from the original PatcheeSection.
>>  +  // For a SyntheticSection Buf already has OutSecOff added, but relocateAlloc
>>  +  // also adds OutSecOff so we need to subtract to avoid double counting.
>>  +  this->relocateAlloc(Buf - OutSecOff, Buf - OutSecOff + getSize());
> 
> I wonder if we could read the already patched output buffer and avoid
>  this? Can we guarantee that the patch is always written after the patchee?

The patch will always be later in the list of InputSections than the patchee, but if the writing of InputSections is multithreaded then I don't think we could. In any case with the current implementation we use a branch relocation on the location of the original patchee instruction to mutate it into a branch. I think that if we were to go down that route we'd need to do something like Gold, it does 2 passes over the relocations, once for non-patch sections and once for patch sections, with the patch section pass responsible for mutating the patchee instruction into a branch after copying it into the patch.

>> +  std::merge(ISD.Sections.begin(), ISD.Sections.end(), Patches.begin(),
>>  +             Patches.end(), std::back_inserter(Tmp), MergeCmp);
> 
> After this the patch sections still have a OutSecOff of the limit of
> where they can be placed, no? Is that OK?

Yes as we only merge into the InputSectionDescription->Sections once using OutSecOff in the comparison function for the merge. At the end of the pass assignAddresses() will give each patch the correct OutSecOff.

>> +void SectionPatcher::patchInputSectionDescription(
>>  +    InputSectionDescription &ISD, std::vector<Patch843419Section *> &Patches) {
> 
> return the std::vector.

Ok, done.


https://reviews.llvm.org/D36749

Files:
  ELF/AArch64ErrataFix.cpp
  ELF/AArch64ErrataFix.h
  ELF/Writer.cpp
  test/ELF/aarch64-cortex-a53-843419-address.s
  test/ELF/aarch64-cortex-a53-843419-large.s
  test/ELF/aarch64-cortex-a53-843419-recognize.s
  test/ELF/aarch64-cortex-a53-843419-thunk.s

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D36749.126368.patch
Type: text/x-patch
Size: 49010 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171211/6e4c4e0c/attachment.bin>