Hi Aaron, I didn't mean to scare you off, but at the same time I think you understand how strongly I prefer to avoid screwing around with the gdb code unless it's absolutely necessary. (Not to mention that the dual-patching of printcmd.c by both gdb-7.6.patch and the gdb-7.6-ppc64le-support.patch, may have made it unworkable.) So I took my own advice, and implemented the "dis -[rf] address count" capability with a simple patch to the top-level kernel.c disassembly code: https://github.com/crash-utility/crash/commit/6c891b667f75543db279e1a29bf3d646a83d175d Also, inspired by your reference to "Add negative repeat count to 'x' command" gdb patch, I added a new "rd -R" option last week: https://github.com/crash-utility/crash/commit/72654f8c62fb1277939a21daa92bc8b1af70ed71 Thanks, Dave ----- Original Message ----- > ----- Original Message ----- > > 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. > > I can't stop you from trying, but I have to say that you're not calming > my nerves, because: > > (1) I don't care whether gdb-7.12 has that capability. > (2) If it is at all possible to refrain from tinkering with > the temperamental beast gdb, I would always rather do so. > > In the interest of simplicity and maintainability, it just seems that > here in cmd_dis(): > > 2019 gdb_pass_through(buf5, NULL, GNU_RETURN_ON_ERROR); > 2020 > 2021 if (req->flags & GNU_COMMAND_FAILED) { > 2022 close_tmpfile(); > 2023 error(FATAL, dis_err, req->addr); > 2024 } > 2025 > 2026 rewind(pc->tmpfile); > 2027 while (fgets(buf2, BUFSIZE, pc->tmpfile)) { > > at line 2025, you could do something like: > > if (count_entered and reverse) > set_reverse_offset(req->count); > else > rewind(pc->tmpfile) > > Your choice... > > Dave > > > > > > > > Kind regards, > > > > -- > > Aaron Tomlin > > -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility
