[ELF] Do not omit debug sections when computing build-id

[phosek]

Summary

The primary use of build-id is in debugging, hence omitting debug sections when computing it significantly reduces its usability as changes in debug section content wouldn't alter the build-id.

[phosek]

This was pointed out by Roland McGrath who proposed the build-id feature and did the original implementation in BFD ld. The original feature proposal can be found here.

[ruiu]

It is unfortunate that this speed hack didn't work.

ELF/Writer.cpp

1323 ↗

(On Diff #69945)

... of the output file.

1328 ↗

(On Diff #69945)

Please simplify writeBuildId so that it takes just a uint8_t pointer and size instead of an array of arrays.

[phosek]

ELF/Writer.cpp

1328 ↗

(On Diff #69945)

I've changed it to use ArrayRef<uint8_t>, do you prefer to use pointer and size instead?

[ruiu]

LGTM

ELF/Writer.cpp

1325–1326 ↗

(On Diff #70080)

I'm fine in either way.

lld/trunk/ELF/OutputSections.h

private:
   std::vector<FdeData> Fdes;
 };
 
 template <class ELFT> class BuildIdSection : public OutputSectionBase<ELFT> {
   typedef OutputSectionBase<ELFT> Base;
 
 public:
   void writeTo(uint8_t *Buf) override;
-  virtual void writeBuildId(ArrayRef<ArrayRef<uint8_t>> Bufs) = 0;
+  virtual void writeBuildId(ArrayRef<uint8_t> Buf) = 0;
   typename Base::Kind getKind() const override { return Base::BuildId; }
   static bool classof(const Base *B) { return B->getKind() == Base::BuildId; }
 
 protected:
   BuildIdSection(size_t HashSize);
   size_t HashSize;
   uint8_t *HashBuf = nullptr;
 };
 
 template <class ELFT> class BuildIdFnv1 final : public BuildIdSection<ELFT> {
 public:
   BuildIdFnv1() : BuildIdSection<ELFT>(8) {}
-  void writeBuildId(ArrayRef<ArrayRef<uint8_t>> Bufs) override;
+  void writeBuildId(ArrayRef<uint8_t> Buf) override;
 };
 
 template <class ELFT> class BuildIdMd5 final : public BuildIdSection<ELFT> {
 public:
   BuildIdMd5() : BuildIdSection<ELFT>(16) {}
-  void writeBuildId(ArrayRef<ArrayRef<uint8_t>> Bufs) override;
+  void writeBuildId(ArrayRef<uint8_t> Buf) override;
 };
 
 template <class ELFT> class BuildIdSha1 final : public BuildIdSection<ELFT> {
 public:
   BuildIdSha1() : BuildIdSection<ELFT>(20) {}
-  void writeBuildId(ArrayRef<ArrayRef<uint8_t>> Bufs) override;
+  void writeBuildId(ArrayRef<uint8_t> Buf) override;
 };
 
 template <class ELFT> class BuildIdUuid final : public BuildIdSection<ELFT> {
 public:
   BuildIdUuid() : BuildIdSection<ELFT>(16) {}
-  void writeBuildId(ArrayRef<ArrayRef<uint8_t>> Bufs) override;
+  void writeBuildId(ArrayRef<uint8_t> Buf) override;
 };
 
 template <class ELFT>
 class BuildIdHexstring final : public BuildIdSection<ELFT> {
 public:
   BuildIdHexstring();
-  void writeBuildId(ArrayRef<ArrayRef<uint8_t>> Bufs) override;
+  void writeBuildId(ArrayRef<uint8_t>) override;
 };
 
 // All output sections that are hadnled by the linker specially are
 // globally accessible. Writer initializes them, so don't use them
 // until Writer is initialized.
 template <class ELFT> struct Out {
   typedef typename ELFT::uint uintX_t;
   typedef typename ELFT::Phdr Elf_Phdr;

lld/trunk/ELF/OutputSections.cpp

template <class ELFT> void BuildIdSection<ELFT>::writeTo(uint8_t *Buf) {
   write32<E>(Buf, 4);                   // Name size
   write32<E>(Buf + 4, HashSize);        // Content size
   write32<E>(Buf + 8, NT_GNU_BUILD_ID); // Type
   memcpy(Buf + 12, "GNU", 4);           // Name string
   HashBuf = Buf + 16;
 }
 
 template <class ELFT>
-void BuildIdFnv1<ELFT>::writeBuildId(ArrayRef<ArrayRef<uint8_t>> Bufs) {
+void BuildIdFnv1<ELFT>::writeBuildId(ArrayRef<uint8_t> Buf) {
   const endianness E = ELFT::TargetEndianness;
 
   // 64-bit FNV-1 hash
   uint64_t Hash = 0xcbf29ce484222325;
-  for (ArrayRef<uint8_t> Buf : Bufs) {
-    for (uint8_t B : Buf) {
-      Hash *= 0x100000001b3;
-      Hash ^= B;
-    }
-  }
   write64<E>(this->HashBuf, Hash);
 }
 
 template <class ELFT>
-void BuildIdMd5<ELFT>::writeBuildId(ArrayRef<ArrayRef<uint8_t>> Bufs) {
+void BuildIdMd5<ELFT>::writeBuildId(ArrayRef<uint8_t> Buf) {
   MD5 Hash;
-  for (ArrayRef<uint8_t> Buf : Bufs)
-    Hash.update(Buf);
   MD5::MD5Result Res;
   Hash.final(Res);
   memcpy(this->HashBuf, Res, 16);
 }
 
 template <class ELFT>
-void BuildIdSha1<ELFT>::writeBuildId(ArrayRef<ArrayRef<uint8_t>> Bufs) {
+void BuildIdSha1<ELFT>::writeBuildId(ArrayRef<uint8_t> Buf) {
   SHA1 Hash;
-  for (ArrayRef<uint8_t> Buf : Bufs)
-    Hash.update(Buf);
   memcpy(this->HashBuf, Hash.final().data(), 20);
 }
 
 template <class ELFT>
-void BuildIdUuid<ELFT>::writeBuildId(ArrayRef<ArrayRef<uint8_t>> Bufs) {
+void BuildIdUuid<ELFT>::writeBuildId(ArrayRef<uint8_t> Buf) {
   if (getRandomBytes(this->HashBuf, 16))
     error("entropy source failure");
 }
 
 template <class ELFT>
 BuildIdHexstring<ELFT>::BuildIdHexstring()
     : BuildIdSection<ELFT>(Config->BuildIdVector.size()) {}
 
 template <class ELFT>
-void BuildIdHexstring<ELFT>::writeBuildId(ArrayRef<ArrayRef<uint8_t>> Bufs) {
+void BuildIdHexstring<ELFT>::writeBuildId(ArrayRef<uint8_t> Buf) {
   memcpy(this->HashBuf, Config->BuildIdVector.data(),
          Config->BuildIdVector.size());
 }
 
 template <class ELFT>
 MipsReginfoOutputSection<ELFT>::MipsReginfoOutputSection()
     : OutputSectionBase<ELFT>(".reginfo", SHT_MIPS_REGINFO, SHF_ALLOC) {
   this->Header.sh_addralign = 4;

lld/trunk/ELF/Writer.cpp

template <class ELFT> void Writer<ELFT>::writeSections() {
   if (!Out<ELFT>::EhFrame->empty() && Out<ELFT>::EhFrameHdr)
     Out<ELFT>::EhFrameHdr->writeTo(Buf + Out<ELFT>::EhFrameHdr->getFileOff());
 }
 
 template <class ELFT> void Writer<ELFT>::writeBuildId() {
   if (!Out<ELFT>::BuildId)
     return;
 
-  // Compute a hash of all sections except .debug_* sections.
-  // We skip debug sections because they tend to be very large
-  // and their contents are very likely to be the same as long as
-  // other sections are the same.
+  // Compute a hash of all sections of the output file.
   uint8_t *Start = Buffer->getBufferStart();
-  uint8_t *Last = Start;
-  std::vector<ArrayRef<uint8_t>> Regions;
-  for (OutputSectionBase<ELFT> *Sec : OutputSections) {
-    uint8_t *End = Start + Sec->getFileOff();
-    if (!Sec->getName().startswith(".debug_"))
-      Regions.push_back({Last, End});
-    Last = End;
-  }
-  Regions.push_back({Last, Start + FileSize});
-  Out<ELFT>::BuildId->writeBuildId(Regions);
+  uint8_t *End = Start + FileSize;
+  Out<ELFT>::BuildId->writeBuildId({Start, End});
 }
 
 template void elf::writeResult<ELF32LE>();
 template void elf::writeResult<ELF32BE>();
 template void elf::writeResult<ELF64LE>();
 template void elf::writeResult<ELF64BE>();
 
 template struct elf::PhdrEntry<ELF32LE>;