Added ELFObjectFileBase::checkMagic() for checking ELF magic word.

[vzakhari]

Summary

Added new method to ELFObjectFileBase to check whether the ELF image used to construct it is actually an ELF image (i.e. it has ELF magic word).

[JonChesterfield]

git-clang-format HEAD^ may be useful for reformatting the already changed parts. LGTM, @jhenderson?

[MaskRay]

We have lib/BinaryFormat/Magic.cpp. Do we need the ELF specific API?

[vzakhari]

In D103546#2799569, @MaskRay wrote:

We have lib/BinaryFormat/Magic.cpp. Do we need the ELF specific API?

Thanks! I was not aware of this code. How about adding something like static inline is_any_elf(file_magic) method in Magic.h? If you agree, then I will abandon this change-set and add the new method in D103545.

[jhenderson]

In D103546#2799634, @vzakhari wrote:

In D103546#2799569, @MaskRay wrote:

We have lib/BinaryFormat/Magic.cpp. Do we need the ELF specific API?

Thanks! I was not aware of this code. How about adding something like static inline is_any_elf(file_magic) method in Magic.h? If you agree, then I will abandon this change-set and add the new method in D103545.

I'm assuming this would be to avoid having to write something like if (objectType == elf || objectType == elf_relocatable || /*all the other elf_* types*/ ...) (or an equivalent switch statement)? Is this something you'd use in more than one place within your code?

[vzakhari]

In D103546#2802020, @jhenderson wrote:

In D103546#2799634, @vzakhari wrote:

In D103546#2799569, @MaskRay wrote:

We have lib/BinaryFormat/Magic.cpp. Do we need the ELF specific API?

Thanks! I was not aware of this code. How about adding something like static inline is_any_elf(file_magic) method in Magic.h? If you agree, then I will abandon this change-set and add the new method in D103545.

I'm assuming this would be to avoid having to write something like if (objectType == elf || objectType == elf_relocatable || /*all the other elf_* types*/ ...) (or an equivalent switch statement)? Is this something you'd use in more than one place within your code?

Exactly! I could add the same helper in the plugin code, but I'd rather keep it close to the file_magic enum definition. Do you suggest adding it in the plugin code?

[MaskRay]

It is rare that an application can process all of elf_relocatable, elf_executable, and elf_shared_object. llvm-objcopy is such a tool, but most others either only handle elf_relocatable or elf_executable+elf_shared_object.

[vzakhari]

Implemented using lib/BinaryFormat/Magic.cpp

llvm/include/llvm/Object/ELFObjectFile.h

public:
   void setARMSubArch(Triple &TheTriple) const override;
 
   virtual uint16_t getEType() const = 0;
 
   virtual uint16_t getEMachine() const = 0;
 
   std::vector<std::pair<Optional<DataRefImpl>, uint64_t>>
   getPltAddresses() const;
+
+  virtual bool checkMagic() const = 0;
 };
 
 class ELFSectionRef : public SectionRef {
 public:
   ELFSectionRef(const SectionRef &B) : SectionRef(B) {
     assert(isa<ELFObjectFileBase>(SectionRef::getObject()));
   }
 

 ELFObjectFileBase::symbols() const {
   return elf_symbol_iterator_range(symbol_begin(), symbol_end());
 }
 
 template <class ELFT> class ELFObjectFile : public ELFObjectFileBase {
   uint16_t getEMachine() const override;
   uint16_t getEType() const override;
   uint64_t getSymbolSize(DataRefImpl Sym) const override;
+  bool checkMagic() const override;
 
 public:
   LLVM_ELF_IMPORT_TYPES_ELFT(ELFT)
 
   SectionRef toSectionRef(const Elf_Shdr *Sec) const {
     return SectionRef(toDRI(Sec), this);
   }
 

 template <class ELFT>
 uint64_t ELFObjectFile<ELFT>::getSymbolSize(DataRefImpl Sym) const {
   Expected<const Elf_Sym *> SymOrErr = getSymbol(Sym);
   if (!SymOrErr)
     report_fatal_error(SymOrErr.takeError());
   return (*SymOrErr)->st_size;
 }
 
 template <class ELFT>
+bool ELFObjectFile<ELFT>::checkMagic() const {
+  return EF.getHeader().checkMagic();
+}
+
+template <class ELFT>
 uint64_t ELFObjectFile<ELFT>::getCommonSymbolSizeImpl(DataRefImpl Symb) const {
   return getSymbolSize(Symb);
 }
 
 template <class ELFT>
 uint8_t ELFObjectFile<ELFT>::getSymbolBinding(DataRefImpl Symb) const {
   Expected<const Elf_Sym *> SymOrErr = getSymbol(Symb);
   if (!SymOrErr)

llvm/unittests/Object/ELFObjectFileTest.cpp

 using namespace llvm::object;
 
 namespace {
 
 // A struct to initialize a buffer to represent an ELF object file.
 struct DataForTest {
   std::vector<uint8_t> Data;
 
-  template <typename T>
+  template <typename T> static
   std::vector<uint8_t> makeElfData(uint8_t Class, uint8_t Encoding,
                                    uint16_t Machine) {
     T Ehdr{}; // Zero-initialise the header.
     Ehdr.e_ident[ELF::EI_MAG0] = 0x7f;
     Ehdr.e_ident[ELF::EI_MAG1] = 'E';
     Ehdr.e_ident[ELF::EI_MAG2] = 'L';
     Ehdr.e_ident[ELF::EI_MAG3] = 'F';
     Ehdr.e_ident[ELF::EI_CLASS] = Class;
)";
   SmallString<128> OverLimitNumBlocks(CommonYamlString);
   OverLimitNumBlocks += R"(
         NumBlocks: 0x100000000
 )";
 
   DoCheck(OverLimitNumBlocks,
           "ULEB128 value at offset 0x8 exceeds UINT32_MAX (0x100000000)");
 }
+
+TEST(ELFObjectFileTest, InvalidMagicTest) {
+  std::vector<uint8_t> Data =
+      DataForTest::makeElfData<ELF::Elf64_Ehdr>(
+          ELF::ELFCLASS32, ELF::ELFDATA2LSB, ELF::EM_NONE);
+  // Replace the ELF with an ORC.
+  Data[ELF::EI_MAG1] = 'O';
+  Data[ELF::EI_MAG2] = 'R';
+  Data[ELF::EI_MAG3] = 'C';
+
+  // In the current implementation the data is still recognized
+  // as an ELF even though it is an ORC.
+  Expected<std::unique_ptr<ObjectFile>> ELFObjOrErr =
+      object::ObjectFile::createELFObjectFile(
+          MemoryBufferRef(toStringRef(Data), "dummyELF"));
+  ASSERT_THAT_EXPECTED(ELFObjOrErr, Succeeded());
+
+  auto *Object = dyn_cast<const ELFObjectFileBase>(ELFObjOrErr->get());
+  // It should be a valid ELFObjectFileBase object.
+  EXPECT_TRUE(Object != nullptr);
+  // checkMagic() recognizes a real ELF.
+  EXPECT_TRUE(!Object->checkMagic());
+}