Re: [PATCH 2/2] gdb: make avr_integer_to_address generate code or data address based on type

[Pedro Alves <palves@redhat.com>]

On 5/24/20 3:20 PM, Simon Marchi via Gdb-patches wrote:
> From: Cristiano De Alti <cristiano_dealti@hotmail.com>
> 
> I (Simon Marchi) am re-sending this patch written by Cristiano De Alti:
> 
>     https://sourceware.org/legacy-ml/gdb-patches/2016-03/msg00318.html
> 
> The commit message is new but the code has not changed.
> 
> The AVR architecture is a Harvard one, meaning it has different memory
> spaces for code and data.  In GDB, this is dealt with by having the data
> (SRAM) addresses start at 0x00800000.  When interpreting an integer as
> an address (converting to a CORE_ADDR), we currently always generate a
> data address.  This doesn't work for some cases described below, where
> the integer is meant to represent a code address.
> 
> This patch changes avr_integer_to_address so that it generates the
> correct type of address (code or data) based on the passed type.
> 
> Using the simavr.exp board, I didn't see any regressions when running
> the gdb.base/*.exp tests.  A few tests go from fail to pass, but none
> from pass to fail.  There are a few new fails and unresolved, but it's
> just because some tests manage to make more progress before failing in a
> different way.
> 
> In practice, it fixes disassembling by address, as described in the PR:
> 
>     - (gdb) disassemble 0x12a,0x12b
>     - Dump of assembler code from 0x12a to 0x12b:
>     -    0x0000012a <main+0>: push    r28
>     - End of assembler dump.
> 
>     + (gdb) disassemble 0x12a,0x12b
>     + Dump of assembler code from 0x80012a to 0x80012b:
>     +    0x0080012a:  nop
>     + End of assembler dump.
> 
> And also, setting a breakpoint by address:
> 
>     - (gdb) p &main
>     - $1 = (int (*)(void)) 0x12a <main>
>     - (gdb) b *0x12a
>     - Breakpoint 1 at 0x80012a
> 
>     + (gdb) p &main
>     + $1 = (int (*)(void)) 0x12a <main>
>     + (gdb) b *0x12a
>     + Breakpoint 1 at 0x12a: file test-avr.c, line 3.
>     + Note: automatically using hardware breakpoints for read-only addresses.
> 
> gdb/ChangeLog:
> 
> 	PR gdb/13519
> 	* avr-tdep.c (avr_integer_to_address): Return data or code
> 	address accordingly to the second 'type' argument of the
> 	function.

Thank you for writing the commit log.  LGTM.

Pedro Alves