David Malcolm <dmalc...@redhat.com> writes:

> On Mon, 2019-07-01 at 12:50 +0300, Vladislav Ivanishin wrote:
>> Hi!
>> 
>> GDB's Python API provides strip_typedefs method that can be
>> instrumental
>> for writing DRY code.  Using it at least partially obviates the need
>> for
>> the add_printer_for_types method we have in gdbhooks.py (it takes a
>> list
>> of typenames to match and is usually used to deal with typedefs).
>> 
>> I think, the code can be simplified further (there's still
>> ['tree_node *', 'const tree_node *'], which is a little awkward) if
>> we
>> deal with pointer types in a uniform fashion (I'll touch on this in
>> the
>> description of the second patch).  But that can be improved
>> separately.
>> 
>> The gimple_statement_cond, etc. part has been dysfunctional for a
>> while
>> (namely since gimple-classes-v2-option-3 branch was merged).  I
>> updated
>> it to use the new classes' names.  That seems to work (though it
>> doesn't
>> output much info anyway: pretty
>>        <gimple_phi 0x7ffff78c0200> vs. 
>>        (gphi *) 0x7ffff78c0200
>> in the raw version).
>> 
>> I changed the name passed to pp.add_printer_for_types() for
>> scalar_mode
>> and friends -- so they all share the same name now -- but I don't
>> believe the name is used in any context where it would matter.
>
> IIRC there's a gdb command to tell you what the registered pretty-
> printers are;

Yeah, it's (gdb) info pretty-printer

> I think the name is only ever used in that context (or maybe for
> fine-grained control of pretty-printing) -

>From the gdb info manual:
'disable pretty-printer [OBJECT-REGEXP [NAME-REGEXP]]'
     Disable pretty-printers matching OBJECT-REGEXP and NAME-REGEXP.

In our case, this is how to do it:
    (gdb) disable pretty-printer global gcc;scalar.*

So, that would be a regression, if different modes were given different
names on purpose (starts with r251467).  Looks unintentional though, and
the subprinter is very simple.

I think, these scenarios exhaust the uses for the name attribute of a
pretty printer.

> and I don't know of anyone who uses that.  I don't think changing the
> name matters.

Good.  Neither do I :) Except after writing this I started using this
feature myself.  It provides a convenient way to isolate a faulty
pretty-printer, or continue debugging without it.  The manual says that
"The consequences of a broken pretty-printer are severe enough that GDB
provides support for enabling and disabling individual printers".  Oh,
were they right ...  (see below).

>> This is just a clean up of gdbhooks.py.  OK to commit?
>
> The only area I wasn't sure about were the removal hunks relating to
> machine modes: is that all covered now by the looking-through-typedefs?

Yes, I checked (using debug output) that all the modes for which I
removed explicit handling are correctly expanded to either opt_mode<>,
or pod_mode<>.

> Otherwise, looks good to me.  I assume you've tested the patch by
> debugging with it.

Yeah, I thought my debugging with the patch should have given it
reasonable coverage.

However, I had not previously actually tested debugging code involving
modes, so I went ahead and did it.  And I am seeing a segfault with
RtxPrinter now (-ex "b reg_nonzero_bits_for_combine" -ex "r" -ex "bt").
Delving in...

The rtx type (for which the unqualified form is also 'rtx') was not
being matched by any of the printers.  The typedef-stripped form is
'rtx_def *'. It matches now and uncovers a bug in the subprinter(?)

I dug into it and the impression I have so far is that the
print_inline_rtx call upsets gdb's frame unwinders... Upon the "bt"
command, GDB itself produces more than 157000 frames before hitting the
segfault.

Have you seen anything like that before?  It would be nice to understand
the root cause of this bug.  I am going to spend some more time on it,
but I've no prior experience in debugging gdb internals.

As a workaround, I'd propose perhaps removing the kludge, but the output
looks nice, while the alternative implementation (after return) is not
as informative.

Thanks,
Vlad

> Thanks
> Dave

Reply via email to