This is an archive of the discontinued LLVM Phabricator instance.

Implement trampoline step-through for Windows-x86.
ClosedPublic

Authored by sas on Jul 11 2016, 11:03 AM.

Details

Summary

This is required to be able to step through calls to external functions
that are not properly marked with __declspec(dllimport). When a call
like this is emitted, the linker will inject a trampoline to produce an
indirect call through the IAT.

Diff Detail

Repository
rL LLVM

Event Timeline

sas updated this revision to Diff 63541.Jul 11 2016, 11:03 AM
sas retitled this revision from to Implement trampoline step-through for Windows-x86..
sas updated this object.
sas added reviewers: clayborg, zturner.
sas added a subscriber: lldb-commits.
zturner edited edge metadata.Jul 11 2016, 11:05 AM

Excited to see this working. I will look at the patch in detail later, do you think you could make a test for it?

jingham requested changes to this revision.Jul 11 2016, 11:22 AM
jingham added a reviewer: jingham.
jingham added a subscriber: jingham.

Can't you just call "thread->QueueThreadPlanForStepSingleInstruction"? For the most part, it doesn't make sense to make a thread plan and not queue it right away. So the "Thread::QueueThreadPlanFor..." API's are the public ones.

This revision now requires changes to proceed.Jul 11 2016, 11:22 AM
sas requested a review of this revision.Jul 11 2016, 11:54 AM
sas edited edge metadata.

@jingham, it looks like the GetStepThroughTrampolinePlan functions do not queue the thread plan themselves. See DynamicLoaderPOSIXDYLD::GetStepThroughTrampolinePlan for instance that just does new ThreadPlanRunToAddress(...).

If I understand correctly, the thread plan is pushed to the stack later on, by the caller. Let me know if I got something wrong there.

Yes, that's sad. It needs to be done that way because the plan that organizes stepping "through" and provides a safety backstop if the step through runs away needs to push the sub-plans after it has been pushed. We could move all the stuff in the ThreadPlanStepThrough constructor to the DidPush method, then let the various GetStepThroughTrampolinePlan methods -> PushStepThroughTrampolinePlans. But it looks like we've already made a bunch of the ThreadPlan constructors public to work around this sort of problem, so we should probably just give in and make all the constructors public, and just document that you should preferentially call QueueThreadPlan if you are in a place where that's possible...

sas added a subscriber: sas.Jul 11 2016, 12:38 PM

Sounds good. I can make a separate patch to make all the constructors
public if you think that's better. See http://reviews.llvm.org/D22230
for a patch that makes one of the constructors public.

Is this patch good to go in its current form then?

clayborg resigned from this revision.Jul 11 2016, 12:44 PM
clayborg removed a reviewer: clayborg.

Jim Ingham should OK any changes. If Jim is happy, then I am OK.

jingham accepted this revision.Nov 7 2016, 6:40 PM
jingham edited edge metadata.

I seem not to close the formal loop on these reviews...

This revision is now accepted and ready to land.Nov 7 2016, 6:40 PM
sas updated this revision to Diff 116440.Sep 22 2017, 4:42 PM

Rebase + clang-format.

This revision was automatically updated to reflect the committed changes.