RE: [patch] Disassembly improvements

Abid, Hafiz
2013-10-11T16:45:17Z

> > But it seems to me that will just disable the optimization for buffer
> > line > 1.
> >
> > LEN here I think will the disassembler considers to be the maximum
> > length of an instruction for the arquitecture it is disassembling.
> > We want to read _more_ than that from memory in one go, otherwise,
> > we'll not be buffering anything. What we do not want, is for that
> > over fetching to read beyond the range that was passed to
> gdb_disassembly.
We come in the else case only if we have exhausted the initial buffer allocated
in gdb_disassembly. You are right that current code will read LEN bytes in that case
instead of reading the larger buffer but I think that should not be very common and
we don't lose the optimization gained from the buffering for the first ' DIS_BUF_SIZE'
bytes. So I think this patch is useful on its own and the other enhancement can be built
on top of it.

Also if user provides a range of address to the disassembly command then it is possible that
end address is in the middle of the instruction and gdb will end up reading beyond the end
address given in the command. So we probably cannot forbid that here anyway.

(gdb) disassemble /r $pc,$pc+12
Dump of assembler code from 0x40292c to 0x402938:
=> 0x000000000040292c <main+8>: c7 45 fc 00 00 00 00 movl $0x0,-0x4(%rbp)
0x0000000000402933 <main+15>: eb 78 jmp 0x4029ad <main+137>
0x0000000000402935 <main+17>: 8b 15 f5 18 20 00 mov 0x2018f5(%rip),%edx # 0x604230 <loops>

As you can see gdb reading beyond 0x402938 here.

> > I think we'll need to derive from "struct disassemble_info", and add
> > the original range to that new struct, or record that info directly in
> > "struct disassemble_info", which is in include/dis-asm.h.
>
> I now noticed there's a struct disassemble_info->application_data field,
> which GDB currently uses to put the gdbarch in. We could put a disasm.c
> specific structure there instead.
I noticed that this field is already being used in spu-tdep.c. Any changes
to it will break that code. We can add new fields but as I described above,
we may not want to check for the high address of the request.

Thanks,
Abid