[RFC][patch] Allow to disassemble line.

[RFC][patch] Allow to disassemble line.

[Paul Pluzhnikov]

Greetings,

When debugging optimized code, I often need to disassemble current line
(with inlining, that could be quite a lot of assembly).

Currently, I do this by using 'info line' followed by 'disas <start> <end>',
where start and end are cut/pasted.

Needless to say, that is quite inconvenient.

Attached patch makes it so 'disas/l' will disassemble current line, and
'disas/l foo.c:22' will disassemble line 22 of foo.c

This is similar to 'set disassemble-next-line', but not exactly the same,
see below.

I'll appreciate comments.

I realize this will need documentation update as well, which I'll do if
this is considered reasonable.

Thanks,

P.S. With 'set disassemble-next-line on', current GDB appears to not work
as described in the manual. In particular, the manual for it says:

     If ON, GDB will display disassembly of the next source line when
     execution of the program being debugged stops.

But what GDB actually does is disassemble from *current instruction* to the
end of line.

If the reason for stopping is a breakpoint on the given line, then the
end result is the same either way. If the reason for stopping is a crash,
then you get disassembly only from crash point to end of line.

I think it's reasonable for GDB to behave either way. If it should
disassemble the *entire* current line, this code should be unified with
my patch. If the current behavior is more desirable, the manual should
be fixed instead.

--
Paul Pluzhnikov

2009-10-01  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* cli/cli-cmds.c (disassemble_sal, disassemble_lines): New function.
	(disassemble_command): Handle /l modifier.
	(init_cli_cmds): Adjust help text.

Index: cli/cli-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.92
diff -u -p -u -r1.92 cli-cmds.c
--- cli/cli-cmds.c	11 Jul 2009 14:04:23 -0000	1.92
+++ cli/cli-cmds.c	1 Oct 2009 22:26:49 -0000
@@ -964,6 +964,71 @@ disassemble_current_function (int flags)
   print_disassembly (gdbarch, name, low, high, flags);
 }
 
+/* Disassemble line of code identified by SAL.  */
+
+static void
+disassemble_sal (struct gdbarch *gdbarch, int flags,
+		 const char *error_text,
+		 const struct symtab_and_line *sal)
+{
+  char *name;
+  CORE_ADDR low, high;
+
+  if (sal->line > 0 && find_line_pc_range (*sal, &low, &high)
+      && find_pc_partial_function (low, &name, NULL, NULL))
+    {
+      if (low < high)
+	print_disassembly (gdbarch, name, low, high, flags);
+      else
+	printf_filtered (_("Line %s:%d contains no code.\n"),
+			 sal->symtab->filename, sal->line);
+    }
+  else
+    error ("%s", error_text);
+}
+
+/* Disassemble line LINE.  If LINE is NULL, disassemle current line.  */
+
+static void
+disassemble_lines (struct gdbarch *gdbarch, int flags, char *line)
+{
+  if (have_full_symbols () == 0 && have_partial_symbols () == 0)
+    error (_("No symbol table is loaded.  Use the \"file\" command."));
+
+  if (line == NULL)
+    {
+      struct frame_info *frame;
+      struct symtab_and_line sal = { 0 };
+
+      frame = get_selected_frame (NULL);
+      find_frame_sal (frame, &sal);
+      disassemble_sal (gdbarch, flags,
+		       _("No line information for current frame."),
+		       &sal);
+      return;
+    }
+  else
+    {
+      struct cleanup *old_cleanups;
+      struct symtabs_and_lines sals;
+
+      sals = decode_line_1 (&line, 0, 0, 0, 0, 0);
+      old_cleanups = make_cleanup (xfree, sals.sals);
+      if (sals.nelts > 1)
+	{
+	  printf_filtered (_("Ambiguous line specification:\n"));
+	  ambiguous_line_spec (&sals);
+	}
+      else if (sals.sals->line > 0)
+	disassemble_sal (gdbarch, flags, _("Unexpected error"), sals.sals);
+      else
+	error (_("Unable to decode line specification."));
+
+      do_cleanups (old_cleanups);
+    }
+  return;
+}
+
 /* Dump a specified section of assembly code.
 
    Usage:
@@ -974,8 +1039,15 @@ disassemble_current_function (int flags)
      disassemble [/mr] low high
        - dump the assembly code in the range [LOW,HIGH)
 
+     disassemble /l[mr]
+       - dump the assembly code for current line
+     disassemble /l[mr] linespec
+       - dump the assembly code for linespec
+
    A /m modifier will include source code with the assembly.
-   A /r modifier will include raw instructions in hex with the assembly.  */
+   A /r modifier will include raw instructions in hex with the assembly.
+   A /l modifier changes the meaning of argument to "line" instead of
+   "address".  */
 
 static void
 disassemble_command (char *arg, int from_tty)
@@ -985,10 +1057,11 @@ disassemble_command (char *arg, int from
   char *name;
   CORE_ADDR pc, pc_masked;
   char *space_index;
-  int flags;
+  int flags, disassemble_line;
 
   name = NULL;
   flags = 0;
+  disassemble_line = 0;
 
   if (arg && *arg == '/')
     {
@@ -1007,6 +1080,9 @@ disassemble_command (char *arg, int from
 	    case 'r':
 	      flags |= DISASSEMBLY_RAW_INSN;
 	      break;
+	    case 'l':
+	      disassemble_line = 1;
+	      break;
 	    default:
 	      error (_("Invalid disassembly modifier."));
 	    }
@@ -1018,7 +1094,10 @@ disassemble_command (char *arg, int from
 
   if (! arg || ! *arg)
     {
-      disassemble_current_function (flags);
+      if (disassemble_line != 0)
+	disassemble_lines (gdbarch, flags, NULL);
+      else
+	disassemble_current_function (flags);
       return;
     }
 
@@ -1028,6 +1107,12 @@ disassemble_command (char *arg, int from
   if (!(space_index = (char *) strchr (arg, ' ')))
     {
       /* One argument.  */
+      if (disassemble_line != 0)
+	{
+	  /* We are given line coordinates rather than addresses.  */
+	  disassemble_lines (gdbarch, flags, arg);
+	  return;
+	}
       pc = parse_and_eval_address (arg);
       if (find_pc_partial_function (pc, &name, &low, &high) == 0)
 	error (_("No function contains specified address."));
@@ -1044,6 +1129,13 @@ disassemble_command (char *arg, int from
     {
       /* Two arguments.  */
       *space_index = '\0';
+      if (disassemble_line != 0)
+	{
+	  /* We are given line coordinates rather than addresses.  */
+	  warning (_("Second parameter ignored with /l modifier.\n"));
+	  disassemble_lines (gdbarch, flags, arg);
+	  return;
+	}
       low = parse_and_eval_address (arg);
       high = parse_and_eval_address (space_index + 1);
     }
@@ -1457,10 +1549,12 @@ With two args if one is empty it stands 
   c = add_com ("disassemble", class_vars, disassemble_command, _("\
 Disassemble a specified section of memory.\n\
 Default is the function surrounding the pc of the selected frame.\n\
+With a /l modifier, disassemble current line.\n\
 With a /m modifier, source lines are included (if available).\n\
 With a /r modifier, raw instructions in hex are included.\n\
 With a single argument, the function surrounding that address is dumped.\n\
-Two arguments are taken as a range of memory to dump."));
+Two arguments are taken as a range of memory to dump.\n\
+With a /l modifier, single argument is parsed as line specification."));
   set_cmd_completer (c, location_completer);
   if (xdb_commands)
     add_com_alias ("va", "disassemble", class_xdb, 0);

Re: [RFC][patch] Allow to disassemble line.

[Joel Brobecker]

> Attached patch makes it so 'disas/l' will disassemble current line, and
> 'disas/l foo.c:22' will disassemble line 22 of foo.c

Without looking at the implementation itself for now, I have to say that
I have been missing this feature very badly.

Questions:

  1. What should we do if there are more than one match for the given SAL?
     Probably, we want to display a multiple-choice menu, and force the
     user to select one and only one choice.

     Eg: disass /l foo.c:22
         Multiple matches for foo.c:22
         [0] cancel
         [1] foo at foo.c:22
         [2] bar at foo.c:22
         >

     This can happen I think with templates (called generics in Ada),
     and probably inlining.

  2. A little trickier: How do we want to handle the case where a line
     of code is split in more than one block of instructions.  This happens
     really often when debugging optimized code.

     Right now, the easy solution is to only disassemble the first
     block.  It'd be nice to have them all, though. Perhaps printing
     the instructions for each block one after the other, with something
     like a little sign in between indicating the next instruction is
     part of another block?

     0x... <fun_name+nn>    bla bla bla
     0x... <fun_name+mm>    bla bla bla
     [...]
     0x... <fun_name+oo>    bla bla bla

Anyway, I really like this new feature.
-- 
Joel

Re: [RFC][patch] Allow to disassemble line.

[Paul Pluzhnikov]

On Thu, Oct 1, 2009 at 11:52 PM, Joel Brobecker <brobecker@adacore.com> wrote:

>> Attached patch makes it so 'disas/l' will disassemble current line, and
>> 'disas/l foo.c:22' will disassemble line 22 of foo.c
>
> Without looking at the implementation itself for now, I have to say that
> I have been missing this feature very badly.
>
> Questions:
>
>   1. What should we do if there are more than one match for the given SAL?

I only ever needed this feature while single-stepping through the code
(i.e. this line crashes, but why?).

Perhaps it is reasonable to get rid of the parameter, and just say that
'disas/l' always disassembles current line, thus eliminating the ambiguity.

Alternatively, if there are two code segments matching 'foo.c:22', but
one of them is inlined into the current function and the other is inlined
somewhere else, then clearly the user is interested in the "current" one
(same for templates).

>   2. A little trickier: How do we want to handle the case where a line
>       of code is split in more than one block of instructions.   This happens
>       really often when debugging optimized code.
>
>       Right now, the easy solution is to only disassemble the first
>       block.   It'd be nice to have them all, though. Perhaps printing
>       the instructions for each block one after the other, with something
>       like a little sign in between indicating the next instruction is
>       part of another block?
>
>       0x... <fun_name+nn>      bla bla bla
>       0x... <fun_name+mm>      bla bla bla
>       [...]
>       0x... <fun_name+oo>      bla bla bla

Some alternatives:

A) determine min(low), max(high) core address for all SALs, then disassemble
   that entire range, but there are discontinuous source lines turn on
   DISASSEMBLY_SOURCE automatically, so it becomes clear which instructions
   come from which line(s).

   The trouble with this approach is that the two blocks could be quite
   far apart. I haven't see GCC do that, but under MSVC I've seen wildly
   discontinuous blocks of code. I think GCC may soon start doing hot/cold
   code splitting as well, especially with FDO.

B) disassemble only the SAL which "covers" current $pc (as that's likely
   what the user is looking for). This would be consistent with proposed
   elimination of parameter, so 'disas/l' always implies 'current $pc'.

-- 
Paul Pluzhnikov

Re: [RFC][patch] Allow to disassemble line.

[Joel Brobecker]

> Perhaps it is reasonable to get rid of the parameter, and just say that
> 'disas/l' always disassembles current line, [...]

Not necessarily. Perhaps we could look at the /l as a qualifier which
allows the user to specify that he's not going to provide a range, or
a start address, but rather one address that's part of a line.  GDB
is to understand this as a request to disassemble the block of
instruction corresponding to the line corresponding to that address.

When looking at it this way, adding support for "foo.c:22" becomes
orthogonal to the introduction of the /l modifier. For instance,
you could do "disass /l" which, without argument, would access
the default value which conceptually is the $pc of the currently
selected frame.

We can also independently add FILE:LINE as a valid location expression
and make it work regardless of whether the /l modifier is used or not.

What I propose we do, for now, is not worry too much if the line of code
is split in multiple instruction blocks. Let's just print the first one,
for instance. There is no reason why we cannot deal with that later if
needed.

-- 
Joel

Re: [RFC][patch] Allow to disassemble line.

[Tom Tromey]

>>>>> "Paul" == Paul Pluzhnikov <ppluzhnikov@google.com> writes:

Paul> I realize this will need documentation update as well, which I'll do if
Paul> this is considered reasonable.

I think the idea is fine.

Tom

Re: [RFC][patch] Allow to disassemble line.

[Paul Pluzhnikov]

On Thu, Oct 1, 2009 at 5:49 PM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:

> P.S. With 'set disassemble-next-line on', current GDB appears to not work
> as described in the manual. In particular, the manual for it says:
>
>   If ON, GDB will display disassembly of the next source line when
>   execution of the program being debugged stops.
>
> But what GDB actually does is disassemble from *current instruction* to the
> end of line.
>
> If the reason for stopping is a breakpoint on the given line, then the
> end result is the same either way. If the reason for stopping is a crash,
> then you get disassembly only from crash point to end of line.
>
> I think it's reasonable for GDB to behave either way. If it should
> disassemble the *entire* current line, this code should be unified with
> my patch. If the current behavior is more desirable, the manual should
> be fixed instead.

Does anybody have an opinion on whether the implementation should be
changed to match the manual, or vice versa?

Thanks,
-- 
Paul Pluzhnikov

Re: [RFC][patch] Allow to disassemble line.

[Daniel Jacobowitz]

On Thu, Oct 08, 2009 at 09:16:23AM -0700, Paul Pluzhnikov wrote:
> On Thu, Oct 1, 2009 at 5:49 PM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> 
> > P.S. With 'set disassemble-next-line on', current GDB appears to not work
> > as described in the manual. In particular, the manual for it says:
> >
> >   If ON, GDB will display disassembly of the next source line when
> >   execution of the program being debugged stops.
> >
> > But what GDB actually does is disassemble from *current instruction* to the
> > end of line.
> >
> > If the reason for stopping is a breakpoint on the given line, then the
> > end result is the same either way. If the reason for stopping is a crash,
> > then you get disassembly only from crash point to end of line.
> >
> > I think it's reasonable for GDB to behave either way. If it should
> > disassemble the *entire* current line, this code should be unified with
> > my patch. If the current behavior is more desirable, the manual should
> > be fixed instead.
> 
> Does anybody have an opinion on whether the implementation should be
> changed to match the manual, or vice versa?

I agree that both are reasonable.

I'd mildly prefer changing the behavior of GDB - but only if we can
get an additional enhancement that I don't think we have yet: "*" at
the PC...

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC][patch] Allow to disassemble line.

[Joel Brobecker]

> I'd mildly prefer changing the behavior of GDB - but only if we can
> get an additional enhancement that I don't think we have yet: "*" at
> the PC...

That would work too...

-- 
Joel

Re: [RFC][patch] Allow to disassemble line.

[Paul Pluzhnikov]

On Thu, Oct 8, 2009 at 9:23 AM, Daniel Jacobowitz <drow@false.org> wrote:

> I'd mildly prefer changing the behavior of GDB - but only if we can
> get an additional enhancement that I don't think we have yet: "*" at
> the PC...

That sounds good. I'll try to implement that.

On Thu, Oct 8, 2009 at 9:24 AM, Joel Brobecker <brobecker@adacore.com> wrote:

> My 2 cents: I *think* the intention of this setting was to display
> the next few instructions that are about to be executed.

Yes, but the context (a couple of instructions which have just executed)
is often important as well.

If 'set disassemble-next-line on' worked as Daniel proposed, that would
significantly reduce the need for 'disas/l', I think.

Thanks,
-- 
Paul Pluzhnikov

Re: [RFC][patch] Allow to disassemble line.

[Daniel Jacobowitz]

On Thu, Oct 08, 2009 at 09:52:35AM -0700, Paul Pluzhnikov wrote:
> If 'set disassemble-next-line on' worked as Daniel proposed, that would
> significantly reduce the need for 'disas/l', I think.

For my two cents, I'd rather have both... consider x/i $pc and
display/i $pc.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC][patch] Allow to disassemble line.

[Joel Brobecker]

On Thu, Oct 08, 2009 at 01:29:26PM -0400, Daniel Jacobowitz wrote:
> On Thu, Oct 08, 2009 at 09:52:35AM -0700, Paul Pluzhnikov wrote:
> > If 'set disassemble-next-line on' worked as Daniel proposed, that would
> > significantly reduce the need for 'disas/l', I think.
> 
> For my two cents, I'd rather have both... consider x/i $pc and
> display/i $pc.

Me too.

-- 
Joel

Re: [RFC][patch] Allow to disassemble line.

[Paul Pluzhnikov]

[-- Attachment #1: Type: text/plain, Size: 5783 bytes --]

On Thu, Oct 8, 2009 at 9:52 AM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:

> On Thu, Oct 8, 2009 at 9:23 AM, Daniel Jacobowitz <drow@false.org> wrote:
>
>> I'd mildly prefer changing the behavior of GDB - but only if we can
>> get an additional enhancement that I don't think we have yet: "*" at
>> the PC...
>
> That sounds good. I'll try to implement that.

Here my attempt to implement that.

I am guessing that it is flawed, as it possibly breaks something in MI
(which I don't understand yet).

It also breaks quite a few test cases -- I'll adjust expected patterns if
it is decided to go ahead with this.

I should say that from CLI perspective I like this very much. On x86,
often you need to examine 10 or so instructions before the crash point.

I usually do 'x/10i $pc-15' (as instruction size is not fixed), and then
"hunt" for the current PC in the resulting output. This patch makes it so
much easier:

(top) x/10i $pc-15
  0x54d212 <catch_errors+54>:   mov    -0x10(%rbp),%edi
  0x54d215 <catch_errors+57>:   mov    $0x1,%esi
  0x54d21a <catch_errors+62>:   callq  0x452c38 <__sigsetjmp@plt>
  0x54d21f <catch_errors+67>:   jmp    0x54d237 <catch_errors+91>
* 0x54d221 <catch_errors+69>:   mov    -0x30(%rbp),%rdi
  0x54d225 <catch_errors+73>:   mov    -0x28(%rbp),%rax
  0x54d229 <catch_errors+77>:   callq  *%rax
  0x54d22b <catch_errors+79>:   mov    %eax,-0x4(%rbp)
  0x54d22e <catch_errors+82>:   callq  0x54c9c0
<exceptions_state_mc_action_iter_1>
  0x54d233 <catch_errors+87>:   test   %eax,%eax

I also like how 'disassemble-next-line on' works:

(top) set disassemble-next-line on
(top) ni
0x000000000054d225      510           val = func (func_args);
  0x000000000054d221 <catch_errors+69>:  48 8b 7d d0    mov    -0x30(%rbp),%rdi
* 0x000000000054d225 <catch_errors+73>:  48 8b 45 d8    mov    -0x28(%rbp),%rax
  0x000000000054d229 <catch_errors+77>:  ff d0  callq  *%rax
  0x000000000054d22b <catch_errors+79>:  89 45 fc       mov    %eax,-0x4(%rbp)
(top) ni
0x000000000054d229      510           val = func (func_args);
  0x000000000054d221 <catch_errors+69>:  48 8b 7d d0    mov    -0x30(%rbp),%rdi
  0x000000000054d225 <catch_errors+73>:  48 8b 45 d8    mov    -0x28(%rbp),%rax
* 0x000000000054d229 <catch_errors+77>:  ff d0  callq  *%rax
  0x000000000054d22b <catch_errors+79>:  89 45 fc       mov    %eax,-0x4(%rbp)

As well as how 'disas' works:

(top) disas
Dump of assembler code for function catch_errors:
  0x000000000054d1dc <catch_errors+0>:  push   %rbp
  0x000000000054d1dd <catch_errors+1>:  mov    %rsp,%rbp
  0x000000000054d1e0 <catch_errors+4>:  sub    $0x40,%rsp
  0x000000000054d1e4 <catch_errors+8>:  mov    %rdi,-0x28(%rbp)
  0x000000000054d1e8 <catch_errors+12>: mov    %rsi,-0x30(%rbp)
  0x000000000054d1ec <catch_errors+16>: mov    %rdx,-0x38(%rbp)
  0x000000000054d1f0 <catch_errors+20>: mov    %ecx,-0x3c(%rbp)
  0x000000000054d1f3 <catch_errors+23>: movl   $0x0,-0x4(%rbp)
  0x000000000054d1fa <catch_errors+30>: mov    0x56e1bf(%rip),%rdi
   # 0xabb3c0 <uiout>
  0x000000000054d201 <catch_errors+37>: mov    -0x3c(%rbp),%edx
  0x000000000054d204 <catch_errors+40>: lea    -0x20(%rbp),%rsi
  0x000000000054d208 <catch_errors+44>: callq  0x54c678
<exceptions_state_mc_init>
  0x000000000054d20d <catch_errors+49>: mov    %rax,-0x10(%rbp)
  0x000000000054d211 <catch_errors+53>: mov    -0x10(%rbp),%rdi
  0x000000000054d215 <catch_errors+57>: mov    $0x1,%esi
  0x000000000054d21a <catch_errors+62>: callq  0x452c38 <__sigsetjmp@plt>
  0x000000000054d21f <catch_errors+67>: jmp    0x54d237 <catch_errors+91>
  0x000000000054d221 <catch_errors+69>: mov    -0x30(%rbp),%rdi
  0x000000000054d225 <catch_errors+73>: mov    -0x28(%rbp),%rax
* 0x000000000054d229 <catch_errors+77>: callq  *%rax
  0x000000000054d22b <catch_errors+79>: mov    %eax,-0x4(%rbp)
  0x000000000054d22e <catch_errors+82>: callq  0x54c9c0
<exceptions_state_mc_action_iter_1>
  0x000000000054d233 <catch_errors+87>: test   %eax,%eax
  0x000000000054d235 <catch_errors+89>: jne    0x54d221 <catch_errors+69>
  0x000000000054d237 <catch_errors+91>: callq  0x54c9b0
<exceptions_state_mc_action_iter>
  0x000000000054d23c <catch_errors+96>: test   %eax,%eax
  0x000000000054d23e <catch_errors+98>: jne    0x54d22e <catch_errors+82>
  0x000000000054d240 <catch_errors+100>:        mov
0x593721(%rip),%rdi        # 0xae0968 <gdb_stderr>
  0x000000000054d247 <catch_errors+107>:        mov    -0x20(%rbp),%rdx
  0x000000000054d24b <catch_errors+111>:        mov    -0x18(%rbp),%rcx
  0x000000000054d24f <catch_errors+115>:        mov    -0x38(%rbp),%rsi
  0x000000000054d253 <catch_errors+119>:        callq  0x54cdea
<print_any_exception>
  0x000000000054d258 <catch_errors+124>:        mov    -0x20(%rbp),%eax
  0x000000000054d25b <catch_errors+127>:        test   %eax,%eax
  0x000000000054d25d <catch_errors+129>:        je     0x54d268
<catch_errors+140>
  0x000000000054d25f <catch_errors+131>:        movl   $0x0,-0x40(%rbp)
  0x000000000054d266 <catch_errors+138>:        jmp    0x54d26e
<catch_errors+146>
  0x000000000054d268 <catch_errors+140>:        mov    -0x4(%rbp),%eax
  0x000000000054d26b <catch_errors+143>:        mov    %eax,-0x40(%rbp)
  0x000000000054d26e <catch_errors+146>:        mov    -0x40(%rbp),%eax
  0x000000000054d271 <catch_errors+149>:        leaveq
  0x000000000054d272 <catch_errors+150>:        retq
End of assembler dump.


Thanks,
-- 
Paul Pluzhnikov

2009-10-16  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* defs.h (pc_prefix): New prototype.
	* disasm.c (dump_insns): Identify instruction address as such.
	* ui-out.c (ui_out_field_core_addr): Highlight current instruction.
	* printcmd.c (do_examine): Likewise.
	(pc_prefix, print_pc_prefix): New function.
	* stack.c (print_frame_info): Disassemble entire current line.

[-- Attachment #2: gdb-disas-20091016.txt --]
[-- Type: text/plain, Size: 3066 bytes --]

Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.255
diff -u -p -u -r1.255 defs.h
--- defs.h	22 Sep 2009 22:34:17 -0000	1.255
+++ defs.h	16 Oct 2009 22:37:36 -0000
@@ -608,6 +608,7 @@ extern int build_address_symbolic (CORE_
 				   int *unmapped);
 
 extern void print_address (struct gdbarch *, CORE_ADDR, struct ui_file *);
+extern const char *pc_prefix (CORE_ADDR);
 
 /* From source.c */
 
Index: disasm.c
===================================================================
RCS file: /cvs/src/src/gdb/disasm.c,v
retrieving revision 1.33
diff -u -p -u -r1.33 disasm.c
--- disasm.c	11 Jul 2009 14:04:23 -0000	1.33
+++ disasm.c	16 Oct 2009 22:37:36 -0000
@@ -113,7 +113,7 @@ dump_insns (struct gdbarch *gdbarch, str
 	    num_displayed++;
 	}
       ui_out_chain = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
-      ui_out_field_core_addr (uiout, "address", gdbarch, pc);
+      ui_out_field_core_addr (uiout, "pc_addr", gdbarch, pc);
 
       if (!build_address_symbolic (pc, 0, &name, &offset, &filename,
 				   &line, &unmapped))
Index: printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.164
diff -u -p -u -r1.164 printcmd.c
--- printcmd.c	2 Jul 2009 17:25:58 -0000	1.164
+++ printcmd.c	16 Oct 2009 22:37:36 -0000
@@ -716,6 +716,32 @@ print_address (struct gdbarch *gdbarch,
   print_address_symbolic (addr, stream, asm_demangle, " ");
 }
 
+/* Return a prefix for instruction address:
+   "* " for current instruction, else "  ".  */
+
+const char *
+pc_prefix (CORE_ADDR addr)
+{
+  if (has_stack_frames ())
+    {
+      struct frame_info *frame;
+      CORE_ADDR pc;
+
+      frame = get_selected_frame (NULL);
+      pc = get_frame_pc (frame);
+
+      if (pc == addr)
+	return "* ";
+    }
+  return "  ";
+}
+
+static void
+print_pc_prefix (CORE_ADDR addr, struct ui_file *stream)
+{
+  fputs_filtered (pc_prefix (addr), stream);
+}
+
 /* Print address ADDR symbolically on STREAM.  Parameter DEMANGLE
    controls whether to print the symbolic name "raw" or demangled.
    Global setting "addressprint" controls whether to print hex address
@@ -808,6 +834,8 @@ do_examine (struct format_data fmt, stru
   while (count > 0)
     {
       QUIT;
+      if (format == 'i')
+	print_pc_prefix (next_address, gdb_stdout);
       print_address (next_gdbarch, next_address, gdb_stdout);
       printf_filtered (":");
       for (i = maxelts;
Index: ui-out.c
===================================================================
RCS file: /cvs/src/src/gdb/ui-out.c,v
retrieving revision 1.43
diff -u -p -u -r1.43 ui-out.c
--- ui-out.c	2 Jul 2009 17:21:07 -0000	1.43
+++ ui-out.c	16 Oct 2009 22:37:36 -0000
@@ -505,6 +505,9 @@ ui_out_field_core_addr (struct ui_out *u
   else
     strcpy (addstr, hex_string_custom (address, 16));
 
+  if (strcmp (fldname, "pc_addr") == 0)
+    ui_out_text (uiout, pc_prefix (address));
+
   ui_out_field_string (uiout, fldname, addstr);
 }
 

Re: [RFC][patch] Allow to disassemble line.

[Paul Pluzhnikov]

[-- Attachment #1: Type: text/plain, Size: 383 bytes --]

On Fri, Oct 16, 2009 at 4:07 PM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:

> I usually do 'x/10i $pc-15' (as instruction size is not fixed), and then
> "hunt" for the current PC in the resulting output. This patch makes it so
> much easier:

It would have helped if gmail didn't mangle the output so much :-(
I've attached actual observed output.

Thanks,
-- 
Paul Pluzhnikov

[-- Attachment #2: pp.txt --]
[-- Type: text/plain, Size: 4386 bytes --]

(top) x/10i $pc-15
  0x54d212 <catch_errors+54>:   mov    -0x10(%rbp),%edi
  0x54d215 <catch_errors+57>:   mov    $0x1,%esi
  0x54d21a <catch_errors+62>:   callq  0x452c38 <__sigsetjmp@plt>
  0x54d21f <catch_errors+67>:   jmp    0x54d237 <catch_errors+91>
* 0x54d221 <catch_errors+69>:   mov    -0x30(%rbp),%rdi
  0x54d225 <catch_errors+73>:   mov    -0x28(%rbp),%rax
  0x54d229 <catch_errors+77>:   callq  *%rax
  0x54d22b <catch_errors+79>:   mov    %eax,-0x4(%rbp)
  0x54d22e <catch_errors+82>:   callq  0x54c9c0 <exceptions_state_mc_action_iter_1>
  0x54d233 <catch_errors+87>:   test   %eax,%eax

(top) set disassemble-next-line on
(top) ni
0x000000000054d225      510           val = func (func_args);
  0x000000000054d221 <catch_errors+69>:  48 8b 7d d0    mov    -0x30(%rbp),%rdi
* 0x000000000054d225 <catch_errors+73>:  48 8b 45 d8    mov    -0x28(%rbp),%rax
  0x000000000054d229 <catch_errors+77>:  ff d0  callq  *%rax
  0x000000000054d22b <catch_errors+79>:  89 45 fc       mov    %eax,-0x4(%rbp)
(top) ni
0x000000000054d229      510           val = func (func_args);
  0x000000000054d221 <catch_errors+69>:  48 8b 7d d0    mov    -0x30(%rbp),%rdi
  0x000000000054d225 <catch_errors+73>:  48 8b 45 d8    mov    -0x28(%rbp),%rax
* 0x000000000054d229 <catch_errors+77>:  ff d0  callq  *%rax
  0x000000000054d22b <catch_errors+79>:  89 45 fc       mov    %eax,-0x4(%rbp)

(top) disas
Dump of assembler code for function catch_errors:
  0x000000000054d1dc <catch_errors+0>:  push   %rbp
  0x000000000054d1dd <catch_errors+1>:  mov    %rsp,%rbp
  0x000000000054d1e0 <catch_errors+4>:  sub    $0x40,%rsp
  0x000000000054d1e4 <catch_errors+8>:  mov    %rdi,-0x28(%rbp)
  0x000000000054d1e8 <catch_errors+12>: mov    %rsi,-0x30(%rbp)
  0x000000000054d1ec <catch_errors+16>: mov    %rdx,-0x38(%rbp)
  0x000000000054d1f0 <catch_errors+20>: mov    %ecx,-0x3c(%rbp)
  0x000000000054d1f3 <catch_errors+23>: movl   $0x0,-0x4(%rbp)
  0x000000000054d1fa <catch_errors+30>: mov    0x56e1bf(%rip),%rdi        # 0xabb3c0 <uiout>
  0x000000000054d201 <catch_errors+37>: mov    -0x3c(%rbp),%edx
  0x000000000054d204 <catch_errors+40>: lea    -0x20(%rbp),%rsi
  0x000000000054d208 <catch_errors+44>: callq  0x54c678 <exceptions_state_mc_init>
  0x000000000054d20d <catch_errors+49>: mov    %rax,-0x10(%rbp)
  0x000000000054d211 <catch_errors+53>: mov    -0x10(%rbp),%rdi
  0x000000000054d215 <catch_errors+57>: mov    $0x1,%esi
  0x000000000054d21a <catch_errors+62>: callq  0x452c38 <__sigsetjmp@plt>
  0x000000000054d21f <catch_errors+67>: jmp    0x54d237 <catch_errors+91>
  0x000000000054d221 <catch_errors+69>: mov    -0x30(%rbp),%rdi
  0x000000000054d225 <catch_errors+73>: mov    -0x28(%rbp),%rax
* 0x000000000054d229 <catch_errors+77>: callq  *%rax
  0x000000000054d22b <catch_errors+79>: mov    %eax,-0x4(%rbp)
  0x000000000054d22e <catch_errors+82>: callq  0x54c9c0 <exceptions_state_mc_action_iter_1>
  0x000000000054d233 <catch_errors+87>: test   %eax,%eax
  0x000000000054d235 <catch_errors+89>: jne    0x54d221 <catch_errors+69>
  0x000000000054d237 <catch_errors+91>: callq  0x54c9b0 <exceptions_state_mc_action_iter>
  0x000000000054d23c <catch_errors+96>: test   %eax,%eax
  0x000000000054d23e <catch_errors+98>: jne    0x54d22e <catch_errors+82>
  0x000000000054d240 <catch_errors+100>:        mov    0x593721(%rip),%rdi        # 0xae0968 <gdb_stderr>
  0x000000000054d247 <catch_errors+107>:        mov    -0x20(%rbp),%rdx
  0x000000000054d24b <catch_errors+111>:        mov    -0x18(%rbp),%rcx
  0x000000000054d24f <catch_errors+115>:        mov    -0x38(%rbp),%rsi
  0x000000000054d253 <catch_errors+119>:        callq  0x54cdea <print_any_exception>
  0x000000000054d258 <catch_errors+124>:        mov    -0x20(%rbp),%eax
  0x000000000054d25b <catch_errors+127>:        test   %eax,%eax
  0x000000000054d25d <catch_errors+129>:        je     0x54d268 <catch_errors+140>
  0x000000000054d25f <catch_errors+131>:        movl   $0x0,-0x40(%rbp)
  0x000000000054d266 <catch_errors+138>:        jmp    0x54d26e <catch_errors+146>
  0x000000000054d268 <catch_errors+140>:        mov    -0x4(%rbp),%eax
  0x000000000054d26b <catch_errors+143>:        mov    %eax,-0x40(%rbp)
  0x000000000054d26e <catch_errors+146>:        mov    -0x40(%rbp),%eax
  0x000000000054d271 <catch_errors+149>:        leaveq 
  0x000000000054d272 <catch_errors+150>:        retq   
End of assembler dump.

Re: [RFC][patch] Allow to disassemble line.

[Eli Zaretskii]

> Date: Fri, 16 Oct 2009 16:11:27 -0700
> From: Paul Pluzhnikov <ppluzhnikov@google.com>
> 
> (top) set disassemble-next-line on
> (top) ni
> 0x000000000054d225      510           val = func (func_args);
>   0x000000000054d221 <catch_errors+69>:  48 8b 7d d0    mov    -0x30(%rbp),%rdi
> * 0x000000000054d225 <catch_errors+73>:  48 8b 45 d8    mov    -0x28(%rbp),%rax
>   0x000000000054d229 <catch_errors+77>:  ff d0  callq  *%rax
>   0x000000000054d22b <catch_errors+79>:  89 45 fc       mov    %eax,-0x4(%rbp)
> (top) ni
> 0x000000000054d229      510           val = func (func_args);
>   0x000000000054d221 <catch_errors+69>:  48 8b 7d d0    mov    -0x30(%rbp),%rdi
>   0x000000000054d225 <catch_errors+73>:  48 8b 45 d8    mov    -0x28(%rbp),%rax
> * 0x000000000054d229 <catch_errors+77>:  ff d0  callq  *%rax
>   0x000000000054d22b <catch_errors+79>:  89 45 fc       mov    %eax,-0x4(%rbp)

The asterisk in both cases is on the same mnemonics, "callq  *%rax".
Is that a bug or a "feature"?

Re: [RFC][patch] Allow to disassemble line.

[Paul Pluzhnikov]

On Sat, Oct 17, 2009 at 1:31 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Fri, 16 Oct 2009 16:11:27 -0700
>> From: Paul Pluzhnikov <ppluzhnikov@google.com>
>>
>> (top) set disassemble-next-line on
>> (top) ni
>> 0x000000000054d225      510           val = func (func_args);
>>   0x000000000054d221 <catch_errors+69>:  48 8b 7d d0    mov    -0x30(%rbp),%rdi
>> * 0x000000000054d225 <catch_errors+73>:  48 8b 45 d8    mov    -0x28(%rbp),%rax
>>   0x000000000054d229 <catch_errors+77>:  ff d0  callq  *%rax
>>   0x000000000054d22b <catch_errors+79>:  89 45 fc       mov    %eax,-0x4(%rbp)
>> (top) ni
>> 0x000000000054d229      510           val = func (func_args);
>>   0x000000000054d221 <catch_errors+69>:  48 8b 7d d0    mov    -0x30(%rbp),%rdi
>>   0x000000000054d225 <catch_errors+73>:  48 8b 45 d8    mov    -0x28(%rbp),%rax
>> * 0x000000000054d229 <catch_errors+77>:  ff d0  callq  *%rax
>>   0x000000000054d22b <catch_errors+79>:  89 45 fc       mov    %eax,-0x4(%rbp)
>
> The asterisk in both cases is on the same mnemonics, "callq  *%rax".

It's the "leading asterisk" that is the marker here, and it moves from
0x000000000054d225 to 0x000000000054d229.

Instruction itself (which is "callq  *%rax") is constant, just like it
should be.

Or did I misunderstand your question?

Thanks,
-- 
Paul Pluzhnikov

Re: [RFC][patch] Allow to disassemble line.

[Eli Zaretskii]

> Date: Sat, 17 Oct 2009 08:49:48 -0700
> From: Paul Pluzhnikov <ppluzhnikov@google.com>
> Cc: gdb-patches@sourceware.org
> 
> It's the "leading asterisk" that is the marker here, and it moves from
> 0x000000000054d225 to 0x000000000054d229.
> 
> Instruction itself (which is "callq  *%rax") is constant, just like it
> should be.
> 
> Or did I misunderstand your question?

No, you didn't misunderstand.

However, in that case, how about using ">" instead of "*"?  That
should make the indication unambiguous.  If it duped me, it could dupe
someone else.

We should also mention this in the manual, I think (assuming the patch
is approved).

Thanks.

Re: [RFC][patch] Allow to disassemble line.

[Paul Pluzhnikov]

On Sat, Oct 17, 2009 at 9:47 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Sat, 17 Oct 2009 08:49:48 -0700

> However, in that case, how about using ">" instead of "*"?

That of course is a trivial one character change to the patch, and
sounds like a good idea to me. Maybe something even more prominent,
like "=> " ?

> We should also mention this in the manual

Will do.

Tnanks,
-- 
Paul Pluzhnikov

Re: [RFC][patch] Allow to disassemble line.

[Eli Zaretskii]

> Date: Sat, 17 Oct 2009 10:08:41 -0700
> From: Paul Pluzhnikov <ppluzhnikov@google.com>
> Cc: gdb-patches@sourceware.org
> 
> On Sat, Oct 17, 2009 at 9:47 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> Date: Sat, 17 Oct 2009 08:49:48 -0700
> 
> > However, in that case, how about using ">" instead of "*"?
> 
> That of course is a trivial one character change to the patch, and
> sounds like a good idea to me. Maybe something even more prominent,
> like "=> " ?

That's even better.

> > We should also mention this in the manual
> 
> Will do.

Thanks.

Re: [RFC][patch] Allow to disassemble line.

[Tom Tromey]

>>>>> "Paul" == Paul Pluzhnikov <ppluzhnikov@google.com> writes:

Paul> -      ui_out_field_core_addr (uiout, "address", gdbarch, pc);
Paul> +      ui_out_field_core_addr (uiout, "pc_addr", gdbarch, pc);

Changing a field name breaks MI clients.
So, I don't think this part is ok.

Paul> Index: ui-out.c
[...]
Paul> +  if (strcmp (fldname, "pc_addr") == 0)
Paul> +    ui_out_text (uiout, pc_prefix (address));

I think magic field names and extra behavior in low-level ui-out
functions is pretty ugly and also breaks an abstraction barrier.

Why not directly call ui_out_text in dump_insns?

Tom

Re: [RFC][patch] Allow to disassemble line.

[Paul Pluzhnikov]

[-- Attachment #1: Type: text/plain, Size: 1318 bytes --]

On Mon, Oct 19, 2009 at 10:47 AM, Tom Tromey <tromey@redhat.com> wrote:

> Why not directly call ui_out_text in dump_insns?

I am not sure how I arrived at the proposed patch ...
Doing it the way you suggesed simplifies it a bit :-)

I've also changed the "current PC marker" from "* " to "=> " (as Eli
suggested), so the output looks like this:

(top) disas
Dump of assembler code for function gdb_main:
   0x0000000000454c9e <gdb_main+0>:     push   %rbp
   0x0000000000454c9f <gdb_main+1>:     mov    %rsp,%rbp
   0x0000000000454ca2 <gdb_main+4>:     sub    $0x10,%rsp
   0x0000000000454ca6 <gdb_main+8>:     mov    %rdi,-0x8(%rbp)
=> 0x0000000000454caa <gdb_main+12>:    mov    -0x8(%rbp),%rax
   0x0000000000454cae <gdb_main+16>:    mov    0x10(%rax),%eax
   0x0000000000454cb1 <gdb_main+19>:    mov    %eax,0x678475(%rip) #
0xacd12c <use_windows>


This patch still lacks documentation and test case updates. I'll work up
the complete patch if this one is OK.

Thanks,
-- 
Paul Pluzhnikov

2009-10-19  Paul Pluzhnikov  <ppluzhnikov@google.com>

       * defs.h (pc_prefix): New prototype.
       * disasm.c (dump_insns): Mark current instruction.
       * printcmd.c (do_examine): Likewise.
       (pc_prefix, print_pc_prefix): New function.
       * stack.c (print_frame_info): Disassemble entire current line.

[-- Attachment #2: gdb-disas-20091019.txt --]
[-- Type: text/plain, Size: 3201 bytes --]

Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.256
diff -u -p -u -r1.256 defs.h
--- defs.h	19 Oct 2009 09:51:40 -0000	1.256
+++ defs.h	19 Oct 2009 18:08:37 -0000
@@ -608,6 +608,7 @@ extern int build_address_symbolic (CORE_
 				   int *unmapped);
 
 extern void print_address (struct gdbarch *, CORE_ADDR, struct ui_file *);
+extern const char *pc_prefix (CORE_ADDR);
 
 /* From source.c */
 
Index: disasm.c
===================================================================
RCS file: /cvs/src/src/gdb/disasm.c,v
retrieving revision 1.33
diff -u -p -u -r1.33 disasm.c
--- disasm.c	11 Jul 2009 14:04:23 -0000	1.33
+++ disasm.c	19 Oct 2009 18:08:37 -0000
@@ -113,6 +113,7 @@ dump_insns (struct gdbarch *gdbarch, str
 	    num_displayed++;
 	}
       ui_out_chain = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
+      ui_out_text (uiout, pc_prefix (pc));
       ui_out_field_core_addr (uiout, "address", gdbarch, pc);
 
       if (!build_address_symbolic (pc, 0, &name, &offset, &filename,
Index: printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.165
diff -u -p -u -r1.165 printcmd.c
--- printcmd.c	19 Oct 2009 09:51:41 -0000	1.165
+++ printcmd.c	19 Oct 2009 18:08:37 -0000
@@ -725,6 +725,32 @@ print_address (struct gdbarch *gdbarch,
   print_address_symbolic (addr, stream, asm_demangle, " ");
 }
 
+/* Return a prefix for instruction address:
+   "=> " for current instruction, else "   ".  */
+
+const char *
+pc_prefix (CORE_ADDR addr)
+{
+  if (has_stack_frames ())
+    {
+      struct frame_info *frame;
+      CORE_ADDR pc;
+
+      frame = get_selected_frame (NULL);
+      pc = get_frame_pc (frame);
+
+      if (pc == addr)
+	return "=> ";
+    }
+  return "   ";
+}
+
+static void
+print_pc_prefix (CORE_ADDR addr, struct ui_file *stream)
+{
+  fputs_filtered (pc_prefix (addr), stream);
+}
+
 /* Print address ADDR symbolically on STREAM.  Parameter DEMANGLE
    controls whether to print the symbolic name "raw" or demangled.
    Global setting "addressprint" controls whether to print hex address
@@ -817,6 +843,8 @@ do_examine (struct format_data fmt, stru
   while (count > 0)
     {
       QUIT;
+      if (format == 'i')
+	print_pc_prefix (next_address, gdb_stdout);
       print_address (next_gdbarch, next_address, gdb_stdout);
       printf_filtered (":");
       for (i = maxelts;
Index: stack.c
===================================================================
RCS file: /cvs/src/src/gdb/stack.c,v
retrieving revision 1.208
diff -u -p -u -r1.208 stack.c
--- stack.c	19 Oct 2009 09:51:42 -0000	1.208
+++ stack.c	19 Oct 2009 18:08:37 -0000
@@ -643,8 +643,7 @@ print_frame_info (struct frame_info *fra
       /* If disassemble-next-line is set to on and there is line debug
          messages, output assembly codes for next line.  */
       if (disassemble_next_line == AUTO_BOOLEAN_TRUE)
-	do_gdb_disassembly (get_frame_arch (frame), -1,
-			    get_frame_pc (frame), sal.end);
+	do_gdb_disassembly (get_frame_arch (frame), -1, sal.pc, sal.end);
     }
 
   if (print_what != LOCATION)

Re: [RFC][patch] Allow to disassemble line.

[Paul Pluzhnikov]

[-- Attachment #1: Type: text/plain, Size: 570 bytes --]

On Mon, Oct 19, 2009 at 11:09 AM, Paul Pluzhnikov
<ppluzhnikov@google.com> wrote:

> Doing it the way you suggesed simplifies it a bit :-)

The one-line print_pc_prefix() with a single caller doesn't really buy
anything either. Eliminated.

Thanks,
-- 
Paul Pluzhnikov

2009-10-19  Paul Pluzhnikov  <ppluzhnikov@google.com>

      * defs.h (pc_prefix): New prototype.
      * disasm.c (dump_insns): Mark current instruction.
      * printcmd.c (do_examine): Likewise.
      (pc_prefix): New function.
      * stack.c (print_frame_info): Disassemble entire current line.

[-- Attachment #2: gdb-disas-20091019-2.txt --]
[-- Type: text/plain, Size: 3087 bytes --]

Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.256
diff -u -p -u -r1.256 defs.h
--- defs.h	19 Oct 2009 09:51:40 -0000	1.256
+++ defs.h	19 Oct 2009 18:16:51 -0000
@@ -608,6 +608,7 @@ extern int build_address_symbolic (CORE_
 				   int *unmapped);
 
 extern void print_address (struct gdbarch *, CORE_ADDR, struct ui_file *);
+extern const char *pc_prefix (CORE_ADDR);
 
 /* From source.c */
 
Index: disasm.c
===================================================================
RCS file: /cvs/src/src/gdb/disasm.c,v
retrieving revision 1.33
diff -u -p -u -r1.33 disasm.c
--- disasm.c	11 Jul 2009 14:04:23 -0000	1.33
+++ disasm.c	19 Oct 2009 18:16:51 -0000
@@ -113,6 +113,7 @@ dump_insns (struct gdbarch *gdbarch, str
 	    num_displayed++;
 	}
       ui_out_chain = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
+      ui_out_text (uiout, pc_prefix (pc));
       ui_out_field_core_addr (uiout, "address", gdbarch, pc);
 
       if (!build_address_symbolic (pc, 0, &name, &offset, &filename,
Index: printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.165
diff -u -p -u -r1.165 printcmd.c
--- printcmd.c	19 Oct 2009 09:51:41 -0000	1.165
+++ printcmd.c	19 Oct 2009 18:16:51 -0000
@@ -725,6 +725,26 @@ print_address (struct gdbarch *gdbarch,
   print_address_symbolic (addr, stream, asm_demangle, " ");
 }
 
+/* Return a prefix for instruction address:
+   "=> " for current instruction, else "   ".  */
+
+const char *
+pc_prefix (CORE_ADDR addr)
+{
+  if (has_stack_frames ())
+    {
+      struct frame_info *frame;
+      CORE_ADDR pc;
+
+      frame = get_selected_frame (NULL);
+      pc = get_frame_pc (frame);
+
+      if (pc == addr)
+	return "=> ";
+    }
+  return "   ";
+}
+
 /* Print address ADDR symbolically on STREAM.  Parameter DEMANGLE
    controls whether to print the symbolic name "raw" or demangled.
    Global setting "addressprint" controls whether to print hex address
@@ -817,6 +837,8 @@ do_examine (struct format_data fmt, stru
   while (count > 0)
     {
       QUIT;
+      if (format == 'i')
+	fputs_filtered (pc_prefix (next_address), gdb_stdout);
       print_address (next_gdbarch, next_address, gdb_stdout);
       printf_filtered (":");
       for (i = maxelts;
Index: stack.c
===================================================================
RCS file: /cvs/src/src/gdb/stack.c,v
retrieving revision 1.208
diff -u -p -u -r1.208 stack.c
--- stack.c	19 Oct 2009 09:51:42 -0000	1.208
+++ stack.c	19 Oct 2009 18:16:51 -0000
@@ -643,8 +643,7 @@ print_frame_info (struct frame_info *fra
       /* If disassemble-next-line is set to on and there is line debug
          messages, output assembly codes for next line.  */
       if (disassemble_next_line == AUTO_BOOLEAN_TRUE)
-	do_gdb_disassembly (get_frame_arch (frame), -1,
-			    get_frame_pc (frame), sal.end);
+	do_gdb_disassembly (get_frame_arch (frame), -1, sal.pc, sal.end);
     }
 
   if (print_what != LOCATION)

Re: [RFC][patch] Allow to disassemble line.

[Tom Tromey]

>>>>> "Paul" == Paul Pluzhnikov <ppluzhnikov@google.com> writes:

Paul> This patch still lacks documentation and test case updates. I'll work up
Paul> the complete patch if this one is OK.

It looks reasonable to me.

Tom

Re: [RFC][patch] Allow to disassemble line.

[Paul Pluzhnikov]

[-- Attachment #1: Type: text/plain, Size: 1047 bytes --]

On Mon, Oct 19, 2009 at 11:30 AM, Tom Tromey <tromey@redhat.com> wrote:

>>>>>> "Paul" == Paul Pluzhnikov <ppluzhnikov@google.com> writes:
>
> Paul> This patch still lacks documentation and test case updates. I'll work up
> Paul> the complete patch if this one is OK.
>
> It looks reasonable to me.

Here is updated patch with documentation and testsuite fixes.

Thanks,
-- 
Paul Pluzhnikov

2009-10-20  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* defs.h (pc_prefix): New prototype.
	* disasm.c (dump_insns): Mark current instruction.
	* printcmd.c (do_examine): Likewise.
	(pc_prefix): New function.
	* stack.c (print_frame_info): Disassemble entire current line.

doc/ChangeLog

2009-10-20  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb.texi (Machine Code): Mention current pc marker, update examples.
	(Memory): Likewise.

testsuite/ChangeLog

2009-10-20  Paul Pluzhnikov  <ppluzhnikov@google.com>

	* gdb.base/consecutive.exp: Adjust.
	* gdb.base/display.exp: Likewise.
	* gdb.base/pc-fp.exp: Likewise.
	* gdb.base/sigbpt.exp: Likewise.

[-- Attachment #2: gdb-disas-20091020.txt --]
[-- Type: text/plain, Size: 11518 bytes --]

Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.634
diff -u -p -u -r1.634 gdb.texinfo
--- doc/gdb.texinfo	19 Oct 2009 09:51:42 -0000	1.634
+++ doc/gdb.texinfo	21 Oct 2009 00:14:17 -0000
@@ -6525,6 +6525,9 @@ program counter of the selected frame.  
 command is a program counter value; @value{GDBN} dumps the function
 surrounding this value.  Two arguments specify a range of addresses
 (first inclusive, second exclusive) to dump.
+
+If the range of memory being disassembled contains current program counter,
+the instruction at that location is shown with a @code{=>} marker.
 @end table
 
 The following example shows the disassembly of a range of addresses of
@@ -6533,38 +6536,39 @@ HP PA-RISC 2.0 code:
 @smallexample
 (@value{GDBP}) disas 0x32c4 0x32e4
 Dump of assembler code from 0x32c4 to 0x32e4:
-0x32c4 <main+204>:      addil 0,dp
-0x32c8 <main+208>:      ldw 0x22c(sr0,r1),r26
-0x32cc <main+212>:      ldil 0x3000,r31
-0x32d0 <main+216>:      ble 0x3f8(sr4,r31)
-0x32d4 <main+220>:      ldo 0(r31),rp
-0x32d8 <main+224>:      addil -0x800,dp
-0x32dc <main+228>:      ldo 0x588(r1),r26
-0x32e0 <main+232>:      ldil 0x3000,r31
+   0x32c4 <main+204>:      addil 0,dp
+   0x32c8 <main+208>:      ldw 0x22c(sr0,r1),r26
+   0x32cc <main+212>:      ldil 0x3000,r31
+   0x32d0 <main+216>:      ble 0x3f8(sr4,r31)
+   0x32d4 <main+220>:      ldo 0(r31),rp
+   0x32d8 <main+224>:      addil -0x800,dp
+   0x32dc <main+228>:      ldo 0x588(r1),r26
+   0x32e0 <main+232>:      ldil 0x3000,r31
 End of assembler dump.
 @end smallexample
 
-Here is an example showing mixed source+assembly for Intel x86:
+Here is an example showing mixed source+assembly for Intel x86, when the
+program is stopped just after function prologue:
 
 @smallexample
 (@value{GDBP}) disas /m main
 Dump of assembler code for function main:
 5       @{
-0x08048330 <main+0>:    push   %ebp
-0x08048331 <main+1>:    mov    %esp,%ebp
-0x08048333 <main+3>:    sub    $0x8,%esp
-0x08048336 <main+6>:    and    $0xfffffff0,%esp
-0x08048339 <main+9>:    sub    $0x10,%esp
+   0x08048330 <main+0>:    push   %ebp
+   0x08048331 <main+1>:    mov    %esp,%ebp
+   0x08048333 <main+3>:    sub    $0x8,%esp
+   0x08048336 <main+6>:    and    $0xfffffff0,%esp
+   0x08048339 <main+9>:    sub    $0x10,%esp
 
 6         printf ("Hello.\n");
-0x0804833c <main+12>:   movl   $0x8048440,(%esp)
-0x08048343 <main+19>:   call   0x8048284 <puts@@plt>
+=> 0x0804833c <main+12>:   movl   $0x8048440,(%esp)
+   0x08048343 <main+19>:   call   0x8048284 <puts@@plt>
 
 7         return 0;
 8       @}
-0x08048348 <main+24>:   mov    $0x0,%eax
-0x0804834d <main+29>:   leave
-0x0804834e <main+30>:   ret
+   0x08048348 <main+24>:   mov    $0x0,%eax
+   0x0804834d <main+29>:   leave
+   0x0804834e <main+30>:   ret
 
 End of assembler dump.
 @end smallexample
@@ -7244,6 +7248,18 @@ with just @samp{x/7}.  If you use @key{R
 the repeat count @var{n} is used again; the other arguments default as
 for successive uses of @code{x}.
 
+When examining machine instructions, instruction at current program counter
+is shown with a @code{=>} marker. For example:
+
+@smallexample
+(@value{GDBP}) x/5i $pc-6
+   0x804837f <main+11>: mov    %esp,%ebp
+   0x8048381 <main+13>: push   %ecx
+   0x8048382 <main+14>: sub    $0x4,%esp
+=> 0x8048385 <main+17>: movl   $0x8048460,(%esp)
+   0x804838c <main+24>: call   0x80482d4 <puts@@plt>
+@end smallexample
+
 @cindex @code{$_}, @code{$__}, and value history
 The addresses and contents printed by the @code{x} command are not saved
 in the value history because there is often too much of them and they
Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.256
diff -u -p -u -r1.256 defs.h
--- defs.h	19 Oct 2009 09:51:40 -0000	1.256
+++ defs.h	21 Oct 2009 00:14:17 -0000
@@ -608,6 +608,7 @@ extern int build_address_symbolic (CORE_
 				   int *unmapped);
 
 extern void print_address (struct gdbarch *, CORE_ADDR, struct ui_file *);
+extern const char *pc_prefix (CORE_ADDR);
 
 /* From source.c */
 
Index: disasm.c
===================================================================
RCS file: /cvs/src/src/gdb/disasm.c,v
retrieving revision 1.33
diff -u -p -u -r1.33 disasm.c
--- disasm.c	11 Jul 2009 14:04:23 -0000	1.33
+++ disasm.c	21 Oct 2009 00:14:17 -0000
@@ -113,6 +113,7 @@ dump_insns (struct gdbarch *gdbarch, str
 	    num_displayed++;
 	}
       ui_out_chain = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
+      ui_out_text (uiout, pc_prefix (pc));
       ui_out_field_core_addr (uiout, "address", gdbarch, pc);
 
       if (!build_address_symbolic (pc, 0, &name, &offset, &filename,
Index: printcmd.c
===================================================================
RCS file: /cvs/src/src/gdb/printcmd.c,v
retrieving revision 1.165
diff -u -p -u -r1.165 printcmd.c
--- printcmd.c	19 Oct 2009 09:51:41 -0000	1.165
+++ printcmd.c	21 Oct 2009 00:14:17 -0000
@@ -725,6 +725,26 @@ print_address (struct gdbarch *gdbarch,
   print_address_symbolic (addr, stream, asm_demangle, " ");
 }
 
+/* Return a prefix for instruction address:
+   "=> " for current instruction, else "   ".  */
+
+const char *
+pc_prefix (CORE_ADDR addr)
+{
+  if (has_stack_frames ())
+    {
+      struct frame_info *frame;
+      CORE_ADDR pc;
+
+      frame = get_selected_frame (NULL);
+      pc = get_frame_pc (frame);
+
+      if (pc == addr)
+	return "=> ";
+    }
+  return "   ";
+}
+
 /* Print address ADDR symbolically on STREAM.  Parameter DEMANGLE
    controls whether to print the symbolic name "raw" or demangled.
    Global setting "addressprint" controls whether to print hex address
@@ -817,6 +837,8 @@ do_examine (struct format_data fmt, stru
   while (count > 0)
     {
       QUIT;
+      if (format == 'i')
+	fputs_filtered (pc_prefix (next_address), gdb_stdout);
       print_address (next_gdbarch, next_address, gdb_stdout);
       printf_filtered (":");
       for (i = maxelts;
Index: stack.c
===================================================================
RCS file: /cvs/src/src/gdb/stack.c,v
retrieving revision 1.208
diff -u -p -u -r1.208 stack.c
--- stack.c	19 Oct 2009 09:51:42 -0000	1.208
+++ stack.c	21 Oct 2009 00:14:17 -0000
@@ -643,8 +643,7 @@ print_frame_info (struct frame_info *fra
       /* If disassemble-next-line is set to on and there is line debug
          messages, output assembly codes for next line.  */
       if (disassemble_next_line == AUTO_BOOLEAN_TRUE)
-	do_gdb_disassembly (get_frame_arch (frame), -1,
-			    get_frame_pc (frame), sal.end);
+	do_gdb_disassembly (get_frame_arch (frame), -1, sal.pc, sal.end);
     }
 
   if (print_what != LOCATION)
Index: testsuite/gdb.base/consecutive.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/consecutive.exp,v
retrieving revision 1.9
diff -u -p -u -r1.9 consecutive.exp
--- testsuite/gdb.base/consecutive.exp	3 Jan 2009 05:58:03 -0000	1.9
+++ testsuite/gdb.base/consecutive.exp	21 Oct 2009 00:14:17 -0000
@@ -64,7 +64,7 @@ set stop_addr 0
 
 send_gdb "x /2i \$pc\n"
 gdb_expect {
-    -re "$hex.*${nl}($hex).*$gdb_prompt $" {
+    -re "=> $hex.*${nl}   ($hex).*$gdb_prompt $" {
 	set bp_addr $expect_out(1,string)
 	pass "get breakpoint address for foo"
     }
Index: testsuite/gdb.base/display.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/display.exp,v
retrieving revision 1.18
diff -u -p -u -r1.18 display.exp
--- testsuite/gdb.base/display.exp	28 May 2009 01:05:14 -0000	1.18
+++ testsuite/gdb.base/display.exp	21 Oct 2009 00:14:17 -0000
@@ -91,7 +91,7 @@ gdb_test "break 19"     ".*Breakpoint 4.
 gdb_test "info disp" ".*There are no auto-display expressions now..*" "inf disp"
 gdb_test "disp i" ".*1: i = 0.*" "display i"
 gdb_test "disp/x j" ".*2: /x j = 0x0.*" "display j"
-gdb_test "disp/i &k" ".*3: x/i &k(\r\n|  )$hex:.*" "display &k"
+gdb_test "disp/i &k" ".*3: x/i &k(\r\n|  )   $hex:.*" "display &k"
 gdb_test "disp/f f" ".*4: /f f = 3.1415*" "display/f f"
 gdb_test "disp/s &sum" ".*5: x/s &sum  $hex.*sum.:.*" "display/s &sum"
 
Index: testsuite/gdb.base/pc-fp.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/pc-fp.exp,v
retrieving revision 1.12
diff -u -p -u -r1.12 pc-fp.exp
--- testsuite/gdb.base/pc-fp.exp	14 Feb 2009 00:27:56 -0000	1.12
+++ testsuite/gdb.base/pc-fp.exp	21 Oct 2009 00:14:17 -0000
@@ -63,7 +63,7 @@ set valueof_fp [get_hexadecimal_valueof 
 # display since that encodes and then decodes the expression parameter
 # (and hence uses the mechanisms we're trying to test).
 
-gdb_test "display/i \$pc" "1: x/i +\\\$pc( +|\r\n)${valueof_pc}.*"
+gdb_test "display/i \$pc" "1: x/i +\\\$pc( +|\r\n)=> ${valueof_pc}.*"
 gdb_test "display/w \$fp" "2: x/xw +\\\$fp +${valueof_fp}.*"
 
 # FIXME: cagney/2002-09-04: Should also check that ``info registers
Index: testsuite/gdb.base/sigbpt.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/sigbpt.exp,v
retrieving revision 1.15
diff -u -p -u -r1.15 sigbpt.exp
--- testsuite/gdb.base/sigbpt.exp	5 Feb 2009 13:59:18 -0000	1.15
+++ testsuite/gdb.base/sigbpt.exp	21 Oct 2009 00:14:17 -0000
@@ -93,12 +93,12 @@ gdb_test "advance *bowler" "bowler.*" "a
 set test "stepping to fault"
 set signame "SIGSEGV"
 gdb_test_multiple "stepi" "$test" {
-    -re "Program received signal (SIGBUS|SIGSEGV).*pc(\r\n| *) *(0x\[0-9a-f\]*).*$gdb_prompt $" {
+    -re "Program received signal (SIGBUS|SIGSEGV).*pc(\r\n| *) *=> (0x\[0-9a-f\]*).*$gdb_prompt $" {
 	set signame $expect_out(1,string)
 	set segv_addr $expect_out(3,string)
 	pass "$test"
     }
-    -re " .*pc(\r\n| *)(0x\[0-9a-f\]*).*bowler.*$gdb_prompt $" {
+    -re " .*pc(\r\n| *)=> (0x\[0-9a-f\]*).*bowler.*$gdb_prompt $" {
 	set bowler_addrs [concat $expect_out(2,string) $bowler_addrs]
 	send_gdb "stepi\n"
 	exp_continue
@@ -110,7 +110,7 @@ gdb_test_multiple "stepi" "$test" {
 
 set test "get insn after fault"
 gdb_test_multiple {x/2i $pc} "$test" {
-    -re "(0x\[0-9a-f\]*).*bowler.*(0x\[0-9a-f\]*).*bowler.*$gdb_prompt $" {
+    -re "=> (0x\[0-9a-f\]*).*bowler.*(0x\[0-9a-f\]*).*bowler.*$gdb_prompt $" {
 	set bowler_addrs [concat $expect_out(2,string) $bowler_addrs]
 	pass "$test"
     }
@@ -199,7 +199,7 @@ proc stepi_out { name args } {
 	-re "pc(\r\n| *)[after_segv] .*bowler.*$gdb_prompt $" {
 	    kfail gdb/1702 "$test (skipped fault insn)"
 	}
-	-re "pc(\r\n| *)0x\[a-z0-9\]* .*bowler.*$gdb_prompt $" {
+	-re "pc(\r\n| *)=> 0x\[a-z0-9\]* .*bowler.*$gdb_prompt $" {
 	    kfail gdb/1702 "$test (corrupt pc)"
 	}
     }
@@ -244,12 +244,12 @@ proc cont_out { name args } {
     # inserted at the faulting instruction.  Note that the breakpoint
     # instruction wasn't executed, rather the inferior was SIGTRAPed
     # with the PC at the breakpoint.
-    gdb_test "continue" "Breakpoint.*pc(\r\n| *)[at_segv] .*" \
+    gdb_test "continue" "Breakpoint.*pc(\r\n| *)=> [at_segv] .*" \
 	"${name}; continue to breakpoint at fault"
 
     # Now single step the faulted instrction at that breakpoint.
     gdb_test "stepi" \
-	"Program received signal ${signame}.*pc(\r\n| *)[at_segv] .*" \
+	"Program received signal ${signame}.*pc(\r\n| *)=> [at_segv] .*" \
 	"${name}; stepi fault"    
 
     # Clear any breakpoints

Re: [RFC][patch] Allow to disassemble line.

[Eli Zaretskii]

> Date: Tue, 20 Oct 2009 17:22:24 -0700
> From: Paul Pluzhnikov <ppluzhnikov@google.com>
> Cc: gdb-patches@sourceware.org
> 
> Here is updated patch with documentation and testsuite fixes.

Thanks.

> 2009-10-20  Paul Pluzhnikov  <ppluzhnikov@google.com>
> 
> 	* gdb.texi (Machine Code): Mention current pc marker, update examples.
          ^^^^^^^^
The file name is wrong.

> +When examining machine instructions, instruction at current program counter
                                        ^^^^^^^^^^^
"the instruction"

The documentation patch is okay with these changes.

I think it would be good to mention this in NEWS as well.

Thanks.

Re: [RFC][patch] Allow to disassemble line.

[Paul Pluzhnikov]

On Tue, Oct 20, 2009 at 9:05 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> The documentation patch is okay with these changes.

Thanks, committed with these fixed.

> I think it would be good to mention this in NEWS as well.

This seems pretty minor correction/adjustment, not worthy of NEWS entry IMHO.

Thanks,
-- 
Paul Pluzhnikov

Re: [RFC][patch] Allow to disassemble line.

[Eli Zaretskii]

> Date: Wed, 21 Oct 2009 11:06:32 -0700
> From: Paul Pluzhnikov <ppluzhnikov@google.com>
> Cc: tromey@redhat.com, gdb-patches@sourceware.org
> 
> > I think it would be good to mention this in NEWS as well.
> 
> This seems pretty minor correction/adjustment, not worthy of NEWS entry IMHO.

What do we have to lose?  GDB's NEWS for each release aren't that
long, so I see no reason to save the users this knowledge.

Re: [RFC][patch] Allow to disassemble line.

[Tom Tromey]

>>>>> "Paul" == Paul Pluzhnikov <ppluzhnikov@google.com> writes:

Paul> 2009-10-20  Paul Pluzhnikov  <ppluzhnikov@google.com>
Paul> 	* defs.h (pc_prefix): New prototype.
Paul> 	* disasm.c (dump_insns): Mark current instruction.
Paul> 	* printcmd.c (do_examine): Likewise.
Paul> 	(pc_prefix): New function.
Paul> 	* stack.c (print_frame_info): Disassemble entire current line.
[...]

Ok.

Tom

Re: [RFC][patch] Allow to disassemble line.

[Daniel Jacobowitz]

On Mon, Oct 19, 2009 at 11:09:44AM -0700, Paul Pluzhnikov wrote:
> I've also changed the "current PC marker" from "* " to "=> " (as Eli
> suggested), so the output looks like this:
> 
> (top) disas
> Dump of assembler code for function gdb_main:
>    0x0000000000454c9e <gdb_main+0>:     push   %rbp
>    0x0000000000454c9f <gdb_main+1>:     mov    %rsp,%rbp
>    0x0000000000454ca2 <gdb_main+4>:     sub    $0x10,%rsp
>    0x0000000000454ca6 <gdb_main+8>:     mov    %rdi,-0x8(%rbp)
> => 0x0000000000454caa <gdb_main+12>:    mov    -0x8(%rbp),%rax
>    0x0000000000454cae <gdb_main+16>:    mov    0x10(%rax),%eax
>    0x0000000000454cb1 <gdb_main+19>:    mov    %eax,0x678475(%rip) #
> 0xacd12c <use_windows>

For whatever it's worth, I miss the extra screen real estate stolen by
the enlarged prompt (or even the narrower prompt).  I use a wider than
average terminal and most of the times I use disassemble it still runs
off the right and wraps unreadably.  It's fine in examples because
they have names like "gdb_main" or "bar"...

We've got this layout here with nicely aligned columns but tons of
whitespace.  In a halfway modern world we could do this with color.
Or bold the address of the current instruction.  This would be a real
interface departure for GDB so I'm curious what others think of it.
(Yes, I'm thinking of Tufte, why do you ask?)

Another possibility would be to factor out the name of the function.
Something like this:

(top) disas
Dump of assembler code for function gdb_main:
   0x0000000000454c9e (+0):     push   %rbp
   0x0000000000454c9f (+1):     mov    %rsp,%rbp
   0x0000000000454ca2 (+4):     sub    $0x10,%rsp
   0x0000000000454ca6 (+8):     mov    %rdi,-0x8(%rbp)
=> 0x0000000000454caa (+12):    mov    -0x8(%rbp),%rax
   0x0000000000454cae (+16):    mov    0x10(%rax),%eax

Or for x, current version:

(gdb) x/2i main
0x44cce0 <main>:        sub    $0x28,%rsp
0x44cce4 <main+4>:      movq   $0x0,(%rsp)
(gdb) <enter>
0x44ccec <main+12>:     mov    %edi,(%rsp)
0x44ccef <main+15>:     mov    %rsp,%rdi

After:

(gdb) x/2i main
Dump of assembler code for function main:
0x44cce0 (+0):          sub    $0x28,%rsp
0x44cce4 (+4):          movq   $0x0,(%rsp)
(gdb) <enter>
0x44ccec (+12):         mov    %edi,(%rsp)
0x44ccef (+15):         mov    %rsp,%rdi
...
(gdb) <enter>
0x44cd1a (+58):     retq
0x44cd1b:           nop
...
(gdb) <enter>
0x44cd1f:           nop
Dump of assembler code for function gdb_main:
0x44cd20 (+0):      sub    $0x8,%rsp


Anyway, I just made that all up.  I'm not asking for anyone to work on
it but I'd like to know what ways we have to reclaim some of this
space before we eat up more of it.

I do observe that disassemble prints leading zeros and examine strips them.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC][patch] Allow to disassemble line.

[Eli Zaretskii]

> Date: Mon, 19 Oct 2009 14:48:53 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: tromey@redhat.com, gdb-patches@sourceware.org
> 
> On Mon, Oct 19, 2009 at 11:09:44AM -0700, Paul Pluzhnikov wrote:
> > I've also changed the "current PC marker" from "* " to "=> " (as Eli
> > suggested), so the output looks like this:
> > 
> > (top) disas
> > Dump of assembler code for function gdb_main:
> >    0x0000000000454c9e <gdb_main+0>:     push   %rbp
> >    0x0000000000454c9f <gdb_main+1>:     mov    %rsp,%rbp
> >    0x0000000000454ca2 <gdb_main+4>:     sub    $0x10,%rsp
> >    0x0000000000454ca6 <gdb_main+8>:     mov    %rdi,-0x8(%rbp)
> > => 0x0000000000454caa <gdb_main+12>:    mov    -0x8(%rbp),%rax
> >    0x0000000000454cae <gdb_main+16>:    mov    0x10(%rax),%eax
> >    0x0000000000454cb1 <gdb_main+19>:    mov    %eax,0x678475(%rip) #
> > 0xacd12c <use_windows>
> 
> For whatever it's worth, I miss the extra screen real estate stolen by
> the enlarged prompt (or even the narrower prompt).

It's really hard to believe that 1 or 2 or 3 extra columns can make
such a difference.  Listing the source, GDB is unable to show most of
the lines in the call stack without wrapping them, and we live with
that.  How on Earth can disassembly do worse?

> I use a wider than average terminal

How wide is it?  Even on a 17" display I can easily have 130-column
text windows with very readable font, and almost 200 with slightly
less readable one.

> and most of the times I use disassemble it still runs
> off the right and wraps unreadably.  It's fine in examples because
> they have names like "gdb_main" or "bar"...

Please show your counter-examples.  I'm curious to see what kind of
symbols do you have there.

> We've got this layout here with nicely aligned columns but tons of
> whitespace.  In a halfway modern world we could do this with color.
> Or bold the address of the current instruction.

I have nothing against this, but it will have to be an option, because
dumb terminals, non-Posix emulations of termcap, and some front ends
will need to turn it off (and then the arrow should re-appear).

> Another possibility would be to factor out the name of the function.
> Something like this:
> 
> (top) disas
> Dump of assembler code for function gdb_main:
>    0x0000000000454c9e (+0):     push   %rbp
>    0x0000000000454c9f (+1):     mov    %rsp,%rbp
>    0x0000000000454ca2 (+4):     sub    $0x10,%rsp
>    0x0000000000454ca6 (+8):     mov    %rdi,-0x8(%rbp)
> => 0x0000000000454caa (+12):    mov    -0x8(%rbp),%rax
>    0x0000000000454cae (+16):    mov    0x10(%rax),%eax

I don't like this: disassembly should look like it looks elsewhere.
However, we could offer an optional removal of the leftmost column,
the address, and the next one, which shows the symbol and offset from
it.  Stripping leading zeros, as you point out, will help even more,
especially on 64-bit platforms.

Re: [RFC][patch] Allow to disassemble line.

[Daniel Jacobowitz]

On Mon, Oct 19, 2009 at 09:37:25PM +0200, Eli Zaretskii wrote:
> It's really hard to believe that 1 or 2 or 3 extra columns can make
> such a difference.  Listing the source, GDB is unable to show most of
> the lines in the call stack without wrapping them, and we live with
> that.  How on Earth can disassembly do worse?

Disassembly can do worse because so much of the line is usually
unintersting.  The three columns won't be a huge difference, even
though they're a noticeable percentage of the screen real estate;
mostly they prompted me to start this conversation.

I usually use a 104 column terminal.  Here's a sample from GDB; the
places most likely to wrap and be hard to write are calls:

0x080ec87f <handle_inferior_event+3087>:        call   0x8101d90 <gdbarch_deprecated_function_start_offset>

GDB wrote that on one line and let it hard wrap.

Or one from GNU libstdc++:

0xc6bc7 <_ZSt17rethrow_exceptionNSt15__exception_ptr13exception_ptrE+87>:
    callq  0x58e20 <_Unwind_RaiseException@plt>

GDB wrapped that before the callq.  I can't figure out when it does
this versus when it lets things run off the end of the line; I've seen
both today.

Neither of these are unusual functions.

> > We've got this layout here with nicely aligned columns but tons of
> > whitespace.  In a halfway modern world we could do this with color.
> > Or bold the address of the current instruction.
> 
> I have nothing against this, but it will have to be an option, because
> dumb terminals, non-Posix emulations of termcap, and some front ends
> will need to turn it off (and then the arrow should re-appear).

Agreed.  I hope that we could do this automatically in 99% of cases,
using the termcap capabilities and isatty.  We do already rely on
readline which requires tgetent.

> > Another possibility would be to factor out the name of the function.
> > Something like this:
> > 
> > (top) disas
> > Dump of assembler code for function gdb_main:
> >    0x0000000000454c9e (+0):     push   %rbp
> >    0x0000000000454c9f (+1):     mov    %rsp,%rbp
> >    0x0000000000454ca2 (+4):     sub    $0x10,%rsp
> >    0x0000000000454ca6 (+8):     mov    %rdi,-0x8(%rbp)
> > => 0x0000000000454caa (+12):    mov    -0x8(%rbp),%rax
> >    0x0000000000454cae (+16):    mov    0x10(%rax),%eax
> 
> I don't like this: disassembly should look like it looks elsewhere.
> However, we could offer an optional removal of the leftmost column,
> the address, and the next one, which shows the symbol and offset from
> it.  Stripping leading zeros, as you point out, will help even more,
> especially on 64-bit platforms.

I use both of those pieces of information... it's the function name
that I don't use.

Joel raised a good point about the function name in x/i though.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC][patch] Allow to disassemble line.

[Joel Brobecker]

> For whatever it's worth, I miss the extra screen real estate stolen by
> the enlarged prompt (or even the narrower prompt).  I use a wider than
> average terminal and most of the times I use disassemble it still runs
> off the right and wraps unreadably.  It's fine in examples because
> they have names like "gdb_main" or "bar"...

Same complaint here...

> We've got this layout here with nicely aligned columns but tons of
> whitespace.  In a halfway modern world we could do this with color.
> Or bold the address of the current instruction.  This would be a real
> interface departure for GDB so I'm curious what others think of it.
> (Yes, I'm thinking of Tufte, why do you ask?)

Wouldn't it be a problem with targets using MI? Or IDEs that run
GDB through a pipe (thinking of Windows in this case)?

I love the idea of starting to take advantage of the terminal capabilities
in order to introduce font properties such as colors or boldness.

> (top) disas
> Dump of assembler code for function gdb_main:
>    0x0000000000454c9e (+0):     push   %rbp
>    0x0000000000454c9f (+1):     mov    %rsp,%rbp

Regardless of the above, this suggestion seems fine to me.  The repeated
function name has always been clutter (more of the time, I disass the
current function, and I know what it is :-).

> (gdb) x/2i main
> Dump of assembler code for function main:
> 0x44cce0 (+0):          sub    $0x28,%rsp
> 0x44cce4 (+4):          movq   $0x0,(%rsp)

This one is a little less obviously nice. I have little user-define
functions such as "ssi" which does "stepi; x /i $pc". Having the
output fit in a couple of lines would really add clutter in this case
(IMO).

> I do observe that disassemble prints leading zeros and examine strips them.

We could start with that...

-- 
Joel

Re: [RFC][patch] Allow to disassemble line.

[Daniel Jacobowitz]

On Mon, Oct 19, 2009 at 09:40:30PM +0200, Joel Brobecker wrote:
> Wouldn't it be a problem with targets using MI? Or IDEs that run
> GDB through a pipe (thinking of Windows in this case)?

I hope they don't use the CLI disassemble command; there's been an MI
equivalent for ages.  But yes, anything we change could break
anything...

> > (gdb) x/2i main
> > Dump of assembler code for function main:
> > 0x44cce0 (+0):          sub    $0x28,%rsp
> > 0x44cce4 (+4):          movq   $0x0,(%rsp)
> 
> This one is a little less obviously nice. I have little user-define
> functions such as "ssi" which does "stepi; x /i $pc". Having the
> output fit in a couple of lines would really add clutter in this case
> (IMO).

Good point; I use display/i $pc all the time.  And I do find the
offsets useful, just not the function name.  I suppose it could be an
option...

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC][patch] Allow to disassemble line.

[Tom Tromey]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

Daniel> On Mon, Oct 19, 2009 at 09:40:30PM +0200, Joel Brobecker wrote:
>> Wouldn't it be a problem with targets using MI? Or IDEs that run
>> GDB through a pipe (thinking of Windows in this case)?

Daniel> I hope they don't use the CLI disassemble command; there's been an MI
Daniel> equivalent for ages.  But yes, anything we change could break
Daniel> anything...

As a general rule, if there is a corresponding MI command, then I think
we should feel free to change CLI output as we like.

Tom

Re: [RFC][patch] Allow to disassemble line.

[Joel Brobecker]

> As a general rule, if there is a corresponding MI command, then I think
> we should feel free to change CLI output as we like.

Agreed. I was mostly thinking of situations where we don't have
a terminal (or we have a dumb one). I like the idea of doing this
nicely on 99% of the cases with a setting to control the output:

  If we have a capable terminal, then use colors/boldness.
  If not, then revert to using the arrow.

The setting could be a general "auto/yes/no" setting for using
terminal capabilities in CLI display.

That being said, saving 3 or 4 characters through the use of font
characteristics may since too little a gain for the extra code?

I really like the idea of not printing the name of the function
in the disassembly output, though.  I've always had to cut it out
when I email portions of a GDB session that includes a disass.

-- 
Joel

Re: [RFC][patch] Allow to disassemble line.

[Paul Pluzhnikov]

On Mon, Oct 19, 2009 at 1:10 PM, Joel Brobecker <brobecker@adacore.com> wrote:

> I really like the idea of not printing the name of the function
> in the disassembly output, though.  I've always had to cut it out
> when I email portions of a GDB session that includes a disass.

I can do that in a separate patch.

-- 
Paul Pluzhnikov

Re: [RFC][patch] Allow to disassemble line.

[Daniel Jacobowitz]

On Mon, Oct 19, 2009 at 01:23:06PM -0700, Paul Pluzhnikov wrote:
> On Mon, Oct 19, 2009 at 1:10 PM, Joel Brobecker <brobecker@adacore.com> wrote:
> 
> > I really like the idea of not printing the name of the function
> > in the disassembly output, though.  I've always had to cut it out
> > when I email portions of a GDB session that includes a disass.
> 
> I can do that in a separate patch.

Paul, sorry for derailing this thread.  The part that matters is that
I'm grateful for your work and I think the new => marker is better
than no marker at all; I'm happy.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [RFC][patch] Allow to disassemble line.

[Tom Tromey]

>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:

Daniel> We've got this layout here with nicely aligned columns but tons of
Daniel> whitespace.  In a halfway modern world we could do this with color.
Daniel> Or bold the address of the current instruction.  This would be a real
Daniel> interface departure for GDB so I'm curious what others think of it.
Daniel> (Yes, I'm thinking of Tufte, why do you ask?)

I like it.  Also, I know there is interest "out there" in colorizing gdb
output:

    http://www.visophyte.org/blog/2009/03/04/gaudily-colorized-gdb-backtraces-woo/

I'm already looking forward to the day when people start posting GDB
color themes. :-)

Daniel> Another possibility would be to factor out the name of the function.

Yeah.  IME, repeating the function name over and over adds little value.

Tom

Re: [RFC][patch] Allow to disassemble line.

[Joel Brobecker]

> Does anybody have an opinion on whether the implementation should be
> changed to match the manual, or vice versa?

My 2 cents: I *think* the intention of this setting was to display
the next few instructions that are about to be executed.  That's the way
I personally would like this feature to work as I'd be able to identify
immediately which instruction is next.  So my vote goes towards updating
the manual to match the current implementation.

-- 
Joel

Re: [RFC][patch] Allow to disassemble line.

[Eli Zaretskii]

> Date: Thu, 8 Oct 2009 09:24:45 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> > Does anybody have an opinion on whether the implementation should be
> > changed to match the manual, or vice versa?
> 
> My 2 cents: I *think* the intention of this setting was to display
> the next few instructions that are about to be executed.  That's the way
> I personally would like this feature to work as I'd be able to identify
> immediately which instruction is next.  So my vote goes towards updating
> the manual to match the current implementation.

If the current implementation is what we want (and I personally don't
have an opinion either way), then I don't think the subtle difference
is important enough to update the manual.  The described behavior is
what most users will see almost all the time; accurately describing
the subtlety of this when you crash will most probably be so confusing
that it is not worth doing.