[PATCH] D62838: [Object] add isExecutableObject member function

[Alex Brachet via Phabricator via llvm-commits]

abrachet marked an inline comment as done.
abrachet added inline comments.


================
Comment at: llvm/include/llvm/Object/ELFObjectFile.h:1203
+template <class ELFT> bool ELFObjectFile<ELFT>::isExecutableObject() const {
+  return EF.getHeader()->e_type == ELF::ET_EXEC;
+}
----------------
MaskRay wrote:
> abrachet wrote:
> > MaskRay wrote:
> > > Some people will be surprised if `ET_DYN` is not considered as executable object.
> > > 
> > > Position-Independent Executables use `ET_DYN`.
> > $ llvm-readelf `which llvm-readelf` -h
> > ...
> > Type:                              DYN (Shared object file)
> > 
> > Oh yeah, good catch, thank you. How would you write this method? Does an executable always have e_entry as non zero? Is there ever a case where execve(2) will not fail if e_entry is 0? 
> > 
> > Also, should I worry about this? https://github.com/freebsd/freebsd/blob/master/sys/kern/imgact_elf.c#L1135 I don't know enough about ELF yet on this one. Thanks.
> Do you have a use case (not a unittest) of this member function?
> 
> If you have one, let's see if relocatable/executable/shared should be trichotomy or not.
> 
> Trichotomy doesn't work in ELF. One may argue a PIE doesn't have PT_INTERP but a DSO doesn't. However, a static pie doesn't have PT_INTERP, either. Its difference from a DSO is if DT_DEBUG exists. However, DT_DEBUG can be disabled in lld with -z rodynamic (Fuchsia people do so)...
> 
> > Also, should I worry about this? https://github.com/freebsd/freebsd/blob/master/sys/kern/imgact_elf.c#L1135
> 
> We should not be bothered by FreeBSD Brandinfo
> Do you have a use case (not a unittest) of this member function?
Yes, D62718 here I have a change which tries to mirror gnu objcopy's handling of output file permissions which is dependent on the file being executable or not. 

Thanks for the help.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62838