[PATCH] [RFC] python: gdb.Type: strip typedefs past pointers too

[PATCH] [RFC] python: gdb.Type: strip typedefs past pointers too

[Paul Bolle]

0) gdb.Type.strip_typedefs() stops stripping types if it encounters a
type that is a pointer to a typedef:

(gdb) ptype wchar_t
type = long int
(gdb) ptype wchar_t *
type = long int *
(gdb) python print gdb.lookup_type("wchar_t").strip_typedefs()
long int
(gdb) python print gdb.lookup_type("wchar_t").pointer().strip_typedefs()
wchar_t *

1) I drafted a patch (pasted below this message) that works around this
limitation:

(gdb) ptype wchar_t
type = long int
(gdb) ptype wchar_t *
type = long int *
(gdb) python print gdb.lookup_type("wchar_t").strip_typedefs()
long int
(gdb) python print gdb.lookup_type("wchar_t").pointer().strip_typedefs()
long int *

2) I'm not comfortable with the code I'm using here (ie, gdb's type
handling code) so I'd appreciate comments on this draft patch.


Paul Bolle
---
Currently gdb.Type.strip_typedefs() doesn't strip typedefs if it
encounters a type that is a pointer to a typedef. In those cases it
doesn't really behave as advertised (well, as I understand the
advertisement).

So learn strip_typedefs() to handle pointers to typedefs too.
---
 gdb/python/py-type.c |   23 ++++++++++++++++++++++-
 1 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 8232436..08602a1 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -267,8 +267,29 @@ static PyObject *
 typy_strip_typedefs (PyObject *self, PyObject *args)
 {
   struct type *type = ((type_object *) self)->type;
+  struct type *real_type;
+  int level = 0;
 
-  return type_to_type_object (check_typedef (type));
+  real_type = check_typedef (type);
+
+  while (TYPE_CODE (real_type) == TYPE_CODE_PTR ||
+	 TYPE_CODE (real_type) == TYPE_CODE_TYPEDEF)
+    {
+      if (TYPE_CODE (real_type) == TYPE_CODE_TYPEDEF)
+	{
+	  CHECK_TYPEDEF (real_type);
+	}
+      else
+       {
+	  real_type = TYPE_TARGET_TYPE (real_type);
+	  level++;
+	}
+    }
+
+  while (level--)
+    real_type = lookup_pointer_type (real_type);
+
+  return type_to_type_object (real_type);
 }
 
 /* Return an array type.  */
-- 
1.7.2.3

Re: [PATCH] [RFC] python: gdb.Type: strip typedefs past pointers too

[Joel Brobecker]

> 1) I drafted a patch (pasted below this message) that works around this
> limitation:

Personally, I think that the current behavior is correct.  But I did
not participate in the design of the API, so others will probably
have a more informed opinion.

If we want to support the behavior you are looking for, I think it
should be done either through the control of a parameter (defaulted
to current behavior), or another method (recursive_strip_typedef).

> +  while (TYPE_CODE (real_type) == TYPE_CODE_PTR ||
> +	 TYPE_CODE (real_type) == TYPE_CODE_TYPEDEF)

Small nit on the coding style: the `||' should be at the beginning
of the next line:

        while (TYPE_CODE (real_type) == TYPE_CODE_PTR
               || TYPE_CODE (real_type) == TYPE_CODE_TYPEDEF)

(how about references?)

> +      if (TYPE_CODE (real_type) == TYPE_CODE_TYPEDEF)
> +	{
> +	  CHECK_TYPEDEF (real_type);
> +	}

When there is only one statement in the block, the prefered style
in GDB is to omit the curly braces:

      if (TYPE_CODE (real_type) == TYPE_CODE_TYPEDEF)
	CHECK_TYPEDEF (real_type);

-- 
Joel

Re: [PATCH] [RFC] python: gdb.Type: strip typedefs past pointers too

[Paul Bolle]

On Sat, 2010-09-18 at 10:12 -0400, Joel Brobecker wrote:
> Personally, I think that the current behavior is correct.  But I did
> not participate in the design of the API, so others will probably
> have a more informed opinion.
> 
> If we want to support the behavior you are looking for, I think it
> should be done either through the control of a parameter (defaulted
> to current behavior), or another method (recursive_strip_typedef).

The documentation reads:
>   -- Method on Type: strip_typedefs
>       Return a new `gdb.Type' that represents the real type, after
>       removing all layers of typedefs.

I feel that both the name of this function (which uses typedefs in
plural) and the documentation imply the behavior my patch tries to
achieve.

> Small nit on the coding style: the `||' should be at the beginning
> of the next line:

This nit (and the later one) I'll try to remember.

> (how about references?)

I have no idea. References are C++ things, aren't they?


Paul Bolle

Re: [PATCH] [RFC] python: gdb.Type: strip typedefs past pointers too

[Daniel Jacobowitz]

On Sat, Sep 18, 2010 at 10:27:28PM +0200, Paul Bolle wrote:
> On Sat, 2010-09-18 at 10:12 -0400, Joel Brobecker wrote:
> > Personally, I think that the current behavior is correct.  But I did
> > not participate in the design of the API, so others will probably
> > have a more informed opinion.
> > 
> > If we want to support the behavior you are looking for, I think it
> > should be done either through the control of a parameter (defaulted
> > to current behavior), or another method (recursive_strip_typedef).
> 
> The documentation reads:
> >   -- Method on Type: strip_typedefs
> >       Return a new `gdb.Type' that represents the real type, after
> >       removing all layers of typedefs.
> 
> I feel that both the name of this function (which uses typedefs in
> plural) and the documentation imply the behavior my patch tries to
> achieve.

I agree with Joel; the current behavior is correct.  I'm surprised by
the CLI example you gave; I kind of consider that a bug too...

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] [RFC] python: gdb.Type: strip typedefs past pointers too

[Paul Bolle]

On Sun, 2010-09-19 at 13:56 -0400, Daniel Jacobowitz wrote:
> I'm surprised by the CLI example you gave; I kind of consider that a
> bug too...

I'm afraid it's unclear to me which of the examples you're referring to
here (and thus how that example could be considered a bug).


Paul Bolle

Re: [PATCH] [RFC] python: gdb.Type: strip typedefs past pointers too

[Daniel Jacobowitz]

On Sun, Sep 19, 2010 at 10:54:08PM +0200, Paul Bolle wrote:
> On Sun, 2010-09-19 at 13:56 -0400, Daniel Jacobowitz wrote:
> > I'm surprised by the CLI example you gave; I kind of consider that a
> > bug too...
> 
> I'm afraid it's unclear to me which of the examples you're referring to
> here (and thus how that example could be considered a bug).

I would not have expected "ptype wchar_t *" to product "int *".

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] [RFC] python: gdb.Type: strip typedefs past pointers too

[Phil Muldoon]

On 09/17/2010 08:55 PM, Paul Bolle wrote:
> 0) gdb.Type.strip_typedefs() stops stripping types if it encounters a
> type that is a pointer to a typedef:
>  
> 1) I drafted a patch (pasted below this message) that works around this
> limitation:

I think the current behaviour is correct.  And we are limited in what
we can change in these situations.  Would it break existing scripts?
Still, I like your patch.  Would you consider submitting it so that it
can be turned on (off by default) via keyword?  (i.e. a recurse=True
keyword).  That would allow us to preserve the existing functionality,
but allow the user an API friendly way for this alternate recursive
behaviour?

Cheers,

Phil

Re: [PATCH] [RFC] python: gdb.Type: strip typedefs past pointers too

[Paul Bolle]

On Mon, 2010-09-20 at 10:32 +0100, Phil Muldoon wrote:
> I think the current behaviour is correct.

Daniel took the time to explain that I basically tried to emulate what
looks to be a bug in the ptype command.

> Would you consider submitting it so that it
> can be turned on (off by default) via keyword?  (i.e. a recurse=True
> keyword).  That would allow us to preserve the existing functionality,
> but allow the user an API friendly way for this alternate recursive
> behaviour[.]

Any thoughts on Joel's question (sent a few days ago):
> (how about references?)
?


Paul Bolle

Re: [PATCH] [RFC] python: gdb.Type: strip typedefs past pointers too

[Jan Kratochvil]

On Fri, 17 Sep 2010 21:55:56 +0200, Paul Bolle wrote:
> 1) I drafted a patch (pasted below this message) that works around this
> limitation:
> 
> (gdb) ptype wchar_t
> type = long int
> (gdb) ptype wchar_t *
> type = long int *

What is the goal of this patch?  strip_typedefs I understand to be the Python
interface to check_typedef.

check_typedef is there to prepare whatever type you get to a point where you
have something you can process as pointer/array/integer/etc. It also sets type
sizeof of the outer typedef to sizeof of the resolved inner type for opaque
types, depending on how they got resolved.  With VLA patch the resolved size
also depends on the current/variable size.

I still do not see a use case why is it useful to resolve typedefs in the
inner sub-types.  Some functions will call check_typedef again while
recursively processing the type anyway.  What is your use case?

The strip_typedefs description should be fixed anyway as the text is now
ambiguous.


Regards,
Jan

Re: [PATCH] [RFC] python: gdb.Type: strip typedefs past pointers too

[André Pönitz]

On Friday 17 September 2010 21:55:56 ext Paul Bolle wrote:
> 0) gdb.Type.strip_typedefs() stops stripping types if it encounters a
> type that is a pointer to a typedef:
> 
> (gdb) ptype wchar_t
> type = long int
> (gdb) ptype wchar_t *
> type = long int *

Funny. What platform and gdb version is that? I seem to get 'wchar_t *'
here with 6.3, 6.8. 7.0, 7.1, and fairly recent git.

>&"ptype wchar_t\n"
>~"type = wchar_t\n"
>^done
>&"ptype wchar_t*\n"
>~"type = wchar_t *\n"


> (gdb) python print gdb.lookup_type("wchar_t").strip_typedefs()
> long int
> (gdb) python print gdb.lookup_type("wchar_t").pointer().strip_typedefs()
> wchar_t *

[This is also in my opinion correct and expected behaviour.]

> 1) I drafted a patch (pasted below this message) that works around this
> limitation:
> 
> (gdb) ptype wchar_t
> type = long int
> (gdb) ptype wchar_t *
> type = long int *
> (gdb) python print gdb.lookup_type("wchar_t").strip_typedefs()
> long int
> (gdb) python print gdb.lookup_type("wchar_t").pointer().strip_typedefs()
> long int *

If you need the functionality you can provide it using a few lines of 
Python by checking for type.code == gdb.TYPE_CODE_PTR and 
recurse if needed.


Andre'

Re: [PATCH] [RFC] python: gdb.Type: strip typedefs past pointers too

[Paul Bolle]

On Mon, 2010-09-20 at 12:04 +0200, Jan Kratochvil wrote:
> On Fri, 17 Sep 2010 21:55:56 +0200, Paul Bolle wrote:
> > 1) I drafted a patch (pasted below this message) that works around this
> > limitation:
> > 
> > (gdb) ptype wchar_t
> > type = long int
> > (gdb) ptype wchar_t *
> > type = long int *
> 
> What is the goal of this patch?  strip_typedefs I understand to be the Python
> interface to check_typedef.

Basically I tried to emulate the current behavior of the ptype command,
which goes past pointers to look whether their target is typedef'd too
(see the second example above). Daniel explained that this seems to be a
bug in the ptype command.

> The strip_typedefs description should be fixed anyway as the text is now
> ambiguous.

Do you mean that the current text is ambiguous? In the commit message of
my draft patch I noted that strip_typedefs() "doesn't really behave as
advertised (well, as I understand the advertisement)". I understood that
text to mean that behavior similar to that of the ptype command was
intended.


Paul Bolle

Re: [PATCH] [RFC] python: gdb.Type: strip typedefs past pointers too

[Paul Bolle]

On Mon, 2010-09-20 at 12:04 +0200, Jan Kratochvil wrote:
> On Fri, 17 Sep 2010 21:55:56 +0200, Paul Bolle wrote:
> > 1) I drafted a patch (pasted below this message) that works around this
> > limitation:
> > 
> > (gdb) ptype wchar_t
> > type = long int
> > (gdb) ptype wchar_t *
> > type = long int *
> 
> What is the goal of this patch?  strip_typedefs I understand to be the Python
> interface to check_typedef.

Basically I tried to emulate the current behavior of the ptype command,
which goes past pointers to look whether their target is typedef'd too
(see the second example above). Daniel explained that this seems to be a
bug in the ptype command.

> The strip_typedefs description should be fixed anyway as the text is now
> ambiguous.

Do you mean that the current text is ambiguous? In the commit message of
my draft patch I noted that strip_typedefs() "doesn't really behave as
advertised (well, as I understand the advertisement)". I understood that
text to mean that behavior similar to that of the ptype command was
intended.


Paul Bolle

Re: [PATCH] [RFC] python: gdb.Type: strip typedefs past pointers too

[Paul Bolle]

On Mon, 2010-09-20 at 12:04 +0200, Jan Kratochvil wrote:
> On Fri, 17 Sep 2010 21:55:56 +0200, Paul Bolle wrote:
> > 1) I drafted a patch (pasted below this message) that works around this
> > limitation:
> > 
> > (gdb) ptype wchar_t
> > type = long int
> > (gdb) ptype wchar_t *
> > type = long int *
> 
> What is the goal of this patch?  strip_typedefs I understand to be the Python
> interface to check_typedef.

Basically I tried to emulate the current behavior of the ptype command,
which goes past pointers to look whether their target is typedef'd too
(see the second example above). Daniel explained that this seems to be a
bug in the ptype command.

> The strip_typedefs description should be fixed anyway as the text is now
> ambiguous.

Do you mean that the current text is ambiguous? In the commit message of
my draft patch I noted that strip_typedefs() "doesn't really behave as
advertised (well, as I understand the advertisement)". I understood that
text to mean that behavior similar to that of the ptype command was
intended.


Paul Bolle

Re: [PATCH] [RFC] python: gdb.Type: strip typedefs past pointers too

[Jan Kratochvil]

On Mon, 20 Sep 2010 12:47:16 +0200, Paul Bolle wrote:
> On Mon, 2010-09-20 at 12:04 +0200, Jan Kratochvil wrote:
> > On Fri, 17 Sep 2010 21:55:56 +0200, Paul Bolle wrote:
> > > (gdb) ptype wchar_t
> > > type = long int
> > > (gdb) ptype wchar_t *
> > > type = long int *

I can confirm it here:

(gdb) ptype wchar_t *
type = int *


> Daniel explained that this seems to be a bug in the ptype command.

I also thought so, therefore tried the fix below but it regresses on:

 (gdb) ptype foop
-type = struct foo {
-    int a;
-    int b;
-} *
-PASS: gdb.base/opaque.exp: ptype on opaque struct pointer (statically)
+type = struct foo *
+FAIL: gdb.base/opaque.exp: ptype on opaque struct pointer (statically)

Still if `a' or `b' are typedefs they are preserved undereferenced.

I would generally prefer to use always whatis; but that does not work for:
(gdb) whatis struct foo
type = struct foo

This is a longterm issue that `ptype' is too much aggreesive while `whatis' is
too little aggresive.


> Do you mean that the current text is ambiguous?

Yes.

> In the commit message of my draft patch I noted that strip_typedefs()
> "doesn't really behave as advertised (well, as I understand the
> advertisement)". I understood that text to mean that behavior similar to
> that of the ptype command was intended.

I understood it as the intended behavior was that of check_typedef.
This is also how the function is implemented.

BTW I am +1 for André's suggested Python wrapper (or even +0.5 for the Phil's
suggested new parameter.)


Thanks,
Jan


--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -723,7 +723,7 @@ c_type_print_base (struct type *type, struct ui_file *stream, int show,
     case TYPE_CODE_FUNC:
     case TYPE_CODE_METHOD:
     case TYPE_CODE_METHODPTR:
-      c_type_print_base (TYPE_TARGET_TYPE (type), stream, show, level);
+      c_type_print_base (TYPE_TARGET_TYPE (type), stream, show - 1, level);
       break;
 
     case TYPE_CODE_STRUCT: