On Tue, 2019-07-02 at 14:29 +0300, Vladislav Ivanishin wrote: > 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).
In any case, I have no objection to the change-of-names part of the patch. > > > 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. Sounds like an infinite recursion (possibly a mutual recursion involving gdb repeatedly injecting new frames into the inferior cc1, or could just be an infinite recursion in the inferior). > 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. Yeah, the rtx-printing is a kludge. The gdbhooks.py code isn't well tested. I wonder if what you're seeing is a regression, or you're simply uncovering an existing bug. Maybe have the stripping-of-typedefs special-case rtx, and not strip typedefs for the specific case of rtx? (or for that printer, given that RtxPrinter is a kludge). Dave
