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

[Michael Spencer]

On Wed, Jan 18, 2012 at 12:42 AM, Bendersky, Eli
<eli.bendersky at intel.com> wrote:
> 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

This looks good. The only concern I have is the following:

> // Template to assign target pointers consistent with endianness and alignment
> // requirements.
> namespace detail {
> template<typename value_type,
>          support::endianness endian,
>          support::alignment align>
> class packed_endian_specific_pointer
>   : public support::detail::packed_endian_specific_integral<
>       value_type, endian, align> {
> public:
>   void operator=(const value_type *newPointer) {
>     value_type newValue = 0;
>
>    memcpy(&newValue, &newPointer, sizeof(value_type *));

This is invalid when sizeof(value_type) != sizeof(value_type *), which
will happen when targeting a different system (ELF32 from a x86-64
system). For now, because ObjectFile doesn't yet support handling
foreign address spaces, this class seems unneeded. You can simply use
"sec->sh_addr = static_cast<value_type>(intptr_t(actual_address));"
along with an assert before it that sizeof(value_type) ==
sizeof(intptr_t). This will be correct once actual_address is of the
correct type on cross system cases.

>    support::detail::packed_endian_specific_integral<
>        value_type, endian, align>::operator=(newValue);
>  }
> };
> } // end namespace detail

Other than that it's good.

- Michael Spencer