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

[Bendersky, 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.
> 

Fixed and committed in r148653
Thanks for the review

Eli


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