[PATCH] D150022: [Object] Fix handling of Elf_Nhdr with sh_addralign=8

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/include/llvm/Object/ELF.h:325
+    return Elf_Note_Iterator(base() + Phdr.p_offset, Phdr.p_filesz,
+                             std::max<size_t>(Phdr.p_align, 4), Err);
   }
----------------
jhenderson wrote:
> Nit: this probably wants to be a `uint64_t` rather than `size_t` max, right (since p_align could be `uint64_t` and therefore a larger type on 32-bit hosts than `size_t`)?
We have excluded larger `p_align` values, so using `size_t` is fine. `ELFTypes.h Elf_Nhdr_Impl::getSize` returns a `size_t` and the choice is to match its return value.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5827
     llvm::function_ref<void(std::optional<StringRef>, typename ELFT::Off,
-                            typename ELFT::Addr)>
+                            typename ELFT::Addr, size_t)>
         StartNotesFn,
----------------
jhenderson wrote:
> Similar to the earlier comment, should this be `uint64_t`? Same goes for other cases in the file.
This should be fine, since we've excluded large `p_align` values and reported an error very early.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150022/new/

https://reviews.llvm.org/D150022