Page MenuHomePhabricator

[CodeView] Initial support for emitting S_THUNK32 symbols for compiler-generated thunk routines
ClosedPublic

Authored by bwyma on Feb 27 2018, 3:59 PM.

Details

Summary

Compiler-generated thunk routines are currently emitted using S_GPROC32_ID symbols. This causes Visual Studio to believe the thunk is user code, and when stepping into them Visual Studio opens a "Source Not Found" window because the thunk is correlated with source line zero (0).

By emitting these as S_THUNK32 symbols instead, Visual Studio will recognize these as compiler-generated thunks and step through them into the user code they call.

This initial support only handles standard thunk ordinals.

Diff Detail

Repository
rL LLVM

Event Timeline

bwyma created this revision.Feb 27 2018, 3:59 PM
rnk added a comment.Feb 28 2018, 2:44 PM

Thanks! Can you add a clang IRGen test that checks the DISubprogram flags set for thunks?

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
869 ↗(On Diff #136165)

This throws away all the local variables and inlined call sites in the thunk. I guess that's good enough for -O0, and optimized debug info is best effort anyway, but it's worth a comment, either here or at the point where we return early for thunks.

874 ↗(On Diff #136165)

I noticed that MSVC emits S_PROC_ID_END. Should we do that too?

llvm/test/DebugInfo/COFF/thunk.ll
20 ↗(On Diff #136165)

Can you add a member pointer thunk to this test? Add something like:

bool (A::*mp)() = &A::MyMethod;

The ??_9 function should be a thunk.

bwyma updated this revision to Diff 138017.Mar 12 2018, 8:36 AM

Can you add a clang IRGen test that checks the DISubprogram flags set for thunks?

I extended the existing test debug-info-thunk.cpp to make sure the DIFlagThunk flag is set on the thunk routines.

... it's worth a comment, either here ...

Comment added.

I noticed that MSVC emits S_PROC_ID_END. Should we do that too?

Fixed.

Can you add a member pointer thunk to this test?

Done.

bwyma marked 3 inline comments as done.Mar 12 2018, 8:40 AM
bwyma added inline comments.
llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
2430 ↗(On Diff #138017)

Some thunks are (correctly) not given any source correlation by CLANG. When this happens it does not have line info and the entire thunk was being thrown away here. This change prevents the compiler from throwing away the thunk.

rnk accepted this revision.Mar 13 2018, 10:57 AM

lgtm

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
2430 ↗(On Diff #138017)

I think this might be a hold-over from when the only reason we emitted ProcSym records was for line table info. We might want to re-evaluate this check completely at some point, but for now, sounds good.

This revision is now accepted and ready to land.Mar 13 2018, 10:57 AM
This revision was automatically updated to reflect the committed changes.