| Message ID | 877e92rslo.fsf@ispras.ru |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] gdbhooks.py: use strip_typedefs to simplify matching type names | expand |
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; I think the name is only ever used in that context (or maybe for fine-grained control of pretty-printing) - and I don't know of anyone who uses that. I don't think changing the name matters. > 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? Otherwise, looks good to me. I assume you've tested the patch by debugging with it. Thanks Dave
David Malcolm <dmalcolm@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
On Tue, 2019-07-02 at 14:29 +0300, Vladislav Ivanishin wrote: > David Malcolm <dmalcolm@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
David Malcolm <dmalcolm@redhat.com> writes: > On Tue, 2019-07-02 at 14:29 +0300, Vladislav Ivanishin wrote: >> David Malcolm <dmalcolm@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). It was an infinite recursion, all inside gdb. >> 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. Talking to Alex made me realize that pretty printing function arguments in a backtrace is fundamentally problematic. Consider a function that is passed a pointer whose pointed-to value gets clobbered in a callee of that function. The pretty-printer would access garbage through that pointer and that would lead to incorrect results at best and to screwing up the whole process at worst (I think, this is the root cause of the bug I described). GDB seems to have some provision for that: Function: pretty_printer.to_string (self) [...] Also, depending on the print settings (see Print Settings), the CLI may print just the result of to_string in a stack trace, omitting the result of children. And in 'Print Settings' I learned that there's the `set print frame-arguments all|scalars|none` command. The default setting is "scalars", and that makes sense in our situation, but the deal is pointers - 'rtx_def *' in particular - are (rightfully) considered scalars. I tried moving the call to print_inline_rtx from RtxPrinter.to_string() to RtxPrinter.children(), but whenever the former is called, the latter gets called, too (regardless of display_hint). So I think, the right direction forward here is to always handle the pointed-to types in pretty printers and only do simple forwarding to them for pointers. That way we stay aligned with GDB's printing only scalar types in frame infos and keep backtraces safe (at the cost of perhaps making them less informative). Hope I make sense. Anyway, I wrote up a patch that does that. Do you like this approach? The patch is against trunk, but it requires the - type_ = gdbval.type.unqualified() + type_ = gdbval.type.unqualified().strip_typedefs() change to reliably show visible effect. We can also get 'const rtx_def *' to be pretty printed with e.g. diff --git a/gcc/gdbhooks.py b/gcc/gdbhooks.py index 4f0f9688b35..98c6e13197a 100644 --- a/gcc/gdbhooks.py +++ b/gcc/gdbhooks.py @@ -600,7 +626,7 @@ def build_pretty_printer(): pp.add_printer_for_types(['edge', 'edge_def *'], 'edge', CfgEdgePrinter) - pp.add_printer_for_regex(r'.* rtx_def *', 'rtx_def-forwarder', PtrPrinter) + pp.add_printer_for_regex(r'.*rtx_def \*', 'rtx_def-forwarder', PtrPrinter) pp.add_printer_for_types(['rtx_def'], 'rtx_def', RtxPrinter) pp.add_printer_for_types(['opt_pass *'], 'opt_pass', PassPrinter) Here's a debugging session and output I got with the patch. $ gdb -ex "b reg_nonzero_bits_for_combine" \ -ex "r" \ -ex "f 4" \ -ex "p x" \ --args ./cc1 -O /mnt/co/gcc/gcc/testsuite/gcc.dg/Wunused-var-3.c Breakpoint 5, reg_nonzero_bits_for_combine (x=0x7ffff78cae70 = {...}, xmode=DImode, mode=DImode, nonzero=0x7fffffffdcf0) at /mnt/co/gcc/gcc/combine.c:10271 10271 { #4 set_nonzero_bits_and_sign_copies (x=0x7ffff78d2018 = {...}, set=<optimized out>, data=<optimized out>) at /mnt/co/gcc/gcc/combine.c:1743 1743 set_nonzero_bits_and_sign_copies (rtx x, const_rtx set, void *data) $1 = 0x7ffff78d2018 = {(reg:DI 86)}
Vladislav Ivanishin <vlad@ispras.ru> writes: > David Malcolm <dmalcolm@redhat.com> writes: > >> On Tue, 2019-07-02 at 14:29 +0300, Vladislav Ivanishin wrote: >>> David Malcolm <dmalcolm@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). > > It was an infinite recursion, all inside gdb. > >>> 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. > > Talking to Alex made me realize that pretty printing function arguments > in a backtrace is fundamentally problematic. Consider a function that > is passed a pointer whose pointed-to value gets clobbered in a callee of > that function. The pretty-printer would access garbage through that > pointer and that would lead to incorrect results at best and to screwing > up the whole process at worst (I think, this is the root cause of the > bug I described). > > GDB seems to have some provision for that: > > Function: pretty_printer.to_string (self) > [...] > Also, depending on the print settings (see Print Settings), the CLI > may print just the result of to_string in a stack trace, omitting > the result of children. > > And in 'Print Settings' I learned that there's the `set print > frame-arguments all|scalars|none` command. The default setting is > "scalars", and that makes sense in our situation, but the deal is > pointers - 'rtx_def *' in particular - are (rightfully) considered > scalars. > > I tried moving the call to print_inline_rtx from RtxPrinter.to_string() > to RtxPrinter.children(), but whenever the former is called, the latter > gets called, too (regardless of display_hint). > > So I think, the right direction forward here is to always handle the > pointed-to types in pretty printers and only do simple forwarding to > them for pointers. That way we stay aligned with GDB's printing only > scalar types in frame infos and keep backtraces safe (at the cost of > perhaps making them less informative). > > Hope I make sense. > > Anyway, I wrote up a patch that does that. Do you like this approach? > > The patch is against trunk, but it requires the > - type_ = gdbval.type.unqualified() > + type_ = gdbval.type.unqualified().strip_typedefs() > change to reliably show visible effect. > > diff --git a/gcc/gdbhooks.py b/gcc/gdbhooks.py > index a7e665cadb9..15f9738aeee 100644 > --- a/gcc/gdbhooks.py > +++ b/gcc/gdbhooks.py > @@ -411,8 +411,8 @@ class RtxPrinter: > string for gdb > """ > # We use print_inline_rtx to avoid a trailing newline > - gdb.execute('call print_inline_rtx (stderr, (const_rtx) %s, 0)' > - % intptr(self.gdbval)) > + my_debug_rtx = gdb.lookup_global_symbol('my_debug_inline_rtx').value() > + my_debug_rtx(self.gdbval) > return '' > > # or by hand; based on gcc/print-rtl.c:print_rtx > @@ -428,6 +428,27 @@ class RtxPrinter: > > ###################################################################### > > +class PtrPrinter: > + def __init__(self, gdbval): > + self.gdbval = gdbval > + > + def to_string (self): > + """ > + Print the pointer value. We forward to the pretty-printer for > + the pointed-to value in children(). > + """ > + return '0x%x' % intptr(self.gdbval) > + > + def display_hint (self): > + # The choice of displayhint is based purely on aesthetics (it is not > + # ideal, but the options are limited). > + return 'array' > + > + def children (self): > + yield ('pointed-to', self.gdbval.dereference()) > + > +###################################################################### > + > class PassPrinter: > def __init__(self, gdbval): > self.gdbval = gdbval > @@ -590,7 +611,8 @@ def build_pretty_printer(): > pp.add_printer_for_types(['edge_def *'], > 'edge', > CfgEdgePrinter) > - pp.add_printer_for_types(['rtx_def *'], 'rtx_def', RtxPrinter) > + pp.add_printer_for_types(['rtx_def *'], 'rtx_def-forwarder', PtrPrinter) > + pp.add_printer_for_types(['rtx_def'], 'rtx_def', RtxPrinter) > pp.add_printer_for_types(['opt_pass *'], 'opt_pass', PassPrinter) > > pp.add_printer_for_regex(r'vec<.*> \*', > diff --git a/gcc/print-rtl.c b/gcc/print-rtl.c > index fbb108568b3..2783e921a75 100644 > --- a/gcc/print-rtl.c > +++ b/gcc/print-rtl.c > @@ -1023,6 +1023,15 @@ debug (const rtx_def &ref) > debug_rtx (&ref); > } > > +/* Copy of the above, inlined and w/o a newline, under a different name. */ > + > +DEBUG_FUNCTION void > +my_debug_inline_rtx (const rtx_def &ref) > +{ > + rtx_writer w (stderr, 0, false, false, NULL); > + w.print_rtx (&ref); > +} > + > DEBUG_FUNCTION void > debug (const rtx_def *ptr) > { > > > We can also get 'const rtx_def *' to be pretty printed with e.g. > > diff --git a/gcc/gdbhooks.py b/gcc/gdbhooks.py > index 4f0f9688b35..98c6e13197a 100644 > --- a/gcc/gdbhooks.py > +++ b/gcc/gdbhooks.py > @@ -600,7 +626,7 @@ def build_pretty_printer(): > pp.add_printer_for_types(['edge', 'edge_def *'], > 'edge', > CfgEdgePrinter) > - pp.add_printer_for_regex(r'.* rtx_def *', 'rtx_def-forwarder', PtrPrinter) > + pp.add_printer_for_regex(r'.*rtx_def \*', 'rtx_def-forwarder', PtrPrinter) > pp.add_printer_for_types(['rtx_def'], 'rtx_def', RtxPrinter) > pp.add_printer_for_types(['opt_pass *'], 'opt_pass', PassPrinter) > > Here's a debugging session and output I got with the patch. > > $ gdb -ex "b reg_nonzero_bits_for_combine" \ > -ex "r" \ > -ex "f 4" \ > -ex "p x" \ > --args ./cc1 -O /mnt/co/gcc/gcc/testsuite/gcc.dg/Wunused-var-3.c > > Breakpoint 5, reg_nonzero_bits_for_combine (x=0x7ffff78cae70 = {...}, xmode=DImode, mode=DImode, nonzero=0x7fffffffdcf0) at /mnt/co/gcc/gcc/combine.c:10271 > 10271 { > #4 set_nonzero_bits_and_sign_copies (x=0x7ffff78d2018 = {...}, set=<optimized out>, data=<optimized out>) at /mnt/co/gcc/gcc/combine.c:1743 > 1743 set_nonzero_bits_and_sign_copies (rtx x, const_rtx set, void *data) > $1 = 0x7ffff78d2018 = {(reg:DI 86)} Hi David, I'd like to ping the patch in the previous message of this thread (https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00188.html). The issue it solves (infinite recursion and crash in gdb upon "bt") blocks the original 1/2 patch. And the 2/2 (vec support) - which you also reviewed <https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00063.html> (thank you), but I haven't yet updated - depends on that.
gcc/
* gdbhooks.py (GdbPrettyPrinters.__call__): canonicalize any variants of
a type name using strip_typdefs.
(build_pretty_printer): Use canonical names for types and otherwise
simplify the code. Clean up gimple_sth, gimple_statement_sth cruft.
---
gcc/gdbhooks.py | 48 +++++++++++++++++++-----------------------------
1 file changed, 19 insertions(+), 29 deletions(-)
diff --git a/gcc/gdbhooks.py b/gcc/gdbhooks.py
index 26a09749aa3..b036704b86a 100644
--- a/gcc/gdbhooks.py
+++ b/gcc/gdbhooks.py
@@ -528,7 +528,7 @@ class GdbPrettyPrinters(gdb.printing.PrettyPrinter):
self.subprinters.append(GdbSubprinterRegex(regex, name, class_))
def __call__(self, gdbval):
- type_ = gdbval.type.unqualified()
+ type_ = gdbval.type.unqualified().strip_typedefs()
str_type = str(type_)
for printer in self.subprinters:
if printer.enabled and printer.handles_type(str_type):
@@ -540,7 +540,7 @@ class GdbPrettyPrinters(gdb.printing.PrettyPrinter):
def build_pretty_printer():
pp = GdbPrettyPrinters('gcc')
- pp.add_printer_for_types(['tree', 'const_tree'],
+ pp.add_printer_for_types(['tree_node *', 'const tree_node *'],
'tree', TreePrinter)
pp.add_printer_for_types(['cgraph_node *', 'varpool_node *', 'symtab_node *'],
'symtab_node', SymtabNodePrinter)
@@ -548,32 +548,25 @@ def build_pretty_printer():
'cgraph_edge', CgraphEdgePrinter)
pp.add_printer_for_types(['ipa_ref *'],
'ipa_ref', IpaReferencePrinter)
- pp.add_printer_for_types(['dw_die_ref'],
+ pp.add_printer_for_types(['die_struct *'],
'dw_die_ref', DWDieRefPrinter)
pp.add_printer_for_types(['gimple', 'gimple *',
# Keep this in the same order as gimple.def:
- 'gimple_cond', 'const_gimple_cond',
- 'gimple_statement_cond *',
- 'gimple_debug', 'const_gimple_debug',
- 'gimple_statement_debug *',
- 'gimple_label', 'const_gimple_label',
- 'gimple_statement_label *',
- 'gimple_switch', 'const_gimple_switch',
- 'gimple_statement_switch *',
- 'gimple_assign', 'const_gimple_assign',
- 'gimple_statement_assign *',
- 'gimple_bind', 'const_gimple_bind',
- 'gimple_statement_bind *',
- 'gimple_phi', 'const_gimple_phi',
- 'gimple_statement_phi *'],
+ 'gcond', 'gcond *',
+ 'gdebug', 'gdebug *',
+ 'glabel', 'glabel *',
+ 'gswitch', 'gswitch *',
+ 'gassign', 'gassign *',
+ 'gbind', 'gbind *',
+ 'gphi', 'gphi *'],
'gimple',
GimplePrinter)
- pp.add_printer_for_types(['basic_block', 'basic_block_def *'],
+ pp.add_printer_for_types(['basic_block_def *'],
'basic_block',
BasicBlockPrinter)
- pp.add_printer_for_types(['edge', 'edge_def *'],
+ pp.add_printer_for_types(['edge_def *'],
'edge',
CfgEdgePrinter)
pp.add_printer_for_types(['rtx_def *'], 'rtx_def', RtxPrinter)
@@ -585,18 +578,15 @@ def build_pretty_printer():
pp.add_printer_for_regex(r'opt_mode<(\S+)>',
'opt_mode', OptMachineModePrinter)
- pp.add_printer_for_types(['opt_scalar_int_mode',
- 'opt_scalar_float_mode',
- 'opt_scalar_mode'],
- 'opt_mode', OptMachineModePrinter)
pp.add_printer_for_regex(r'pod_mode<(\S+)>',
'pod_mode', MachineModePrinter)
- pp.add_printer_for_types(['scalar_int_mode_pod',
- 'scalar_mode_pod'],
- 'pod_mode', MachineModePrinter)
- for mode in ('scalar_mode', 'scalar_int_mode', 'scalar_float_mode',
- 'complex_mode'):
- pp.add_printer_for_types([mode], mode, MachineModePrinter)
+
+ pp.add_printer_for_types(['scalar_mode',
+ 'scalar_int_mode',
+ 'scalar_float_mode',
+ 'complex_mode'],
+ 'scalar & complex modes',
+ MachineModePrinter)
return pp
--
2.22.0
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. This is just a clean up of gdbhooks.py. OK to commit?