Re: [RFAv2 1/3] Implement 'set print frame-info|frame-arguments presence'.

[Philippe Waroquiers <philippe.waroquiers@skynet.be>]

On Tue, 2019-06-04 at 11:11 -0600, Tom Tromey wrote:
> > > > > > "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:
> 
> Philippe> New settings allow to better control what frame information is printed.
> Philippe> 'set print frame-info' allows to override the default frame information
> Philippe> printed when a GDB command prints a frame.
> 
> Thanks for the patch.
> 
> I wanted to mention - in the last series, I noticed that the ChangeLog
> entries didn't wind up in the commit messages.  I think it is a gdb
> standard to do that, so please make sure it happens for future pushes.
Sorry, I thought that having the ChangeLog file was sufficient.
I will ensure that it is done in the future.

> 
> I realize this is a bit of a pain, but various people have scripts for
> automating it, I believe, and so you can pretty much pick one approach
> that appeals to you.
> 
> Philippe> +  if (args_type == CLI_PRESENCE)
> Philippe> +    {
> Philippe> +      if (args_iter != Py_None)
> Philippe> +	{
> Philippe> +	  if (PyIter_Next (args_iter.get ()) != NULL)
> 
> This causes a memory leak, because PyIter_Next returns a new reference.
> You can wrap it in a gdbpy_ref<> to avoid this problem.
Fixed.
(Note that also for this patch series, I will wait for Pedro to push cli-option
to avoid pushing merge conflicts to him).

> 
> Sometimes I think we just use wrapper functions for the Python API that
> let us spell out this stuff in the type system.
> 
> Philippe> +  /* Note that this print_what default implies that 'bt' and 'bt no-filters'
> Philippe> +     shows different information, as the default for 'bt no-filters
> Philippe> +     is LOCATION.  */
> Philippe> +  enum print_what print_what = LOC_AND_ADDRESS;
> 
> Is this a pre-existing bug?  It seems like something we should change,
> since my believe is that "no filters" should produce the same output as
> the situation where there are actually no filters installed.
Yes, it is a pre-existing "bug", but the difference of behaviour is
between 'bt' and 'bt no-filters' *when there are some filters*.

here is a log with the HEAD:

(gdb) bt
#0  0x000055555555548e in niam (argc=1, argv=0x7fffffffe0d8) at sleepers.c:194
(gdb) bt no-filters
#0  main (argc=1, argv=0x7fffffffe0d8) at sleepers.c:194
(gdb) p $pc
$1 = (void (*)()) 0x55555555548e <main+823>
(gdb) frame
#0  main (argc=1, argv=0x7fffffffe0d8) at sleepers.c:194
194	  sleeper_or_burner(&m);
(gdb) info line
Line 194 of "sleepers.c" starts at address 0x55555555548e <main+823> and ends at
0x55555555549d <main+838>.
(gdb) 

As you can see, the first 'bt' is applying a filter (the 'Reverse' filter
of gdb/testsuite/gdb.python/py-framefilter.py), and shows the address,
while the 'bt no-filters' does not show the address.

This difference of behaviour will be visible as long as there is
one filter active, even if the filter is a 'no effect' filter.

This is because backtrace_command_1 calls apply_ext_lang_frame_filter.
If this finds at least one active filter, it will print the frame itself
by calling py_print_frame.
If there are no active filter or with 'bt no-filters', backtrace_command_1
calls 'print_frame_info (..., LOCATION, ...).

So, the GDB HEAD has a different default way to print a frame
when it is printed by py_print_frame or by stack.c print_frame_info.

The 'set print frame-info auto' setting aims at keeping a backward
compatible behaviour : in GDB HEAD, different commands have different
behaviours, and 'auto' means to let each command decide by itself
what to do.

If the user chooses a specific value (e.g. 'set print frame-info location'),
then with this patch, the python frame filtering will do what the user wants.

So, the question is: do we want to change the default output behaviour
of python frame filters to be LOCATION ?
That is of course a user visible change.

Philippe