[Object] add isExecutableObject member function

[abrachet]

Summary

added object::ObjectFile::isExecutableObject() which checks from an ObjectFiles header whether it is marked as executable.

[jhenderson]

Hi @abrachet - you forgot to include context with this diff, so it's somewhat harder to review as is. Please update with context.

What is the purpose of this function? Do you have a use-case for it yet?

You should also add some unit tests for this. I don't think it would be too hard, but you might need to create some new unit test files.

llvm/include/llvm/BinaryFormat/XCOFF.h

25

Missing trailing full stop.

llvm/lib/Object/WasmObjectFile.cpp

1540

Could this just be !isRelocatableObject() && !isSharedObject()? I don't know anything about wasm though, so this might not make sense.

[sbc100]

llvm/lib/Object/WasmObjectFile.cpp

1540

Yes that sounds correct to me.

[abrachet]

I felt this would be a useful change because in llvm-objcopy I am planning on using isExecutableObject() when deciding on output file permissions. I also think the change gives more continuity in the ObjectFile class, which already has the isRelocatableObject() method.

[jhenderson]

The code change looks fine now, but there's still no context in the diff.

Are you going to add unit tests too?

[rupprecht]

llvm/include/llvm/BinaryFormat/XCOFF.h

33

This is 0x0100, not 0x1000

[abrachet]

Added test case

[abrachet]

llvm/include/llvm/BinaryFormat/XCOFF.h

33

Good catch had an extra 0 at the end there, thanks!

[rupprecht]

llvm/unittests/Object/CMakeLists.txt

10

nit: I'd put this in a file called ELFObjectFileTest.cpp. Usually unit test files have a 1:1 correspondence with the file they're testing, and individual test cases for the methods/scenarios they're testing.

llvm/unittests/Object/IsExecutableTest.cpp

31 ↗

(On Diff #203504)

It'd be good to parameterize (i.e. with a loop) this on other ELF types too: ET_NONE, ET_DYN, ET_CORE, and maybe a few non-enum types (pick an arbitrary number between ET_LOPROC and ET_HIPROC).

Note: within a loop, you can use stream logging w/ ASSERT_*/EXPECT_* macros to print the type value if the assertion fails (since gtest won't print it otherwise), e.g.

for (...) {
  Header.e_type = ElfType;
  ...
  EXPECT_FALSE(ObjFileOrErr.get()->isExecutableObject()) << " for " << ElfType;
}

38–39 ↗

(On Diff #203504)

For ASSERT (which aborts the test case) vs EXPECT (which causes failure but continues with the test) in gtest, you usually want to use ASSERT only when the condition being false will cause fatal errors later, and EXPECT when the test can continue to check other things.

ASSERT_TRUE(!!ObjFileOrErr) is a good check because calling ObjFileOrErr.get()->isExecutableObject() on the next line would (probably) cause the test to crash anyway (I think ObjFileOrErr.get() would be nullptr in that case?)
However, ASSERT_FALSE(ObjFileOrErr.get()->isExecutableObject()) should be EXPECT_FALSE instead -- it's not a fatal error, and you might want to see what the result is when you change the type to ET_EXEC. (e.g. maybe that will also fail because the check is backwards, or maybe it will pass because isExecutableObject() always returns true, etc.)

[abrachet]

Updated test case

[jhenderson]

llvm/unittests/Object/ObjectFileTest.cpp

18–19

Do you need both these headers? Note that in LLVM, the standard is to only include the headers that are actually required.

48

Nit: capital letter at start of comment.

49–52

You probably only need one of these, but you should also add a value between LOOS and HIOS.

You should probably even add an unused value from the standard range (e.g. ET_CORE + 1), or similar.

110

',' -> '.'

111

Change the rest of the comment to:
"They don't make a copy, so there's no reason to call ObjectFile::createObjectFile every time."

121–124

Sorry, I don't understand this?

163–164

Delete the bit after the last comma.

166

Add "files" after "MachO". Add a full stop at the end of this line.

168–169

Delete the last sentence. I don't think it needs explaining in the test.

180

Why the double '!'? Does ASSERT_TRUE not work directly on ObjectFileOrErr?

[abrachet]

llvm/unittests/Object/ObjectFileTest.cpp

18–19

I do use both std::function and std::vector. I suppose that std::vector was not 100% required, but it makes things somewhat cleaner I would say. Would you have a strong preference that I take it out?

121–124

I probably over complicated things with this entire function. A lot of work to not copy and paste. The idea behind these lines was that although most likely it would be tested in the upper loop, this explicitly tests for example if a ObjectFile::isExecutableObject() is true when ET_EXEC is set as its type. I say this isn't mandatory because ET_EXEC would have already been tested going through the loop on 112 as it is in ELF_ETypes.

180

Seems like gtest doesn't play well with operator bool? I'm not really sure but it didn't work for me without the manual bool conversion. They explicitly mention this in the ErrorOr unit tests, actually: https://github.com/llvm/llvm-project/blob/master/llvm/unittests/Support/ErrorOrTest.cpp#L23

[jhenderson]

llvm/unittests/Object/ObjectFileTest.cpp

18–19

See https://llvm.org/docs/CodingStandards.html#include-as-little-as-possible. So, if one of the headers includes them, my understanding is that you don't need to explicitly mention them in the source too.

121–124

Yes, exactly, you don't need this block, because you are already testing it above. I don't think you need a separate test that does the same thing.

Sometimes a little copy-pasting might make it easier to read, just be careful how much. In this case, I don't think you need to.

[abrachet]

llvm/unittests/Object/ObjectFileTest.cpp

49–52

Which LOOS and HIOS should I use in the context of e_type?

[abrachet]

llvm/unittests/Object/ObjectFileTest.cpp

18–19

I don’t personally interpret this in the same way you do. It seems to me that is mainly targeting when a forward declaring would suffice which is not the case here. I’m sure that one of the headers that I have included itself includes vector, but the compile time performance cost mentioned in that standard I would guess isn’t concerned with the cost of an ifdef. Also, if one of those files no longer needs vector then the build breaks. I wrote a lot more than is representative of how much I care. I’m not putting my foot down or anything, it’ll gladly remove the include if you still want

[rupprecht]

llvm/unittests/Object/ObjectFileTest.cpp

24

For these create*Header methods, it looks like these can all create their own header variables instead of making the caller pass a blank one in?

93–118

This is a bit too clever; it looks great but it makes it harder to read. Tests, even more than code, should be simple to read to make sure the test is actually doing the right thing. Otherwise, if it gets too complex, you have to start writing tests for the test itself :)

I think a good way to make it simpler would be to break it apart, e.g. one that reinterprets to an ObjectFile, and break apart the true/false types into different loops (avoiding the need for KnownTrue), like:

template <typename header_t>
static void createObjectFile(header_t Header, file_magic magic = file_magic::unknown) {
  StringRef Bytes(reinterpret_cast<const char *>(&Header), sizeof(Header));
  auto ObjFileOrErr =
      ObjectFile::createObjectFile(MemoryBufferRef(Bytes, ""), magic);
  ASSERT_TRUE(!!ObjFileOrErr) << "invalid object file format";

  return *ObjFileOrErr.get();
}

TEST(IsExecutableObject, ELF) {
  ELF64LE::EHdr Header = createGenericELFHeader();
  ObjectFile Obj = createObjectFile(Header);
  for (auto ElfType : { ELF::ET_NONE, ELF::ET_REL, ... }) {
    Obj.Header = ElfType;
    EXPECT_FALSE(Obj.isExecutable());
  }
  // Same for EXPECT_TRUE
}

Depending on how you refactor, you might also want to put these into a test fixture (i.e. create a test class and use TEST_F), though I'm not sure there's any data to share (the usual reason for creating a test fixture).

[jhenderson]

llvm/unittests/Object/ObjectFileTest.cpp

18–19

I agree with basically everything you just said, but ultimately I'm going on what common practice seems to be. I really don't care that much, and would happily back bringing this up on the mailing list as an inconsistency in how the coding standard is applied etc, with a suggestion to either change the standard or seeking agreement to start pushing more concretely for it.

Also, if one of those files no longer needs vector then the build breaks.

This is not really an issue to worry about, since people should build their code before committing anyway.

49–52

Any arbitrary value between ET_LOOS and ET_HIOS.

Reference in case you need it: http://www.sco.com/developers/gabi/latest/ch4.eheader.html

110

Missing trailing full stop.

[abrachet]

Made tests simpler.

[abrachet]

llvm/unittests/Object/ObjectFileTest.cpp

49–52

Thanks, I was confused because these values were not in BinaryFormat/ELF.h, I have added them.

93–118

Yup the classic saved a few minutes of copy and pasting for a couple of hours fighting the compiler. You should have seen how bad it was before :)

[jhenderson]

llvm/unittests/Object/ObjectFileTest.cpp

77

Don't use first person in comments. If you feel you need a comment, rephrase this as "Omit a scope..." etc, or "This macro omits a scope creating..."

It would also be easier to read this comment if you surround code snippets with quotation marks.

80

What do they return? What's stopping your function returning something and then asserting in the code on that? E.g:

std::string my_function(bool b) {
  if (b)
    return "";
  return "something went wrong";
}

TEST(a_test) {
  auto result = my_function(somecall());
  ASSERT_TRUE(result.empty()) << result;
  ...
}

[rupprecht]

llvm/unittests/Object/ObjectFileTest.cpp

81

Why a macro? Can you create a templated method instead?

[abrachet]

Use function instead of macro in unit test.

[abrachet]

Added period at the end of comment.

[abrachet]

Ran clang-format

[jhenderson]

Couple of minor nits, but otherwise LGTM.

llvm/unittests/Object/ObjectFileTest.cpp

77

Can Header be a const &? Also, I think header_t should follow standard LLVM type naming (i.e. UpperCamelCase, e.g. HeaderType).

[rupprecht]

(just the one comment, then lgtm)

llvm/unittests/Object/ObjectFileTest.cpp

83

This should just be returned, i.e. the method should have a signature:

template <typename HeaderType>
std::unique_ptr<ObjectFile> createObjectFile(const HeaderType &Header, file_magic Magic = file_magic::unknown)

(+1 to using the name suggestion from James)

[abrachet]

llvm/unittests/Object/ObjectFileTest.cpp

83

If I returned the unique_ptr then I couldn’t have the ASSERT_TRUE which returns void on error. I would like to keep it, but it isn’t totally necessary I suppose.

[rupprecht]

LGTM just with the naming nit from James (header_t&->const HeaderType&)

llvm/unittests/Object/ObjectFileTest.cpp

83

Oh, yeah. I knew there was a weird gotcha with ASSERT_TRUE, but couldn't remember specifically. :(

Using a test fixture would workaround this issue; e.g. the test class would hold an empty std::unique_pt<ObjectFile> ObjPtr that the test method would initialize (and would not assign as an out-parameter as you have here). But I think this is fine.

[abrachet]

Refactored unit test.

[abrachet]

llvm/unittests/Object/ObjectFileTest.cpp

83

How does this look now?

[jhenderson]

llvm/unittests/Object/ObjectFileTest.cpp

48

You've marked this as private (implicitly) here, but it's protected in the base class. Is that intended?

64

Do you really need to make the constructor protected?

153

I wonder if it would make more sense to fold this into the test case above? i.e. the check becomes:

if (Type == MachO::MH_EXECUTE) {
  EXPECT_TRUE(ObjPtr->isExecutableObject());
  EXPECT_FALSE(ObjPtr->isRelocatableObject());
} else if (Type == MachO::MH_OBJECT) {
  EXPECT_FALSE(ObjPtr->isExecutableObject());
  EXPECT_TRUE(ObjPtr->isRelocatableObject());
} else { // Don't know if this is really necessary or not, but you get the idea
  EXPECT_FALSE(ObjPtr->isExecutableObject());
  EXPECT_FALSE(ObjPtr->isRelocatableObject());
}

On the other hand, if you think the separate test-cases is easier to read, I'm okay with it.

Same comment applies to ELF.

[MaskRay]

llvm/include/llvm/Object/ELFObjectFile.h

1203

Some people will be surprised if ET_DYN is not considered as executable object.

Position-Independent Executables use ET_DYN.

[abrachet]

llvm/include/llvm/Object/ELFObjectFile.h

1203

$ 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.

llvm/unittests/Object/ObjectFileTest.cpp

48

Well createGenericHeader is only ever called by SetUp in TestBase, it isn't necessary to be called outside of the class.

153

Hmm, I think that the current way is better for when a failure occurs because the output will be more clear on which test it failed.

[abrachet]

Would it make sense to add something like this? It might catch a case I haven't thought of yet on obscure systems (assuming we end up changing this to do more than just checking e_type and e_entry, otherwise this won't catch much), assuming build bots are running tests on such systems.

if (!sys::fs::exists("/usr/bin/true") {
    // Try other paths?
    // If it doesn't exist then just return.
}

// Ensure it isn't a symlink, I think we don't deal with these currently?

auto ObjFile = createObjectFile(Path);
ASSERT_TRUE(ObjFile->isExecutableObject());

Also, for more reference this is how elf executables are loaded by execve in Linux I believe. Obviously there is more to check than just e_type, e_entry or program headers but how much is reasonable? Is it worth commenting in ObjectFile::isExecutableObject() that a false return may not necessarily mean the OS can't load the object as an executable?

[MaskRay]

llvm/include/llvm/Object/ELFObjectFile.h

1203

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

[abrachet]

llvm/include/llvm/Object/ELFObjectFile.h

1203

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.

[MaskRay]

llvm/include/llvm/Object/ELFObjectFile.h

1203

Replied on D62718. The idea to use isExecutableObject() to set file permissions looks very bad to me. That would bring more issues than it tried to solve. It is a good example that copying every corner case of GNU objcopy is harmful.

If D62718 is the only use case, I suggest we wait until a more reasonable use case arises.

[abrachet]

This method is no longer needed, D62718 will be going in a new direction.

llvm/include/llvm/BinaryFormat/ELF.h

 
 // File types
 enum {
   ET_NONE = 0,        // No file type
   ET_REL = 1,         // Relocatable file
   ET_EXEC = 2,        // Executable file
   ET_DYN = 3,         // Shared object file
   ET_CORE = 4,        // Core file
+  ET_LOOS = 0xfe00,   // Beginning of OS-specific codes
+  ET_HIOS = 0xfeff,   // OS-specific
   ET_LOPROC = 0xff00, // Beginning of processor-specific codes
   ET_HIPROC = 0xffff  // Processor-specific
 };
 
 // Versioning
 enum { EV_NONE = 0, EV_CURRENT = 1 };
 
 // Machine architectures

llvm/include/llvm/BinaryFormat/XCOFF.h

 
 namespace llvm {
 namespace XCOFF {
 
 // Constants used in the XCOFF definition.
 enum { SectionNameSize = 8, SymbolNameSize = 8 };
 enum ReservedSectionNum { N_DEBUG = -2, N_ABS = -1, N_UNDEF = 0 };
 
+// Flags for the headers f_flag field.
+enum HeaderFlags {
+  F_RELFLG = 0x0001,
+  F_EXEC = 0x0002,
+  F_LNNO = 0x0004,
+  F_FDPR_PROF = 0x0010,
+  F_FDPR_OPTI = 0x0020,
+  F_DSA = 0x0040,
+  F_VARPG = 0x0100,
+  F_DYNLOAD = 0x1000,
+  F_SHROBJ = 0x2000,
+  F_LOADONLY = 0x4000
+};
+
 // Flags for defining the section type. Used for the s_flags field of
 // the section header structure. Defined in the system header `scnhdr.h`.
 enum SectionTypeFlags {
   STYP_PAD = 0x0008,
   STYP_DWARF = 0x0010,
   STYP_TEXT = 0x0020,
   STYP_DATA = 0x0040,
   STYP_BSS = 0x0080,

llvm/include/llvm/Object/COFF.h

public:
   /// Get PDB information from an executable. If the information is not present,
   /// Info will be set to nullptr and PDBFileName will be empty. An error is
   /// returned only on corrupt object files. Convenience accessor that can be
   /// used if the debug directory is not already handy.
   std::error_code getDebugPDBInfo(const codeview::DebugInfo *&Info,
                                   StringRef &PDBFileName) const;
 
   bool isRelocatableObject() const override;
+  bool isExecutableObject() const override;
   bool is64() const { return PE32PlusHeader; }
 
   StringRef mapDebugSectionName(StringRef Name) const override;
 
   static bool classof(const Binary *v) { return v->isCOFF(); }
 };
 
 // The iterator for the import directory table.

llvm/include/llvm/Object/ELFObjectFile.h

public:
   static bool classof(const Binary *v) {
     return v->getType() == getELFType(ELFT::TargetEndianness == support::little,
                                       ELFT::Is64Bits);
   }
 
   elf_symbol_iterator_range getDynamicSymbolIterators() const override;
 
   bool isRelocatableObject() const override;
+  bool isExecutableObject() const override;
 };
 
 using ELF32LEObjectFile = ELFObjectFile<ELF32LE>;
 using ELF64LEObjectFile = ELFObjectFile<ELF64LE>;
 using ELF32BEObjectFile = ELFObjectFile<ELF32BE>;
 using ELF64BEObjectFile = ELFObjectFile<ELF64BE>;
 
 template <class ELFT>

 ELFObjectFile<ELFT>::getDynamicSymbolIterators() const {
   return make_range(dynamic_symbol_begin(), dynamic_symbol_end());
 }
 
 template <class ELFT> bool ELFObjectFile<ELFT>::isRelocatableObject() const {
   return EF.getHeader()->e_type == ELF::ET_REL;
 }
 
+template <class ELFT> bool ELFObjectFile<ELFT>::isExecutableObject() const {
+  return EF.getHeader()->e_type == ELF::ET_EXEC;
+}
+
 } // end namespace object
 } // end namespace llvm
 
 #endif // LLVM_OBJECT_ELFOBJECTFILE_H

llvm/include/llvm/Object/MachO.h

public:
   static Triple::ArchType getArch(uint32_t CPUType);
   static Triple getArchTriple(uint32_t CPUType, uint32_t CPUSubType,
                               const char **McpuDefault = nullptr,
                               const char **ArchFlag = nullptr);
   static bool isValidArch(StringRef ArchFlag);
   static Triple getHostArch();
 
   bool isRelocatableObject() const override;
+  bool isExecutableObject() const override;
 
   StringRef mapDebugSectionName(StringRef Name) const override;
 
   bool hasPageZeroSegment() const { return HasPageZeroSegment; }
 
   static bool classof(const Binary *v) {
     return v->isMachO();
   }

llvm/include/llvm/Object/ObjectFile.h

public:
   Triple makeTriple() const;
 
   /// Maps a debug section name to a standard DWARF section name.
   virtual StringRef mapDebugSectionName(StringRef Name) const { return Name; }
 
   /// True if this is a relocatable object (.o/.obj).
   virtual bool isRelocatableObject() const = 0;
 
+  /// True if this is an executable object.
+  virtual bool isExecutableObject() const = 0;
+
   /// @returns Pointer to ObjectFile subclass to handle this type of object.
   /// @param ObjectPath The path to the object file. ObjectPath.isObject must
   ///        return true.
   /// Create ObjectFile from path.
   static Expected<OwningBinary<ObjectFile>>
   createObjectFile(StringRef ObjectPath);
 
   static Expected<std::unique_ptr<ObjectFile>>

llvm/include/llvm/Object/Wasm.h

public:
 
   section_iterator section_begin() const override;
   section_iterator section_end() const override;
   uint8_t getBytesInAddress() const override;
   StringRef getFileFormatName() const override;
   Triple::ArchType getArch() const override;
   SubtargetFeatures getFeatures() const override;
   bool isRelocatableObject() const override;
+  bool isExecutableObject() const override;
   bool isSharedObject() const;
 
   struct ReadContext {
     const uint8_t *Start;
     const uint8_t *Ptr;
     const uint8_t *End;
   };
 

llvm/include/llvm/Object/XCOFFObjectFile.h

public:
   section_iterator section_begin() const override;
   section_iterator section_end() const override;
   uint8_t getBytesInAddress() const override;
   StringRef getFileFormatName() const override;
   Triple::ArchType getArch() const override;
   SubtargetFeatures getFeatures() const override;
   Expected<uint64_t> getStartAddress() const override;
   bool isRelocatableObject() const override;
+  bool isExecutableObject() const override;
 
   XCOFFObjectFile(MemoryBufferRef Object, std::error_code &EC);
 
   const XCOFFFileHeader *getFileHeader() const { return FileHdrPtr; }
   const XCOFFSymbolEntry *getPointerToSymbolTable() const {
     return SymbolTblPtr;
   }
 

llvm/lib/Object/COFFObjectFile.cpp

void COFFObjectFile::getRelocationTypeName(
   StringRef Res = getRelocationTypeName(Reloc->Type);
   Result.append(Res.begin(), Res.end());
 }
 
 bool COFFObjectFile::isRelocatableObject() const {
   return !DataDirectory;
 }
 
+bool COFFObjectFile::isExecutableObject() const {
+  return getCharacteristics() & COFF::IMAGE_FILE_EXECUTABLE_IMAGE;
+}
+
 StringRef COFFObjectFile::mapDebugSectionName(StringRef Name) const {
   return StringSwitch<StringRef>(Name)
       .Case("eh_fram", "eh_frame")
       .Default(Name);
 }
 
 bool ImportDirectoryEntryRef::
 operator==(const ImportDirectoryEntryRef &Other) const {

llvm/lib/Object/MachOObjectFile.cpp

while (uint64_t delta = extractor.getULEB128(&offset)) {
     Out.push_back(data);
   }
 }
 
 bool MachOObjectFile::isRelocatableObject() const {
   return getHeader().filetype == MachO::MH_OBJECT;
 }
 
+bool MachOObjectFile::isExecutableObject() const {
+  return getHeader().filetype == MachO::MH_EXECUTE;
+}
+
 Expected<std::unique_ptr<MachOObjectFile>>
 ObjectFile::createMachOObjectFile(MemoryBufferRef Buffer,
                                   uint32_t UniversalCputype,
                                   uint32_t UniversalIndex) {
   StringRef Magic = Buffer.getBuffer().slice(0, 4);
   if (Magic == "\xFE\xED\xFA\xCE")
     return MachOObjectFile::create(Buffer, false, false,
                                    UniversalCputype, UniversalIndex);

llvm/lib/Object/WasmObjectFile.cpp

 Triple::ArchType WasmObjectFile::getArch() const { return Triple::wasm32; }
 
 SubtargetFeatures WasmObjectFile::getFeatures() const {
   return SubtargetFeatures();
 }
 
 bool WasmObjectFile::isRelocatableObject() const { return HasLinkingSection; }
 
+bool WasmObjectFile::isExecutableObject() const {
+  return !isRelocatableObject() && !isSharedObject();
+}
+
 bool WasmObjectFile::isSharedObject() const { return HasDylinkSection; }
 
 const WasmSection &WasmObjectFile::getWasmSection(DataRefImpl Ref) const {
   assert(Ref.d.a < Sections.size());
   return Sections[Ref.d.a];
 }
 
 const WasmSection &

llvm/lib/Object/XCOFFObjectFile.cpp

 }
 
 bool XCOFFObjectFile::isRelocatableObject() const {
   bool Result = false;
   llvm_unreachable("Not yet implemented!");
   return Result;
 }
 
+bool XCOFFObjectFile::isExecutableObject() const {
+  return getFileHeader()->Flags == XCOFF::F_EXEC;
+}
+
 Expected<uint64_t> XCOFFObjectFile::getStartAddress() const {
   // TODO FIXME Should get from auxiliary_header->o_entry when support for the
   // auxiliary_header is added.
   return 0;
 }
 
 std::error_code
 XCOFFObjectFile::getSectionByNum(int16_t Num,

llvm/unittests/Object/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   BinaryFormat
   Object
   )
 
 add_llvm_unittest(ObjectTests
   MinidumpTest.cpp
   SymbolSizeTest.cpp
   SymbolicFileTest.cpp
+  ObjectFileTest.cpp
   )
 
 target_link_libraries(ObjectTests PRIVATE LLVMTestingSupport)

llvm/unittests/Object/ObjectFileTest.cpp

+//===- ObjectFileTest.cpp - Tests for object::ObjectFile ------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Object/ObjectFile.h"
+#include "llvm/Object/COFF.h"
+#include "llvm/Object/ELFObjectFile.h"
+#include "llvm/Object/ELFTypes.h"
+#include "llvm/Object/MachO.h"
+#include "llvm/Support/raw_ostream.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace object;
+
+static ELF64LE::Ehdr createGenericELFHeader() {
+  ELF64LE::Ehdr Header;
+  memset(&Header, 0, sizeof(Header));
+  const char e_ident[16] = {
+    '\177', 'E', 'L', 'F',
+    ELF::ELFCLASS64,
+    ELF::ELFDATA2LSB,
+    ELF::EV_CURRENT,
+    0,
+    0,
+    9, 0, 0, 0, 0, 0,
+    16
+  };
+  memcpy(&Header.e_ident, e_ident, 16);
+  Header.e_machine = ELF::EM_X86_64;
+  Header.e_version = ELF::EV_CURRENT;
+  Header.e_ehsize = sizeof(Header);
+  return Header;
+}
+
+std::vector<unsigned> ELF_ETypes{
+    ELF::ET_NONE, ELF::ET_REL, ELF::ET_DYN, ELF::ET_EXEC, ELF::ET_CORE,
+    ELF::ET_CORE + 1,
+    // This is arbitarily chosen between LOPROC and HIPROC.
+    ELF::ET_LOPROC + 20,
+    // Arbitraily chosen between LOOS and HIOS.
+    ELF::ET_LOOS + 17};
+
+static coff_file_header createGenericCOFFHeader() {
+  coff_file_header Header;
+  memset(&Header, 0, sizeof(Header));
+  Header.Machine = COFF::MachineTypes::IMAGE_FILE_MACHINE_AMD64;
+  return Header;
+}
+
+std::vector<unsigned> COFF_Characteristcs{
+    COFF::IMAGE_FILE_EXECUTABLE_IMAGE | COFF::IMAGE_FILE_DEBUG_STRIPPED,
+    COFF::IMAGE_FILE_LARGE_ADDRESS_AWARE | COFF::IMAGE_FILE_DLL |
+        COFF::IMAGE_FILE_LOCAL_SYMS_STRIPPED,
+    COFF::IMAGE_FILE_BYTES_REVERSED_HI | COFF::IMAGE_FILE_LINE_NUMS_STRIPPED,
+    COFF::IMAGE_FILE_AGGRESSIVE_WS_TRIM};
+
+static MachO::mach_header_64 createGenericMachHeader() {
+  MachO::mach_header_64 Header;
+  memset(&Header, 0, sizeof(Header));
+  Header.magic = MachO::MH_MAGIC_64;
+  Header.cputype = MachO::CPU_TYPE_X86;
+  return Header;
+}
+
+std::vector<unsigned> MachO_Filetypes{
+    MachO::MH_OBJECT, MachO::MH_EXECUTE, MachO::MH_FVMLIB,
+    MachO::MH_CORE,   MachO::MH_PRELOAD, MachO::MH_DYLINKER,
+    MachO::MH_BUNDLE, MachO::MH_DSYM,    MachO::MH_KEXT_BUNDLE};
+
+// This function must return void because ASSERT_* macros return void.
+template <typename header_t>
+void createObjectFile(std::unique_ptr<ObjectFile> &ObjPtr, header_t &Header,
+                      file_magic Magic = file_magic::unknown) {
+  StringRef Bytes(reinterpret_cast<const char *>(&Header), sizeof(Header));
+  auto ObjFileOrErr =
+      ObjectFile::createObjectFile(MemoryBufferRef(Bytes, ""), Magic);
+  ASSERT_TRUE(!!ObjFileOrErr) << "invalid object file format";
+  ObjPtr = std::move(ObjFileOrErr.get());
+}
+
+TEST(IsExecutableObject, ELF) {
+  ELF64LE::Ehdr Header = createGenericELFHeader();
+  std::unique_ptr<ObjectFile> ObjPtr;
+  createObjectFile(ObjPtr, Header, file_magic::elf);
+  for (auto CurrType : ELF_ETypes) {
+    Header.e_type = CurrType;
+    if (CurrType == ELF::ET_EXEC)
+      EXPECT_TRUE(ObjPtr->isExecutableObject());
+    else
+      EXPECT_FALSE(ObjPtr->isExecutableObject());
+  }
+}
+
+TEST(IsRelocatableObject, ELF) {
+  ELF64LE::Ehdr Header = createGenericELFHeader();
+  std::unique_ptr<ObjectFile> ObjPtr;
+  createObjectFile(ObjPtr, Header, file_magic::elf);
+  for (auto CurrType : ELF_ETypes) {
+    Header.e_type = CurrType;
+    if (CurrType == ELF::ET_REL)
+      EXPECT_TRUE(ObjPtr->isRelocatableObject());
+    else
+      EXPECT_FALSE(ObjPtr->isRelocatableObject());
+  }
+}
+
+TEST(IsExecutableObject, COFF) {
+  coff_file_header Header = createGenericCOFFHeader();
+  std::unique_ptr<ObjectFile> ObjPtr;
+  createObjectFile(ObjPtr, Header, file_magic::coff_object);
+  for (auto CurrChar : COFF_Characteristcs) {
+    Header.Characteristics = CurrChar;
+    if (CurrChar & COFF::IMAGE_FILE_EXECUTABLE_IMAGE)
+      EXPECT_TRUE(ObjPtr->isExecutableObject());
+    else
+      EXPECT_FALSE(ObjPtr->isExecutableObject());
+  }
+}
+
+// A test for isRelocatableObject for COFF is notably missing because
+// this isn't identified in the header for COFF and is more involved.
+
+TEST(IsExecutableObject, MachO) {
+  MachO::mach_header_64 Header = createGenericMachHeader();
+  for (auto Type : MachO_Filetypes) {
+    Header.filetype = Type;
+    std::unique_ptr<ObjectFile> ObjPtr;
+    createObjectFile(ObjPtr, Header, file_magic::macho_object);
+    if (Type == MachO::MH_EXECUTE)
+      EXPECT_TRUE(ObjPtr->isExecutableObject());
+    else
+      EXPECT_FALSE(ObjPtr->isExecutableObject());
+  }
+}
+
+TEST(IsRelocatableObject, MachO) {
+  MachO::mach_header_64 Header = createGenericMachHeader();
+  for (auto Type : MachO_Filetypes) {
+    Header.filetype = Type;
+    std::unique_ptr<ObjectFile> ObjPtr;
+    createObjectFile(ObjPtr, Header, file_magic::macho_object);
+    if (Type == MachO::MH_OBJECT)
+      EXPECT_TRUE(ObjPtr->isRelocatableObject());
+    else
+      EXPECT_FALSE(ObjPtr->isRelocatableObject());
+  }
+}