[PATCH] Display full file path in MI style disassembly listing

[PATCH] Display full file path in MI style disassembly listing

[Andrew Burgess]

When producing an MI style disassembly listing we use the shorted symtab filename, rather than computing the fullname.  This can make it harder for an MI consumer to figure out which file to open.

The patch below tries to use the fullname when it can, and falls back to the shorter name if it can't figure out the full name.

Ok to apply?

Thanks,
Andrew

gdb/ChangeLog

2012-10-04  Andrew Burhess  <aburgess@broadcom.com>

	* source.c (print_source_lines_base): Display full file name when
	producing MI style disassembly listings.

diff --git a/gdb/source.c b/gdb/source.c
index 31e104f..2a02382 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1298,9 +1298,19 @@ print_source_lines_base (struct symtab *s, int line, int stopline, int noerror)
 	}
       else
 	{
+	  char *filename;
+
+	  filename = s->fullname;
+	  if (filename == NULL)
+	    {
+	      filename = symtab_to_fullname (s);
+	      if (filename == NULL)
+		filename = s->filename;
+	    }
+
 	  ui_out_field_int (uiout, "line", line);
 	  ui_out_text (uiout, "\tin ");
-	  ui_out_field_string (uiout, "file", s->filename);
+	  ui_out_field_string (uiout, "file", filename);
 	  ui_out_text (uiout, "\n");
 	}
 

Re: [PATCH] Display full file path in MI style disassembly listing

[Jan Kratochvil]

On Thu, 04 Oct 2012 18:09:28 +0200, Andrew Burgess wrote:
> The patch below tries to use the fullname when it can, and falls back to the
> shorter name if it can't figure out the full name.

FYI this is a variant of not yet checked in more featured pathset:
[patch] GDB 7.2: new feature for "backtrace" that cuts path to file (remain filename)


Jan

Re: [PATCH] Display full file path in MI style disassembly listing

[Andrew Burgess]

On 05/10/2012 1:43 PM, Jan Kratochvil wrote:
> On Thu, 04 Oct 2012 18:09:28 +0200, Andrew Burgess wrote:
>> The patch below tries to use the fullname when it can, and falls back to the
>> shorter name if it can't figure out the full name.
> 
> FYI this is a variant of not yet checked in more featured pathset:
> [patch] GDB 7.2: new feature for "backtrace" that cuts path to file (remain filename)
> 

Thanks for taking the time to look through this patch.

I'm not sure where this leaves my patch though.  The patch you reference
has not had a post from the author for 6 months, so I'm not sure when or
if it will be merged.

Ideally I'd still like to merge this patch, what do you think?

Thanks,
Andrew

Re: [PATCH] Display full file path in MI style disassembly listing

[Jan Kratochvil]

On Sun, 07 Oct 2012 16:27:52 +0200, Andrew Burgess wrote:
> On 05/10/2012 1:43 PM, Jan Kratochvil wrote:
> > FYI this is a variant of not yet checked in more featured pathset:
> > [patch] GDB 7.2: new feature for "backtrace" that cuts path to file (remain filename)
> 
> Thanks for taking the time to look through this patch.
> 
> I'm not sure where this leaves my patch though.  The patch you reference
> has not had a post from the author for 6 months, so I'm not sure when or
> if it will be merged.
> 
> Ideally I'd still like to merge this patch, what do you think?

I have invested already many days into the patch above so will takeover it
myself.  Unfortunately I have more serious work to do now or even up to
January.


Jan

Re: [PATCH] Display full file path in MI style disassembly listing

[Joel Brobecker]

> I have invested already many days into the patch above so will takeover it
> myself.  Unfortunately I have more serious work to do now or even up to
> January.

In that case, let's have someone take the time to look at this patch
as a temporary measure.  It would be discouraging to have your patch
blocked for 2 months just because another more complete patch has been
put on hold.

-- 
Joel

Re: [PATCH] Display full file path in MI style disassembly listing

[Tom Tromey]

Jan> I have invested already many days into the patch above so will
Jan> takeover it myself.  Unfortunately I have more serious work to do
Jan> now or even up to January.

Joel> In that case, let's have someone take the time to look at this patch
Joel> as a temporary measure.  It would be discouraging to have your patch
Joel> blocked for 2 months just because another more complete patch has been
Joel> put on hold.

I agree with this approach.  However, I see this specific case as
borderline.  Andrew could in theory pick up the incomplete patch (but I
realize there are plenty of reasons why not); I think we're generally in
agreement that it is the direction we want to go.  Also I wonder whether
we'll have a release before Jan gets to the patch -- if not then it
seems ok-ish to say "we'd rather wait".

Tom

Re: [PATCH] Display full file path in MI style disassembly listing

[Pedro Alves]

On 10/05/2012 01:43 PM, Jan Kratochvil wrote:
> On Thu, 04 Oct 2012 18:09:28 +0200, Andrew Burgess wrote:
>> The patch below tries to use the fullname when it can, and falls back to the
>> shorter name if it can't figure out the full name.
> 
> FYI this is a variant of not yet checked in more featured pathset:
> [patch] GDB 7.2: new feature for "backtrace" that cuts path to file (remain filename)

I'm not sure I understand the relation between the patches.  I think we should
output a "fullname" field for MI, like we do for breakpoints.

-- 
Pedro Alves

Re: [PATCH] Display full file path in MI style disassembly listing

[Jan Kratochvil]

On Wed, 17 Oct 2012 20:13:40 +0200, Pedro Alves wrote:
> On 10/05/2012 01:43 PM, Jan Kratochvil wrote:
> > On Thu, 04 Oct 2012 18:09:28 +0200, Andrew Burgess wrote:
> >> The patch below tries to use the fullname when it can, and falls back to the
> >> shorter name if it can't figure out the full name.
> > 
> > FYI this is a variant of not yet checked in more featured pathset:
> > [patch] GDB 7.2: new feature for "backtrace" that cuts path to file (remain filename)
> 
> I'm not sure I understand the relation between the patches.  I think we should
> output a "fullname" field for MI, like we do for breakpoints.

Probably, it is true I do not know much how MI frontends display the data.


Thanks,
Jan

Re: [PATCH] Display full file path in MI style disassembly listing

[Andrew Burgess]

On 18/10/2012 7:48 AM, Jan Kratochvil wrote:
> On Wed, 17 Oct 2012 20:13:40 +0200, Pedro Alves wrote:
>> On 10/05/2012 01:43 PM, Jan Kratochvil wrote:
>>> On Thu, 04 Oct 2012 18:09:28 +0200, Andrew Burgess wrote:
>>>> The patch below tries to use the fullname when it can, and falls back to the
>>>> shorter name if it can't figure out the full name.
>>>
>>> FYI this is a variant of not yet checked in more featured pathset:
>>> [patch] GDB 7.2: new feature for "backtrace" that cuts path to file (remain filename)
>>
>> I'm not sure I understand the relation between the patches.

I agree that though both patches relate to display of paths they are not
directly related.

I took a look at the original patch.  I it changes one place (in
backtrace) where we display symtab->filename.  There are several places
where we display symtab->filename, including the one I changed.

Either, (a) we plan to merge the original patch almost as is, in which
case it only changes the backtrace code, leaving my patch free to be
applied, or (b) we plan to extend the original patch to cover more/all
of the places we display symtab->filename, in which case, we'd have to
consider each of those places in turn and make a suitable change.

If (a) then my patch could go in, if (b) then it doesn't feel like my
patch changes things that much, I certainly don't add any new functions
or anything like that, that would make doing task (b) any harder later on.

>>                                                              I think we should
>> output a "fullname" field for MI, like we do for breakpoints.

I would be happy to take this approach as a compromise.

> Probably, it is true I do not know much how MI frontends display the data.

The frontend (eclipse in this case) wants to open the file in order to
display the contents.  Using just symtab->filename is not always
specific enough to identify a unique file.

I originally switched to only displaying the full path as this code is
only used for MI type output or when we fail to open the file, but I can
see that displaying the fullpath in addition might be cleaner.

Jan, if I post a patch that adds a fullpath would you be happy with that
as a solution?

Thanks,
Andrew








> 
> 
> Thanks,
> Jan
> 
> 
> 

Re: [PATCH] Display full file path in MI style disassembly listing

[Pedro Alves]

On 10/18/2012 10:48 AM, Andrew Burgess wrote:

>>>                                                              I think we should
>>> output a "fullname" field for MI, like we do for breakpoints.
> 
> I would be happy to take this approach as a compromise.

IMO, it's not really a compromise.  It's all about giving all the data
to the frontend, so it can do whatever it pleases.

 - the file path as recorded in the compilation.  This is useful to have
   so the user can identify issues with fullpath mappings.
 - the file path gdb thinks that maps to in the filesystem, using the current
   settings of "directory/set directories/etc".

I'd even argue that the backtrace path in question should not change the "file"
field in MI, but instead it would add a new field to the output, if that's
also useful for MI.

-- 
Pedro Alves

Re: [PATCH] Display full file path in MI style disassembly listing

[André Pönitz]

On Thu, Oct 18, 2012 at 11:17:22AM +0100, Pedro Alves wrote:
> On 10/18/2012 10:48 AM, Andrew Burgess wrote:
> 
> >>> I think we should
> >>> output a "fullname" field for MI, like we do for breakpoints.
> > 
> > I would be happy to take this approach as a compromise.
> 
> IMO, it's not really a compromise.  It's all about giving all the data
> to the frontend, so it can do whatever it pleases.
> 
>  - the file path as recorded in the compilation.  This is useful to have
>    so the user can identify issues with fullpath mappings.
>  - the file path gdb thinks that maps to in the filesystem, using the current
>    settings of "directory/set directories/etc".

I'd personally also call it a proper solution, not a compromise.

> I'd even argue that the backtrace path in question should not change the "file"
> field in MI, but instead it would add a new field to the output, if that's
> also useful for MI.

From an MI consumer's point of view anything that provides the raw data and 
a "best effort interpretation" at the same time is perfect. Not providing
"raw data" leaves no room for fixups in case the "cooked" version is "wrong",
not providing the "cooked" version leaves the consumer with re-implementing
the "cooking", often enough without access to helpful auxilliary infomation.

Andre'

Re: [PATCH] Display full file path in MI style disassembly listing

[Jan Kratochvil]

On Thu, 18 Oct 2012 11:48:56 +0200, Andrew Burgess wrote:
> I took a look at the original patch.  I it changes one place (in
> backtrace) where we display symtab->filename.  There are several places
> where we display symtab->filename, including the one I changed.
> 
> Either, (a) we plan to merge the original patch almost as is, in which
> case it only changes the backtrace code, leaving my patch free to be
> applied, or (b) we plan to extend the original patch to cover more/all
> of the places we display symtab->filename, in which case, we'd have to
> consider each of those places in turn and make a suitable change.

(b) but this seems to be irrelevant to this patch now.


> Jan, if I post a patch that adds a fullpath would you be happy with that
> as a solution?

Pedro IIUC has shown that if anywhere in MI is missing the "fullname" field
you can add it, so far there is no need to change the content of "file" field
for the MI purposes.  I support this Pedro's proposal.


Thanks,
Jan

Re: [PATCH] Display full file path in MI style disassembly listing

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@broadcom.com> writes:

Andrew> +	  char *filename;

I think this could be const.

Andrew> +
Andrew> +	  filename = s->fullname;
Andrew> +	  if (filename == NULL)
Andrew> +	    {
Andrew> +	      filename = symtab_to_fullname (s);
Andrew> +	      if (filename == NULL)
Andrew> +		filename = s->filename;
Andrew> +	    }
Andrew> +
Andrew>  	  ui_out_field_int (uiout, "line", line);
Andrew>  	  ui_out_text (uiout, "\tin ");
Andrew> -	  ui_out_field_string (uiout, "file", s->filename);
Andrew> +	  ui_out_field_string (uiout, "file", filename);
Andrew>  	  ui_out_text (uiout, "\n");

It isn't obvious to me that this only applies to the MI case.
Can you explain that?

Tom

Re: [PATCH] Display full file path in MI style disassembly listing

[Andrew Burgess]

On 17/10/2012 6:15 PM, Tom Tromey wrote:
>>>>>> "Andrew" == Andrew Burgess <aburgess@broadcom.com> writes:
> 
> Andrew> +	  char *filename;
> 
> I think this could be const.
> 
> Andrew> +
> Andrew> +	  filename = s->fullname;
> Andrew> +	  if (filename == NULL)
> Andrew> +	    {
> Andrew> +	      filename = symtab_to_fullname (s);
> Andrew> +	      if (filename == NULL)
> Andrew> +		filename = s->filename;
> Andrew> +	    }
> Andrew> +
> Andrew>  	  ui_out_field_int (uiout, "line", line);
> Andrew>  	  ui_out_text (uiout, "\tin ");
> Andrew> -	  ui_out_field_string (uiout, "file", s->filename);
> Andrew> +	  ui_out_field_string (uiout, "file", filename);
> Andrew>  	  ui_out_text (uiout, "\n");
> 
> It isn't obvious to me that this only applies to the MI case.
> Can you explain that?

You are correct, and my subject line is miss-leading.  It is possible to
hit this code from cli gdb, but I believe this only happens if we fail
to open the file.

 - we first test "ui_out_test_flags (uiout, ui_source_list)" which I
believe is always true for cli gdb.  I couldn't see a way this can be
turned off, but maybe I missed something.
 - we then try to open the file in question.

If we're not printing source (mi) or the open failed (mi/cli) then we'll
pass through the code I changed.

Thanks,

Andrew

Re: [PATCH] Display full file path in MI style disassembly listing

[Jan Kratochvil]

On Thu, 18 Oct 2012 11:34:15 +0200, Andrew Burgess wrote:
> On 17/10/2012 6:15 PM, Tom Tromey wrote:
> >>>>>> "Andrew" == Andrew Burgess <aburgess@broadcom.com> writes:
> > It isn't obvious to me that this only applies to the MI case.
> > Can you explain that?
> 
> You are correct, and my subject line is miss-leading.  It is possible to
> hit this code from cli gdb, but I believe this only happens if we fail
> to open the file.

This discussion is difficult without backing the patch with a testcase, at
least for the cases you want to change (cases you do not want to change would
be also nice to check but I understand it may be too broad to cover).

Otherwise the "sleeping" patch I mentioned I would like to push again later
may likely break this new functionality without noticing it.


Thanks,
Jan

Re: [PATCH] Display full file path in MI style disassembly listing

[Pedro Alves]

On 10/04/2012 05:09 PM, Andrew Burgess wrote:
> When producing an MI style disassembly listing we use the shorted symtab filename, rather than computing the fullname.  This can make it harder for an MI consumer to figure out which file to open.
> 
> The patch below tries to use the fullname when it can, and falls back to the shorter name if it can't figure out the full name.
> 
> Ok to apply?

At least with breakpoints, we leave "file" to whatever is recorded in the symtab,
and output a "fullname" MI field in addition, with whatever GDB resolves as the file's
path in the filesystem using its current source location rules:

b main
&"b main\n"
~"Breakpoint 6 at 0x457ceb: file ../../src/gdb/gdb.c, line 29.\n"
=breakpoint-created,bkpt={number="6",type="breakpoint",disp="keep",enabled="y",addr="0x0000000000457ceb",func="main",file="../../src/gdb/gdb.c",fullname="/home/pedro/gdb/mygit/src/gdb/gdb.c",line="29",times="0",original-location="main"}

I'd think it better to be consistent here.

> 
> Thanks,
> Andrew
> 
> gdb/ChangeLog
> 
> 2012-10-04  Andrew Burhess  <aburgess@broadcom.com>
> 
> 	* source.c (print_source_lines_base): Display full file name when
> 	producing MI style disassembly listings.
> 
> diff --git a/gdb/source.c b/gdb/source.c
> index 31e104f..2a02382 100644
> --- a/gdb/source.c
> +++ b/gdb/source.c
> @@ -1298,9 +1298,19 @@ print_source_lines_base (struct symtab *s, int line, int stopline, int noerror)
>  	}
>        else
>  	{
> +	  char *filename;
> +
> +	  filename = s->fullname;
> +	  if (filename == NULL)
> +	    {
> +	      filename = symtab_to_fullname (s);
> +	      if (filename == NULL)
> +		filename = s->filename;
> +	    }
> +
>  	  ui_out_field_int (uiout, "line", line);
>  	  ui_out_text (uiout, "\tin ");
> -	  ui_out_field_string (uiout, "file", s->filename);
> +	  ui_out_field_string (uiout, "file", filename);
>  	  ui_out_text (uiout, "\n");
>  	}
>  
> 


-- 
Pedro Alves

Add fullname field in disassembly output (Was Re: [PATCH] Display full file path in MI style disassembly listing)

[Andrew Burgess]

So after feedback here's a new patch to add a fullname field to the disassembly output when in mi mode.  I've updated the mi-disassembly test to match.

Ok to commit?

Thanks,

Andrew


gdb/ChangeLog

2012-10-22  Andrew Burgess  <aburgess@broadcom.com>

	* source.c (print_source_lines_base): Add fullname field giving
	full path to file in mi output.

diff --git a/gdb/source.c b/gdb/source.c
index bd11c63..465382e 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1301,6 +1301,13 @@ print_source_lines_base (struct symtab *s, int line, int stopline, int noerror)
 	  ui_out_field_int (uiout, "line", line);
 	  ui_out_text (uiout, "\tin ");
 	  ui_out_field_string (uiout, "file", s->filename);
+	  if (ui_out_is_mi_like_p (uiout))
+	    {
+	      const char *fullname = symtab_to_fullname (s);
+
+	      if (fullname != NULL)
+		ui_out_field_string (uiout, "fullname", fullname);
+	    }
 	  ui_out_text (uiout, "\n");
 	}


 
gdb/testsuite/ChangeLog

2012-10-22  Andrew Burgess  <aburgess@broadcom.com>

	* gdb.mi/mi-disassemble.exp: Expect fullname field in mi
	disassembly output.

diff --git a/gdb/testsuite/gdb.mi/mi-disassemble.exp b/gdb/testsuite/gdb.mi/mi-disassemble.exp
index 377ffde..695521a 100644
--- a/gdb/testsuite/gdb.mi/mi-disassemble.exp
+++ b/gdb/testsuite/gdb.mi/mi-disassemble.exp
@@ -115,6 +115,7 @@ proc test_disassembly_mixed {} {
     global mi_gdb_prompt
     global hex
     global decimal
+    global fullname_syntax
 
     set line_callee2_head       [gdb_get_line_number "callee2 ("]
     set line_callee2_open_brace [expr $line_callee2_head + 1]
@@ -125,7 +126,7 @@ proc test_disassembly_mixed {} {
     # -data-disassembly -s $pc -e "$pc+8" -- 1
 
     mi_gdb_test "002-data-disassemble -f basics.c -l $line_callee2_open_brace -- 1" \
-	    "002\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$line_callee2_open_brace\",file=\".*basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"callee2\",offset=\"0\",inst=\".*\"\}.*\\\]\}.*,src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",line_asm_insn=\\\[.*\{address=\"$hex\",func-name=\"callee2\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\\\]" \
+	    "002\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$line_callee2_open_brace\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"callee2\",offset=\"0\",inst=\".*\"\}.*\\\]\}.*,src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[.*\{address=\"$hex\",func-name=\"callee2\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\\\]" \
              "data-disassemble file, line assembly mixed"
 
     #
@@ -134,7 +135,7 @@ proc test_disassembly_mixed {} {
     # which we are now, even if we have specified that the range is only 2 insns.
     #
     mi_gdb_test "003-data-disassemble -s \$pc -e \"\$pc+4\" -- 1" \
-	    "003\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}.*\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\\\]" \
+	    "003\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}.*\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\\\]" \
              "data-disassemble range assembly mixed"
 }
 
@@ -142,6 +143,7 @@ proc test_disassembly_mixed_with_opcodes {} {
     global mi_gdb_prompt
     global hex
     global decimal
+    global fullname_syntax
 
     set line_callee2_head       [gdb_get_line_number "callee2 ("]
     set line_callee2_open_brace [expr $line_callee2_head + 1]
@@ -152,7 +154,7 @@ proc test_disassembly_mixed_with_opcodes {} {
     # -data-disassembly -s $pc -e "$pc+8" -- 3
 
     mi_gdb_test "002-data-disassemble -f basics.c -l $line_callee2_open_brace -- 3" \
-           "002\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$line_callee2_open_brace\",file=\".*basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"callee2\",offset=\"0\",opcodes=\".*\",inst=\".*\"\}.*\\\]\}.*,src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",line_asm_insn=\\\[.*\{address=\"$hex\",func-name=\"callee2\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}\\\]\}\\\]" \
+           "002\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$line_callee2_open_brace\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"callee2\",offset=\"0\",opcodes=\".*\",inst=\".*\"\}.*\\\]\}.*,src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[.*\{address=\"$hex\",func-name=\"callee2\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}\\\]\}\\\]" \
              "data-disassemble file, line assembly mixed with opcodes"
 
     #
@@ -161,7 +163,7 @@ proc test_disassembly_mixed_with_opcodes {} {
     # which we are now, even if we have specified that the range is only 2 insns.
     #
     mi_gdb_test "003-data-disassemble -s \$pc -e \"\$pc+4\" -- 3" \
-           "003\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}.*\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}\\\]\}\\\]" \
+           "003\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}.*\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}\\\]\}\\\]" \
              "data-disassemble range assembly mixed with opcodes"
 }
 
@@ -169,6 +171,7 @@ proc test_disassembly_mixed_lines_limit {} {
     global mi_gdb_prompt
     global hex
     global decimal
+    global fullname_syntax
 
     set line_main_head       [gdb_get_line_number "main ("]
     set line_main_open_brace [expr $line_main_head + 1]
@@ -182,15 +185,15 @@ proc test_disassembly_mixed_lines_limit {} {
 
     mi_gdb_test "print/x \$pc" "" ""
     mi_gdb_test "222-data-disassemble  -f basics.c -l $line_main_body -n 20 -- 1" \
-	    "222\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",inst=\".*\"\},.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\]" \
+	    "222\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",inst=\".*\"\},.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\]" \
               "data-disassemble file, line, number assembly mixed"
 
     mi_gdb_test "222-data-disassemble  -f basics.c -l $line_main_body -n 0 -- 1" \
-	    "222\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$line_main_open_brace\",file=\".*basics.c\",line_asm_insn=\\\[\\\]\}\\\]" \
+	    "222\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$line_main_open_brace\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\\\]\}\\\]" \
               "data-disassemble file, line, number (zero lines) assembly mixed"
 
     mi_gdb_test "222-data-disassemble  -f basics.c -l $line_main_body -n 50 -- 1" \
-	    "222\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",inst=\".*\"\}.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\]" \
+	    "222\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",inst=\".*\"\}.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\]" \
               "data-disassemble file, line, number (more than main lines) assembly mixed"
 }
 

Re: Add fullname field in disassembly output

[Pedro Alves]

On 10/22/2012 10:25 PM, Andrew Burgess wrote:
> So after feedback here's a new patch to add a fullname field to the disassembly output when in mi mode.  I've updated the mi-disassembly test to match.

Thanks.

> 
> Ok to commit?

Not yet, sorry.  This new field needs to be documented in the manual, and mentioned in NEWS.

-- 
Pedro Alves

Re: Add fullname field in disassembly output

[Andrew Burgess]

On 31/10/2012 2:53 PM, Pedro Alves wrote:

>> Ok to commit?
> 
> Not yet, sorry.  This new field needs to be documented in the manual, and mentioned in NEWS.

Sorry, I should have realised I'd need to do these things.

Latest version of patch, includes fullname field, test
updates, noted in documentation and in NEWS file.

Is this Ok?


Thanks,

Andrew



gdb/ChangeLog

2012-11-02  Andrew Burhess  <aburgess@broadcom.com>

	* source.c (print_source_lines_base): Add fullname field giving
	full path to file in mi output.
	* NEWS: Mention the new fullname field.

diff --git a/gdb/NEWS b/gdb/NEWS
index 069b84f..f953602 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -64,6 +64,9 @@ py [command]
      async record "=record-started" and "=record-stopped".
   ** Memory changes are now notified using new async record
      "=memory-changed".
+  ** The data-disassemble command response will include a "fullname" field
+     containing the full path to the source file when gdb can determine the
+     full path, and source has been requested in the output.
 
 *** Changes in GDB 7.5

diff --git a/gdb/source.c b/gdb/source.c
index bd11c63..465382e 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1301,6 +1301,13 @@ print_source_lines_base (struct symtab *s, int line, int stopline, int noerror)
 	  ui_out_field_int (uiout, "line", line);
 	  ui_out_text (uiout, "\tin ");
 	  ui_out_field_string (uiout, "file", s->filename);
+	  if (ui_out_is_mi_like_p (uiout))
+	    {
+	      const char *fullname = symtab_to_fullname (s);
+
+	      if (fullname != NULL)
+		ui_out_field_string (uiout, "fullname", fullname);
+	    }
 	  ui_out_text (uiout, "\n");
 	}



 
gdb/doc/ChangeLog

2012-11-02  Andrew Burgess  <aburgess@broadcom.com>

	* gdb.texinfo (GDB/MI Data Manipulation): Add fullname field to
	the example -data-disassemble output.

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 21db475..88ee695 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -30795,13 +30795,15 @@ Disassemble 3 instructions from the start of @code{main} in mixed mode:
 -data-disassemble -f basics.c -l 32 -n 3 -- 1
 ^done,asm_insns=[
 src_and_asm_line=@{line="31",
-file="/kwikemart/marge/ezannoni/flathead-dev/devo/gdb/ \
-  testsuite/gdb.mi/basics.c",line_asm_insn=[
+file="../../../src/gdb/testsuite/gdb.mi/basics.c",
+fullname="/absolute/path/to/src/gdb/testsuite/ \
+gdb.mi/basics.c",line_asm_insn=[
 @{address="0x000107bc",func-name="main",offset="0",
 inst="save  %sp, -112, %sp"@}]@},
 src_and_asm_line=@{line="32",
-file="/kwikemart/marge/ezannoni/flathead-dev/devo/gdb/ \
-  testsuite/gdb.mi/basics.c",line_asm_insn=[
+file="../../../src/gdb/testsuite/gdb.mi/basics.c",
+fullname="/absolute/path/to/src/gdb/testsuite/ \
+gdb.mi/basics.c",line_asm_insn=[
 @{address="0x000107c0",func-name="main",offset="4",
 inst="mov  2, %o0"@},
 @{address="0x000107c4",func-name="main",offset="8",



 
gdb/testsuite/ChangeLog

2012-11-02  Andrew Burgess  <aburgess@broadcom.com>

	* gdb.mi/mi-disassemble.exp: Expect fullname field in mi
	disassembly output.

diff --git a/gdb/testsuite/gdb.mi/mi-disassemble.exp b/gdb/testsuite/gdb.mi/mi-disassemble.exp
index 377ffde..695521a 100644
--- a/gdb/testsuite/gdb.mi/mi-disassemble.exp
+++ b/gdb/testsuite/gdb.mi/mi-disassemble.exp
@@ -115,6 +115,7 @@ proc test_disassembly_mixed {} {
     global mi_gdb_prompt
     global hex
     global decimal
+    global fullname_syntax
 
     set line_callee2_head       [gdb_get_line_number "callee2 ("]
     set line_callee2_open_brace [expr $line_callee2_head + 1]
@@ -125,7 +126,7 @@ proc test_disassembly_mixed {} {
     # -data-disassembly -s $pc -e "$pc+8" -- 1
 
     mi_gdb_test "002-data-disassemble -f basics.c -l $line_callee2_open_brace -- 1" \
-	    "002\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$line_callee2_open_brace\",file=\".*basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"callee2\",offset=\"0\",inst=\".*\"\}.*\\\]\}.*,src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",line_asm_insn=\\\[.*\{address=\"$hex\",func-name=\"callee2\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\\\]" \
+	    "002\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$line_callee2_open_brace\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"callee2\",offset=\"0\",inst=\".*\"\}.*\\\]\}.*,src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[.*\{address=\"$hex\",func-name=\"callee2\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\\\]" \
              "data-disassemble file, line assembly mixed"
 
     #
@@ -134,7 +135,7 @@ proc test_disassembly_mixed {} {
     # which we are now, even if we have specified that the range is only 2 insns.
     #
     mi_gdb_test "003-data-disassemble -s \$pc -e \"\$pc+4\" -- 1" \
-	    "003\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}.*\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\\\]" \
+	    "003\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}.*\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\\\]" \
              "data-disassemble range assembly mixed"
 }
 
@@ -142,6 +143,7 @@ proc test_disassembly_mixed_with_opcodes {} {
     global mi_gdb_prompt
     global hex
     global decimal
+    global fullname_syntax
 
     set line_callee2_head       [gdb_get_line_number "callee2 ("]
     set line_callee2_open_brace [expr $line_callee2_head + 1]
@@ -152,7 +154,7 @@ proc test_disassembly_mixed_with_opcodes {} {
     # -data-disassembly -s $pc -e "$pc+8" -- 3
 
     mi_gdb_test "002-data-disassemble -f basics.c -l $line_callee2_open_brace -- 3" \
-           "002\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$line_callee2_open_brace\",file=\".*basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"callee2\",offset=\"0\",opcodes=\".*\",inst=\".*\"\}.*\\\]\}.*,src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",line_asm_insn=\\\[.*\{address=\"$hex\",func-name=\"callee2\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}\\\]\}\\\]" \
+           "002\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$line_callee2_open_brace\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"callee2\",offset=\"0\",opcodes=\".*\",inst=\".*\"\}.*\\\]\}.*,src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[.*\{address=\"$hex\",func-name=\"callee2\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}\\\]\}\\\]" \
              "data-disassemble file, line assembly mixed with opcodes"
 
     #
@@ -161,7 +163,7 @@ proc test_disassembly_mixed_with_opcodes {} {
     # which we are now, even if we have specified that the range is only 2 insns.
     #
     mi_gdb_test "003-data-disassemble -s \$pc -e \"\$pc+4\" -- 3" \
-           "003\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}.*\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}\\\]\}\\\]" \
+           "003\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}.*\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}\\\]\}\\\]" \
              "data-disassemble range assembly mixed with opcodes"
 }
 
@@ -169,6 +171,7 @@ proc test_disassembly_mixed_lines_limit {} {
     global mi_gdb_prompt
     global hex
     global decimal
+    global fullname_syntax
 
     set line_main_head       [gdb_get_line_number "main ("]
     set line_main_open_brace [expr $line_main_head + 1]
@@ -182,15 +185,15 @@ proc test_disassembly_mixed_lines_limit {} {
 
     mi_gdb_test "print/x \$pc" "" ""
     mi_gdb_test "222-data-disassemble  -f basics.c -l $line_main_body -n 20 -- 1" \
-	    "222\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",inst=\".*\"\},.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\]" \
+	    "222\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",inst=\".*\"\},.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\]" \
               "data-disassemble file, line, number assembly mixed"
 
     mi_gdb_test "222-data-disassemble  -f basics.c -l $line_main_body -n 0 -- 1" \
-	    "222\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$line_main_open_brace\",file=\".*basics.c\",line_asm_insn=\\\[\\\]\}\\\]" \
+	    "222\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$line_main_open_brace\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\\\]\}\\\]" \
               "data-disassemble file, line, number (zero lines) assembly mixed"
 
     mi_gdb_test "222-data-disassemble  -f basics.c -l $line_main_body -n 50 -- 1" \
-	    "222\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",inst=\".*\"\}.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\]" \
+	    "222\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",inst=\".*\"\}.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\]" \
               "data-disassemble file, line, number (more than main lines) assembly mixed"
 }
 

Re: Add fullname field in disassembly output

[Pedro Alves]

On 11/02/2012 10:59 AM, Andrew Burgess wrote:
> On 31/10/2012 2:53 PM, Pedro Alves wrote:
> 
>>> Ok to commit?
>>
>> Not yet, sorry.  This new field needs to be documented in the manual, and mentioned in NEWS.
> 
> Sorry, I should have realised I'd need to do these things.
> 
> Latest version of patch, includes fullname field, test
> updates, noted in documentation and in NEWS file.
> 
> Is this Ok?

The code looks good to me, thanks.

But it looks strange to me that NEWS has more detail on the new output
than the docs.  IMO, it would be good if, e.g., something around the
part that reads:

  The output for each instruction is composed of four fields:

was updated to reflect the new field.  The short examples alone
don't explain what the field is.

In any case, Eli will give you the final word on the docs bits.

Thanks,

-- 
Pedro Alves

Re: Add fullname field in disassembly output

[Andrew Burgess]

On 02/11/2012 3:32 PM, Pedro Alves wrote:
> 
> But it looks strange to me that NEWS has more detail on the new output
> than the docs.  IMO, it would be good if, e.g., something around the
> part that reads:
> 
>   The output for each instruction is composed of four fields:
> 
> was updated to reflect the new field.  The short examples alone
> don't explain what the field is.
> 
> In any case, Eli will give you the final word on the docs bits.

So I agree that the documentation for -data-disassemble has been allowed to get out of date with all the new features available.  As such, just adding the new field to the list you suggest would be more confusing I think.

So, new patch below, the code changes are the same, but I've attempted to improve the documentation for -data-disassemble, this now includes not only the new field I've added, but all the other fields that were not documented.

I've also filled in the cli 'disassemble' command as being the related cli command as it offers most (all?) of the features that -data-disassemble offers.

I tried to draw inspiration from surrounding documentation, but I suspect this will need a few changes to get right :) All feeback welcome.

Thanks,
Andrew

gdb/ChangeLog

2012-11-02  Andrew Burgess  <aburgess@broadcom.com>

	* source.c (print_source_lines_base): Add fullname field giving
	full path to file in mi output.
	* NEWS: Mention the new fullname field.


diff --git a/gdb/NEWS b/gdb/NEWS
index 069b84f..ccc9ba1 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -64,6 +64,9 @@ py [command]
      async record "=record-started" and "=record-stopped".
   ** Memory changes are now notified using new async record
      "=memory-changed".
+  ** The data-disassemble command response will include a "fullname" field
+     containing the absolute file name when gdb can determine it and source
+     has been requested.
 
 *** Changes in GDB 7.5

diff --git a/gdb/source.c b/gdb/source.c
index bd11c63..465382e 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1301,6 +1301,13 @@ print_source_lines_base (struct symtab *s, int line, int stopline, int noerror)
 	  ui_out_field_int (uiout, "line", line);
 	  ui_out_text (uiout, "\tin ");
 	  ui_out_field_string (uiout, "file", s->filename);
+	  if (ui_out_is_mi_like_p (uiout))
+	    {
+	      const char *fullname = symtab_to_fullname (s);
+
+	      if (fullname != NULL)
+		ui_out_field_string (uiout, "fullname", fullname);
+	    }
 	  ui_out_text (uiout, "\n");
 	}



gdb/doc/ChangeLog

2012-11-02  Andrew Burgess  <aburgess@broadcom.com>

	* gdb.texinfo (GDB/MI Data Manipulation): Add fullname field to
	the example -data-disassemble output.  Extend the description of
	the -data-disassemble results to document all fields.  Document
	the cli disassemble command as being related to -data-disassemble.

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 21db475..ebec4e5 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -30717,21 +30717,65 @@ mixed source and disassembly with raw opcodes).
 
 @subsubheading Result
 
-The output for each instruction is composed of four fields:
+The result of the -data-disassemble command will be a list named
+@samp{asm_insns}, the contents of this list depend on the @var{mode}
+used with the -data-disassemble command.
 
-@itemize @bullet
-@item Address
-@item Func-name
-@item Offset
-@item Instruction
-@end itemize
+For modes 0 and 2 the @samp{asm_insns} list contains tuples with the
+following fields:
 
-Note that whatever included in the instruction field, is not manipulated
-directly by @sc{gdb/mi}, i.e., it is not possible to adjust its format.
+@table @code
+@item address
+The address at which this instruction was disassembled.
+
+@item func-name
+The name of the function this instruction is within.
+
+@item offset
+The decimal offset in bytes from the start of @samp{func-name}.
+
+@item inst
+The text disassembly for this @samp{address}.
+
+@item opcodes
+This field is only present for mode 2.  This contains the raw opcode
+bytes for the @samp{inst} field.
+
+@end table
+
+For modes 1 and 3 the @samp{asm_insns} list contains tuples names
+@samp{src_and_asm_line}, each of which has the following fields:
+
+@table @code
+@item line
+The line number within @samp{file}.
+
+@item file
+The file name from the compilation unit.  This might be an absolute
+file name or a relative file name depending on the compile command
+used.
+
+@item fullname
+This field is optional.  If it is present it will contain an absolute
+file name to @samp{file}.  If this field is not present then gdb was
+unable to determine the absolute file name.
+
+@item line_asm_insn
+This is a list of tuples containing the disassembly for @samp{line} in
+@samp{file}.  The fields of each tuple are the same as for
+-data-disassemble in @var{mode} 0 and 2, so @samp{address},
+@samp{func-name}, @samp{offset}, @samp{inst}, and optionally
+@samp{opcodes}.
+
+@end table
+
+Note that whatever included in the @samp{inst} field, is not
+manipulated directly by @sc{gdb/mi}, i.e., it is not possible to
+adjust its format.
 
 @subsubheading @value{GDBN} Command
 
-There's no direct mapping from this command to the CLI.
+The corresponding @value{GDBN} command is @samp{disassemble}.
 
 @subsubheading Example
 
@@ -30795,15 +30839,15 @@ Disassemble 3 instructions from the start of @code{main} in mixed mode:
 -data-disassemble -f basics.c -l 32 -n 3 -- 1
 ^done,asm_insns=[
 src_and_asm_line=@{line="31",
-file="/kwikemart/marge/ezannoni/flathead-dev/devo/gdb/ \
-  testsuite/gdb.mi/basics.c",line_asm_insn=[
-@{address="0x000107bc",func-name="main",offset="0",
-inst="save  %sp, -112, %sp"@}]@},
+file="../../../src/gdb/testsuite/gdb.mi/basics.c",
+fullname="/absolute/path/to/src/gdb/testsuite/gdb.mi/basics.c",
+line_asm_insn=[@{address="0x000107bc",
+func-name="main",offset="0",inst="save  %sp, -112, %sp"@}]@},
 src_and_asm_line=@{line="32",
-file="/kwikemart/marge/ezannoni/flathead-dev/devo/gdb/ \
-  testsuite/gdb.mi/basics.c",line_asm_insn=[
-@{address="0x000107c0",func-name="main",offset="4",
-inst="mov  2, %o0"@},
+file="../../../src/gdb/testsuite/gdb.mi/basics.c",
+fullname="/absolute/path/to/src/gdb/testsuite/gdb.mi/basics.c",
+line_asm_insn=[@{address="0x000107c0",
+func-name="main",offset="4",inst="mov  2, %o0"@},
 @{address="0x000107c4",func-name="main",offset="8",
 inst="sethi  %hi(0x11800), %o2"@}]@}]
 (gdb)


gdb/testsuite/ChangeLog

2012-11-02  Andrew Burgess  <aburgess@broadcom.com>

	* gdb.mi/mi-disassemble.exp: Expect fullname field in mi
	disassembly output.

diff --git a/gdb/testsuite/gdb.mi/mi-disassemble.exp b/gdb/testsuite/gdb.mi/mi-disassemble.exp
index 377ffde..695521a 100644
--- a/gdb/testsuite/gdb.mi/mi-disassemble.exp
+++ b/gdb/testsuite/gdb.mi/mi-disassemble.exp
@@ -115,6 +115,7 @@ proc test_disassembly_mixed {} {
     global mi_gdb_prompt
     global hex
     global decimal
+    global fullname_syntax
 
     set line_callee2_head       [gdb_get_line_number "callee2 ("]
     set line_callee2_open_brace [expr $line_callee2_head + 1]
@@ -125,7 +126,7 @@ proc test_disassembly_mixed {} {
     # -data-disassembly -s $pc -e "$pc+8" -- 1
 
     mi_gdb_test "002-data-disassemble -f basics.c -l $line_callee2_open_brace -- 1" \
-	    "002\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$line_callee2_open_brace\",file=\".*basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"callee2\",offset=\"0\",inst=\".*\"\}.*\\\]\}.*,src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",line_asm_insn=\\\[.*\{address=\"$hex\",func-name=\"callee2\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\\\]" \
+	    "002\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$line_callee2_open_brace\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"callee2\",offset=\"0\",inst=\".*\"\}.*\\\]\}.*,src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[.*\{address=\"$hex\",func-name=\"callee2\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\\\]" \
              "data-disassemble file, line assembly mixed"
 
     #
@@ -134,7 +135,7 @@ proc test_disassembly_mixed {} {
     # which we are now, even if we have specified that the range is only 2 insns.
     #
     mi_gdb_test "003-data-disassemble -s \$pc -e \"\$pc+4\" -- 1" \
-	    "003\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}.*\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\\\]" \
+	    "003\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}.*\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\\\]" \
              "data-disassemble range assembly mixed"
 }
 
@@ -142,6 +143,7 @@ proc test_disassembly_mixed_with_opcodes {} {
     global mi_gdb_prompt
     global hex
     global decimal
+    global fullname_syntax
 
     set line_callee2_head       [gdb_get_line_number "callee2 ("]
     set line_callee2_open_brace [expr $line_callee2_head + 1]
@@ -152,7 +154,7 @@ proc test_disassembly_mixed_with_opcodes {} {
     # -data-disassembly -s $pc -e "$pc+8" -- 3
 
     mi_gdb_test "002-data-disassemble -f basics.c -l $line_callee2_open_brace -- 3" \
-           "002\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$line_callee2_open_brace\",file=\".*basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"callee2\",offset=\"0\",opcodes=\".*\",inst=\".*\"\}.*\\\]\}.*,src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",line_asm_insn=\\\[.*\{address=\"$hex\",func-name=\"callee2\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}\\\]\}\\\]" \
+           "002\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$line_callee2_open_brace\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"callee2\",offset=\"0\",opcodes=\".*\",inst=\".*\"\}.*\\\]\}.*,src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[.*\{address=\"$hex\",func-name=\"callee2\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}\\\]\}\\\]" \
              "data-disassemble file, line assembly mixed with opcodes"
 
     #
@@ -161,7 +163,7 @@ proc test_disassembly_mixed_with_opcodes {} {
     # which we are now, even if we have specified that the range is only 2 insns.
     #
     mi_gdb_test "003-data-disassemble -s \$pc -e \"\$pc+4\" -- 3" \
-           "003\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}.*\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}\\\]\}\\\]" \
+           "003\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}.*\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",opcodes=\".*\",inst=\".*\"\}\\\]\}\\\]" \
              "data-disassemble range assembly mixed with opcodes"
 }
 
@@ -169,6 +171,7 @@ proc test_disassembly_mixed_lines_limit {} {
     global mi_gdb_prompt
     global hex
     global decimal
+    global fullname_syntax
 
     set line_main_head       [gdb_get_line_number "main ("]
     set line_main_open_brace [expr $line_main_head + 1]
@@ -182,15 +185,15 @@ proc test_disassembly_mixed_lines_limit {} {
 
     mi_gdb_test "print/x \$pc" "" ""
     mi_gdb_test "222-data-disassemble  -f basics.c -l $line_main_body -n 20 -- 1" \
-	    "222\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",inst=\".*\"\},.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\]" \
+	    "222\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",inst=\".*\"\},.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\]" \
               "data-disassemble file, line, number assembly mixed"
 
     mi_gdb_test "222-data-disassemble  -f basics.c -l $line_main_body -n 0 -- 1" \
-	    "222\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$line_main_open_brace\",file=\".*basics.c\",line_asm_insn=\\\[\\\]\}\\\]" \
+	    "222\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$line_main_open_brace\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\\\]\}\\\]" \
               "data-disassemble file, line, number (zero lines) assembly mixed"
 
     mi_gdb_test "222-data-disassemble  -f basics.c -l $line_main_body -n 50 -- 1" \
-	    "222\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",inst=\".*\"\}.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\]" \
+	    "222\\^done,asm_insns=\\\[src_and_asm_line=\{line=\"$decimal\",file=\".*basics.c\",fullname=\"${fullname_syntax}basics.c\",line_asm_insn=\\\[\{address=\"$hex\",func-name=\"main\",offset=\"0\",inst=\".*\"\}.*,\{address=\"$hex\",func-name=\"main\",offset=\"$decimal\",inst=\".*\"\}\\\]\}\]" \
               "data-disassemble file, line, number (more than main lines) assembly mixed"
 }
 

Re: Add fullname field in disassembly output

[Eli Zaretskii]

> Date: Tue, 6 Nov 2012 12:13:36 +0000
> From: "Andrew Burgess" <aburgess@broadcom.com>
> cc: gdb-patches@sourceware.org,	"Eli Zaretskii" <eliz@gnu.org>
> 
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -64,6 +64,9 @@ py [command]
>       async record "=record-started" and "=record-stopped".
>    ** Memory changes are now notified using new async record
>       "=memory-changed".
> +  ** The data-disassemble command response will include a "fullname" field
> +     containing the absolute file name when gdb can determine it and source
> +     has been requested.

This part is OK, except that I believe we use "GDB", not "gdb".

> +The result of the -data-disassemble command will be a list named

"-data-disassemble" should be in @code.

> +@samp{asm_insns}, the contents of this list depend on the @var{mode}
> +used with the -data-disassemble command.

Same here.

> +For modes 1 and 3 the @samp{asm_insns} list contains tuples names
                                                               ^^^^^
"named", I guess.

> +@samp{src_and_asm_line}, each of which has the following fields:
> +
> +@table @code
> +@item line
> +The line number within @samp{file}.
> +
> +@item file
> +The file name from the compilation unit.  This might be an absolute
> +file name or a relative file name depending on the compile command
> +used.
> +
> +@item fullname
> +This field is optional.  If it is present it will contain an absolute
> +file name to @samp{file}.  If this field is not present then gdb was
             ^^                                                 ^^^
"of", not "to", and "@value{GDBN}" instead of "gdb".

> +-data-disassemble in @var{mode} 0 and 2, so @samp{address},
   ^^^^^^^^^^^^^^^^^
@code

The patch for the manual is OK with these changes.

> +file="../../../src/gdb/testsuite/gdb.mi/basics.c",
> +fullname="/absolute/path/to/src/gdb/testsuite/gdb.mi/basics.c",
> +line_asm_insn=[@{address="0x000107bc",
> +func-name="main",offset="0",inst="save  %sp, -112, %sp"@}]@},
>  src_and_asm_line=@{line="32",
> -file="/kwikemart/marge/ezannoni/flathead-dev/devo/gdb/ \
> -  testsuite/gdb.mi/basics.c",line_asm_insn=[
> -@{address="0x000107c0",func-name="main",offset="4",
> -inst="mov  2, %o0"@},
> +file="../../../src/gdb/testsuite/gdb.mi/basics.c",
> +fullname="/absolute/path/to/src/gdb/testsuite/gdb.mi/basics.c",
> +line_asm_insn=[@{address="0x000107c0",
> +func-name="main",offset="4",inst="mov  2, %o0"@},
>  @{address="0x000107c4",func-name="main",offset="8",
>  inst="sethi  %hi(0x11800), %o2"@}]@}]
>  (gdb)

Why do we sometimes use names with a hyphen, like func-name, and
sometimes with underscores, like line_asm_insn?  Shouldn't we pick one
and use it consistently?

Thanks.

Re: Add fullname field in disassembly output

[Andrew Burgess]

On 06/11/2012 5:44 PM, Eli Zaretskii wrote:
>> Date: Tue, 6 Nov 2012 12:13:36 +0000
>> From: "Andrew Burgess" <aburgess@broadcom.com>
>> cc: gdb-patches@sourceware.org,	"Eli Zaretskii" <eliz@gnu.org>
>>
> 
> The patch for the manual is OK with these changes.

Thanks for the review, I'll make those changes.

> 
>> +file="../../../src/gdb/testsuite/gdb.mi/basics.c",
>> +fullname="/absolute/path/to/src/gdb/testsuite/gdb.mi/basics.c",
>> +line_asm_insn=[@{address="0x000107bc",
>> +func-name="main",offset="0",inst="save  %sp, -112, %sp"@}]@},
>>  src_and_asm_line=@{line="32",
>> -file="/kwikemart/marge/ezannoni/flathead-dev/devo/gdb/ \
>> -  testsuite/gdb.mi/basics.c",line_asm_insn=[
>> -@{address="0x000107c0",func-name="main",offset="4",
>> -inst="mov  2, %o0"@},
>> +file="../../../src/gdb/testsuite/gdb.mi/basics.c",
>> +fullname="/absolute/path/to/src/gdb/testsuite/gdb.mi/basics.c",
>> +line_asm_insn=[@{address="0x000107c0",
>> +func-name="main",offset="4",inst="mov  2, %o0"@},
>>  @{address="0x000107c4",func-name="main",offset="8",
>>  inst="sethi  %hi(0x11800), %o2"@}]@}]
>>  (gdb)
> 
> Why do we sometimes use names with a hyphen, like func-name, and
> sometimes with underscores, like line_asm_insn?  Shouldn't we pick one
> and use it consistently?

That would probably be a good thing, but given I didn't add the code I'd
rather not start changing these things.  Also, given that this exists in
the wild and the MI interface is supposed to remain consistent I suspect
we're stuck with what we have...

Just so I'm clear, should I consider this a block to committing this patch?

Thanks,
Andrew

Re: Add fullname field in disassembly output

[Pedro Alves]

On 11/07/2012 03:08 PM, Andrew Burgess wrote:
> That would probably be a good thing, but given I didn't add the code I'd
> rather not start changing these things.  Also, given that this exists in
> the wild and the MI interface is supposed to remain consistent I suspect
> we're stuck with what we have...

Yes.

> Just so I'm clear, should I consider this a block to committing this patch?

No.

-- 
Pedro Alves

Re: Add fullname field in disassembly output

[Tom Tromey]

Andrew> Just so I'm clear, should I consider this a block to committing
Andrew> this patch?

Pedro> No.

I didn't see this go in yet, but based on reading the thread, I think
everything has been approved (docs conditional on some minor tweaks).
Just FYI.

Tom

Re: Add fullname field in disassembly output

[Andrew Burgess]

On 08/11/2012 9:30 PM, Tom Tromey wrote:
> Andrew> Just so I'm clear, should I consider this a block to committing
> Andrew> this patch?
> 
> Pedro> No.
> 
> I didn't see this go in yet, but based on reading the thread, I think
> everything has been approved (docs conditional on some minor tweaks).
> Just FYI.

Committed.  Sorry for the delay before committing, other work stuff came up.

Thanks for all your efforts reviewing this patch.

Andrew

Re: Add fullname field in disassembly output

[Eli Zaretskii]

> Date: Fri, 2 Nov 2012 10:59:17 +0000
> From: "Andrew Burgess" <aburgess@broadcom.com>
> cc: "Pedro Alves" <palves@redhat.com>
> 
> On 31/10/2012 2:53 PM, Pedro Alves wrote:
> 
> >> Ok to commit?
> > 
> > Not yet, sorry.  This new field needs to be documented in the manual, and mentioned in NEWS.
> 
> Sorry, I should have realised I'd need to do these things.
> 
> Latest version of patch, includes fullname field, test
> updates, noted in documentation and in NEWS file.

Thanks.

> +  ** The data-disassemble command response will include a "fullname" field
> +     containing the full path to the source file when gdb can determine the
> +     full path, and source has been requested in the output.

Please use "absolute file name" or just "file name" instead of
"path".  GNU coding standards frown upon use of "path" for anything
except PATH-style directory lists.

> -file="/kwikemart/marge/ezannoni/flathead-dev/devo/gdb/ \
> -  testsuite/gdb.mi/basics.c",line_asm_insn=[
> +file="../../../src/gdb/testsuite/gdb.mi/basics.c",
> +fullname="/absolute/path/to/src/gdb/testsuite/ \
> +gdb.mi/basics.c",line_asm_insn=[

It would be better not to break the last line in the middle of a file
name, as that could confuse the reader into thinking that there's a
blank in the middle.

> -file="/kwikemart/marge/ezannoni/flathead-dev/devo/gdb/ \
> -  testsuite/gdb.mi/basics.c",line_asm_insn=[
> +file="../../../src/gdb/testsuite/gdb.mi/basics.c",
> +fullname="/absolute/path/to/src/gdb/testsuite/ \
> +gdb.mi/basics.c",line_asm_insn=[

Same here.

The documentation parts are OK with those changes.