Re: [PATCH v2] Fix raw-frame-arguments in combination with frame-filters

[Hannes Domani <ssbssa@yahoo.de>]

 Am Mittwoch, 2. Juni 2021, 15:17:51 MESZ hat Andrew Burgess <andrew.burgess@embecosm.com> Folgendes geschrieben:

> * Hannes Domani <ssbssa@yahoo.de> [2021-05-17 10:54:08 +0000]:
>
> >  Am Montag, 17. Mai 2021, 11:48:27 MESZ hat Andrew Burgess <andrew.burgess@embecosm.com> Folgendes geschrieben:
> >
> > > * Hannes Domani via Gdb-patches <gdb-patches@sourceware.org> [2021-01-31 14:46:11 +0100]:
> > >
> > > > Currently, if frame-filters are active, raw-values is used instead of
> > > > raw-frame-arguments to decide if a pretty-printer should be invoked for
> > > > frame arguments in a backtrace.
> > > >
> > > > In this example, "super struct" is the output of the pretty-printer:
> > > >
> > > >    (gdb) disable frame-filter global BasicFrameFilter
> > > >    (gdb) bt
> > > >    #0  foo (x=42, ss=super struct = {...}) at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:47
> > > >    #1  0x004016aa in main () at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:57
> > > >
> > > > If no frame-filter is active, then the raw-values print option does not
> > > > affect the backtrace output:
> > > >
> > > >    (gdb) set print raw-values on
> > > >    (gdb) bt
> > > >    #0  foo (x=42, ss=super struct = {...}) at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:47
> > > >    #1  0x004016aa in main () at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:57
> > > >    (gdb) set print raw-values off
> > > >
> > > > Instead, the raw-frame-arguments option disables the pretty-printer in the
> > > > backtrace:
> > > >
> > > >    (gdb) bt -raw-frame-arguments on
> > > >    #0  foo (x=42, ss=...) at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:47
> > > >    #1  0x004016aa in main () at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:57
> > > >
> > > > But if a frame-filter is active, the same rules don't apply.
> > > > The option raw-frame-arguments is ignored, but raw-values decides if the
> > > > pretty-printer is used:
> > > >
> > > >    (gdb) enable frame-filter global BasicFrameFilter
> > > >    (gdb) bt
> > > >    #0  foo (x=42, ss=super struct = {...}) at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:47
> > > >    #1  0x004016aa in main () at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:57
> > > >    (gdb) set print raw-values on
> > > >    (gdb) bt
> > > >    #0  foo (x=42, ss=...) at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:47
> > > >    #1  0x004016aa in main () at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:57
> > > >    (gdb) set print raw-values off
> > > >    (gdb) bt -raw-frame-arguments on
> > > >    #0  foo (x=42, ss=super struct = {...}) at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:47
> > > >    #1  0x004016aa in main () at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:57
> > > >
> > > > So this adds the PRINT_RAW_FRAME_ARGUMENTS flag to frame_filter_flag, which
> > > > is then used in the frame-filter to override the raw flag in enumerate_args.
> > > >
> > > > Then the output is the same if a frame-filter is active, the pretty-printer
> > > > for backtraces is only disabled with the raw-frame-arguments option:
> > > >
> > > >    (gdb) enable frame-filter global BasicFrameFilter
> > > >    (gdb) bt
> > > >    #0  foo (x=42, ss=super struct = {...}) at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:47
> > > >    #1  0x004016aa in main () at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:57
> > > >    (gdb) set print raw-values on
> > > >    (gdb) bt
> > > >    #0  foo (x=42, ss=super struct = {...}) at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:47
> > > >    #1  0x004016aa in main () at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:57
> > > >    (gdb) set print raw-values off
> > > >    (gdb) bt -raw-frame-arguments on
> > > >    #0  foo (x=42, ss=...) at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:47
> > > >    #1  0x004016aa in main () at C:/src/repos/gdb-testsuite/gdb/testsuite/gdb.python/py-frame-args.c:57
> > >
> > > Thanks for working on improving this behaviour.
> > >
> > > I think that there's something weird going on with the MI behaviour.
> > >
> > > I ran the py-frame-args.c test program, stopped at line 47, then I see
> > > this:
> > >
> > >   (gdb) set print raw-frame-arguments off
> > >   (gdb) set print raw-values off
> > >   (gdb) interpreter-exec mi "-stack-list-variables 1"
> > >   ^done,variables=[{name="x",arg="1",value="42"},{name="ss",arg="1",value="super struct = {\n  a = m=<1>,\n  b = m=<2>\n}"}]
> > >   (gdb) interpreter-exec mi "-stack-list-arguments 1"
> > >   ^done,stack-args=[frame={level="0",args=[{name="x",value="42"},{name="ss",value="super struct = {\n  a = m=<1>,\n  b = m=<2>\n}"}]},frame={level="1",args=[]}]
> > >
> > >   (gdb) set print raw-frame-arguments on
> > >   (gdb) interpreter-exec mi "-stack-list-arguments 1"
> > >   ^done,stack-args=[frame={level="0",args=[{name="x",value="42"},{name="ss",value="super struct = {\n  a = m=<1>,\n  b = m=<2>\n}"}]},frame={level="1",args=[]}]
> > >   (gdb) interpreter-exec mi "-stack-list-variables 1"
> > >   ^done,variables=[{name="x",arg="1",value="42"},{name="ss",arg="1",value="super struct = {\n  a = m=<1>,\n  b = m=<2>\n}"}]
> > >
> > >   (gdb) set print raw-values on
> > >   (gdb) interpreter-exec mi "-stack-list-arguments 1"
> > >   ^done,stack-args=[frame={level="0",args=[{name="x",value="42"},{name="ss",value="super struct = {\n  a = m=<1>,\n  b = m=<2>\n}"}]},frame={level="1",args=[]}]
> > >   (gdb) interpreter-exec mi "-stack-list-variables 1"
> > >   ^done,variables=[{name="x",arg="1",value="42"},{name="ss",arg="1",value="{\n  a = {\n    m = 1\n  },\n  b = {\n    m = 2\n  }\n}"}]
> > >   (gdb)
> > >
> > > The summary is:
> > >
> > >   - when just doing '-stack-list-arguments' neither 'set print
> > >     raw-values' or 'set print raw-frame-arguments' seem to effect what
> > >     is printed, and
> > >
> > >   - when doing '-stack-list-variables', 'set print raw-values' seems
> > >     to effect how arguments are printed, but 'set print
> > >     raw-frame-arguments' does not.
> > >
> > > I guess I would expect 'set print raw-frame-arguments' to be the
> > > controlling option in both these cases.
> >
> > I'm not completely sure now, is that something this patch breaks?
> > Or do you mean this should be fixed as well, since it's a very similar problem?
>
> All of the above :)
>
> There is I believe at least one regression with this patch, the
> following session is using GDB without your patch, break in your test
> program at line 47 then:
>
>   (gdb) interpreter-exec mi "-enable-frame-filters"
>   ^done
>   (gdb) set print raw-values off
>   (gdb) set print raw-frame-arguments off
>   (gdb) interpreter-exec mi "-stack-list-arguments 1"
>   ^done,stack-args=[frame={level="0",args=[{name="x",value="42"},{name="ss",value="super struct = {\n  a = m=<1>,\n  b = m=<2>\n}"}]},frame={level="1",args=[]}]
>   (gdb) set print raw-values on
>   (gdb) interpreter-exec mi "-stack-list-arguments 1"
>   ^done,stack-args=[frame={level="0",args=[{name="x",value="42"},{name="ss",value="{\n  a = {\n    m = 1\n  },\n  b = {\n    m = 2\n  }\n}"}]},frame={level="1",args=[]}]
>   (gdb) set print raw-values off
>   (gdb) set print raw-frame-arguments on
>   (gdb) interpreter-exec mi "-stack-list-arguments 1"
>   ^done,stack-args=[frame={level="0",args=[{name="x",value="42"},{name="ss",value="super struct = {\n  a = m=<1>,\n  b = m=<2>\n}"}]},frame={level="1",args=[]}]
>
> Notice that 'set print raw-values' controls whether 'ss' is printed
> raw or not.
>
> This is an existing bug, but it would be neat if this got fixed in
> this commit as it is so close to what you're already fixing.
>
> BUT....
>
> With your patch applied the above behaviour changes, I now see this:
>
>   (gdb) interpreter-exec mi "-enable-frame-filters"
>   ^done
>   (gdb) set print raw-values off
>   (gdb) set print raw-frame-arguments off
>   (gdb) interpreter-exec mi "-stack-list-arguments 1"
>   ^done,stack-args=[frame={level="0",args=[{name="x",value="42"},{name="ss",value="super struct = {\n  a = m=<1>,\n  b = m=<2>\n}"}]},frame={level="1",args=[]}]
>   (gdb) set print raw-values on
>   (gdb) interpreter-exec mi "-stack-list-arguments 1"
>   ^done,stack-args=[frame={level="0",args=[{name="x",value="42"},{name="ss",value="super struct = {\n  a = m=<1>,\n  b = m=<2>\n}"}]},frame={level="1",args=[]}]
>   (gdb) set print raw-values off
>   (gdb) set print raw-frame-arguments on
>   (gdb) interpreter-exec mi "-stack-list-arguments 1"
>   ^done,stack-args=[frame={level="0",args=[{name="x",value="42"},{name="ss",value="super struct = {\n  a = m=<1>,\n  b = m=<2>\n}"}]},frame={level="1",args=[]}]
>
> Notice now that even 'set print raw-values on|off' doesn't change how
> 'ss' is printed, it's always pretty-printed.
>
> I think as a minimum you need to fix this regression, and ideally add
> a test for this case, as clearly it's not covered in the existing tests.
>
> However, while trying to figure out how reasonable it was to ask you
> to fix the above, I think I probably fixed it (see the patch below,
> which applies on top of yours), the only thing missing would be some
> tests, which would still need to be written.  I'll take a look when I
> have time, but if you wanted to write some then this would be great.
>
> Let me know how you'd feel about including this fix with yours,

I'm fine with including this in my patch.

So with your changes I get the expected result:

  (gdb) interpreter-exec mi "-enable-frame-filters"
  ^done
  (gdb) set print raw-values off
  (gdb) set print raw-frame-arguments off
  (gdb) interpreter-exec mi "-stack-list-arguments 1"
  ^done,stack-args=[frame={level="0",args=[{name="x",value="42"},{name="ss",value="super struct = {a = m=<1>, b = m=<2>}"}]},frame={level="1",args=[]}]
  (gdb) set print raw-values on
  (gdb) interpreter-exec mi "-stack-list-arguments 1"
  ^done,stack-args=[frame={level="0",args=[{name="x",value="42"},{name="ss",value="super struct = {a = m=<1>, b = m=<2>}"}]},frame={level="1",args=[]}]
  (gdb) set print raw-values off
  (gdb) set print raw-frame-arguments on
  (gdb) interpreter-exec mi "-stack-list-arguments 1"
  ^done,stack-args=[frame={level="0",args=[{name="x",value="42"},{name="ss",value="{a = {m = 1}, b = {m = 2}}"}]},frame={level="1",args=[]}]

But if I disable the frame-filter, it's again 'set print raw-values' that
controls how 'ss' is printed:

  (gdb) disable frame-filter global BasicFrameFilter
  (gdb) interpreter-exec mi "-enable-frame-filters"
  ^done
  (gdb) set print raw-values off
  (gdb) set print raw-frame-arguments off
  (gdb) interpreter-exec mi "-stack-list-arguments 1"
  ^done,stack-args=[frame={level="0",args=[{name="x",value="42"},{name="ss",value="super struct = {a = m=<1>, b = m=<2>}"}]},frame={level="1",args=[]}]
  (gdb) set print raw-values on
  (gdb) interpreter-exec mi "-stack-list-arguments 1"
  ^done,stack-args=[frame={level="0",args=[{name="x",value="42"},{name="ss",value="{a = {m = 1}, b = {m = 2}}"}]},frame={level="1",args=[]}]
  (gdb) set print raw-values off
  (gdb) set print raw-frame-arguments on
  (gdb) interpreter-exec mi "-stack-list-arguments 1"
  ^done,stack-args=[frame={level="0",args=[{name="x",value="42"},{name="ss",value="super struct = {a = m=<1>, b = m=<2>}"}]},frame={level="1",args=[]}]

I didn't see an obvious fix for this, maybe you can take a look at this?

Regarding the tests, I would have just integrated it to py-frame-args.exp,
see patch below.


Hannes


diff --git a/gdb/testsuite/gdb.python/py-frame-args.exp b/gdb/testsuite/gdb.python/py-frame-args.exp
index dae0c0ac29e..3711e2f5c54 100644
--- a/gdb/testsuite/gdb.python/py-frame-args.exp
+++ b/gdb/testsuite/gdb.python/py-frame-args.exp
@@ -35,6 +35,8 @@ gdb_breakpoint [gdb_get_line_number "break-here"]
 gdb_continue_to_breakpoint "break-here" ".* break-here .*"
 
 # Test raw-frame-arguments on backtrace with and without frame-filter
+gdb_test "interpreter-exec mi \"-enable-frame-filters\"" \
+    "done"
 foreach_with_prefix filtered {enable disable} {
     gdb_test_no_output "$filtered frame-filter global BasicFrameFilter"
 
@@ -46,11 +48,24 @@ foreach_with_prefix filtered {enable disable} {
     ".*foo \\(x=42, ss=\[.\]{3}\\).*" \
     "bt raw"
 
+    gdb_test "interpreter-exec mi \"-stack-list-arguments 1\"" \
+    ".*name=\"ss\",value=\"super struct =.*" \
+    "mi bt pretty"
+
+    gdb_test_no_output "set print raw-frame-arguments on"
+    gdb_test "interpreter-exec mi \"-stack-list-arguments 1\"" \
+    ".*name=\"ss\",value=\".a =.*" \
+    "mi bt raw"
+    gdb_test_no_output "set print raw-frame-arguments off"
+
     # "set print raw-values" should not affect frame arguments
     gdb_test_no_output "set print raw-values on"
     gdb_test "bt 1" \
     ".*foo \\(x=42, ss=super struct = {\[.\]{3}}\\).*" \
     "bt pretty,raw-values"
+    gdb_test "interpreter-exec mi \"-stack-list-arguments 1\"" \
+    ".*name=\"ss\",value=\"super struct =.*" \
+    "mi bt pretty,raw-values"
     gdb_test_no_output "set print raw-values off"
 }