[Bug mi/23312] New: MI producing invalid records following redirection (m_suppress_field_separator)

[rfj at google dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=23312

Bug ID: 23312
Summary: MI producing invalid records following redirection
(m_suppress_field_separator)
Product: gdb
Version: HEAD
Status: UNCONFIRMED
Severity: normal
Priority: P2
Component: mi
Assignee: unassigned at sourceware dot org
Reporter: rfj at google dot com
Target Milestone: ---

Release: GNU gdb (Debian 7.12-6) 7.12.0.20161007-git, GNU gdb (GDB)
8.1.50.20180618-git
Environment: Linux 4.5.18-x86, gcc 7.3.0, This GDB was configured as
"i686-linux-gnu".

The issues was noticed when a "-var-update" command was issued, and that
triggered pretty-printers that generated async "cmd-param-changed" records
before the changelist in reply to the "-var-update" command is emitted.

The "-var-update" command sets "m_suppress_field_separator" in the m_ui_out
instance, intending the first element of the emitted changelist is not prefixed
with a ",". A pretty-printer is invoked, and that issues a command that causes
an async record ("cmd-param-changed") to be emitted, which consumes the
"m_suppress_field_separator" setting, causing a necessary "," to be suppressed,
and clears the "m_suppress_field_separator" flag.

A small repro of the problem (3 files: "foo_test.c", "foo_test-gdb.py" and
"mi_commands.txt"):

foo_test.c:
struct foo { int a; };

int main(int argc, char *argv[]) {
struct foo f = {0};
f.a = 1;
return 0;
}

foo_test-gdb.py:
import gdb

# this function is used to generate some async cmd-param-changed records
def toggle_print_symbol():
current_value = gdb.parameter('print symbol')
gdb.execute("set print symbol on")
gdb.execute("set print symbol off")
gdb.execute("set print symbol %s" % ("on" if current_value else "off"))

class fooPrinter:
"""Print a foo object."""

def __init__(self, val):
self.val = val

def children(self):
toggle_print_symbol()
return [('a', self.val['a'])]

def to_string(self):
toggle_print_symbol()
return ('a=<' + str(self.val['a']) +'>')

def build_pretty_printer():
pp = gdb.printing.RegexpCollectionPrettyPrinter('my_library')
pp.add_printer('foo', '^foo$', fooPrinter)
return pp

if __name__ == '__main__':
gdb.printing.register_pretty_printer(
gdb.current_objfile(),
build_pretty_printer())

mi_commands.txt:
-enable-pretty-printing
-break-insert -f "foo_test.c:5"
-exec-run
-var-create "f" * "f"
-var-list-children --all-values "f" -1 -1
-exec-next
-var-update --all-values *

Build and run:
gcc -ggdb foo_test.c -o foo_test
gdb -iex "set auto-load safe-path ." --interpreter=mi2 foo_test <
mi_commands.txt

Towards the end of the output should be something similar to:
=cmd-param-changedparam="print symbol",value="off"
=cmd-param-changed,param="print symbol",value="on"
^done,changelist=[,{name="f.a",value="1",in_scope="true",type_changed="false",has_more="0"}]

I understand this should be:
=cmd-param-changed,param="print symbol",value="off"
=cmd-param-changed,param="print symbol",value="on"
^done,changelist=[{name="f.a",value="1",in_scope="true",type_changed="false",has_more="0"}]

I think this occurs because "mi_ui_out::do_redirect" does not save (and
restore) the state of "m_suppress_field_separator" when it redirects to the
event_channel. The emitted async "cmd-param-changed" record consumes the
"m_suppress_field_separator", missing a necessary ",", and then the flag is no
longer set for when the "changelist" record is emitted.