This is an archive of the discontinued LLVM Phabricator instance.

[ORC] Emit i386 indirections for 32-bit processes on x86_64 hosts
AbandonedPublic

Authored by sgraenitz on Aug 5 2021, 7:24 AM.

Details

Reviewers
mgorny
lhames
Summary

The ORC ABI determines the machine instructions emitted for compiler-generated indirections. We use the target triple to select the right ABI, but the triple detection does not respect the process bitness. Thus, ORC emitted x86_64 instructions instead of i386 ones for 32-bit processes on x86_64 hosts. This caused segfaults in two tests as reported with https://llvm.org/PR51292.

Diff Detail

Event Timeline

sgraenitz created this revision.Aug 5 2021, 7:24 AM
sgraenitz requested review of this revision.Aug 5 2021, 7:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 5 2021, 7:24 AM
mgorny added inline comments.Aug 5 2021, 7:49 AM
llvm/lib/ExecutionEngine/Orc/IndirectionUtils.cpp
151

As I said on the bug, we normally don't use LLVM_BUILD_32_BITS. I think this is somewhat ugly but how about just using __i386__?

I did debug the misbehavior in LazyCallThroughManager specifically and verified that this change fixes the issue. I assume that the approach can/should be used in IndirectStubsManager and JITCompileCallbackManager as well, but these cases don't seem to have test coverage right now.

Note that this may not be sufficient to fix https://llvm.org/PR51292. There seems to be another issue with i386 call-through stubs that causes a subsequent segfault.

llvm/lib/ExecutionEngine/Orc/IndirectionUtils.cpp
226

TODO: clang-format proposed this new indentation, but it doesn't look correct

sgraenitz added inline comments.Aug 5 2021, 8:24 AM
llvm/lib/ExecutionEngine/Orc/IndirectionUtils.cpp
151

Didn't tests it yet, but if it works that's fine for me. It would avoid the CMake change. What do you think @lhames?

lhames added a comment.Aug 5 2021, 6:24 PM

Why are we getting Triple::x86_64 rather than Triple::x86? I think we really want to add a Triple::x86 case and then figure out how to make sure the right triple is fed in.

sgraenitz added a comment.EditedAug 6 2021, 2:29 AM

Why are we getting Triple::x86_64 rather than Triple::x86? I think we really want to add a Triple::x86 case and then figure out how to make sure the right triple is fed in.

sys::getProcessTriple() returns LLVM_HOST_TRIPLE which contains Triple::x86_64 on 64-bit hosts (even in 32-bit processes). We could patch the getProcessTriple() function, but I am afraid it's going to be a bigger chunk of work. Please find some more detail from my investigation in the ticket: https://bugs.llvm.org/show_bug.cgi?id=51292#c3

sgraenitz abandoned this revision.Aug 6 2021, 6:24 AM

I am still seeing segfaults in 32-bit builds even with the appropriate triple. Not sure what else is missing and right now I don't have the time to investigate the details. For the moment D107640 limits the debug object tests to 64-bit processes on x86_64 hosts. This is what the debug plugin and tests were made for.