r189320 - PR14569: Omit debug info for thunks

[David Blaikie]

On Aug 28, 2013 1:46 AM, "Alexey Samsonov" <samsonov at google.com> wrote:
>
>
> On Wed, Aug 28, 2013 at 4:03 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> On Tue, Aug 27, 2013 at 1:45 AM, Dmitry Vyukov <dvyukov at google.com>
wrote:
>> > On Tue, Aug 27, 2013 at 12:19 PM, Alexey Samsonov <samsonov at google.com>
wrote:
>> >>
>> >> On Tue, Aug 27, 2013 at 9:21 AM, David Blaikie <dblaikie at gmail.com>
wrote:
>> >>>
>> >>> Author: dblaikie
>> >>> Date: Tue Aug 27 00:21:11 2013
>> >>> New Revision: 189320
>> >>>
>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=189320&view=rev
>> >>> Log:
>> >>> PR14569: Omit debug info for thunks
>> >>>
>> >>> This was added in r166676 based on PR13942 on the basis that tools
may
>> >>> need debug information for any executable code/function for some
fairly
>> >>> broad/non-specific purposes. It seems to me (as noted in PR14569)
that
>> >>> the major/only purpose is in backtraces, which should generally not
>> >>> apply to thunks as they won't appear in the stack themselves.
>> >>
>> >>
>> >> Are you sure that thunks may not appear in backtraces? I think I saw
them in
>> >> stack traces,
>> >> more specifically - in stack traces produced by ThreadSanitizer.
>> >> It's possible that I'm wrong, though...
>> >
>> > Debug info for thunks is necessary for sampling-based profilers and
>> > gdb (if you step or stop in thunk).
>>
>> In theory I would think this could just be handled using the symbol
>> table - the only advantage of emitting debug info is to allow inlining
>> to correctly attribute the inlined instructions to the original
>> function, as I understand it (for tasks such as this anyway - where
>> there's no user code that maps into the function, etc).
>>
>> Please let me know what your thoughts on that are.
>
>
> I guess debug info can be necessary to match generated thunk with
> source file (this is also true for generated functions that initialize
global variables etc).

In theory, at least, the debug info for the global variable or class should
still be available, if a tool demangles the think/initializer name, gets
the class/variable name, it should be able to use that. Maybe that's
insufficient or just not the lengths these tools go to yet/today.

It would be rather useful to have concrete examples that break when this
debug info is missing (perhaps file a GCC bug for it to be added there,
too), but given how little size it costs, it seems reasonable to leave in
for some future/current/speculated use.

>
>>
>>
>> In the mean time, given that there was no observable debug info size
>> savings (which I still don't really understand/believe, but can't
>> refute), I have reverted & reworked this patch to keep thunk debug
>> info but to encode the name only in the linkage name in the debug
>> info, not the plain name. This is sufficient to fix the relevant GDB
>> test case.
>>
>> - David
>>
>> >
>> >
>> >>> By
>> >>> removing them we fix PR14569 and reduce the size of Clang's debug
info.
>> >>>
>> >>> Strangely enough this doesn't seem to have a substantial impact on
>> >>> Clang's self-hosted debug info (at least looking at DWO file size)
size
>> >>> at all. Not sure if I failed to test this correctly but I only
observed
>> >>> a 0.004% change in DWO file size over Clang+LLVM.
>> >>>
>> >>> With thanks to Dinesh Dwivedi for work on this PR.
>> >>>
>> >>> Modified:
>> >>>     cfe/trunk/lib/CodeGen/CGVTables.cpp
>> >>>     cfe/trunk/test/CodeGenCXX/debug-info-thunk.cpp
>> >>>
>> >>> Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp
>> >>> URL:
>> >>>
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=189320&r1=189319&r2=189320&view=diff
>> >>>
>> >>>
==============================================================================
>> >>> --- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)
>> >>> +++ cfe/trunk/lib/CodeGen/CGVTables.cpp Tue Aug 27 00:21:11 2013
>> >>> @@ -292,6 +292,7 @@ void CodeGenFunction::GenerateVarArgsThu
>> >>>  void CodeGenFunction::GenerateThunk(llvm::Function *Fn,
>> >>>                                      const CGFunctionInfo &FnInfo,
>> >>>                                      GlobalDecl GD, const ThunkInfo
>> >>> &Thunk) {
>> >>> +  DebugInfo = NULL; // debug info for thunks is not required or
desired
>> >>>    const CXXMethodDecl *MD = cast<CXXMethodDecl>(GD.getDecl());
>> >>>    const FunctionProtoType *FPT =
>> >>> MD->getType()->getAs<FunctionProtoType>();
>> >>>    QualType ThisType = MD->getThisType(getContext());
>> >>>
>> >>> Modified: cfe/trunk/test/CodeGenCXX/debug-info-thunk.cpp
>> >>> URL:
>> >>>
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-thunk.cpp?rev=189320&r1=189319&r2=189320&view=diff
>> >>>
>> >>>
==============================================================================
>> >>> --- cfe/trunk/test/CodeGenCXX/debug-info-thunk.cpp (original)
>> >>> +++ cfe/trunk/test/CodeGenCXX/debug-info-thunk.cpp Tue Aug 27
00:21:11
>> >>> 2013
>> >>> @@ -14,4 +14,4 @@ struct C : A, B {
>> >>>
>> >>>  void C::f() { }
>> >>>
>> >>> -// CHECK: [ DW_TAG_subprogram ] [line 15] [def]
[_ZThn{{4|8}}_N1C1fEv]
>> >>> +// CHECK-NOT: [ DW_TAG_subprogram ] [line 15] [def]
>> >>> [_ZThn{{4|8}}_N1C1fEv]
>> >>>
>> >>>
>> >>> _______________________________________________
>> >>> cfe-commits mailing list
>> >>> cfe-commits at cs.uiuc.edu
>> >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> >>
>> >>
>> >>
>> >>
>> >> --
>> >> Alexey Samsonov, MSK
>
>
>
>
> --
> Alexey Samsonov, MSK
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130828/1f2367d4/attachment.html>