radareorg / radare2

UNIX-like reverse engineering framework and command-line toolset
https://www.radare.org/
GNU Lesser General Public License v3.0
20.8k stars 3.01k forks source link

Read/write memory by blocks in remote debug engine (GDB) #4038

Closed XVilka closed 7 years ago

XVilka commented 8 years ago

Currently, for operations like scrolling in visual mode, or single stepping, via GDB protocol, it requests memory readings byte-by-byte, this is one of the reasons it is so slow.

radare commented 8 years ago

Iirc it does reads in blocks of 4096 bytes. Maybe im wrong...

On 27 Jan 2016, at 09:31, Anton Kochkov notifications@github.com wrote:

Currently, for operations like scrolling in visual mode, or single stepping, via GDB protocol, it requests memory readings byte-by-byte, this is one of the reasons it is so slow.

— Reply to this email directly or view it on GitHub.

XVilka commented 8 years ago

I can attach the log. It supposed to use blocks, but...

radare commented 8 years ago

r_io can do reads at any size, the io plugin can do another thing internally, so it is possible that maybe r2 requests a 4 byte read, but io-gdb is internally doing a 4096 byte read. but from what i’ve seen it looks like the main bottleneck is in the poll() performed in libgdbr, because it needs to wait 250ms on every read in order to not miss any packet.

On 27 Jan 2016, at 10:17, Anton Kochkov notifications@github.com wrote:

I can attach the log. It supposed to use blocks, but...

— Reply to this email directly or view it on GitHub https://github.com/radare/radare2/issues/4038#issuecomment-175502242.

XVilka commented 8 years ago

I'll attach the log of the GDB reads once I'll get an access to Linux machine

radare commented 8 years ago

ping

XVilka commented 8 years ago

http://xvilka.me/example23.txt - it shows scrolling in r2 -a avr -D gdb gdb://localhost:1212 attached to simulavr.

XVilka commented 7 years ago

This is what @SrimantaBarua is doing, from what I understood.

radare commented 7 years ago

Yep

On 18 Jun 2017, at 23:21, Anton Kochkov notifications@github.com wrote:

This is what @SrimantaBarua is doing, from what I understood.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

SrimantaBarua commented 7 years ago

Right now gdb reads in blocks limited by max packet size specified by gdbserver. If gdbserver doesn't specify max packet size, default is 16 bytes (for something like simavr). So if reads of 1-2 bytes are happening, that is because radare wants to make small reads. This will be solved by io caching.

radare commented 7 years ago

Isnt 16 too small? I guess that combined with noack it shouldnt be a big issue but i assume this is a way to be safe. Maybe having an =! Io system command to get and set that packet size

On 21 Jun 2017, at 10:12, Srimanta Barua notifications@github.com wrote:

Right now gdb reads in blocks limited by max packet size specified by gdbserver. If gdbserver doesn't specify max packet size, default is 16 bytes (for something like simavr). So if reads of 1-2 bytes are happening, that is because radare wants to make small reads. This will be solved by io caching.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

SrimantaBarua commented 7 years ago

It probably is. I only did 16 because in #7692 you had limited memory read size to 8. But yes, having it configurable makes sense

XVilka commented 7 years ago

I agree on adding an option. May be just do something like e dbg.gdb.pktsz = 1024? Instead of inventing commands.

radare commented 7 years ago

Io plugins cant access eval vars. Better to just use the io-system interface for this

On 21 Jun 2017, at 11:24, Anton Kochkov notifications@github.com wrote:

I agree on adding an option. May be just do something like e dbg.gdb.pktsz = 1024? Instead of inventing commands.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

SrimantaBarua commented 7 years ago

Or maybe e dbg.pktsz with a callback that sets something like core->io->pktsz

XVilka commented 7 years ago

@SrimantaBarua @radare this is exactly what I meant

XVilka commented 7 years ago

@SrimantaBarua so what you decided to do with that?

SrimantaBarua commented 7 years ago

Now I think it's actually better to use the IO system with =!. I hadn't read @radare 's comment properly and yeah, it will be messy to access the IO plugin on setting the config var. Will send fix by tonight

SrimantaBarua commented 7 years ago

Ref #7961

XVilka commented 7 years ago

@SrimantaBarua so can be this safely closed? Or there are some missing pieces?

SrimantaBarua commented 7 years ago

I'd say it can be closed