[PATCH] D54369: [llvm-size][libobject] Add explicit "inTextSegment" methods similar to "isText" section methods to calculate size correctly.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: include/llvm/Object/ELFObjectFile.h:767
+         (getSection(Sec)->sh_flags & ELF::SHF_EXECINSTR ||
+          (!(getSection(Sec)->sh_flags & ELF::SHF_WRITE)));
+}
----------------
MaskRay wrote:
> The parentheses outside of `!` are redundant.
> 
> `GNU size` uses Berkeley format by default (`static int berkeley_format = BSD_DEFAULT;`)
> I'd say it uses a weird counting https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=binutils/size.c;h=3c72e484752d0272a24ac628f497f89ecf36d547;hb=refs/heads/master#l459
> 
>  459   if ((flags & SEC_CODE) != 0 || (flags & SEC_READONLY) != 0)
>  460     textsize += size;
>  461   else if ((flags & SEC_HAS_CONTENTS) != 0)
>  462     datasize += size;
>  463   else
>  464     bsssize += size;
> 
> Basically `SHF_EXECINSTR` sections (`.text`) + `!SHF_WRITE` sections (`.rodata .eh_frame` ...).
> 
> I feel the name `inTextSegment` is a bit misleading. How about `isBerkeleyText`?
> 
> `inDataSegment` may be renamed to `isBerkeleyData`.
The Berkeley format is not that weird. It uses some simple condition to approximate how the traditional linkers partition sections into segments.

With the advent of fine-grained segments (split of `R` and `RX`): `-z separate-code` in ld.bfd and lld's default (unless `--no-rosegment`), the computation may be less relevant. `isBerkeleyText` makes more sense to me.


Repository:
  rL LLVM

https://reviews.llvm.org/D54369