Re: [PATCH v3 24/34] Push thread->control.command_interp to the struct thread_fsm

[Thomas Preudhomme <thomas dot preudhomme at foss dot arm dot com>]

Hi Pedro,

On Friday 06 May 2016 13:34:54 Pedro Alves wrote:
> I noticed that if we step into an inline function, step_1 never
> reaches proceed, and thus nevers sets the thread's
> tp->control.command_interp.  Because of that,
> should_print_stop_to_console fails to determine that is should print
> stop output to the console.
> 
> The fix is to set the thread's command_interp earlier.  However, I
> realized that we can move that field to the thread_fsm, given that its
> lifetime is exactly the same as thread_fsm.  So the patch plumbs all
> fsms constructors to take the command interp and store it in the
> thread_fsm.
> 
> We can see the fix in action, with e.g., the gdb.opt/inline-cmds.exp
> test, and issuing a step when stopped at line 67:
> 
>  &"s\n"
>  ^running
>  *running,thread-id="all"
>  (gdb)
>  ~"67\t  result = func2 ();\n"
>  *stopped,reason="end-stepping-
> range",frame={addr="0x00000000004004d0",func="main",args=[],file="/home/pedr
> o/gdb/mygit/src/gdb/testsuite/gdb.opt/inline-
> cmds.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.opt/inline-
> cmds.c",line="67"},thread-id="1",stopped-threads="all",core="0"
>  (gdb)
>  s
>  &"s\n"
>  ^running
>  *running,thread-id="all"
>  (gdb)
> + ~"func2 () at /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.opt/inline-
> cmds.c:67\n"
> + ~"67\t  result = func2 ();\n"
>  *stopped,reason="end-stepping-
> range",frame={addr="0x00000000004004d0",func="func2",args=[],file="/home/ped
> ro/gdb/mygit/src/gdb/testsuite/gdb.opt/inline-
> cmds.c",fullname="/home/pedro/gdb/mygit/src/gdb/testsuite/gdb.opt/inline-
> cmds.c",line="67"},thread-id="1",stopped-threads="all",core="0"
>  (gdb)
> 
> (The inline-cmds.exp command is adjusted to exercise this.)
> 
> (Due to the follow_fork change, this also fixes "next N" across a fork
> with "set follow-fork child" with "set detach-on-fork on".  Commands
> that rely on internal breakpoints, like "finish" will still require
> more work to migrate breakpoints etc. to the child thread.)

The new tests added by this patch fail for arm-none-eabi targets because -O2 
leads to instructions to be reordered widely. In this case, the instruction 
that follows the first one for line 64 is related to line 70 so the test is 
failing. Shouldn't this test be compiled with -Og and probably also -finline-
small-functions -findirect-inlining -fpartial-inlining which relates to 
inlining and are included in -O2.

Best regards,

Thomas