Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Object,ELFType] Rename TargetEndianness to Endianness #86604

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Mar 26, 2024

TargetEndianness is a long name and using it is unwieldy.
"Target" in the name is confusing. Rename it to "Endianness".

I cannot find noticeable out-of-tree users of TargetEndianness, but
keep TargetEndianness to make this patch safer. TargetEndianness
will be removed by a subsequent change.

Created using spr 1.3.5-bogner
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 26, 2024

@llvm/pr-subscribers-llvm-binary-utilities
@llvm/pr-subscribers-lld-elf

@llvm/pr-subscribers-objectyaml

Author: Fangrui Song (MaskRay)

Changes

TargetEndianness is a long name and using it is unwieldy. This PR
updates a few places to demonstrate the readability improvement.


Full diff: https://github.com/llvm/llvm-project/pull/86604.diff

6 Files Affected:

  • (modified) lld/ELF/SyntheticSections.cpp (+2-2)
  • (modified) llvm/include/llvm/Object/ELFTypes.h (+1)
  • (modified) llvm/lib/ObjCopy/ELF/ELFObject.cpp (+7-8)
  • (modified) llvm/lib/ObjectYAML/ELFEmitter.cpp (+29-29)
  • (modified) llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h (+4-5)
  • (modified) llvm/tools/llvm-readobj/ELFDumper.cpp (+19-25)
diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index 650bd6cd390060..3a290eaa9bd3e5 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -415,7 +415,7 @@ void EhFrameSection::addRecords(EhInputSection *sec, ArrayRef<RelTy> rels) {
   for (EhSectionPiece &cie : sec->cies)
     offsetToCie[cie.inputOff] = addCie<ELFT>(cie, rels);
   for (EhSectionPiece &fde : sec->fdes) {
-    uint32_t id = endian::read32<ELFT::TargetEndianness>(fde.data().data() + 4);
+    uint32_t id = endian::read32<ELFT::Endian>(fde.data().data() + 4);
     CieRecord *rec = offsetToCie[fde.inputOff + 4 - id];
     if (!rec)
       fatal(toString(sec) + ": invalid CIE reference");
@@ -448,7 +448,7 @@ void EhFrameSection::iterateFDEWithLSDAAux(
     if (hasLSDA(cie))
       ciesWithLSDA.insert(cie.inputOff);
   for (EhSectionPiece &fde : sec.fdes) {
-    uint32_t id = endian::read32<ELFT::TargetEndianness>(fde.data().data() + 4);
+    uint32_t id = endian::read32<ELFT::Endian>(fde.data().data() + 4);
     if (!ciesWithLSDA.contains(fde.inputOff + 4 - id))
       continue;
 
diff --git a/llvm/include/llvm/Object/ELFTypes.h b/llvm/include/llvm/Object/ELFTypes.h
index 4986ecf8323d02..7fa850e218b7e7 100644
--- a/llvm/include/llvm/Object/ELFTypes.h
+++ b/llvm/include/llvm/Object/ELFTypes.h
@@ -52,6 +52,7 @@ template <endianness E, bool Is64> struct ELFType {
 
 public:
   static const endianness TargetEndianness = E;
+  static const endianness Endian = E;
   static const bool Is64Bits = Is64;
 
   using uint = std::conditional_t<Is64, uint64_t, uint32_t>;
diff --git a/llvm/lib/ObjCopy/ELF/ELFObject.cpp b/llvm/lib/ObjCopy/ELF/ELFObject.cpp
index 9547cc10d2a0ba..3fd81bfea93c80 100644
--- a/llvm/lib/ObjCopy/ELF/ELFObject.cpp
+++ b/llvm/lib/ObjCopy/ELF/ELFObject.cpp
@@ -33,6 +33,7 @@ using namespace llvm;
 using namespace llvm::ELF;
 using namespace llvm::objcopy::elf;
 using namespace llvm::object;
+using namespace llvm::support;
 
 template <class ELFT> void ELFWriter<ELFT>::writePhdr(const Segment &Seg) {
   uint8_t *B = reinterpret_cast<uint8_t *>(Buf->getBufferStart()) +
@@ -1175,9 +1176,9 @@ template <class ELFT>
 Error ELFSectionWriter<ELFT>::visit(const GroupSection &Sec) {
   ELF::Elf32_Word *Buf =
       reinterpret_cast<ELF::Elf32_Word *>(Out.getBufferStart() + Sec.Offset);
-  support::endian::write32<ELFT::TargetEndianness>(Buf++, Sec.FlagWord);
+  endian::write32<ELFT::Endian>(Buf++, Sec.FlagWord);
   for (SectionBase *S : Sec.GroupMembers)
-    support::endian::write32<ELFT::TargetEndianness>(Buf++, S->Index);
+    endian::write32<ELFT::Endian>(Buf++, S->Index);
   return Error::success();
 }
 
@@ -1522,10 +1523,9 @@ Error ELFBuilder<ELFT>::initGroupSection(GroupSection *GroupSec) {
       reinterpret_cast<const ELF::Elf32_Word *>(GroupSec->Contents.data());
   const ELF::Elf32_Word *End =
       Word + GroupSec->Contents.size() / sizeof(ELF::Elf32_Word);
-  GroupSec->setFlagWord(
-      support::endian::read32<ELFT::TargetEndianness>(Word++));
+  GroupSec->setFlagWord(endian::read32<ELFT::Endian>(Word++));
   for (; Word != End; ++Word) {
-    uint32_t Index = support::endian::read32<ELFT::TargetEndianness>(Word);
+    uint32_t Index = support::endian::read32<ELFT::Endian>(Word);
     Expected<SectionBase *> Sec = SecTable.getSection(
         Index, "group member index " + Twine(Index) + " in section '" +
                    GroupSec->Name + "' is invalid");
@@ -1993,9 +1993,8 @@ template <class ELFT> void ELFWriter<ELFT>::writeEhdr() {
   Ehdr.e_ident[EI_MAG2] = 'L';
   Ehdr.e_ident[EI_MAG3] = 'F';
   Ehdr.e_ident[EI_CLASS] = ELFT::Is64Bits ? ELFCLASS64 : ELFCLASS32;
-  Ehdr.e_ident[EI_DATA] = ELFT::TargetEndianness == llvm::endianness::big
-                              ? ELFDATA2MSB
-                              : ELFDATA2LSB;
+  Ehdr.e_ident[EI_DATA] =
+      ELFT::Endian == llvm::endianness::big ? ELFDATA2MSB : ELFDATA2LSB;
   Ehdr.e_ident[EI_VERSION] = EV_CURRENT;
   Ehdr.e_ident[EI_OSABI] = Obj.OSABI;
   Ehdr.e_ident[EI_ABIVERSION] = Obj.ABIVersion;
diff --git a/llvm/lib/ObjectYAML/ELFEmitter.cpp b/llvm/lib/ObjectYAML/ELFEmitter.cpp
index 58a725f8d87788..890a24d98afc94 100644
--- a/llvm/lib/ObjectYAML/ELFEmitter.cpp
+++ b/llvm/lib/ObjectYAML/ELFEmitter.cpp
@@ -1314,7 +1314,7 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
     if (!ELFT::Is64Bits && E > UINT32_MAX)
       reportError(Section.Name + ": the value is too large for 32-bits: 0x" +
                   Twine::utohexstr(E));
-    CBA.write<uintX_t>(E, ELFT::TargetEndianness);
+    CBA.write<uintX_t>(E, ELFT::Endian);
   }
 
   SHeader.sh_size = sizeof(uintX_t) * Section.Entries->size();
@@ -1333,7 +1333,7 @@ void ELFState<ELFT>::writeSectionContent(
     return;
 
   for (uint32_t E : *Shndx.Entries)
-    CBA.write<uint32_t>(E, ELFT::TargetEndianness);
+    CBA.write<uint32_t>(E, ELFT::Endian);
   SHeader.sh_size = Shndx.Entries->size() * SHeader.sh_entsize;
 }
 
@@ -1357,7 +1357,7 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
       SectionIndex = llvm::ELF::GRP_COMDAT;
     else
       SectionIndex = toSectionIndex(Member.sectionNameOrType, Section.Name);
-    CBA.write<uint32_t>(SectionIndex, ELFT::TargetEndianness);
+    CBA.write<uint32_t>(SectionIndex, ELFT::Endian);
   }
   SHeader.sh_size = SHeader.sh_entsize * Section.Members->size();
 }
@@ -1370,7 +1370,7 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
     return;
 
   for (uint16_t Version : *Section.Entries)
-    CBA.write<uint16_t>(Version, ELFT::TargetEndianness);
+    CBA.write<uint16_t>(Version, ELFT::Endian);
   SHeader.sh_size = Section.Entries->size() * SHeader.sh_entsize;
 }
 
@@ -1382,7 +1382,7 @@ void ELFState<ELFT>::writeSectionContent(
     return;
 
   for (const ELFYAML::StackSizeEntry &E : *Section.Entries) {
-    CBA.write<uintX_t>(E.Address, ELFT::TargetEndianness);
+    CBA.write<uintX_t>(E.Address, ELFT::Endian);
     SHeader.sh_size += sizeof(uintX_t) + CBA.writeULEB128(E.Size);
   }
 }
@@ -1444,7 +1444,7 @@ void ELFState<ELFT>::writeSectionContent(
     uint64_t TotalNumBlocks = 0;
     for (const ELFYAML::BBAddrMapEntry::BBRangeEntry &BBR : *E.BBRanges) {
       // Write the base address of the range.
-      CBA.write<uintX_t>(BBR.BaseAddress, ELFT::TargetEndianness);
+      CBA.write<uintX_t>(BBR.BaseAddress, ELFT::Endian);
       // Write number of BBEntries (number of basic blocks in this basic block
       // range). This is overridden by the 'NumBlocks' YAML field when
       // specified.
@@ -1558,7 +1558,7 @@ void ELFState<ELFT>::writeSectionContent(
     return;
 
   for (const ELFYAML::CallGraphEntryWeight &E : *Section.Entries) {
-    CBA.write<uint64_t>(E.Weight, ELFT::TargetEndianness);
+    CBA.write<uint64_t>(E.Weight, ELFT::Endian);
     SHeader.sh_size += sizeof(object::Elf_CGProfile_Impl<ELFT>);
   }
 }
@@ -1572,15 +1572,15 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
 
   CBA.write<uint32_t>(
       Section.NBucket.value_or(llvm::yaml::Hex64(Section.Bucket->size())),
-      ELFT::TargetEndianness);
+      ELFT::Endian);
   CBA.write<uint32_t>(
       Section.NChain.value_or(llvm::yaml::Hex64(Section.Chain->size())),
-      ELFT::TargetEndianness);
+      ELFT::Endian);
 
   for (uint32_t Val : *Section.Bucket)
-    CBA.write<uint32_t>(Val, ELFT::TargetEndianness);
+    CBA.write<uint32_t>(Val, ELFT::Endian);
   for (uint32_t Val : *Section.Chain)
-    CBA.write<uint32_t>(Val, ELFT::TargetEndianness);
+    CBA.write<uint32_t>(Val, ELFT::Endian);
 
   SHeader.sh_size = (2 + Section.Bucket->size() + Section.Chain->size()) * 4;
 }
@@ -1687,8 +1687,8 @@ void ELFState<ELFT>::writeSectionContent(
     return;
 
   for (const ELFYAML::ARMIndexTableEntry &E : *Section.Entries) {
-    CBA.write<uint32_t>(E.Offset, ELFT::TargetEndianness);
-    CBA.write<uint32_t>(E.Value, ELFT::TargetEndianness);
+    CBA.write<uint32_t>(E.Offset, ELFT::Endian);
+    CBA.write<uint32_t>(E.Value, ELFT::Endian);
   }
   SHeader.sh_size = Section.Entries->size() * 8;
 }
@@ -1729,8 +1729,8 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
     return;
 
   for (const ELFYAML::DynamicEntry &DE : *Section.Entries) {
-    CBA.write<uintX_t>(DE.Tag, ELFT::TargetEndianness);
-    CBA.write<uintX_t>(DE.Val, ELFT::TargetEndianness);
+    CBA.write<uintX_t>(DE.Tag, ELFT::Endian);
+    CBA.write<uintX_t>(DE.Val, ELFT::Endian);
   }
   SHeader.sh_size = 2 * sizeof(uintX_t) * Section.Entries->size();
 }
@@ -1758,18 +1758,18 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
   for (const ELFYAML::NoteEntry &NE : *Section.Notes) {
     // Write name size.
     if (NE.Name.empty())
-      CBA.write<uint32_t>(0, ELFT::TargetEndianness);
+      CBA.write<uint32_t>(0, ELFT::Endian);
     else
-      CBA.write<uint32_t>(NE.Name.size() + 1, ELFT::TargetEndianness);
+      CBA.write<uint32_t>(NE.Name.size() + 1, ELFT::Endian);
 
     // Write description size.
     if (NE.Desc.binary_size() == 0)
-      CBA.write<uint32_t>(0, ELFT::TargetEndianness);
+      CBA.write<uint32_t>(0, ELFT::Endian);
     else
-      CBA.write<uint32_t>(NE.Desc.binary_size(), ELFT::TargetEndianness);
+      CBA.write<uint32_t>(NE.Desc.binary_size(), ELFT::Endian);
 
     // Write type.
-    CBA.write<uint32_t>(NE.Type, ELFT::TargetEndianness);
+    CBA.write<uint32_t>(NE.Type, ELFT::Endian);
 
     // Write name, null terminator and padding.
     if (!NE.Name.empty()) {
@@ -1803,35 +1803,35 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
   // be used to override this field, which is useful for producing broken
   // objects.
   if (Section.Header->NBuckets)
-    CBA.write<uint32_t>(*Section.Header->NBuckets, ELFT::TargetEndianness);
+    CBA.write<uint32_t>(*Section.Header->NBuckets, ELFT::Endian);
   else
-    CBA.write<uint32_t>(Section.HashBuckets->size(), ELFT::TargetEndianness);
+    CBA.write<uint32_t>(Section.HashBuckets->size(), ELFT::Endian);
 
   // Write the index of the first symbol in the dynamic symbol table accessible
   // via the hash table.
-  CBA.write<uint32_t>(Section.Header->SymNdx, ELFT::TargetEndianness);
+  CBA.write<uint32_t>(Section.Header->SymNdx, ELFT::Endian);
 
   // Write the number of words in the Bloom filter. As above, the "MaskWords"
   // property can be used to set this field to any value.
   if (Section.Header->MaskWords)
-    CBA.write<uint32_t>(*Section.Header->MaskWords, ELFT::TargetEndianness);
+    CBA.write<uint32_t>(*Section.Header->MaskWords, ELFT::Endian);
   else
-    CBA.write<uint32_t>(Section.BloomFilter->size(), ELFT::TargetEndianness);
+    CBA.write<uint32_t>(Section.BloomFilter->size(), ELFT::Endian);
 
   // Write the shift constant used by the Bloom filter.
-  CBA.write<uint32_t>(Section.Header->Shift2, ELFT::TargetEndianness);
+  CBA.write<uint32_t>(Section.Header->Shift2, ELFT::Endian);
 
   // We've finished writing the header. Now write the Bloom filter.
   for (llvm::yaml::Hex64 Val : *Section.BloomFilter)
-    CBA.write<uintX_t>(Val, ELFT::TargetEndianness);
+    CBA.write<uintX_t>(Val, ELFT::Endian);
 
   // Write an array of hash buckets.
   for (llvm::yaml::Hex32 Val : *Section.HashBuckets)
-    CBA.write<uint32_t>(Val, ELFT::TargetEndianness);
+    CBA.write<uint32_t>(Val, ELFT::Endian);
 
   // Write an array of hash values.
   for (llvm::yaml::Hex32 Val : *Section.HashValues)
-    CBA.write<uint32_t>(Val, ELFT::TargetEndianness);
+    CBA.write<uint32_t>(Val, ELFT::Endian);
 
   SHeader.sh_size = 16 /*Header size*/ +
                     Section.BloomFilter->size() * sizeof(typename ELFT::uint) +
diff --git a/llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h b/llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h
index 2e89463e68d519..d14b0b1bec34d8 100644
--- a/llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h
+++ b/llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h
@@ -113,7 +113,7 @@ void PrinterContext<ELFT>::printEHFrameHdr(const Elf_Phdr *EHFramePHdr) const {
   if (!Content)
     reportError(Content.takeError(), ObjF.getFileName());
 
-  DataExtractor DE(*Content, ELFT::TargetEndianness == llvm::endianness::little,
+  DataExtractor DE(*Content, ELFT::Endian == llvm::endianness::little,
                    ELFT::Is64Bits ? 8 : 4);
 
   DictScope D(W, "Header");
@@ -186,10 +186,9 @@ void PrinterContext<ELFT>::printEHFrame(const Elf_Shdr *EHFrameShdr) const {
   // Construct DWARFDataExtractor to handle relocations ("PC Begin" fields).
   std::unique_ptr<DWARFContext> DICtx = DWARFContext::create(
       ObjF, DWARFContext::ProcessDebugRelocations::Process, nullptr);
-  DWARFDataExtractor DE(DICtx->getDWARFObj(),
-                        DICtx->getDWARFObj().getEHFrameSection(),
-                        ELFT::TargetEndianness == llvm::endianness::little,
-                        ELFT::Is64Bits ? 8 : 4);
+  DWARFDataExtractor DE(
+      DICtx->getDWARFObj(), DICtx->getDWARFObj().getEHFrameSection(),
+      ELFT::Endian == llvm::endianness::little, ELFT::Is64Bits ? 8 : 4);
   DWARFDebugFrame EHFrame(Triple::ArchType(ObjF.getArch()), /*IsEH=*/true,
                           /*EHFrameAddress=*/Address);
   if (Error E = EHFrame.parse(DE))
diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index d1c05f437042de..954c07c634abfd 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -74,6 +74,7 @@
 
 using namespace llvm;
 using namespace llvm::object;
+using namespace llvm::support;
 using namespace ELF;
 
 #define LLVM_READOBJ_ENUM_CASE(ns, enum)                                       \
@@ -3419,13 +3420,12 @@ template <class ELFT> void ELFDumper<ELFT>::printStackMap() const {
     return;
   }
 
-  if (Error E = StackMapParser<ELFT::TargetEndianness>::validateHeader(
-          *ContentOrErr)) {
+  if (Error E = StackMapParser<ELFT::Endian>::validateHeader(*ContentOrErr)) {
     Warn(std::move(E));
     return;
   }
 
-  prettyPrintStackMap(W, StackMapParser<ELFT::TargetEndianness>(*ContentOrErr));
+  prettyPrintStackMap(W, StackMapParser<ELFT::Endian>(*ContentOrErr));
 }
 
 template <class ELFT>
@@ -5145,7 +5145,7 @@ static std::string getGNUProperty(uint32_t Type, uint32_t DataSize,
       OS << format("<corrupt length: 0x%x>", DataSize);
       return OS.str();
     }
-    PrData = support::endian::read32<ELFT::TargetEndianness>(Data.data());
+    PrData = endian::read32<ELFT::Endian>(Data.data());
     if (PrData == 0) {
       OS << "<None>";
       return OS.str();
@@ -5169,7 +5169,7 @@ static std::string getGNUProperty(uint32_t Type, uint32_t DataSize,
       OS << format("<corrupt length: 0x%x>", DataSize);
       return OS.str();
     }
-    PrData = support::endian::read32<ELFT::TargetEndianness>(Data.data());
+    PrData = endian::read32<ELFT::Endian>(Data.data());
     if (PrData == 0) {
       OS << "<None>";
       return OS.str();
@@ -5195,7 +5195,7 @@ static std::string getGNUProperty(uint32_t Type, uint32_t DataSize,
       OS << format("<corrupt length: 0x%x>", DataSize);
       return OS.str();
     }
-    PrData = support::endian::read32<ELFT::TargetEndianness>(Data.data());
+    PrData = endian::read32<ELFT::Endian>(Data.data());
     if (PrData == 0) {
       OS << "<None>";
       return OS.str();
@@ -5374,10 +5374,8 @@ static bool printAArch64Note(raw_ostream &OS, uint32_t NoteType,
     return false;
   }
 
-  uint64_t Platform =
-      support::endian::read64<ELFT::TargetEndianness>(Desc.data() + 0);
-  uint64_t Version =
-      support::endian::read64<ELFT::TargetEndianness>(Desc.data() + 8);
+  uint64_t Platform = endian::read64<ELFT::Endian>(Desc.data() + 0);
+  uint64_t Version = endian::read64<ELFT::Endian>(Desc.data() + 8);
   OS << format("platform 0x%" PRIx64 ", version 0x%" PRIx64, Platform, Version);
 
   if (Desc.size() > 16)
@@ -5457,16 +5455,14 @@ getFreeBSDNote(uint32_t NoteType, ArrayRef<uint8_t> Desc, bool IsCore) {
   case ELF::NT_FREEBSD_ABI_TAG:
     if (Desc.size() != 4)
       return std::nullopt;
-    return FreeBSDNote{
-        "ABI tag",
-        utostr(support::endian::read32<ELFT::TargetEndianness>(Desc.data()))};
+    return FreeBSDNote{"ABI tag",
+                       utostr(endian::read32<ELFT::Endian>(Desc.data()))};
   case ELF::NT_FREEBSD_ARCH_TAG:
     return FreeBSDNote{"Arch tag", toStringRef(Desc).str()};
   case ELF::NT_FREEBSD_FEATURE_CTL: {
     if (Desc.size() != 4)
       return std::nullopt;
-    unsigned Value =
-        support::endian::read32<ELFT::TargetEndianness>(Desc.data());
+    unsigned Value = endian::read32<ELFT::Endian>(Desc.data());
     std::string FlagsStr;
     raw_string_ostream OS(FlagsStr);
     printFlags(Value, ArrayRef(FreeBSDFeatureCtlFlags), OS);
@@ -6052,9 +6048,9 @@ template <class ELFT> void GNUELFDumper<ELFT>::printNotes() {
         return Error::success();
     } else if (Name == "CORE") {
       if (Type == ELF::NT_FILE) {
-        DataExtractor DescExtractor(
-            Descriptor, ELFT::TargetEndianness == llvm::endianness::little,
-            sizeof(Elf_Addr));
+        DataExtractor DescExtractor(Descriptor,
+                                    ELFT::Endian == llvm::endianness::little,
+                                    sizeof(Elf_Addr));
         if (Expected<CoreNote> NoteOrErr = readCoreNote(DescExtractor)) {
           printCoreNote<ELFT>(OS, *NoteOrErr);
           return Error::success();
@@ -7714,10 +7710,8 @@ static bool printAarch64NoteLLVMStyle(uint32_t NoteType, ArrayRef<uint8_t> Desc,
   if (Desc.size() < 16)
     return false;
 
-  uint64_t platform =
-      support::endian::read64<ELFT::TargetEndianness>(Desc.data() + 0);
-  uint64_t version =
-      support::endian::read64<ELFT::TargetEndianness>(Desc.data() + 8);
+  uint64_t platform = endian::read64<ELFT::Endian>(Desc.data() + 0);
+  uint64_t version = endian::read64<ELFT::Endian>(Desc.data() + 8);
   W.printNumber("Platform", platform);
   W.printNumber("Version", version);
 
@@ -7851,9 +7845,9 @@ template <class ELFT> void LLVMELFDumper<ELFT>::printNotes() {
         return Error::success();
     } else if (Name == "CORE") {
       if (Type == ELF::NT_FILE) {
-        DataExtractor DescExtractor(
-            Descriptor, ELFT::TargetEndianness == llvm::endianness::little,
-            sizeof(Elf_Addr));
+        DataExtractor DescExtractor(Descriptor,
+                                    ELFT::Endian == llvm::endianness::little,
+                                    sizeof(Elf_Addr));
         if (Expected<CoreNote> N = readCoreNote(DescExtractor)) {
           printCoreNoteLLVMStyle(*N, W);
           return Error::success();

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 26, 2024

@llvm/pr-subscribers-lld

Author: Fangrui Song (MaskRay)

Changes

TargetEndianness is a long name and using it is unwieldy. This PR
updates a few places to demonstrate the readability improvement.


Full diff: https://github.com/llvm/llvm-project/pull/86604.diff

6 Files Affected:

  • (modified) lld/ELF/SyntheticSections.cpp (+2-2)
  • (modified) llvm/include/llvm/Object/ELFTypes.h (+1)
  • (modified) llvm/lib/ObjCopy/ELF/ELFObject.cpp (+7-8)
  • (modified) llvm/lib/ObjectYAML/ELFEmitter.cpp (+29-29)
  • (modified) llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h (+4-5)
  • (modified) llvm/tools/llvm-readobj/ELFDumper.cpp (+19-25)
diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index 650bd6cd390060..3a290eaa9bd3e5 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -415,7 +415,7 @@ void EhFrameSection::addRecords(EhInputSection *sec, ArrayRef<RelTy> rels) {
   for (EhSectionPiece &cie : sec->cies)
     offsetToCie[cie.inputOff] = addCie<ELFT>(cie, rels);
   for (EhSectionPiece &fde : sec->fdes) {
-    uint32_t id = endian::read32<ELFT::TargetEndianness>(fde.data().data() + 4);
+    uint32_t id = endian::read32<ELFT::Endian>(fde.data().data() + 4);
     CieRecord *rec = offsetToCie[fde.inputOff + 4 - id];
     if (!rec)
       fatal(toString(sec) + ": invalid CIE reference");
@@ -448,7 +448,7 @@ void EhFrameSection::iterateFDEWithLSDAAux(
     if (hasLSDA(cie))
       ciesWithLSDA.insert(cie.inputOff);
   for (EhSectionPiece &fde : sec.fdes) {
-    uint32_t id = endian::read32<ELFT::TargetEndianness>(fde.data().data() + 4);
+    uint32_t id = endian::read32<ELFT::Endian>(fde.data().data() + 4);
     if (!ciesWithLSDA.contains(fde.inputOff + 4 - id))
       continue;
 
diff --git a/llvm/include/llvm/Object/ELFTypes.h b/llvm/include/llvm/Object/ELFTypes.h
index 4986ecf8323d02..7fa850e218b7e7 100644
--- a/llvm/include/llvm/Object/ELFTypes.h
+++ b/llvm/include/llvm/Object/ELFTypes.h
@@ -52,6 +52,7 @@ template <endianness E, bool Is64> struct ELFType {
 
 public:
   static const endianness TargetEndianness = E;
+  static const endianness Endian = E;
   static const bool Is64Bits = Is64;
 
   using uint = std::conditional_t<Is64, uint64_t, uint32_t>;
diff --git a/llvm/lib/ObjCopy/ELF/ELFObject.cpp b/llvm/lib/ObjCopy/ELF/ELFObject.cpp
index 9547cc10d2a0ba..3fd81bfea93c80 100644
--- a/llvm/lib/ObjCopy/ELF/ELFObject.cpp
+++ b/llvm/lib/ObjCopy/ELF/ELFObject.cpp
@@ -33,6 +33,7 @@ using namespace llvm;
 using namespace llvm::ELF;
 using namespace llvm::objcopy::elf;
 using namespace llvm::object;
+using namespace llvm::support;
 
 template <class ELFT> void ELFWriter<ELFT>::writePhdr(const Segment &Seg) {
   uint8_t *B = reinterpret_cast<uint8_t *>(Buf->getBufferStart()) +
@@ -1175,9 +1176,9 @@ template <class ELFT>
 Error ELFSectionWriter<ELFT>::visit(const GroupSection &Sec) {
   ELF::Elf32_Word *Buf =
       reinterpret_cast<ELF::Elf32_Word *>(Out.getBufferStart() + Sec.Offset);
-  support::endian::write32<ELFT::TargetEndianness>(Buf++, Sec.FlagWord);
+  endian::write32<ELFT::Endian>(Buf++, Sec.FlagWord);
   for (SectionBase *S : Sec.GroupMembers)
-    support::endian::write32<ELFT::TargetEndianness>(Buf++, S->Index);
+    endian::write32<ELFT::Endian>(Buf++, S->Index);
   return Error::success();
 }
 
@@ -1522,10 +1523,9 @@ Error ELFBuilder<ELFT>::initGroupSection(GroupSection *GroupSec) {
       reinterpret_cast<const ELF::Elf32_Word *>(GroupSec->Contents.data());
   const ELF::Elf32_Word *End =
       Word + GroupSec->Contents.size() / sizeof(ELF::Elf32_Word);
-  GroupSec->setFlagWord(
-      support::endian::read32<ELFT::TargetEndianness>(Word++));
+  GroupSec->setFlagWord(endian::read32<ELFT::Endian>(Word++));
   for (; Word != End; ++Word) {
-    uint32_t Index = support::endian::read32<ELFT::TargetEndianness>(Word);
+    uint32_t Index = support::endian::read32<ELFT::Endian>(Word);
     Expected<SectionBase *> Sec = SecTable.getSection(
         Index, "group member index " + Twine(Index) + " in section '" +
                    GroupSec->Name + "' is invalid");
@@ -1993,9 +1993,8 @@ template <class ELFT> void ELFWriter<ELFT>::writeEhdr() {
   Ehdr.e_ident[EI_MAG2] = 'L';
   Ehdr.e_ident[EI_MAG3] = 'F';
   Ehdr.e_ident[EI_CLASS] = ELFT::Is64Bits ? ELFCLASS64 : ELFCLASS32;
-  Ehdr.e_ident[EI_DATA] = ELFT::TargetEndianness == llvm::endianness::big
-                              ? ELFDATA2MSB
-                              : ELFDATA2LSB;
+  Ehdr.e_ident[EI_DATA] =
+      ELFT::Endian == llvm::endianness::big ? ELFDATA2MSB : ELFDATA2LSB;
   Ehdr.e_ident[EI_VERSION] = EV_CURRENT;
   Ehdr.e_ident[EI_OSABI] = Obj.OSABI;
   Ehdr.e_ident[EI_ABIVERSION] = Obj.ABIVersion;
diff --git a/llvm/lib/ObjectYAML/ELFEmitter.cpp b/llvm/lib/ObjectYAML/ELFEmitter.cpp
index 58a725f8d87788..890a24d98afc94 100644
--- a/llvm/lib/ObjectYAML/ELFEmitter.cpp
+++ b/llvm/lib/ObjectYAML/ELFEmitter.cpp
@@ -1314,7 +1314,7 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
     if (!ELFT::Is64Bits && E > UINT32_MAX)
       reportError(Section.Name + ": the value is too large for 32-bits: 0x" +
                   Twine::utohexstr(E));
-    CBA.write<uintX_t>(E, ELFT::TargetEndianness);
+    CBA.write<uintX_t>(E, ELFT::Endian);
   }
 
   SHeader.sh_size = sizeof(uintX_t) * Section.Entries->size();
@@ -1333,7 +1333,7 @@ void ELFState<ELFT>::writeSectionContent(
     return;
 
   for (uint32_t E : *Shndx.Entries)
-    CBA.write<uint32_t>(E, ELFT::TargetEndianness);
+    CBA.write<uint32_t>(E, ELFT::Endian);
   SHeader.sh_size = Shndx.Entries->size() * SHeader.sh_entsize;
 }
 
@@ -1357,7 +1357,7 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
       SectionIndex = llvm::ELF::GRP_COMDAT;
     else
       SectionIndex = toSectionIndex(Member.sectionNameOrType, Section.Name);
-    CBA.write<uint32_t>(SectionIndex, ELFT::TargetEndianness);
+    CBA.write<uint32_t>(SectionIndex, ELFT::Endian);
   }
   SHeader.sh_size = SHeader.sh_entsize * Section.Members->size();
 }
@@ -1370,7 +1370,7 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
     return;
 
   for (uint16_t Version : *Section.Entries)
-    CBA.write<uint16_t>(Version, ELFT::TargetEndianness);
+    CBA.write<uint16_t>(Version, ELFT::Endian);
   SHeader.sh_size = Section.Entries->size() * SHeader.sh_entsize;
 }
 
@@ -1382,7 +1382,7 @@ void ELFState<ELFT>::writeSectionContent(
     return;
 
   for (const ELFYAML::StackSizeEntry &E : *Section.Entries) {
-    CBA.write<uintX_t>(E.Address, ELFT::TargetEndianness);
+    CBA.write<uintX_t>(E.Address, ELFT::Endian);
     SHeader.sh_size += sizeof(uintX_t) + CBA.writeULEB128(E.Size);
   }
 }
@@ -1444,7 +1444,7 @@ void ELFState<ELFT>::writeSectionContent(
     uint64_t TotalNumBlocks = 0;
     for (const ELFYAML::BBAddrMapEntry::BBRangeEntry &BBR : *E.BBRanges) {
       // Write the base address of the range.
-      CBA.write<uintX_t>(BBR.BaseAddress, ELFT::TargetEndianness);
+      CBA.write<uintX_t>(BBR.BaseAddress, ELFT::Endian);
       // Write number of BBEntries (number of basic blocks in this basic block
       // range). This is overridden by the 'NumBlocks' YAML field when
       // specified.
@@ -1558,7 +1558,7 @@ void ELFState<ELFT>::writeSectionContent(
     return;
 
   for (const ELFYAML::CallGraphEntryWeight &E : *Section.Entries) {
-    CBA.write<uint64_t>(E.Weight, ELFT::TargetEndianness);
+    CBA.write<uint64_t>(E.Weight, ELFT::Endian);
     SHeader.sh_size += sizeof(object::Elf_CGProfile_Impl<ELFT>);
   }
 }
@@ -1572,15 +1572,15 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
 
   CBA.write<uint32_t>(
       Section.NBucket.value_or(llvm::yaml::Hex64(Section.Bucket->size())),
-      ELFT::TargetEndianness);
+      ELFT::Endian);
   CBA.write<uint32_t>(
       Section.NChain.value_or(llvm::yaml::Hex64(Section.Chain->size())),
-      ELFT::TargetEndianness);
+      ELFT::Endian);
 
   for (uint32_t Val : *Section.Bucket)
-    CBA.write<uint32_t>(Val, ELFT::TargetEndianness);
+    CBA.write<uint32_t>(Val, ELFT::Endian);
   for (uint32_t Val : *Section.Chain)
-    CBA.write<uint32_t>(Val, ELFT::TargetEndianness);
+    CBA.write<uint32_t>(Val, ELFT::Endian);
 
   SHeader.sh_size = (2 + Section.Bucket->size() + Section.Chain->size()) * 4;
 }
@@ -1687,8 +1687,8 @@ void ELFState<ELFT>::writeSectionContent(
     return;
 
   for (const ELFYAML::ARMIndexTableEntry &E : *Section.Entries) {
-    CBA.write<uint32_t>(E.Offset, ELFT::TargetEndianness);
-    CBA.write<uint32_t>(E.Value, ELFT::TargetEndianness);
+    CBA.write<uint32_t>(E.Offset, ELFT::Endian);
+    CBA.write<uint32_t>(E.Value, ELFT::Endian);
   }
   SHeader.sh_size = Section.Entries->size() * 8;
 }
@@ -1729,8 +1729,8 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
     return;
 
   for (const ELFYAML::DynamicEntry &DE : *Section.Entries) {
-    CBA.write<uintX_t>(DE.Tag, ELFT::TargetEndianness);
-    CBA.write<uintX_t>(DE.Val, ELFT::TargetEndianness);
+    CBA.write<uintX_t>(DE.Tag, ELFT::Endian);
+    CBA.write<uintX_t>(DE.Val, ELFT::Endian);
   }
   SHeader.sh_size = 2 * sizeof(uintX_t) * Section.Entries->size();
 }
@@ -1758,18 +1758,18 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
   for (const ELFYAML::NoteEntry &NE : *Section.Notes) {
     // Write name size.
     if (NE.Name.empty())
-      CBA.write<uint32_t>(0, ELFT::TargetEndianness);
+      CBA.write<uint32_t>(0, ELFT::Endian);
     else
-      CBA.write<uint32_t>(NE.Name.size() + 1, ELFT::TargetEndianness);
+      CBA.write<uint32_t>(NE.Name.size() + 1, ELFT::Endian);
 
     // Write description size.
     if (NE.Desc.binary_size() == 0)
-      CBA.write<uint32_t>(0, ELFT::TargetEndianness);
+      CBA.write<uint32_t>(0, ELFT::Endian);
     else
-      CBA.write<uint32_t>(NE.Desc.binary_size(), ELFT::TargetEndianness);
+      CBA.write<uint32_t>(NE.Desc.binary_size(), ELFT::Endian);
 
     // Write type.
-    CBA.write<uint32_t>(NE.Type, ELFT::TargetEndianness);
+    CBA.write<uint32_t>(NE.Type, ELFT::Endian);
 
     // Write name, null terminator and padding.
     if (!NE.Name.empty()) {
@@ -1803,35 +1803,35 @@ void ELFState<ELFT>::writeSectionContent(Elf_Shdr &SHeader,
   // be used to override this field, which is useful for producing broken
   // objects.
   if (Section.Header->NBuckets)
-    CBA.write<uint32_t>(*Section.Header->NBuckets, ELFT::TargetEndianness);
+    CBA.write<uint32_t>(*Section.Header->NBuckets, ELFT::Endian);
   else
-    CBA.write<uint32_t>(Section.HashBuckets->size(), ELFT::TargetEndianness);
+    CBA.write<uint32_t>(Section.HashBuckets->size(), ELFT::Endian);
 
   // Write the index of the first symbol in the dynamic symbol table accessible
   // via the hash table.
-  CBA.write<uint32_t>(Section.Header->SymNdx, ELFT::TargetEndianness);
+  CBA.write<uint32_t>(Section.Header->SymNdx, ELFT::Endian);
 
   // Write the number of words in the Bloom filter. As above, the "MaskWords"
   // property can be used to set this field to any value.
   if (Section.Header->MaskWords)
-    CBA.write<uint32_t>(*Section.Header->MaskWords, ELFT::TargetEndianness);
+    CBA.write<uint32_t>(*Section.Header->MaskWords, ELFT::Endian);
   else
-    CBA.write<uint32_t>(Section.BloomFilter->size(), ELFT::TargetEndianness);
+    CBA.write<uint32_t>(Section.BloomFilter->size(), ELFT::Endian);
 
   // Write the shift constant used by the Bloom filter.
-  CBA.write<uint32_t>(Section.Header->Shift2, ELFT::TargetEndianness);
+  CBA.write<uint32_t>(Section.Header->Shift2, ELFT::Endian);
 
   // We've finished writing the header. Now write the Bloom filter.
   for (llvm::yaml::Hex64 Val : *Section.BloomFilter)
-    CBA.write<uintX_t>(Val, ELFT::TargetEndianness);
+    CBA.write<uintX_t>(Val, ELFT::Endian);
 
   // Write an array of hash buckets.
   for (llvm::yaml::Hex32 Val : *Section.HashBuckets)
-    CBA.write<uint32_t>(Val, ELFT::TargetEndianness);
+    CBA.write<uint32_t>(Val, ELFT::Endian);
 
   // Write an array of hash values.
   for (llvm::yaml::Hex32 Val : *Section.HashValues)
-    CBA.write<uint32_t>(Val, ELFT::TargetEndianness);
+    CBA.write<uint32_t>(Val, ELFT::Endian);
 
   SHeader.sh_size = 16 /*Header size*/ +
                     Section.BloomFilter->size() * sizeof(typename ELFT::uint) +
diff --git a/llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h b/llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h
index 2e89463e68d519..d14b0b1bec34d8 100644
--- a/llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h
+++ b/llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h
@@ -113,7 +113,7 @@ void PrinterContext<ELFT>::printEHFrameHdr(const Elf_Phdr *EHFramePHdr) const {
   if (!Content)
     reportError(Content.takeError(), ObjF.getFileName());
 
-  DataExtractor DE(*Content, ELFT::TargetEndianness == llvm::endianness::little,
+  DataExtractor DE(*Content, ELFT::Endian == llvm::endianness::little,
                    ELFT::Is64Bits ? 8 : 4);
 
   DictScope D(W, "Header");
@@ -186,10 +186,9 @@ void PrinterContext<ELFT>::printEHFrame(const Elf_Shdr *EHFrameShdr) const {
   // Construct DWARFDataExtractor to handle relocations ("PC Begin" fields).
   std::unique_ptr<DWARFContext> DICtx = DWARFContext::create(
       ObjF, DWARFContext::ProcessDebugRelocations::Process, nullptr);
-  DWARFDataExtractor DE(DICtx->getDWARFObj(),
-                        DICtx->getDWARFObj().getEHFrameSection(),
-                        ELFT::TargetEndianness == llvm::endianness::little,
-                        ELFT::Is64Bits ? 8 : 4);
+  DWARFDataExtractor DE(
+      DICtx->getDWARFObj(), DICtx->getDWARFObj().getEHFrameSection(),
+      ELFT::Endian == llvm::endianness::little, ELFT::Is64Bits ? 8 : 4);
   DWARFDebugFrame EHFrame(Triple::ArchType(ObjF.getArch()), /*IsEH=*/true,
                           /*EHFrameAddress=*/Address);
   if (Error E = EHFrame.parse(DE))
diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index d1c05f437042de..954c07c634abfd 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -74,6 +74,7 @@
 
 using namespace llvm;
 using namespace llvm::object;
+using namespace llvm::support;
 using namespace ELF;
 
 #define LLVM_READOBJ_ENUM_CASE(ns, enum)                                       \
@@ -3419,13 +3420,12 @@ template <class ELFT> void ELFDumper<ELFT>::printStackMap() const {
     return;
   }
 
-  if (Error E = StackMapParser<ELFT::TargetEndianness>::validateHeader(
-          *ContentOrErr)) {
+  if (Error E = StackMapParser<ELFT::Endian>::validateHeader(*ContentOrErr)) {
     Warn(std::move(E));
     return;
   }
 
-  prettyPrintStackMap(W, StackMapParser<ELFT::TargetEndianness>(*ContentOrErr));
+  prettyPrintStackMap(W, StackMapParser<ELFT::Endian>(*ContentOrErr));
 }
 
 template <class ELFT>
@@ -5145,7 +5145,7 @@ static std::string getGNUProperty(uint32_t Type, uint32_t DataSize,
       OS << format("<corrupt length: 0x%x>", DataSize);
       return OS.str();
     }
-    PrData = support::endian::read32<ELFT::TargetEndianness>(Data.data());
+    PrData = endian::read32<ELFT::Endian>(Data.data());
     if (PrData == 0) {
       OS << "<None>";
       return OS.str();
@@ -5169,7 +5169,7 @@ static std::string getGNUProperty(uint32_t Type, uint32_t DataSize,
       OS << format("<corrupt length: 0x%x>", DataSize);
       return OS.str();
     }
-    PrData = support::endian::read32<ELFT::TargetEndianness>(Data.data());
+    PrData = endian::read32<ELFT::Endian>(Data.data());
     if (PrData == 0) {
       OS << "<None>";
       return OS.str();
@@ -5195,7 +5195,7 @@ static std::string getGNUProperty(uint32_t Type, uint32_t DataSize,
       OS << format("<corrupt length: 0x%x>", DataSize);
       return OS.str();
     }
-    PrData = support::endian::read32<ELFT::TargetEndianness>(Data.data());
+    PrData = endian::read32<ELFT::Endian>(Data.data());
     if (PrData == 0) {
       OS << "<None>";
       return OS.str();
@@ -5374,10 +5374,8 @@ static bool printAArch64Note(raw_ostream &OS, uint32_t NoteType,
     return false;
   }
 
-  uint64_t Platform =
-      support::endian::read64<ELFT::TargetEndianness>(Desc.data() + 0);
-  uint64_t Version =
-      support::endian::read64<ELFT::TargetEndianness>(Desc.data() + 8);
+  uint64_t Platform = endian::read64<ELFT::Endian>(Desc.data() + 0);
+  uint64_t Version = endian::read64<ELFT::Endian>(Desc.data() + 8);
   OS << format("platform 0x%" PRIx64 ", version 0x%" PRIx64, Platform, Version);
 
   if (Desc.size() > 16)
@@ -5457,16 +5455,14 @@ getFreeBSDNote(uint32_t NoteType, ArrayRef<uint8_t> Desc, bool IsCore) {
   case ELF::NT_FREEBSD_ABI_TAG:
     if (Desc.size() != 4)
       return std::nullopt;
-    return FreeBSDNote{
-        "ABI tag",
-        utostr(support::endian::read32<ELFT::TargetEndianness>(Desc.data()))};
+    return FreeBSDNote{"ABI tag",
+                       utostr(endian::read32<ELFT::Endian>(Desc.data()))};
   case ELF::NT_FREEBSD_ARCH_TAG:
     return FreeBSDNote{"Arch tag", toStringRef(Desc).str()};
   case ELF::NT_FREEBSD_FEATURE_CTL: {
     if (Desc.size() != 4)
       return std::nullopt;
-    unsigned Value =
-        support::endian::read32<ELFT::TargetEndianness>(Desc.data());
+    unsigned Value = endian::read32<ELFT::Endian>(Desc.data());
     std::string FlagsStr;
     raw_string_ostream OS(FlagsStr);
     printFlags(Value, ArrayRef(FreeBSDFeatureCtlFlags), OS);
@@ -6052,9 +6048,9 @@ template <class ELFT> void GNUELFDumper<ELFT>::printNotes() {
         return Error::success();
     } else if (Name == "CORE") {
       if (Type == ELF::NT_FILE) {
-        DataExtractor DescExtractor(
-            Descriptor, ELFT::TargetEndianness == llvm::endianness::little,
-            sizeof(Elf_Addr));
+        DataExtractor DescExtractor(Descriptor,
+                                    ELFT::Endian == llvm::endianness::little,
+                                    sizeof(Elf_Addr));
         if (Expected<CoreNote> NoteOrErr = readCoreNote(DescExtractor)) {
           printCoreNote<ELFT>(OS, *NoteOrErr);
           return Error::success();
@@ -7714,10 +7710,8 @@ static bool printAarch64NoteLLVMStyle(uint32_t NoteType, ArrayRef<uint8_t> Desc,
   if (Desc.size() < 16)
     return false;
 
-  uint64_t platform =
-      support::endian::read64<ELFT::TargetEndianness>(Desc.data() + 0);
-  uint64_t version =
-      support::endian::read64<ELFT::TargetEndianness>(Desc.data() + 8);
+  uint64_t platform = endian::read64<ELFT::Endian>(Desc.data() + 0);
+  uint64_t version = endian::read64<ELFT::Endian>(Desc.data() + 8);
   W.printNumber("Platform", platform);
   W.printNumber("Version", version);
 
@@ -7851,9 +7845,9 @@ template <class ELFT> void LLVMELFDumper<ELFT>::printNotes() {
         return Error::success();
     } else if (Name == "CORE") {
       if (Type == ELF::NT_FILE) {
-        DataExtractor DescExtractor(
-            Descriptor, ELFT::TargetEndianness == llvm::endianness::little,
-            sizeof(Elf_Addr));
+        DataExtractor DescExtractor(Descriptor,
+                                    ELFT::Endian == llvm::endianness::little,
+                                    sizeof(Elf_Addr));
         if (Expected<CoreNote> N = readCoreNote(DescExtractor)) {
           printCoreNoteLLVMStyle(*N, W);
           return Error::success();

Copy link
Collaborator

@jh7370 jh7370 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know the aim is to shorten the name, but if I'm not mistaken, the term should be "Endianness". I'm pretty sure I've never seen the term "Endian" used outside the context of "big-endian" or "little-endian".

Also, if you want to shorten the name, why create a separate alias, rather than just renaming the original term?

@MaskRay
Copy link
Member Author

MaskRay commented Mar 26, 2024

I know the aim is to shorten the name, but if I'm not mistaken, the term should be "Endianness". I'm pretty sure I've never seen the term "Endian" used outside the context of "big-endian" or "little-endian".

Thanks for the comment. I see that "endian" is used as an adjective. However, it seems used widely, e.g. the system header file endian.h. If I do a case-insensitive search for [ ]endian;, there are quite a few in-tree variables.

There are other cases phrases that are not nouns are used as variable names, e.g. old/previous/next, and Is64Bits. Do I have any chance to get the shorter Endian instead of Endianness? :)

Also, if you want to shorten the name, why create a separate alias, rather than just renaming the original term?

There are quite a few in-tree uses of TargetEndianness. While my long term goal is indeed to remove them, I plan to keep it here.

Edit: I have searched around and so far just found one out-of-tree ELFT::TargetEndianness user: github.com/ydb-platform/nbs

@jh7370
Copy link
Collaborator

jh7370 commented Mar 27, 2024

TL;DR: I don't think we should be using the term "endian" in this context, and I think existing usages of it as a variable name is a mistake/not-conformant with our coding standards. It also saves a whole 4 characters, which, frankly, you shouldn't be worrying about. I can get behind dropping the "Target" prefix, since it is redundant in the context. "Endianness" is the more correct term and is much more widely used in this kind of context than "endian".

I also think it would be better to just do a blanket rename rather than create an alias and have a migration period. Out-of-tree targets are very unlikely to switch until forced. In-tree code is entirely within our control and there's no benefit in staggering the rename locally.


I fell down a linguistic rabbit hole: I did some searching of various online dictionaries, including one computing-specific one, and the term "endian" either a) doesn't appear at all, b) appears but only in the form "big-endian" and "little-endian", or c) in one sole case, standalone, but only as a suffix (so not really standalone). Wikipedia's page on the topic uses the term "endianness" only. I was about to declare that "endian" isn't even really a word, and the usage within LLVM's codebase is a mistake. Then I stumbled upon the C++20 std::endian enum. So it looks like the committee has decreed that "endian" is a legitimate term (though note that the discussion https://groups.google.com/a/isocpp.org/g/std-proposals/c/Hx4ipachs1E/m/yCxriKIvEgAJ and linked proposal primarily used "endianness").

LLVM coding standard states that variables should be nouns. There's a clear precedent for ignoring this for boolean-like types, to allow these to be phrases (e.g. Is64Bits), so something like IsLittleEndian is fine, but that's not applicable here. The case of next/previous is a prior art thing due to traditional linked list-like structures. I'm not sure I'd personally ever use a member name old (I'd use a full name like oldThing). Either way, that case doesn't apply here either.

@MaskRay MaskRay changed the title [Object] Define ELFType::Endian as alias for TargetEndianness [Object,ELFType] Rename TargetEndianness to Endianness Mar 27, 2024
@MaskRay
Copy link
Member Author

MaskRay commented Mar 27, 2024

Thank you for the research. I've switched from "Endian" to "Endianness". While 4 character longer, this PR will still remove 6 characters (Target, which puzzled me: Is there a host != target difference?)...

@arichardson
Copy link
Member

I guess the slightly shorter version could be ByteOrder but endianess is probably better. I agree that the Target prefix does not make any sense since it's the endianess for a given elf type and is unrelated to host/target.

Copy link
Collaborator

@jh7370 jh7370 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@MaskRay MaskRay merged commit 2763353 into main Mar 28, 2024
4 checks passed
@MaskRay MaskRay deleted the users/MaskRay/spr/object-define-elftypeendian-as-alias-for-targetendianness branch March 28, 2024 16:10
MaskRay added a commit that referenced this pull request Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants