[PATCH] D64987: [Object/ELF.h] - Improve testing of the fields in ELFFile<ELFT>::sections().

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: include/llvm/Object/ELF.h:522
+  if (SectionTableOffset + SectionTableSize < SectionTableOffset)
+    return createError("invalid number of sections specified in the NULL "
+                       "section's sh_size field (" +
----------------
I think this error might be easier to understand with some rewording. How about:

"Invalid section header table offset (e_shoff = 0x1234) or invalid number of sections specified in the first section header's sh_size field (0x4321)."


================
Comment at: include/llvm/Object/ELF.h:525
+                       Twine(NumSections) + ") or e_shoff (" +
+                       Twine(SectionTableOffset) + ")");
   // Section table goes past end of file!
----------------
e_shoff should probably be printed in hex, since it's an offset.


================
Comment at: test/Object/invalid.test:556
+
+## We report a error if the number of sections stored in sh_size
+## is greater than UINT64_MAX / sizeof(Elf_Shdr) = 288230376151711743.
----------------
an error


================
Comment at: test/Object/invalid.test:557
+## We report a error if the number of sections stored in sh_size
+## is greater than UINT64_MAX / sizeof(Elf_Shdr) = 288230376151711743.
+## Here we check that do not crash on a border value.
----------------
How about == instead of =?


================
Comment at: test/Object/invalid.test:593
+  - Type: SHT_NULL
+    Size: 288230376151711744
----------------
I think there's one more case to test, if possible: a very high e_shoff value (e.g. 0xfffffffffffffff), combined with a small number of section headers (e.g. 1).

This assumes it's possible in yaml2obj to explicitly specify an e_shoff field without affecting the file size (I can't remember if you added that ability).


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

https://reviews.llvm.org/D64987