[llvm-readelf/obj] - Deduplicate the logic that prints notes. NFCI.

[grimar]

Summary

We have a similar logic for LLVM/GNU styles that can be deduplicated.
This will allow to replace reportError calls with reportUniqueWarning
calls in a single place.

[jhenderson]

llvm/tools/llvm-readobj/ELFDumper.cpp

6847

I might be missing something, but I don't see this scoping in the old behaviour? Without it, we also wouldn't need the EndNotes heper.

6847

Never mind... spotted it. Will look a bit more.

[jhenderson]

Looks okay. I'm not a massive fan of having three lambdas though, and I wonder whether they'd be better off as virtual functions, at least to some extent, of the dump style classes (probably only for the ProcessNote one though, I accept).

[nlguillemot]

llvm/tools/llvm-readobj/ELFDumper.cpp

5562

This line gives a warning with clang's "range-loop analysis" diagnostic:

error: loop variable 'Note' is always a copy because the range of type 'iterator_range<llvm::object::ELFFile<llvm::object::ELFType<llvm::support::big, true> >::Elf_Note_Iterator>' (aka 'iterator_range<Elf_Note_Iterator_Impl<ELFType<(llvm::support::endianness)0U, true> > >') does not return a reference [-Werror,-Wrange-loop-analysis]
      for (const typename ELFT::Note &Note : Obj.notes(S, Err))

It can be fixed by removing the ampersand.

[grimar]

llvm/tools/llvm-readobj/ELFDumper.cpp

5562

Fixed in rGc3673ea65df576535c85397dfe36624bd09fe47b, thanks!

llvm/tools/llvm-readobj/ELFDumper.cpp

const StringRef getNoteTypeName(const typename ELFT::Note &Note,
   if (Name == "AMDGPU")
     return FindNote(AMDGPUNoteTypes);
 
   if (ELFType == ELF::ET_CORE)
     return FindNote(CoreNoteTypes);
   return FindNote(GenericNoteTypes);
 }
 
+template <class ELFT>
+static void printNotesHelper(
+    const ELFDumper<ELFT> &Dumper,
+    llvm::function_ref<void(Optional<StringRef>, typename ELFT::Off,
+                            typename ELFT::Addr)>
+        StartNotesFn,
+    llvm::function_ref<void(const typename ELFT::Note &)> ProcessNoteFn,
+    llvm::function_ref<void()> FinishNotesFn = []() {}) {
+  const ELFFile<ELFT> &Obj = *Dumper.getElfObject().getELFFile();
+
+  ArrayRef<typename ELFT::Shdr> Sections = cantFail(Obj.sections());
+  if (Obj.getHeader().e_type != ELF::ET_CORE && !Sections.empty()) {
+    for (const typename ELFT::Shdr &S : Sections) {
+      if (S.sh_type != SHT_NOTE)
+        continue;
+      StartNotesFn(expectedToOptional(Obj.getSectionName(S)), S.sh_offset,
+                   S.sh_size);
+      Error Err = Error::success();
+      for (const typename ELFT::Note &Note : Obj.notes(S, Err))
+        ProcessNoteFn(Note);
+      if (Err)
+        reportError(std::move(Err), Dumper.getElfObject().getFileName());
+      FinishNotesFn();
+    }
+    return;
+  }
+
+  Expected<ArrayRef<typename ELFT::Phdr>> PhdrsOrErr = Obj.program_headers();
+  if (!PhdrsOrErr) {
+    Dumper.reportUniqueWarning(createError(
+        "unable to read program headers to locate the PT_NOTE segment: " +
+        toString(PhdrsOrErr.takeError())));
+    return;
+  }
+
+  for (const typename ELFT::Phdr &P : *PhdrsOrErr) {
+    if (P.p_type != PT_NOTE)
+      continue;
+    StartNotesFn(/*SecName=*/None, P.p_offset, P.p_filesz);
+    Error Err = Error::success();
+    for (const typename ELFT::Note Note : Obj.notes(P, Err))
+      ProcessNoteFn(Note);
+    if (Err)
+      reportError(std::move(Err), Dumper.getElfObject().getFileName());
+    FinishNotesFn();
+  }
+}
+
 template <class ELFT> void GNUStyle<ELFT>::printNotes() {
   auto PrintHeader = [&](Optional<StringRef> SecName,
                          const typename ELFT::Off Offset,
                          const typename ELFT::Addr Size) {
     OS << "Displaying notes found ";
 
     if (SecName)
       OS << "in: " << *SecName << "\n";
auto ProcessNote = [&](const Elf_Note &Note) {
     } else if (!Descriptor.empty()) {
       OS << "   description data:";
       for (uint8_t B : Descriptor)
         OS << " " << format("%02x", B);
       OS << '\n';
     }
   };
 
-  ArrayRef<Elf_Shdr> Sections = cantFail(this->Obj.sections());
-  if (this->Obj.getHeader().e_type != ELF::ET_CORE && !Sections.empty()) {
-    for (const Elf_Shdr &S : Sections) {
-      if (S.sh_type != SHT_NOTE)
-        continue;
-      PrintHeader(expectedToOptional(this->Obj.getSectionName(S)), S.sh_offset,
-                  S.sh_size);
-      Error Err = Error::success();
-      for (const Elf_Note Note : this->Obj.notes(S, Err))
-        ProcessNote(Note);
-      if (Err)
-        reportError(std::move(Err), this->FileName);
-    }
-  } else {
-    Expected<ArrayRef<Elf_Phdr>> PhdrsOrErr = this->Obj.program_headers();
-    if (!PhdrsOrErr) {
-      this->reportUniqueWarning(createError(
-          "unable to read program headers to locate the PT_NOTE segment: " +
-          toString(PhdrsOrErr.takeError())));
-      return;
-    }
-
-    for (const Elf_Phdr &P : *PhdrsOrErr) {
-      if (P.p_type != PT_NOTE)
-        continue;
-      PrintHeader(/*SecName=*/None, P.p_offset, P.p_filesz);
-      Error Err = Error::success();
-      for (const Elf_Note Note : this->Obj.notes(P, Err))
-        ProcessNote(Note);
-      if (Err)
-        reportError(std::move(Err), this->FileName);
-    }
-  }
+  printNotesHelper(this->dumper(), PrintHeader, ProcessNote);
 }
 
 template <class ELFT> void GNUStyle<ELFT>::printELFLinkerOptions() {
   OS << "printELFLinkerOptions not implemented!\n";
 }
 
 template <class ELFT>
 void DumpStyle<ELFT>::printDependentLibsHelper(
for (const CoreFileMapping &Mapping : Note.Mappings) {
     W.printHex("Offset", Mapping.Offset);
     W.printString("Filename", Mapping.Filename);
   }
 }
 
 template <class ELFT> void LLVMStyle<ELFT>::printNotes() {
   ListScope L(W, "Notes");
 
-  auto PrintHeader = [&](Optional<StringRef> SecName,
-                         const typename ELFT::Off Offset,
-                         const typename ELFT::Addr Size) {
+  std::unique_ptr<DictScope> NoteScope;
+  auto StartNotes = [&](Optional<StringRef> SecName,
+    NoteScope = std::make_unique<DictScope>(W, "NoteSection");
     W.printString("Name", SecName ? *SecName : "<?>");
     W.printHex("Offset", Offset);
     W.printHex("Size", Size);
   };
 
+  auto EndNotes = [&] { NoteScope.reset(); };
+
   auto ProcessNote = [&](const Elf_Note &Note) {
     DictScope D2(W, "Note");
     StringRef Name = Note.getName();
     ArrayRef<uint8_t> Descriptor = Note.getDesc();
     Elf_Word Type = Note.getType();
 
     // Print the note owner/type.
     W.printString("Owner", Name);
if (Name == "GNU") {
         else
           reportWarning(Note.takeError(), this->FileName);
       }
     } else if (!Descriptor.empty()) {
       W.printBinaryBlock("Description data", Descriptor);
     }
   };
 
-  ArrayRef<Elf_Shdr> Sections = cantFail(this->Obj.sections());
-  if (this->Obj.getHeader().e_type != ELF::ET_CORE && !Sections.empty()) {
-    for (const Elf_Shdr &S : Sections) {
-      if (S.sh_type != SHT_NOTE)
-        continue;
-      DictScope D(W, "NoteSection");
-      PrintHeader(expectedToOptional(this->Obj.getSectionName(S)), S.sh_offset,
-                  S.sh_size);
-      Error Err = Error::success();
-      for (auto Note : this->Obj.notes(S, Err))
-        ProcessNote(Note);
-      if (Err)
-        reportError(std::move(Err), this->FileName);
-    }
-  } else {
-    Expected<ArrayRef<Elf_Phdr>> PhdrsOrErr = this->Obj.program_headers();
-    if (!PhdrsOrErr) {
-      this->reportUniqueWarning(createError(
-          "unable to read program headers to locate the PT_NOTE segment: " +
-          toString(PhdrsOrErr.takeError())));
-      return;
-    }
-
-    for (const Elf_Phdr &P : *PhdrsOrErr) {
-      if (P.p_type != PT_NOTE)
-        continue;
-      DictScope D(W, "NoteSection");
-      PrintHeader(/*SecName=*/None, P.p_offset, P.p_filesz);
-      Error Err = Error::success();
-      for (auto Note : this->Obj.notes(P, Err))
-        ProcessNote(Note);
-      if (Err)
-        reportError(std::move(Err), this->FileName);
-    }
-  }
+  printNotesHelper(this->dumper(), StartNotes, ProcessNote, EndNotes);
 }
 
 template <class ELFT> void LLVMStyle<ELFT>::printELFLinkerOptions() {
   ListScope L(W, "LinkerOptions");
 
   unsigned I = -1;
   for (const Elf_Shdr &Shdr : cantFail(this->Obj.sections())) {
     ++I;