[llvm] r243891 - [MCJIT] Fix a cast warning in the unit-test introduced in r243589.

[Lang Hames]

Aaron - I think the API design could be improved, but that's something that
we will need discussion before we change it.

Dave - Yeah, this is gross, but I'm pretty sure you're right on both
counts: Technically UB, but expected to behave as intended. I'm very open
to changing this if we can come up with something saner.

Cheers,
Lang.


On Mon, Aug 3, 2015 at 11:36 AM, Aaron Ballman <aaron at aaronballman.com>
wrote:

> On Mon, Aug 3, 2015 at 2:25 PM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> > On Mon, Aug 3, 2015 at 11:03 AM, Lang Hames <lhames at gmail.com> wrote:
> >>
> >> Author: lhames
> >> Date: Mon Aug  3 13:03:40 2015
> >> New Revision: 243891
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=243891&view=rev
> >> Log:
> >> [MCJIT] Fix a cast warning in the unit-test introduced in r243589.
> >>
> >> Thanks to Aaron Ballman for spotting this.
> >>
> >> Modified:
> >>     llvm/trunk/unittests/ExecutionEngine/MCJIT/MCJITCAPITest.cpp
> >>
> >> Modified: llvm/trunk/unittests/ExecutionEngine/MCJIT/MCJITCAPITest.cpp
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/MCJIT/MCJITCAPITest.cpp?rev=243891&r1=243890&r2=243891&view=diff
> >>
> >>
> ==============================================================================
> >> --- llvm/trunk/unittests/ExecutionEngine/MCJIT/MCJITCAPITest.cpp
> >> (original)
> >> +++ llvm/trunk/unittests/ExecutionEngine/MCJIT/MCJITCAPITest.cpp Mon Aug
> >> 3 13:03:40 2015
> >> @@ -514,7 +514,13 @@ TEST_F(MCJITCAPITest, addGlobalMapping)
> >>    buildMCJITOptions();
> >>    buildMCJITEngine();
> >>
> >> -  LLVMAddGlobalMapping(Engine, MappedFn,
> >> reinterpret_cast<void*>(&localTestFunc));
> >> +  union {
> >> +    int (*raw)();
> >> +    void *usable;
> >> +  } functionPointer;
> >> +  functionPointer.raw = &localTestFunc;
> >
> >
> > This is actually UB, but I think it's somewhere in the UB subset that
> LLVM
> > intentionally preserves the expected behavior of (& I'm not sure if we
> rely
> > on this in other parts of LLVM/assume that all compilers used to build
> LLVM
> > support this behavior).
>
> I don't think we should rely on it when alternatives exist that aren't
> UB, such as use of intptr_t and static_assert. I think the API for
> LLVMAddGlobalMapping() is broken if it's requiring a function pointer
> to be convertible to void * (note, I know nothing of
> LLVMAddGlobalMappings, so I may also be off-base)...
>
> ~Aaron
>
> >
> >>
> >> +
> >> +  LLVMAddGlobalMapping(Engine, MappedFn, functionPointer.usable);
> >>
> >>    buildAndRunPasses();
> >>
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at cs.uiuc.edu
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150803/e20c8e73/attachment.html>