| Message ID | 20191114123554.1291122-3-Hi-Angel@yandex.ru |
|---|---|
| State | New |
| Headers | show |
| Series | gdbinit.in fixes | expand |
On Thu, 14 Nov 2019, Konstantin Kharlamov wrote: > Generally, people expect functions to accept arguments directly. But > ones defined in gdbinit did not use the argument, which may be confusing > for newcomers. But we can't change behavior to use the argument without > breaking existing users of the gdbinit. Let's fix this by adding a check > for whether a user passed an argument, and either use it or go with > older behavior. Thank you for working on this. I think it's possible to avoid code duplication, see below. > 2019-11-14 Konstantin Kharlamov <Hi-Angel@yandex.ru> > > * gdbinit.in (pp, pr, prl, pt, pct, pgg, pgq, pgs, pge, pmz, ptc, pdn, > ptn, pdd, prc, pi, pbm, pel, trt): > Make use of $arg0 if a user passed it (note: no need to start a new line, "Make use ..." can go immediately after the colon) > --- a/gcc/gdbinit.in > +++ b/gcc/gdbinit.in > @@ -17,7 +17,11 @@ > # <http://www.gnu.org/licenses/>. > > define pp > -call debug ($) > + if ($argc == 0) > + call debug ($) > + else > + call debug ($arg0) > + end > end I think here you can use simply use call debug ($argc ? $arg0 : $) and likewise in other instances. Where it would make the line too complicated, I think you can introduce a convenience variable, e.g. instead of > + if ($argc == 0) > + output $.decl_minimal.name->identifier.id.str > + else > + output $arg0.decl_minimal.name->identifier.id.str > + end have something like set $dbgarg = $argc ? $arg0 : $ output $dbgarg.decl_minimal.name->identifier.id.str WDYT? Alexander
On Чт, ноя 14, 2019 at 15:55, Alexander Monakov <amonakov@ispras.ru> wrote: > On Thu, 14 Nov 2019, Konstantin Kharlamov wrote: > >> Generally, people expect functions to accept arguments directly. But >> ones defined in gdbinit did not use the argument, which may be >> confusing >> for newcomers. But we can't change behavior to use the argument >> without >> breaking existing users of the gdbinit. Let's fix this by adding a >> check >> for whether a user passed an argument, and either use it or go with >> older behavior. > > Thank you for working on this. I think it's possible to avoid code > duplication, > see below. > >> 2019-11-14 Konstantin Kharlamov <Hi-Angel@yandex.ru> >> >> * gdbinit.in (pp, pr, prl, pt, pct, pgg, pgq, pgs, pge, >> pmz, ptc, pdn, >> ptn, pdd, prc, pi, pbm, pel, trt): >> Make use of $arg0 if a user passed it > > (note: no need to start a new line, "Make use ..." can go immediately > after the colon) > >> --- a/gcc/gdbinit.in >> +++ b/gcc/gdbinit.in >> @@ -17,7 +17,11 @@ >> # <http://www.gnu.org/licenses/>. >> >> define pp >> -call debug ($) >> + if ($argc == 0) >> + call debug ($) >> + else >> + call debug ($arg0) >> + end >> end > > I think here you can use simply use > > call debug ($argc ? $arg0 : $) > > and likewise in other instances. Where it would make the line too > complicated, I > think you can introduce a convenience variable, e.g. instead of > >> + if ($argc == 0) >> + output $.decl_minimal.name->identifier.id.str >> + else >> + output $arg0.decl_minimal.name->identifier.id.str >> + end > > have something like > > set $dbgarg = $argc ? $arg0 : $ > output $dbgarg.decl_minimal.name->identifier.id.str > > WDYT? Thanks! Unfortunately AFAIK ternary expressions are broken in gdb https://sourceware.org/bugzilla/show_bug.cgi?id=22466 :c
On Thu, 14 Nov 2019, Konstantin Kharlamov wrote: > Thanks! Unfortunately AFAIK ternary expressions are broken in gdb > https://sourceware.org/bugzilla/show_bug.cgi?id=22466 :c Indeed, I didn't notice that. But it still would be nice to avoid duplicating the commands over and over again. Can we use something like define pp if $argc p $arg0 end call debug ($) end this way pp something is simply equivalent to what people already use: p something pp Alexander
On Чт, ноя 14, 2019 at 16:13, Alexander Monakov <amonakov@ispras.ru> wrote: > On Thu, 14 Nov 2019, Konstantin Kharlamov wrote: > >> Thanks! Unfortunately AFAIK ternary expressions are broken in gdb >> https://sourceware.org/bugzilla/show_bug.cgi?id=22466 :c > > Indeed, I didn't notice that. But it still would be nice to avoid > duplicating > the commands over and over again. Can we use something like > > define pp > if $argc > p $arg0 > end > call debug ($) > end > > this way > > pp something > > is simply equivalent to what people already use: > > p something > pp Though, this wouldn't be equivalent to what new people, who just want to call `debug()` with the arg, would expect :) If you want to deduplicate the function call, I can reorganize the code to be like this: define pp if ($argc == 0) set $arg = $ else set $arg = $arg0 end call debug ($arg) end
On Thu, 14 Nov 2019, Konstantin Kharlamov wrote: > Though, this wouldn't be equivalent to what new people, who just want to call > `debug()` with the arg, would expect :) If you want to deduplicate the > function call, I can reorganize the code to be like this: > > define pp > if ($argc == 0) > set $arg = $ > else > set $arg = $arg0 > end > call debug ($arg) > end I wish there was a less verbose way :) I don't insist on this deduplication, so at this point I guess it's up to the reviewers. Your suggestion seems slightly preferable to duplicating commands, but please wait for feedback from others. Alexander
On Чт, ноя 14, 2019 at 16:57, Alexander Monakov <amonakov@ispras.ru> wrote: > On Thu, 14 Nov 2019, Konstantin Kharlamov wrote: > >> Though, this wouldn't be equivalent to what new people, who just >> want to call >> `debug()` with the arg, would expect :) If you want to deduplicate >> the >> function call, I can reorganize the code to be like this: >> >> define pp >> if ($argc == 0) >> set $arg = $ >> else >> set $arg = $arg0 >> end >> call debug ($arg) >> end > > I wish there was a less verbose way :) I don't insist on this > deduplication, so > at this point I guess it's up to the reviewers. Your suggestion > seems slightly > preferable to duplicating commands, but please wait for feedback from > others. Sure; you know, actually I just found out how we can deduplicate the code! Can we use this or not depends on whether we can tolerate dependency on python (I dunno, I can ‾\_(ツ)_/‾). The code: py def choose_arg0_into_gdbarg(): if gdb.execute('output $argc', to_string=True) == '1': gdb.execute('set debug_arg = $arg0', to_string=True) else: gdb.execute('set debug_arg = $', to_string=True) end define pp py choose_arg0_into_gdbarg() call debug (debug_arg) end I also named `dbgarg` as `debug_arg` because I figured I confuse whether it was `gdbarg` or `gdbarg` :D
On Nov 14 2019, Konstantin Kharlamov wrote:
> python (I dunno, I can ‾\_(ツ)_/‾). The code:
Python support is optional.
Andreas.
On Thu, 14 Nov 2019, Konstantin Kharlamov wrote: > I also named `dbgarg` as `debug_arg` because I figured I confuse whether it > was `gdbarg` or `gdbarg` :D It should begin with a dollar ($debug_arg), otherwise GDB will attempt to locate and use a variable named 'debug_arg' in the program being debugged. In any case, I feel we should avoid adding a dependency on GDB-Python here. Alexander
On Thu, 14 Nov 2019, Alexander Monakov wrote: > On Thu, 14 Nov 2019, Konstantin Kharlamov wrote: > > > I also named `dbgarg` as `debug_arg` because I figured I confuse whether it > > was `gdbarg` or `gdbarg` :D > > It should begin with a dollar ($debug_arg), otherwise GDB will attempt to locate > and use a variable named 'debug_arg' in the program being debugged. > > In any case, I feel we should avoid adding a dependency on GDB-Python here. Here's a one-liner that uses eval instead: eval "set $debug_arg = $%s", $argc ? "arg0" : "" call debug ($debug_arg) (but oddly with '$argc ? "$arg0" : ""' it doesn't work). Can you rework your patch to use this approach? Alexander
On Чт, ноя 14, 2019 at 18:00, Alexander Monakov <amonakov@ispras.ru> wrote: > On Thu, 14 Nov 2019, Alexander Monakov wrote: > >> On Thu, 14 Nov 2019, Konstantin Kharlamov wrote: >> >> > I also named `dbgarg` as `debug_arg` because I figured I confuse >> whether it >> > was `gdbarg` or `gdbarg` :D >> >> It should begin with a dollar ($debug_arg), otherwise GDB will >> attempt to locate >> and use a variable named 'debug_arg' in the program being debugged. >> >> In any case, I feel we should avoid adding a dependency on >> GDB-Python here. > > Here's a one-liner that uses eval instead: > > eval "set $debug_arg = $%s", $argc ? "arg0" : "" > call debug ($debug_arg) > > (but oddly with '$argc ? "$arg0" : ""' it doesn't work). Can you > rework your > patch to use this approach? Haha, this is amazing! Will do. A newbish question: shall I send the updated patch "in reply" here, or should I resend the patchset?
On Thu, 14 Nov 2019, Konstantin Kharlamov wrote: > Haha, this is amazing! Will do. A newbish question: shall I send the updated > patch "in reply" here, or should I resend the patchset? Your choice, GCC doesn't have a hard rule for this. Personally I feel it's more appropriate to send patches "in reply" when new revisions are coming fast, and start a new thread when it's been some time since the previous discussion. Alexander
diff --git a/gcc/gdbinit.in b/gcc/gdbinit.in index a933ddc6141..043dfc909df 100644 --- a/gcc/gdbinit.in +++ b/gcc/gdbinit.in @@ -17,7 +17,11 @@ # <http://www.gnu.org/licenses/>. define pp -call debug ($) + if ($argc == 0) + call debug ($) + else + call debug ($arg0) + end end document pp @@ -26,7 +30,11 @@ Works only when an inferior is executing. end define pr -call debug_rtx ($) + if ($argc == 0) + call debug_rtx ($) + else + call debug_rtx ($arg0) + end end document pr @@ -35,7 +43,11 @@ Works only when an inferior is executing. end define prl -call debug_rtx_list ($, debug_rtx_count) + if ($argc == 0) + call debug_rtx_list ($, debug_rtx_count) + else + call debug_rtx_list ($arg0, debug_rtx_count) + end end document prl @@ -50,7 +62,11 @@ it using debug_rtx_list. Usage example: set $foo=debug_rtx_find(first, 42) end define pt -call debug_tree ($) + if ($argc == 0) + call debug_tree ($) + else + call debug_tree ($arg0) + end end document pt @@ -59,7 +75,11 @@ Works only when an inferior is executing. end define pct -call debug_c_tree ($) + if ($argc == 0) + call debug_c_tree ($) + else + call debug_c_tree ($arg0) + end end document pct @@ -68,7 +88,11 @@ Works only when an inferior is executing. end define pgg -call debug_gimple_stmt ($) + if ($argc == 0) + call debug_gimple_stmt ($) + else + call debug_gimple_stmt ($arg0) + end end document pgg @@ -77,7 +101,11 @@ Works only when an inferior is executing. end define pgq -call debug_gimple_seq ($) + if ($argc == 0) + call debug_gimple_seq ($) + else + call debug_gimple_seq ($arg0) + end end document pgq @@ -86,7 +114,11 @@ Works only when an inferior is executing. end define pgs -call debug_generic_stmt ($) + if ($argc == 0) + call debug_generic_stmt ($) + else + call debug_generic_stmt ($arg0) + end end document pgs @@ -95,7 +127,11 @@ Works only when an inferior is executing. end define pge -call debug_generic_expr ($) + if ($argc == 0) + call debug_generic_expr ($) + else + call debug_generic_expr ($arg0) + end end document pge @@ -104,7 +140,11 @@ Works only when an inferior is executing. end define pmz -call mpz_out_str(stderr, 10, $) + if ($argc == 0) + call mpz_out_str(stderr, 10, $) + else + call mpz_out_str(stderr, 10, $arg0) + end end document pmz @@ -113,7 +153,11 @@ Works only when an inferior is executing. end define ptc -output (enum tree_code) $.base.code + if ($argc == 0) + output (enum tree_code) $.base.code + else + output (enum tree_code) $arg0.base.code + end echo \n end @@ -122,7 +166,11 @@ Print the tree-code of the tree node that is $. end define pdn -output $.decl_minimal.name->identifier.id.str + if ($argc == 0) + output $.decl_minimal.name->identifier.id.str + else + output $arg0.decl_minimal.name->identifier.id.str + end echo \n end @@ -131,7 +179,11 @@ Print the name of the decl-node that is $. end define ptn -output $.type.name->decl_minimal.name->identifier.id.str + if ($argc == 0) + output $.type.name->decl_minimal.name->identifier.id.str + else + output $arg0.type.name->decl_minimal.name->identifier.id.str + end echo \n end @@ -140,7 +192,11 @@ Print the name of the type-node that is $. end define pdd -call debug_dwarf_die ($) + if ($argc == 0) + call debug_dwarf_die ($) + else + call debug_dwarf_die ($arg0) + end end document pdd @@ -148,7 +204,11 @@ Print the dw_die_ref that is in $. end define prc -output (enum rtx_code) $.code + if ($argc == 0) + output (enum rtx_code) $.code + else + output (enum rtx_code) $arg0.code + end echo \ ( output $.mode echo )\n @@ -159,7 +219,11 @@ Print the rtx-code and machine mode of the rtx that is $. end define pi -print $.u.fld[0].rt_rtx@7 + if ($argc == 0) + print $.u.fld[0].rt_rtx@7 + else + print $arg0.u.fld[0].rt_rtx@7 + end end document pi @@ -176,7 +240,11 @@ including the global binding level. end define pbm -call bitmap_print (stderr, $, "", "\n") + if ($argc == 0) + call bitmap_print (stderr, $, "", "\n") + else + call bitmap_print (stderr, $arg0, "", "\n") + end end document pbm @@ -184,7 +252,11 @@ Dump the bitmap that is in $ as a comma-separated list of numbers. end define pel -output expand_location ($) + if ($argc == 0) + output expand_location ($) + else + output expand_location ($arg0) + end echo \n end @@ -202,7 +274,11 @@ Print current function. end define trt -print ($.typed.type) + if ($argc == 0) + print ($.typed.type) + else + print ($arg0.typed.type) + end end document trt
Generally, people expect functions to accept arguments directly. But ones defined in gdbinit did not use the argument, which may be confusing for newcomers. But we can't change behavior to use the argument without breaking existing users of the gdbinit. Let's fix this by adding a check for whether a user passed an argument, and either use it or go with older behavior. 2019-11-14 Konstantin Kharlamov <Hi-Angel@yandex.ru> * gdbinit.in (pp, pr, prl, pt, pct, pgg, pgq, pgs, pge, pmz, ptc, pdn, ptn, pdd, prc, pi, pbm, pel, trt): Make use of $arg0 if a user passed it --- gcc/gdbinit.in | 114 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 95 insertions(+), 19 deletions(-) v2: instead of unconditionally using $arg0, check whether a user passed it.