Re: [RFC] mi: print frame architecture when printing frames on an MI channel

[Pedro Alves <palves@redhat.com>]

On 08/13/2018 11:52 AM, Jan Vrany wrote:
> When printing frames on an MI channel also print frame architecture
> like in:
> 
>     (gdb)
>     -stack-list-frames 3 3
>     ^done,stack=
>     [frame={level="3",addr="0x000107a4",func="foo",
>       file="recursive2.c",fullname="/home/foo/bar/recursive2.c",
>       line="14",arch="i386:x86_64"}]
>    (gdb)
> 
> This is useful for MI clients that need to know the architecture in order
> to perform further analysis, for example to use their own disassembler to
> analyze machine code.
> 
> gdb/Changelog:
> 
> 	* stack.c (print_frame): Print frame architecture when printing on
>         an MI output.

Missing ChangeLog entry for the NEWS change.

> 
> gdb/testsuite/Changelog
> 
> 	* lib/mi-support.exp (mi_expect_stop): Update regexp to
> 	accommodate new "arch" field in frame output.
> 	* gdb.mi/mi-return.exp: Likewise.
> 	* gdb.mi/mi-stack.exp: Likewise.
> 	* gdb.mi/mi-syn-frame.exp: Likewise.
> 	* gdb.mi/user-selected-context-sync.exp: Likewise.
> 
> gdb/doc/Changelog
> 
> 	* gdb.texinfo (The -stack-list-frames Command): Update description
> 	to mention "arch".
> 	Update MI examples thorough the document to contain "arch" in frame
> 	output.

> ---
>  gdb/ChangeLog                                 |  5 ++
>  gdb/NEWS                                      |  2 +
>  gdb/doc/ChangeLog                             |  7 ++
>  gdb/doc/gdb.texinfo                           | 90 ++++++++++---------
>  gdb/stack.c                                   |  5 +-
>  gdb/testsuite/ChangeLog                       |  9 ++
>  gdb/testsuite/gdb.mi/mi-return.exp            |  2 +-
>  gdb/testsuite/gdb.mi/mi-stack.exp             | 12 ++-
>  gdb/testsuite/gdb.mi/mi-syn-frame.exp         | 11 +--
>  .../gdb.mi/user-selected-context-sync.exp     |  6 +-
>  gdb/testsuite/lib/mi-support.exp              |  6 +-
>  11 files changed, 96 insertions(+), 59 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 2f5d1ef8b7..c961ce5cbc 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,8 @@
> +2018-08-08  Jan Vrany  <jan.vrany@fit.cvut.cz>
> +
> +	* stack.c (print_frame): Print frame architecture when printing on
> +	an MI output.
> +
>  2018-08-08  Szabolcs Nagy  <szabolcs.nagy@arm.com>
>  
>  	* remote.c (remote_target::download_tracepoint): Change char* to
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 7f9466567f..e71340ad8e 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -3,6 +3,8 @@
>  
>  *** Changes since GDB 8.2
>  
> +* MI now reports frame architecture in command responses and notifications.

This should go under the new "MI changes" section.

I'd suggest rewording:

* Command responses and notifications that include a frame now include
  the frame's architecture in a new "arch" attribute.



>    uiout->text ("\n");
>  }
> -\f
> +

Spurious change here.

> --- a/gdb/testsuite/gdb.mi/mi-stack.exp
> +++ b/gdb/testsuite/gdb.mi/mi-stack.exp
> @@ -46,6 +46,8 @@ proc test_stack_frame_listing {} {
>      global mi_gdb_prompt
>      global hex fullname_syntax srcfile
>  
> +    set any "\[^\"\]+"
> +
>      set callee4_begin [gdb_get_line_number "callee4 begin"]
>      mi_continue_to_line $callee4_begin "continue to callee4 begin"
>  
> @@ -56,7 +58,7 @@ proc test_stack_frame_listing {} {
>      # -stack-list-frames 1 3
>      # -stack-info-frame
>      mi_gdb_test "231-stack-list-frames" \
> -	"231\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*${srcfile}\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$callee4_begin\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"callee2\",.*\},frame=\{level=\"3\",addr=\"$hex\",func=\"callee1\",.*\},frame=\{level=\"4\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
> +	"231\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*${srcfile}\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$callee4_begin\",arch=\"$any\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"callee2\",.*\},frame=\{level=\"3\",addr=\"$hex\",func=\"callee1\",.*\},frame=\{level=\"4\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
>  	"stack frame listing"
>      mi_gdb_test "232-stack-list-frames 1 1" \
>  	"232\\^done,stack=\\\[frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\}\\\]" \
> @@ -70,7 +72,7 @@ proc test_stack_frame_listing {} {
>  	"stack frame listing wrong"
>  
>      mi_gdb_test "235-stack-info-frame" \
> -	"235\\^done,frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*${srcfile}\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$callee4_begin\"\}" \
> +	"235\\^done,frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*${srcfile}\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$callee4_begin\",arch=\"$any\"\}" \
>  	"selected frame listing"
>  
>      mi_gdb_test "236-stack-list-frames 1 300" \
> @@ -82,6 +84,8 @@ proc test_stack_args_listing {} {
>      global mi_gdb_prompt
>      global hex
>  
> +    set any "\[^\"\]+"
> +
>      # Obtain lists for args for the stack frames
>      # Tests:
>      # -stack-list-arguments 0
> @@ -129,6 +133,8 @@ proc test_stack_info_depth {} {
>      global mi_gdb_prompt
>      global hex
>  
> +    set any "\[^\"\]+"
> +
>      # Obtain depth of stack
>      # Tests:
>      # -stack-info-depth
> @@ -156,6 +162,8 @@ proc test_stack_locals_listing {} {
>      global mi_gdb_prompt
>      global hex fullname_syntax srcfile
>  
> +    set any "\[^\"\]+"
> +

Are these three "set any" necessary?  Off hand the variable doesn't
appear used.

>      # Obtain lists for locals for the stack frames
>      # Tests:
>      # -stack-list-locals 0 (--no-values)
Thanks,
Pedro Alves