[Crash-utility] [PATCH v2] crash: dis: introduce count in reverse and forward mode

[Aaron Tomlin]

On Fri 2019-06-21 11:45 -0400, Dave Anderson wrote:
> 
> 
> ----- Original Message -----
> > Changes since v1:
> > 
> >  - Update 'dis' help page
> >  - Resolve patch fuzz
> > 
> > 
> > The purpose of this patch is to add support for a count value in reverse
> > or forward mode, during disassembly:
> > 
> > 	'dis [-r|-f] [symbol|address] [count]'
> > 
> > For example:
> > 
> >   crash> dis -f list_del+0x16 4
> >   0xffffffff812b3346 <list_del+22>:       jne    0xffffffff812b3381  <list_del+81>
> >   0xffffffff812b3348 <list_del+24>:       mov    (%rbx),%rax
> >   0xffffffff812b334b <list_del+27>:       mov    0x8(%rax),%r8
> >   0xffffffff812b334f <list_del+31>:       cmp    %r8,%rbx
> > 
> >   crash> dis -r list_del+0x16 4
> >   0xffffffff812b333c <list_del+12>:       mov    0x8(%rdi),%rax
> >   0xffffffff812b3340 <list_del+16>:       mov    (%rax),%r8
> >   0xffffffff812b3343 <list_del+19>:       cmp    %r8,%rdi
> >   0xffffffff812b3346 <list_del+22>:       jne    0xffffffff812b3381  <list_del+81>
[ ... ]

Hi Dave,

> To be honest, it's not clear how that can be addressed, because any patches
> to printcmd.c must be applicable to the non-ppc64le and the ppc64le versions
> of the file. 

Unfortunately, I believe this is the underlying issue.
I did not compile test under ppc64le.

> So let's take a step back, and consider the risk/reward of this quite intrusive
> patchset to such a crucial gdb file.

In my opinion and if I understand correctly, the original GDB commit
bb556f1facb ("Add negative repeat count to 'x' command") is rather
self-contained and should not cause an issue. I was particularly careful in
this regard. Moving forward, I could strip out all but absolutely necessary
changes from the original GDB commit, to support this feature.

> First, "dis -f <address> <count>" is pretty much meaningless.  In order
> to accomplish that, just don't use the "-f" argument! 

I agree - this is superficial; count in forward mode was added since
reverse mode also accepts a count.

> Secondly, for "dis -r <address> count", you could simply run "dis -r <address> | tail -4".
> For that matter, is it really all that onerous to see the fully disassembled function
> up to the target address?

Personally, I would prefer to avoid using an external command in this
particular case. Especially since the GDB command examine supports a
positive or negative (i.e. in GDB release 7.12 and above) count.

> And again, even if you wanted to support it, it also seems like it could be accomplished
> within the cmd_dis() function, given that the full output is pre-gathered into a tmpfile
> prior to being displayed.  

We can indeed but it would be easier to utilise GDB for this as explained above.

If you don't mind another iteration, I'll attempt to address the above
build issues. Please let me know your thoughts.


Kind regards,

-- 
Aaron Tomlin