Re: [PATCH] Fix alignment of disassemble /r

[Doug Evans <dje@google.com>]

On Thu, Apr 17, 2014 at 10:27 AM, Daniel Gutson
<daniel.gutson@tallertechnologies.com> wrote:
> On Thu, Apr 17, 2014 at 1:27 PM, Doug Evans <dje@google.com> wrote:
>> On Thu, Apr 17, 2014 at 7:37 AM, Daniel Gutson
>> <daniel.gutson@tallertechnologies.com> wrote:
>>> So what if I add a new configuration variable, such as
>>>      set disassemble-raw-alignment
>>> with "off" as default, and if set to on, pad to gdbarch_max_insn_length ?
>>
>> Presumably some frontends will do their own alignment.
>>
>> If we went with disassemble-raw-alignment, a boolean value won't help
>> x86 much, it's either no alignment or (in general) too much
>> whitespace.
>> An improvement would be a value from min-insn-length to
>> max-insn-length, but that would be problematic in a multi-arch
>> debugging scenario.
>>
>> If we could agree on some minimum alignment for each variable-length
>> ISA (5 would be fine for me for x86) then maybe a boolean value could
>> be useful ("off" = no alignment, "on" = employ arch-specific minimum).
>>
>> OTOH, what if we made two passes over the instructions, with the first
>> pass computing the maximum instruction length that is present?
>
> I already considered this, but thought that it would be going to be
> rejected due to be too much non-performant. Wouldn't each pass
> translate in a lot of MI messaging in a case of a remote server?

[nit: If by "remote server" you mean things like gdbserver, then MI
isn't involved.]
Yes, it *could* involve extra traffic between gdb and gdbserver (or
whatever stub is being used) but we have caching now that should
mitigate that.
Still, it's a valid concern.

> And,
> what about screen paginig? I shouldn't iterate over all the range, but
> the screen height range only.

I guess one could go that route but IMO it's not necessary.
OTOH, One *could* also do the max-length computation in bunches that
took advantage of the cache size thus guaranteeing no additional
gdb/gdbserver traffic.
Not my first choice, just a possibility.
[gdb has a lot of knobs, just want to make sure no stone is left
unturned before we add this one :-)]

> I can go for any of the proposed solutions. 5 insn-length was fine for
> me. On a side note, I did this since I was debugging some
> self-modifying-code where the mis-alignment was driving me crazy.
> The two-passes solution is the one I dislike more IMVHO.
>
> Yet another option is to set the disassembly raw alignment width, something like
>     set disassemble-raw-width 5
> (5 is the amount of insn; 0 means no padding equal to the current
> behavior and that would be the default value).

The problem with this is that the setting is arch-specific.
Not that one can do this today (or maybe one can in certain
situations, e.g. Cell), but if I'm debugging two different programs
from two different architectures it's possible any one value of
disassemble-raw-width is going to be painful.
One *could* have prefix commands for each architecture, and then one could have

set arch-$arch disassemble-raw-width N
or
set arch $arch disassemble-raw-width N

[again, just a possibility, not sure I prefer it]

Sorry for not having a definitive suggestion.
Maybe others will express an opinion and we can go from there.