Bug 14998 - GDB cannot handle pointer to member functions being encoded with DW_TAG_ptr_to_member_type
Summary: GDB cannot handle pointer to member functions being encoded with DW_TAG_ptr_t...
Status: RESOLVED FIXED
Alias: None
Product: gdb
Classification: Unclassified
Component: c++ (show other bugs)
Version: 7.5
: P2 normal
Target Milestone: 7.6
Assignee: Tom Tromey
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-07 01:10 UTC by David Blaikie
Modified: 2013-01-31 17:42 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
C++ source (47 bytes, text/x-c++src)
2013-01-07 17:57 UTC, David Blaikie
Details
x86 assembly from Clang (1.22 KB, application/octet-stream)
2013-01-07 17:58 UTC, David Blaikie
Details
x86 assembly from Clang (1.29 KB, application/octet-stream)
2013-01-08 03:08 UTC, David Blaikie
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Blaikie 2013-01-07 01:10:17 UTC
In experimenting with adding support for encoding pointers to members (functions and variables) in Clang I've prototyped support for encoding these as DW_TAG_ptr_to_member_type. For member variables this works fine & appears to be exactly as GCC (4.7) implements debug info.

For pointers to member functions GCC output (& GDB's (7.5) expectation) is a completely different representation. It seems the member function pointer is represented as a struct with two members:
1) a non-member function pointer of a type equivalent to the non-member type of the member function pointer (ie: it includes the hidden initial 'this' parameter, duly marked as artificial, though GCC doesn't always honor this in places it seems it should)
2) a long int offset

I would like to encode member function pointers the same way member variable pointers are encoded, using DW_TAG_ptr_to_member_type. It's simpler to implement in Clang and the debug info is much smaller.

Also, if this was done, it might address a quirk in the way GDB prints out the type of member function pointers. Given:

struct foo {
  int bar(int, float) {
  }
};

int (foo::*x)(int, float);

compiled with GCC and debugged in GDB, "ptype x" prints "type = int (foo::*)(foo * const, int, float)", note the spurious first parameter ("foo *const") which should not be displayed.

For now, with my experimental Clang output, GDB prints "int foo::*(int, float)" (missing the extra "()" around "foo::*") and doesn't understand that the type is a pointer to member function (so it doesn't print out the function name of the pointers value when printing the pointer, it doesn't support calling the member function pointer with the usual syntax, etc)

(Related Clang bug: http://llvm.org/bugs/show_bug.cgi?id=14759 )
Comment 1 Tom Tromey 2013-01-07 15:41:00 UTC
Could you attach the source and .S files for a simple test case?
That would help a lot.  Thanks.
Comment 2 Tom Tromey 2013-01-07 17:11:18 UTC
BTW the corresponding GCC bug is
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28767
Comment 3 David Blaikie 2013-01-07 17:57:17 UTC
Created attachment 6797 [details]
C++ source
Comment 4 David Blaikie 2013-01-07 17:58:24 UTC
Created attachment 6798 [details]
x86 assembly from Clang

clang++ -g -S simple.cpp
Comment 5 David Blaikie 2013-01-07 17:59:48 UTC
(In reply to comment #1)
> Could you attach the source and .S files for a simple test case?
> That would help a lot.  Thanks.

Attached source & assembly generated by Clang. I hope that helps - let me know if there's anything else I can provide/answer.

Thanks for mentioning the associated GCC bug - good to know that someone had the same idea at some point.
Comment 6 Tom Tromey 2013-01-07 19:01:17 UTC
This .s seems a little weird to me, but maybe I am confused.

When I look at the DWARF I see:

 <1><47>: Abbrev Number: 6 (DW_TAG_ptr_to_member_type)
    <48>   DW_AT_type        : <0x34>	
    <4c>   DW_AT_containing_type: <0x3f>	

which points to

 <1><34>: Abbrev Number: 3 (DW_TAG_subroutine_type)
    <35>   DW_AT_type        : <0x26>	
 <2><39>: Abbrev Number: 4 (DW_TAG_formal_parameter)
    <3a>   DW_AT_type        : <0x2d>	

0x2d is just

 <1><2d>: Abbrev Number: 2 (DW_TAG_base_type)
    <2e>   DW_AT_name        : (indirect string, offset: 0x46): float	
    <32>   DW_AT_encoding    : 4	(float)
    <33>   DW_AT_byte_size   : 4	


I was expecting the subroutine type to have an artificial parameter
for the "this" pointer.

I think if that was added then calling would work.


Printing the artificial argument appears to be a conscious decision.
From c-typeprint.c:

Artificial arguments, such as "this"
   in non-static methods, are displayed if LINKAGE_NAME is zero

I think this is a bit weird though.  I wouldn't mind changing it or
perhaps adding a ptype flag to control it.
Comment 7 David Blaikie 2013-01-07 19:11:41 UTC
> I was expecting the subroutine type to have an artificial parameter
> for the "this" pointer.

Your reading of the DWARF is correct. Clang currently doesn't emit the 'this' parameter. The DWARF standard is a bit vague on this:

"The pointer to member entry has a DW_AT_type attribute to describe the type of the class or structure member to which objects of this type may point."

but it seems like if the DW_AT_type of a pointer-to-member variable "int foo::*x;" is simply "int", then the DW_AT_type of a pointer-to-member function "int (foo::*x)(float)" would be "int(float)".

> I think if that was added then calling would work.

Curious - I could try this, though it's a little non-trivial to construct that artificial type in Clang.

If it works, though, then I'd consider this bug to be lower priority but still 'nice to have' (essentially GDB would detect whether the first parameter is artificial (& the same type as the DW_AT_containing_type) & if so, use that, otherwise insert such a parameter based on the DW_AT_containing_type if that's the representation it prefers).

What are your thoughts on this?

> Printing the artificial argument appears to be a conscious decision.
> From c-typeprint.c:
> 
> Artificial arguments, such as "this"
>    in non-static methods, are displayed if LINKAGE_NAME is zero
> 
> I think this is a bit weird though.  

I guess LINKAGE_NAME is DW_AT_MIPS_linkage_name? Though in any case a function pointer won't have a known/fixed name for the function, so I guess it's always 'zero'/always displayed?

> I wouldn't mind changing it or perhaps adding a ptype flag to control it.

Would there be a need to? It's implied by the containing_type mentioned within the name:

int (foo::*)(float)
     ^^^

Mentioning it again in the parameter list seems redundant. (though I guess some tools might've come to rely on this which might be a justification for keeping it in some form?)
Comment 8 Tom Tromey 2013-01-07 19:35:46 UTC
(In reply to comment #7)
> The DWARF standard is a bit vague on this:
> 
> "The pointer to member entry has a DW_AT_type attribute to describe the type of
> the class or structure member to which objects of this type may point."
> 
> but it seems like if the DW_AT_type of a pointer-to-member variable "int
> foo::*x;" is simply "int", then the DW_AT_type of a pointer-to-member function
> "int (foo::*x)(float)" would be "int(float)".

I searched the DWARF standard and didn't see anything requiring this.
However, it seems strange to emit the artificial parameter for a method
but not for a pointer-to-method.

If you really don't want to emit it then I think we ought to file a
bug report for the DWARF standard.

> If it works, though, then I'd consider this bug to be lower priority but still
> 'nice to have' (essentially GDB would detect whether the first parameter is
> artificial (& the same type as the DW_AT_containing_type) & if so, use that,
> otherwise insert such a parameter based on the DW_AT_containing_type if that's
> the representation it prefers).
> 
> What are your thoughts on this?

Certainly doable in gdb, but I think it is nicer to agree on a single
standard interpretation and try to avoid this kind of adaptive behavior.

> I guess LINKAGE_NAME is DW_AT_MIPS_linkage_name? Though in any case a function
> pointer won't have a known/fixed name for the function, so I guess it's always
> 'zero'/always displayed?

It seems to be a boolean flag (to my surprise, I didn't actually read
it closely the first time).
There's probably some way to arrange for this to work properly.

Offhand I don't know why it is this way.  It might just be so that people
can tell "what is really going on" -- but a flag would suffice for this.
Comment 9 David Blaikie 2013-01-07 19:41:03 UTC
(In reply to comment #8)
> (In reply to comment #7)
> > The DWARF standard is a bit vague on this:
> > 
> > "The pointer to member entry has a DW_AT_type attribute to describe the type of
> > the class or structure member to which objects of this type may point."
> > 
> > but it seems like if the DW_AT_type of a pointer-to-member variable "int
> > foo::*x;" is simply "int", then the DW_AT_type of a pointer-to-member function
> > "int (foo::*x)(float)" would be "int(float)".
> 
> I searched the DWARF standard and didn't see anything requiring this.
> However, it seems strange to emit the artificial parameter for a method
> but not for a pointer-to-method.

That's a fair point & at least convinces me to spend the time trying to get the extra artificial parameter from Clang.
 
> Certainly doable in gdb, but I think it is nicer to agree on a single
> standard interpretation and try to avoid this kind of adaptive behavior.

Fair enough/agreed. Since I'm going to try to at least experiment with this (to verify that GDB will play nicely once it has the implicit first parameter) there's no real reason for GDB to be adaptive - once I've done the work I might as well just commit it to Clang & thus be backwards compatible with GDB versions & there's no real reason to rip it out again later.

> 
> > I guess LINKAGE_NAME is DW_AT_MIPS_linkage_name? Though in any case a function
> > pointer won't have a known/fixed name for the function, so I guess it's always
> > 'zero'/always displayed?
> 
> It seems to be a boolean flag (to my surprise, I didn't actually read
> it closely the first time).
> There's probably some way to arrange for this to work properly.
> 
> Offhand I don't know why it is this way.  It might just be so that people
> can tell "what is really going on" -- but a flag would suffice for this.

OK, sounds good.
Comment 10 David Blaikie 2013-01-08 03:08:05 UTC
Created attachment 6800 [details]
x86 assembly from Clang

Seems even with the extra artificial parameter GDB still doesn't handle it as member function pointer. (prints out as a number rather than a function, type doesn't print with the expected (), etc)
Comment 11 Tom Tromey 2013-01-08 04:48:26 UTC
(In reply to comment #10)
> Created attachment 6800 [details]
> x86 assembly from Clang
> 
> Seems even with the extra artificial parameter GDB still doesn't handle it as
> member function pointer. (prints out as a number rather than a function, type
> doesn't print with the expected (), etc)

Oops, yeah, I see the problem.
read_tag_ptr_to_member_type doesn't handle TYPE_CODE_FUNC properly.
It seems to think we'll see TYPE_CODE_METHOD here, which I think we won't.
I have a patch.
Comment 12 Tom Tromey 2013-01-08 17:12:29 UTC
Patches sent.
Comment 13 David Blaikie 2013-01-19 19:22:07 UTC
(In reply to comment #12)
> Patches sent.

Verified that these patches cause Clang to pass gdb.cp/member-ptr.exp once Clang emits the first parameter as artificial (done in Clang r172911).
Comment 14 Sourceware Commits 2013-01-31 17:41:13 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	tromey@sourceware.org	2013-01-31 17:41:09

Modified files:
	gdb            : ChangeLog dwarf2read.c 
	gdb/testsuite  : ChangeLog 
Added files:
	gdb/testsuite/gdb.dwarf2: method-ptr.cc method-ptr.exp 

Log message:
	PR c++/14998:
	* dwarf2read.c (read_tag_ptr_to_member_type): Handle
	TYPE_CODE_FUNC.
	gdb/testsuite
	* gdb.dwarf2/method-ptr.cc: New file.
	* gdb.dwarf2/method-ptr.exp: New file.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/ChangeLog.diff?cvsroot=src&r1=1.15087&r2=1.15088
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/dwarf2read.c.diff?cvsroot=src&r1=1.745&r2=1.746
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/ChangeLog.diff?cvsroot=src&r1=1.3538&r2=1.3539
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.dwarf2/method-ptr.cc.diff?cvsroot=src&r1=NONE&r2=1.1
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.dwarf2/method-ptr.exp.diff?cvsroot=src&r1=NONE&r2=1.1
Comment 15 Tom Tromey 2013-01-31 17:42:49 UTC
Fixed