[PATCH] D67547: [llvm-readobj/llvm-objdump] - Improve how tool locate the dynamic table and report warnings about that.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:1500
                   DynamicSec->sh_size, sizeof(Elf_Dyn), ObjF->getFileName()});
-    parseDynamicTable();
+    IsSecTableValid = !FromSec.getAsArrayRef<Elf_Dyn>().empty();
   }
----------------
grimar wrote:
> MaskRay wrote:
> > The logic is:
> > 
> > ```
> > findDynamic
> >   find PT_DYNAMIC
> >   find .dynamic
> >   if p_offset is out of bounds
> >     reportWarning
> >   if both PT_DYNAMIC and .dynamic exist
> >     check if the information matches
> > 
> > loadDynamicTable
> >   findDynamic()
> >   if PT_DYNAMIC exists
> >     IsPhdrTableValid = PT_DYNAMIC's DynRegionInfo is not empty
> >   if .dynamic exists
> >     IsSecTableValid = .dynamic's DynRegionInfo is not empty
> >   if either does not exist
> >     if (IsPhdrTableValid && IsSecTableValid)
> >       parseDynamicTable()
> >     else
> >       warn
> >     return
> > 
> >   validate information matches
> >   ...
> > ```
> > 
> > Shall we check whether p_offset is out of bounds and whether DynRegionInfo of PT_DYNAMIC is empty together?
> > 
> > ```
> > findDynamic
> >   find PT_DYNAMIC
> >   IsPhdrTableValid = whether PT_DYNAMIC exists
> >   if p_offset is out of bounds or DynRegionInfo not empty
> >     reportWarning
> >     IsPhdrTableValid = false
> > 
> >   find .dynamic
> >   validate .dynamic
> > 
> >   if either does not exist
> >     if ((DynamicPhdr && IsPhdrTableValid) || (DynamicSec && IsSecTableValid))
> >       parseDynamicTable()
> >     else
> >       warn
> >     return
> > 
> >   ...
> >   validate information matches
> >   parseDynamicTable()
> > ```
> I am not sure I understand what you suggest, sorry, Do you want to merge 2 existing methods into one large one?
> (I do not understand why your `findDynamic` might call `arseDynamicTable()` for example).
> Do you want to merge 2 existing methods into one large one?

Yes. Locate PT_DYNAMIC, and do all PT_DYNAMIC specific checks. Then, locate .dynamic, and do all .dynamic related checks. Last, check whether PT_DYNAMIC and .dynamic mismatch.


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

https://reviews.llvm.org/D67547