[llvm-commits] [PATCH] ELFObjectFile with dynamic loading support

[Michael Spencer]

On Mon, Jan 9, 2012 at 7:15 AM, Bendersky, Eli <eli.bendersky at intel.com> wrote:
> Hello,
>
>
>
> Following the email I sent to LLVMdev earlier today
> (http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-January/046671.html),
> please find attached the first patch in the MCJIT/ELF series. It presents a
> subclass of ELFObjectFile, named DyldELFObject, which supports basic dynamic
> loading. This class is used by MCJIT/ELF to load an ELF image generated by
> MC into memory and executing it.
>
>
>
> Please note that there are no stand-alone tests for this class yet. It is
> being tested extensively in the ExecutionEngine tests run on MCJIT/ELF,
> which will be part of the next patch in the series, once this one is
> accepted and committed.
>
>
>
> Thanks in advance,
>
> Eli

I have a couple issues with this patch. They are inline below.

> +    DyldELFObject(MemoryBuffer *Object, std::vector<uint8_t*> *MemoryMap,
> +                  error_code &ec);
> +
> +    static inline bool classof(const Binary *v) {
> +      return v->getType() == Binary::isELF;

This is incorrect. An ELFObjectFile is not a DyldELFObject.

> +    }
> +    static inline bool classof(
> +        const ELFObjectFile<target_endianness, is64Bits> *v) { return true; }
> +    static inline bool classof(const DyldELFObject *v) { return true; }
> +  };

> +
> +  // Mark the image as a dynamic shared library
> +  const int32_t temp = ELF::ET_DYN;
> +  memcpy(&(Header->e_type), &temp, sizeof(Elf_Half));

This bypasses the purpose of packed_endian_specific_integral. Two
things need to happen to fix this. operator =(IntegralT) needs to be
overloaded for PESI. And DyldELFObject needs a way to get access to
non-const versions of these structs. This also occurs other places.

> +
> +  rebaseObject(MemoryMap);
> +}

I'm also not sure about the main interfaces. Although I can't really
think of anything better. I would like Daniel Dunbar to take a look
too.

- Michael Spencer