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

[Pedro Alves <palves@redhat.com>]

Hi Jan,

On 08/10/2018 06:55 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 usefull for MI clients that need to know the architecture in order

Typo: "useful"

> to perform further analysis, for example to use their own disassembler to
> analyze mechinbe code.

Typo: "machine"

Sounds like a good idea.  Could be useful too for multi-arch debugging (like mixed
SPU and Power frames on Cell).

This needs a NEWS entry.

> 
> gdb/Changelog:
> 
> 	* stack.c (print_frame): print frame architecture when printing on
>         an MI output.

Uppercase "Print".

> 
> gdb/testsuite/Changelog
> 
> 	* lib/mi-support.exp (mi_expect_stop): Regexp updated tp aaccomodate

Typo: "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): Updated description
>         to mention "arch".
>         Updated MI examples thorough the document to contain "arch" in frame
>         output.

(Nit: avoid past tense, "Updated" -> "Update" throughout.)


> diff --git a/gdb/testsuite/gdb.mi/user-selected-context-sync.exp b/gdb/testsuite/gdb.mi/user-selected-context-sync.exp
> index 05624deb0a..df3ee42af3 100644
> --- a/gdb/testsuite/gdb.mi/user-selected-context-sync.exp
> +++ b/gdb/testsuite/gdb.mi/user-selected-context-sync.exp
> @@ -156,11 +156,11 @@ proc make_mi_re { mode thread frame type } {
>      set thread_event_re "=thread-selected,id=\"$thread\""
>      set thread_answer_re "\\^done,new-thread-id=\"$thread\""
>  
> -    set frame_re(0) ",frame=\{level=\"0\",addr=\"$hex\",func=\"child_sub_function\",args=\\\[\\\],file=\"${any}${srcfile}\",fullname=\"${any}${srcfile}\",line=\"$thread_loop_line\"\}"
> -    set frame_re(1) ",frame=\{level=\"1\",addr=\"$hex\",func=\"child_function\",args=\\\[\{name=\"args\",value=\"0x0\"\}\\\],file=\"${any}${srcfile}\",fullname=\"${any}${srcfile}\",line=\"$thread_caller_line\"\}"
> +    set frame_re(0) ",frame=\{level=\"0\",addr=\"$hex\",func=\"child_sub_function\",args=\\\[\\\],file=\"${any}${srcfile}\",fullname=\"${any}${srcfile}\",line=\"$thread_loop_line\",arch=\".*\"\}"
> +    set frame_re(1) ",frame=\{level=\"1\",addr=\"$hex\",func=\"child_function\",args=\\\[\{name=\"args\",value=\"0x0\"\}\\\],file=\"${any}${srcfile}\",fullname=\"${any}${srcfile}\",line=\"$thread_caller_line\",arch=\".*\"\}"

I'd rather use a stricter \[^\r\n\]+ or better \[^"\]+ instead of ".*" throughout,
in case ".*" ends up too greedy and manages to consume further attributes,
like a bug that results in:

   ....arch="foo", arch="foo"}

or a bug/regression that causes arch to come out empty, like:

   ....arch=""}

or if .* makes a test manage to consume one frame too many,
or other things like that.  

(In this file, notice the "$any" variable.)

Thanks,
Pedro Alves