[LLD][AARCH64] Small refactor of AArchErrataFix to match changes in ARMErrataFix NFC.

[peter.smith]

Summary

D67284 introduced ARMErrataFix.cpp which was derived from AArch64ErrataFix.cpp. There were some useful refactoring changes made to ARMErrataFix.cpp made as part of the review. This change applies the relevant changes back to AArch64ErrataFix.cpp. Main changes are:

• Old style variable names in comments like IS, are now new style isec.
• Simplify init() collection of mappingSymbols to always start with a code mapping symbol.
• Simplify logic in mergeCmp().
• Fix one 80 column overflow caused by IS -> isec transformation.

[MaskRay]

ELF/AArch64ErrataFix.cpp

467 ↗

(On Diff #220342)

The outer () can probably be removed.

470 ↗

(On Diff #220342)

Missing full stop.

[peter.smith]

Thanks for the comments. Uploaded new patch with fixes.

[MaskRay]

Thanks for the clean-up!

lld/trunk/ELF/AArch64ErrataFix.cpp

return isLoadStoreClass(instr2) &&
          (isLoadStoreExclusive(instr2) || isLoadLiteral(instr2) ||
           isV8SingleRegisterNonStructureLoadStore(instr2) || isSTP(instr2) ||
           isSTNP(instr2) || isST1(instr2)) &&
          !doesLoadStoreWriteToReg(instr2, rn) &&
          isLoadStoreRegisterUnsigned(instr4) && getRn(instr4) == rn;
 }
 
 // Scan the instruction sequence starting at Offset Off from the base of
-// InputSection IS. We update Off in this function rather than in the caller as
-// we can skip ahead much further into the section when we know how many
+// InputSection isec. We update Off in this function rather than in the caller
+// as we can skip ahead much further into the section when we know how many
 // instructions we've scanned.
-// Return the offset of the load or store instruction in IS that we want to
+// Return the offset of the load or store instruction in isec that we want to
 // patch or 0 if no patch required.
 static uint64_t scanCortexA53Errata843419(InputSection *isec, uint64_t &off,
                                           uint64_t limit) {
   uint64_t isecAddr = isec->getVA(0);
 
-  // Advance Off so that (ISAddr + Off) modulo 0x1000 is at least 0xff8.
+  // Advance Off so that (isecAddr + Off) modulo 0x1000 is at least 0xff8.
   uint64_t initialPageOff = (isecAddr + off) & 0xfff;
   if (initialPageOff < 0xff8)
     off += 0xff8 - initialPageOff;
 
   bool optionalAllowed = limit - off > 12;
   if (off >= limit || limit - off < 12) {
     // Need at least 3 4-byte sized instructions to trigger erratum.
     off = limit;
public:
   void writeTo(uint8_t *buf) override;
 
   size_t getSize() const override { return 8; }
 
   uint64_t getLDSTAddr() const;
 
   // The Section we are patching.
   const InputSection *patchee;
-  // The offset of the instruction in the Patchee section we are patching.
+  // The offset of the instruction in the patchee section we are patching.
   uint64_t patcheeOffset;
   // A label for the start of the Patch that we can use as a relocation target.
   Symbol *patchSym;
 };
 
 Patch843419Section::Patch843419Section(InputSection *p, uint64_t off)
     : SyntheticSection(SHF_ALLOC | SHF_EXECINSTR, SHT_PROGBITS, 4,
                        ".text.patch"),
       patchee(p), patcheeOffset(off) {
   this->parent = p->getParent();
   patchSym = addSyntheticLocal(
       saver.save("__CortexA53843419_" + utohexstr(getLDSTAddr())), STT_FUNC, 0,
       getSize(), *this);
   addSyntheticLocal(saver.save("$x"), STT_NOTYPE, 0, 0, *this);
 }
 
 uint64_t Patch843419Section::getLDSTAddr() const {
   return patchee->getVA(patcheeOffset);
 }
 
 void Patch843419Section::writeTo(uint8_t *buf) {
   // Copy the instruction that we will be replacing with a branch in the
-  // Patchee Section.
+  // patchee Section.
   write32le(buf, read32le(patchee->data().begin() + patcheeOffset));
 
-  // Apply any relocation transferred from the original PatcheeSection.
+  // Apply any relocation transferred from the original patchee section.
   // 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());
 
   // Return address is the next instruction after the one we have just copied.
   uint64_t s = getLDSTAddr() + 4;
   uint64_t p = patchSym->getVA() + 4;
   target->relocateOne(buf + 4, R_AARCH64_JUMP26, s - p);
void AArch64Err843419Patcher::init() {
   for (auto &kv : sectionMap) {
     std::vector<const Defined *> &mapSyms = kv.second;
     llvm::stable_sort(mapSyms, [](const Defined *a, const Defined *b) {
       return a->value < b->value;
     });
     mapSyms.erase(
         std::unique(mapSyms.begin(), mapSyms.end(),
                     [=](const Defined *a, const Defined *b) {
-                      return (isCodeMapSymbol(a) && isCodeMapSymbol(b)) ||
-                             (isDataMapSymbol(a) && isDataMapSymbol(b));
+                      return isCodeMapSymbol(a) == isCodeMapSymbol(b);
                     }),
         mapSyms.end());
+    // Always start with a Code Mapping Symbol.
+    if (!mapSyms.empty() && !isCodeMapSymbol(mapSyms.front()))
+      mapSyms.erase(mapSyms.begin());
   }
   initialized = true;
 }
 
 // Insert the PatchSections we have created back into the
 // InputSectionDescription. As inserting patches alters the addresses of
 // InputSections that follow them, we try and place the patches after all the
 // executable sections, although we may need to insert them earlier if the
void AArch64Err843419Patcher::insertPatches(
 
   // Merge all patch sections. We use the outSecOff assigned above to
   // determine the insertion point. This is ok as we only merge into an
   // InputSectionDescription once per pass, and at the end of the pass
   // assignAddresses() will recalculate all the outSecOff values.
   std::vector<InputSection *> tmp;
   tmp.reserve(isd.sections.size() + patches.size());
   auto mergeCmp = [](const InputSection *a, const InputSection *b) {
-    if (a->outSecOff < b->outSecOff)
-      return true;
-    if (a->outSecOff == b->outSecOff && isa<Patch843419Section>(a) &&
-        !isa<Patch843419Section>(b))
-      return true;
-    return false;
+    if (a->outSecOff != b->outSecOff)
+      return a->outSecOff < b->outSecOff;
+    return isa<Patch843419Section>(a) && !isa<Patch843419Section>(b);
   };
   std::merge(isd.sections.begin(), isd.sections.end(), patches.begin(),
              patches.end(), std::back_inserter(tmp), mergeCmp);
   isd.sections = std::move(tmp);
 }
 
 // Given an erratum sequence that starts at address adrpAddr, with an
 // instruction that we need to patch at patcheeOffset from the start of
-// InputSection IS, create a Patch843419 Section and add it to the
+// InputSection isec, create a Patch843419 Section and add it to the
 // Patches that we need to insert.
 static void implementPatch(uint64_t adrpAddr, uint64_t patcheeOffset,
                            InputSection *isec,
                            std::vector<Patch843419Section *> &patches) {
   // There may be a relocation at the same offset that we are patching. There
   // are four cases that we need to consider.
   // Case 1: R_AARCH64_JUMP26 branch relocation. We have already patched this
   // instance of the erratum on a previous patch and altered the relocation. We
ps->relocations.push_back(
         {relIt->expr, relIt->type, 0, relIt->addend, relIt->sym});
     *relIt = makeRelToPatch(patcheeOffset, ps->patchSym);
   } else
     isec->relocations.push_back(makeRelToPatch(patcheeOffset, ps->patchSym));
 }
 
 // Scan all the instructions in InputSectionDescription, for each instance of
 // the erratum sequence create a Patch843419Section. We return the list of
-// Patch843419Sections that need to be applied to ISD.
+// Patch843419Sections that need to be applied to the InputSectionDescription.
 std::vector<Patch843419Section *>
 AArch64Err843419Patcher::patchInputSectionDescription(
     InputSectionDescription &isd) {
   std::vector<Patch843419Section *> patches;
   for (InputSection *isec : isd.sections) {
     //  LLD doesn't use the erratum sequence in SyntheticSections.
     if (isa<SyntheticSection>(isec))
       continue;
     // Use sectionMap to make sure we only scan code and not inline data.
     // We have already sorted MapSyms in ascending order and removed consecutive
     // mapping symbols of the same type. Our range of executable instructions to
     // scan is therefore [codeSym->value, dataSym->value) or [codeSym->value,
     // section size).
     std::vector<const Defined *> &mapSyms = sectionMap[isec];
 
-    auto codeSym = llvm::find_if(mapSyms, [&](const Defined *ms) {
-      return ms->getName().startswith("$x");
-    });
-
+    auto codeSym = mapSyms.begin();
     while (codeSym != mapSyms.end()) {
       auto dataSym = std::next(codeSym);
       uint64_t off = (*codeSym)->value;
       uint64_t limit =
           (dataSym == mapSyms.end()) ? isec->data().size() : (*dataSym)->value;
 
       while (off < limit) {
         uint64_t startAddr = isec->getVA(off);
-        if (uint64_t patcheeOffset = scanCortexA53Errata843419(isec, off, limit))
+        if (uint64_t patcheeOffset =
+                scanCortexA53Errata843419(isec, off, limit))
           implementPatch(startAddr, patcheeOffset, isec, patches);
       }
       if (dataSym == mapSyms.end())
         break;
       codeSym = std::next(dataSym);
     }
   }
   return patches;