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

[Bendersky, Eli]

Hello,

Find attached an updated patch for ELFObjectFile, with modifications done for the sake of MCJIT-on-ELF implementation. The update addresses the reviews made by Xerxes Rånby and Michael Spencer.

Please let me know if it's OK to commit.

Eli






-----Original Message-----
From: Michael Spencer [mailto:bigcheesegs at gmail.com] 
Sent: Tuesday, January 10, 2012 03:05
To: Bendersky, Eli
Cc: llvm-commits at cs.uiuc.edu; Daniel Dunbar
Subject: Re: [llvm-commits] [PATCH] ELFObjectFile with dynamic loading support

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
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: elfobjectfile.2.patch
Type: application/octet-stream
Size: 19691 bytes
Desc: elfobjectfile.2.patch
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120118/2bd39081/attachment.obj>