[PATCH] Switch to support::endian::read/write in X86-specific JIT and RuntimeDyld.

Alexey Samsonov vonosmas at gmail.com
Thu Aug 21 17:54:21 PDT 2014


================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:264-265
@@ -262,4 +263,4 @@
   case ELF::R_X86_64_64: {
-    uint64_t *Target = reinterpret_cast<uint64_t *>(Section.Address + Offset);
-    *Target = Value + Addend;
+    void *Target = Section.Address + Offset;
+    support::endian::write<uint64_t, support::little, 1>(Target, Value + Addend);
     DEBUG(dbgs() << "Writing " << format("%p", (Value + Addend)) << " at "
----------------
Lang Hames wrote:
> Juergen Ributzka wrote:
> > Lang Hames wrote:
> > > Alexey Samsonov wrote:
> > > > Juergen Ributzka wrote:
> > > > > What about using something like this instead?
> > > > > auto *Target = reinterpret_cast<support::ulittle64_t *>(Section.Address + Offset);
> > > > > *Target = Value + Addend;
> > > > It would still be an unaligned write of ulittle_64_t, which is UB.
> > > This might be a good argument for adding some extra types to Endian.h that take the address of the storage. Then we could write something like:
> > > 
> > > ulittle8_ref(Address) = <expr> ;
> > operator= is overloaded for support::ulittle64_t and should call support::endian::write<uint64_t, support::little, 1> for you.
> Juergen - I believe you're right. I had misread your original code.
> 
> Samsonov - The assignment in Juergen's suggested code should go to the explicit assignment operator for ulittle64_t, no?
> 
> Though in either case, I'd prefer to provide a convenience wrapper along the lines I described. Having explicit reinterpret_casts everywhere would be awful.
Oh, right, my bad =/
Thanks for pointing this out.

http://reviews.llvm.org/D5011






More information about the llvm-commits mailing list