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

[George Rimar via Phabricator via llvm-commits]

grimar marked an inline comment as done.
grimar 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();
   }
----------------
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).


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

https://reviews.llvm.org/D67547