Re: [RFA 4/7] Use ui_out_emit_tuple in disasm.c

Simon Marchi
2017-10-12T16:06:00Z

On 2017-10-12 11:36 AM, Simon Marchi wrote:
> On 2017-09-09 02:35 PM, Simon Marchi wrote:
>> On 2017-09-09 17:35, Tom Tromey wrote:
>>> This changes one spot in disasm.c to use ui_out_emit_tuple.Â  This
>>> patch required a large reindentation, so I've separated it out.
>>
>> "git show -w" does wonders!Â  The hard part is figuring out if there's something else in this big function using cleanups (and therefore if the cleanup variable should be kept).
>>
>> Anyway, this LGTM.
>>
>> Simon
>>
>
> I found a regression, bisect pointed to this patch (which makes sense).
>
> 1. Run "disassemble /s" on a small enough terminal so that pagination kicks in.
> 2. Type q <return> to quit pagination.
>
> I get:
>
> ---Type <return> to continue, or q <return> to quit---q
> /home/emaisin/src/binutils-gdb/gdb/ui-out.c:344: internal-error: void ui_out::pop_level(ui_out_type): Assertion `current_level ()->type () == type' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Quit this debugging session? (y or n)
>
> Simon
>

Mixing RAII and cleanups probably changed the order in which the uiout elements
are closed.

In an outer scope (likely do_mixed_source_and_assembly), some uiout elements are
opened and cleanups are installed. In pretty_print_insn, some other uiout elements
are opened and RAII closers are used. When typing 'q', the cleanups are ran, just
before the quit exception would be thrown. The RAII closers have not ran yet, because
they would be when the exception is actually thrown. The solution would be to finish
converting the uiout cleanups to RAII (I won't have time to do that in the near future,
feel free to pick it up).

Simon