[PATCH] D49369: [llvm-readobj] - Teach tool to dump objects with >= SHN_LORESERVE of sections.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:2492-2493
+  ArrayRef<typename ELFT::Shdr> Arr = unwrapOrError(Obj->sections());
+  if (Arr.empty())
+    return "0 (corrupt: out of range)";
+  return to_string(ElfHeader->e_shstrndx) + " (" + to_string(Arr[0].sh_link) + ")";
----------------
jhenderson wrote:
> grimar wrote:
> > jhenderson wrote:
> > > I'm not seeing this behaviour in readelf? If there are no section headers, I just get a value of 0 for the section header string table index, and no warning. A non-zero value on the other hand, when there are no sections (or indeed any value outside that range) does give me that error.
> > I tested many-sections-stripped.elf-x86_64
> > (shared it here: https://drive.google.com/open?id=1HgUSb15a6WQY_lwMIUCDXBxD25VX8I-S)
> > which has e_shoff set to zero. And it has e_shstrndx == SHN_XINDEX.
> > 
> > It reports the following for me:
> > 
> > ```
> > readelf -v
> > GNU readelf (GNU Binutils for Ubuntu) 2.28
> > ...
> > 
> > readelf -a many-sections-stripped.elf-x86_64 
> > ELF Header:
> > ...
> >   Number of section headers:         0
> >   Section header string table index: 65535 <corrupt: out of range>
> > ```
> > 
> > I think when we have e_shstrndx == SHN_XINDEX, we expect that "The actual number of section header table entries is contained in the sh_size field of the section header at index 0". And since actual number is always > 0 ("If the number of sections is greater than or equal to SHN_LORESERVE...").
> > (https://docs.oracle.com/cd/E19120-01/open.solaris/819-0690/chapter6-43405/index.html),
> > we are fine reporting "out of range" here when having no sections. Isn't that look valid behavior?
> > 
> I think we might be misunderstanding each other. I think these are the cases we need to be aware of:
> 
> 1) The regular case where 0 < e_shstrndx < SHN_XINDEX. Here, we should (and do already) print the correct number.
> 2) The regular large number of sections case where e_shrstndx == SHN_XINDEX. Here we should look up the number and print it along with SHN_XINDEX. This is what your patch implements, and is tested by LLVM1 etc.
> 3) The case where e_shstrndx is zero. This means that there is no section header string table. We should just print zero in this case.
> 
> In cases one and two, if the index is greater than the number of sections, we should print an error. In a stripped ELF, there is no reason for a section header string table, so the index should be 0, like case 3), and we should not print an error, even if there were many section headers previously. Currently you are printing "0 (corrupt: out of range)", even though the e_shstrndx field is non-zero. I would expect it to print "65535 (corrupt: out of range)" or similar. So I think the printing of '0' is the bad part, because it hides the actual value in the field.
> 
> Does that make sense?
> So I think the printing of '0' is the bad part, because it hides the actual value in the field.

Ah, I missed that I am printing `0 (corrupt: out of range)` now.
My intention sure was to print the value, so to print the `65535 (corrupt: out of range)`.

I'll update the patch.


https://reviews.llvm.org/D49369