[PATCH] D78005: [yaml2obj] - Reimplement how tool calculates memory sizes of segments.

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM, but I'm happy to dicuss @MaskRay's suggestion too.

In D78005#2093321 <https://reviews.llvm.org/D78005#2093321>, @MaskRay wrote:

>   for (size_t I = 0, E = Fragments.size(); I != E; ++I) {
>     const Fragment &Cur = Fragments[I];
>     const Fragment *Next = (I + 1 < E) ? &Fragments[I + 1] : nullptr;
>   
>     if (Next)
>       MemSize += std::max(Next->Offset - Cur.Offset, Cur.Size); ///////////// [1]
>     else
>       MemSize += Cur.Size;
>   }
>   
>
> [1] is probably less than ideal.  If a middle section has erroneous large sh_size (for example, due to ShSize:), we may want to ignore it when computing p_memsz. If we take `Next->Offset - Cur.Offset` contribution from one section and `Cur.Size` contribution from another, the sum will represent a value which is hard to explain.
>
> We probably should ignore sh_offset and use sh_addr when computing p_memsz:
>
>   p->p_memsz = last->addr + last->size - first->addr;
>   


My personal opinion is that we should not pay any attention to ShSize (or indeed any similar properties like ShOffset etc) for any calculation except for the value that is finally written to the section header table. Thus, I don't think we need to worry about the case you're suggesting (i.e. with an erroneous size).


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

https://reviews.llvm.org/D78005