[PATCH] D78709: [llvm-readobj] - Simplify conditions used for printing segment mappings. NFCI.
George Rimar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 24 05:22:23 PDT 2020
grimar marked an inline comment as done.
grimar added inline comments.
================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:4083
+
+ // No section with zero size must be at the start or at the end of PT_DYNAMIC.
return ((Sec.sh_type == ELF::SHT_NOBITS) ||
----------------
grimar wrote:
> jhenderson wrote:
> > grimar wrote:
> > > jhenderson wrote:
> > > > This comment belongs with the above if, I think.
> > > I think no. It seems to be about
> > >
> > > "(Sec.sh_offset > Phdr.p_offset && Sec.sh_offset < Phdr.p_offset + Phdr.p_filesz)"
> > >
> > > One of conditions to get here is to have "Sec.sh_size == 0".
> > This comment is to do with sections with zero size and PT_DYNAMIC, so the comment belongs with the zero size/PT_DYNAMIC check surely?
> See:
>
> `if (Phdr.p_type != ELF::PT_DYNAMIC || Phdr.p_memsz == 0 || Sec.sh_size != 0)` above means that
> we can get here only when a segment is `PT_DYNAMIC` and section has zero size.
>
> The comment says "No section with zero size must be at the start or at the end of PT_DYNAMIC"
>
> It is related to the following condition:
> ```
> (Sec.sh_offset > Phdr.p_offset && Sec.sh_offset < Phdr.p_offset + Phdr.p_filesz)
> ```
>
> Note that `>` and `<` are used here instead of `>=` and `<=`. I.e. it checks that the empty section is not at the
> start or at the end. Just like comment says.
>
> Right?
I think I'll be able to improve comments in this method to avoid confusion.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78709/new/
https://reviews.llvm.org/D78709
More information about the llvm-commits
mailing list