[Bug gdb/13519] New: [avr] disassemble command attempts to disassemble SRAM rather than program memory

[Bug gdb/13519] New: [avr] disassemble command attempts to disassemble SRAM rather than program memory

[j at uriah dot heep.sax.de]

http://sourceware.org/bugzilla/show_bug.cgi?id=13519

             Bug #: 13519
           Summary: [avr] disassemble command attempts to disassemble SRAM
                    rather than program memory
           Product: gdb
           Version: 7.2
            Status: NEW
          Severity: normal
          Priority: P2
         Component: gdb
        AssignedTo: unassigned@sourceware.org
        ReportedBy: j@uriah.heep.sax.de
                CC: eric.weddington@atmel.com
    Classification: Unclassified
            Target: avr-*-*


The dissassemble command now attempts to disassemble SRAM locations
rather than program memory:

% ./gdb-7.2/gdb/gdb -q foo.elf
Reading symbols from /simtemp/jw/foo.elf...done.

(gdb) disas 0x1, 0x3
Dump of assembler code from 0x800001 to 0x800003:
   0x00800001:  Cannot access memory at address 0x800001

(Due to the AVR's Harvard architecture, SRAM is remapped internally in
the GNU tools to an offset of 0x800000 which is beyond the addressable
program memory area of an AVR.  In contrast, program memory starts at
offset 0.)

Previous versions of GDB didn't experience this problem:

% ./gdb-7.0.1/gdb/gdb -q foo.elf
Reading symbols from /simtemp/jw/foo.elf...done.
(gdb) disas 1 3
Dump of assembler code from 0x1 to 0x3:
0x00000001 <__vectors+1>:       sbci    r25, 0x64       ; 100
End of assembler dump.

The difference is that, obviously, the start and end locations are now
being treated as expressions rather than numbers (which also caused
the requirement to separate them by a comma rather than a space).
Somehow, during evaluation of the expression, the offset 0x800000 gets
added.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug gdb/13519] [avr] disassemble command attempts to disassemble SRAM rather than program memory

[j at uriah dot heep.sax.de]

http://sourceware.org/bugzilla/show_bug.cgi?id=13519

--- Comment #1 from Joerg Wunsch <j at uriah dot heep.sax.de> 2011-12-19 13:36:13 UTC ---
Some more analysis:

disassemble_command() calls value_as_address() on the first argument
(and eventually parse_and_eval_address() on the second one).

This function has a lengthy comment about Harvard-architecture address
mapping and its side effects.  It then calls
gdbarch_integer_to_address(), from there avr_integer_to_address(),
which eventually calls avr_make_saddr().  This functions adds
AVR_SMEM_START to everything except 0.

Something in that sequence is not quite right ;-), but I can't decide
what.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug gdb/13519] [avr][Regression 7.1/7.2/7.3/7.4] disassemble command attempts to disassemble SRAM rather than program memory

[eric.weddington at atmel dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=13519

Eric  Weddington <eric.weddington at atmel dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|7.2                         |7.1
            Summary|[avr] disassemble command   |[avr][Regression
                   |attempts to disassemble     |7.1/7.2/7.3/7.4]
                   |SRAM rather than program    |disassemble command
                   |memory                      |attempts to disassemble
                   |                            |SRAM rather than program
                   |                            |memory

--- Comment #2 from Eric  Weddington <eric.weddington at atmel dot com> 2011-12-19 14:55:19 UTC ---
Note: The user reporting this problem stated that the bug showed up originally
in version 7.1:
http://lists.nongnu.org/archive/html/avr-chat/2011-12/msg00007.html

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug gdb/13519] [avr][Regression 7.1/7.2/7.3/7.4] disassemble command attempts to disassemble SRAM rather than program memory

[mnl at mnl dot de]

http://sourceware.org/bugzilla/show_bug.cgi?id=13519

--- Comment #3 from Michael N. Lipp <mnl at mnl dot de> 2012-02-24 11:27:25 UTC ---
Created attachment 6243
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6243
The patch

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug gdb/13519] [avr][Regression 7.1/7.2/7.3/7.4] disassemble command attempts to disassemble SRAM rather than program memory

[mnl at mnl dot de]

http://sourceware.org/bugzilla/show_bug.cgi?id=13519

Michael N. Lipp <mnl at mnl dot de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mnl at mnl dot de

--- Comment #4 from Michael N. Lipp <mnl at mnl dot de> 2012-02-24 11:28:04 UTC ---
The question is: how clever should gdb be. If you say

(gdb) disas (void (*)())0x1,(void (*)())0x3

it works. But of course, you'd expect gdb to do that cast automatically in the
context of a disassemble.

I have attached a patch that does that cast for disassemble, both from the
command line as well as for mi. gdb is rathr complicated. I hope my approach is
correct.

Actually, the automatic cast should also be applied if you "x/i addr". But I
didn't bother to look that up as I'm using gdb from Eclipse anyway.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug gdb/13519] [avr][Regression 7.1/7.2/7.3/7.4] disassemble command attempts to disassemble SRAM rather than program memory

[mnl at mnl dot de]

http://sourceware.org/bugzilla/show_bug.cgi?id=13519

--- Comment #5 from Michael N. Lipp <mnl at mnl dot de> 2012-02-24 11:31:00 UTC ---
Sorry, forgot: the patch is against the 7.4 branch as of today.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug gdb/13519] [avr][Regression 7.1/7.2/7.3/7.4] disassemble command attempts to disassemble SRAM rather than program memory

[tromey at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=13519

Tom Tromey <tromey at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tromey at redhat dot com

--- Comment #6 from Tom Tromey <tromey at redhat dot com> 2012-02-27 19:33:45 UTC ---
The best way to get a patch in is to follow the contribution
instructions: http://sourceware.org/gdb/contribute/

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug gdb/13519] [avr][Regression 7.1/7.2/7.3/7.4] disassemble command attempts to disassemble SRAM rather than program memory

[eric.weddington at atmel dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=13519

--- Comment #7 from Eric  Weddington <eric.weddington at atmel dot com> 2012-03-09 16:00:59 UTC ---
(In reply to comment #5)
> Sorry, forgot: the patch is against the 7.4 branch as of today.

Hi Michael,

Do you have a FSF Copyright Assignment on file? You'll need that in order to
contribute a non-trivial patch.

Thanks,
Eric Weddington

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug gdb/13519] [avr][Regression 7.1/7.2/7.3/7.4] disassemble command attempts to disassemble SRAM rather than program memory

[dwt at pobox dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=13519

Dennis Tokarski <dwt at pobox dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dwt at pobox dot com

--- Comment #8 from Dennis Tokarski <dwt at pobox dot com> ---
Hi all,

Sorry for reviving a bug that's over two years old, but as
far as I can tell this problem is well understood yet has
never been fixed.

I ran into this a couple of weeks ago using avr-gdb-7.1 on
Fedora 18. More recently I've rebuilt the package using the
source from gdb 7.7 and the problem is still there.

Michael Lipp's patch is readily adapted to 7.7 and works, at
least in the situations he intended.

This issue was raised again on the gdb mailing list by
petr.hluzin@gmail.com in August of 2012. Eric seemed to
stop the thread short by stating "Adding 0x800000 to all
code-space addresses has been part of AVR GCC for many years",
which is not correct. I expect he meant to say "sram addresses"?

Regardless, is there some issue blocking inclusion of Michael's
patch? If it's a question of the maintainers wanting a different
approach for the sake of "doing it right", I'll be happy to work
something up given a little guidance.

Disassembly seems like a fairly major feature to leave broken
for this long even if AVR is kind of a corner use case.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/13519] [avr][Regression 7.1/7.2/7.3/7.4] disassemble command attempts to disassemble SRAM rather than program memory

[palves at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=13519

Pedro Alves <palves at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |palves at redhat dot com

--- Comment #9 from Pedro Alves <palves at redhat dot com> ---
Someone needs to champion the patch upstream.  Please see:

 http://sourceware.org/gdb/contribute/

and:

 https://sourceware.org/gdb/wiki/ContributionChecklist

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/13519] [avr][Regression 7.1/7.2/7.3/7.4] disassemble command attempts to disassemble SRAM rather than program memory

[dwt at pobox dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=13519

--- Comment #10 from Dennis Tokarski <dwt at pobox dot com> ---
Thanks for the links, Pedro, those are helpful.

I'll see what I can put together. It will be a few days.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/13519] [avr][Regression 7.1/7.2/7.3/7.4] disassemble command attempts to disassemble SRAM rather than program memory

[cristiano.dealti at eurotech dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=13519

cristiano.dealti at eurotech dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |cristiano.dealti at eurotech dot c
                   |                            |om
            Version|7.1                         |7.7

--- Comment #11 from cristiano.dealti at eurotech dot com ---
Of course the bug is still present in the latest gdb 7.7.1 release.
It is also exacerbated when avr-gdb is used in Eclipse Juno because its
Disassembly CDT plugin specifies source and end addresses in the
-data-disassemble gdb command.

By enabling logging, it can be seen that gdb adds the 0x80000 offset to the
requests to the avarice backend which reads data from sRAM instead from flash.

The response returned to gdb from avarice contains addresses with the 0x80000
offset (and wrong disassembled instructions of course). These responses are
then rejected, due to address mismatch in the response, by the Eclipse DCT
Disassembly plugin which retries with different parameters and eventually gives
up.

All this significantly slows down debugging from Eclipse and makes the entire
experience unpleasant.

Indeed, this two years old bug should be fixed. It cannot be that difficult.
Otherwise one has to spend a whole day figuring out what's going on, find the
bug report and adapt the attached patch to the latest release. 

Dennis, did you succeed submitting the patch or something?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/13519] [avr][Regression 7.1/7.2/7.3/7.4] disassemble command attempts to disassemble SRAM rather than program memory

[cristiano.dealti at eurotech dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=13519

--- Comment #12 from cristiano.dealti at eurotech dot com ---
Created attachment 7692
  --> https://sourceware.org/bugzilla/attachment.cgi?id=7692&action=edit
patch against gdb-7.7.1

Adaptation of the previous patch to gdb 7.7.1

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/13519] [avr][Regression 7.1/7.2/7.3/7.4] disassemble command attempts to disassemble SRAM rather than program memory

[cristiano.dealti at eurotech dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=13519

cristiano.dealti at eurotech dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #6243|0                           |1
        is obsolete|                            |
   Attachment #7692|0                           |1
        is obsolete|                            |

--- Comment #13 from cristiano.dealti at eurotech dot com ---
Created attachment 8330
  --> https://sourceware.org/bugzilla/attachment.cgi?id=8330&action=edit
Final patch to broken AVR arch with the latest gdbs

This patch obsoletes all previous patches attached to this bug.
The patch is trivial and only affects avr-tdep.c.
As noted by others, avr_integer_to_address() always returns addresses in SRAM.
However, the second parameter of this function, struct type *type, allows to
understand if a code or data address is desired.

Indeed, patch is so simple that the patched function is simply:

static CORE_ADDR
avr_integer_to_address (struct gdbarch *gdbarch,
                        struct type *type, const gdb_byte *buf)
{
  ULONGEST addr = unpack_long (type, buf);

  if (TYPE_DATA_SPACE (type))
    return avr_make_saddr (addr);
  else
    return avr_make_iaddr (addr);
}

Everything works: disassemble, breakpoints at numeric address, breakpoints at
source line and watchpoints.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/13519] [avr][Regression 7.1/7.2/7.3/7.4] disassemble command attempts to disassemble SRAM rather than program memory

[msquirogac at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=13519

Mauro Quiroga <msquirogac at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |msquirogac at gmail dot com

--- Comment #21 from Mauro Quiroga <msquirogac at gmail dot com> ---
Any news regarding this bug and patch? Years has passed and still nothing has
changed.
Also, after reviewing the bug list I found that there is relationship with
other bugs that were filed before and after, like:

https://sourceware.org/bugzilla/show_bug.cgi?id=9511
https://sourceware.org/bugzilla/show_bug.cgi?id=12105
https://sourceware.org/bugzilla/show_bug.cgi?id=12299
https://sourceware.org/bugzilla/show_bug.cgi?id=16606

The effects reported in all of them are the same, being the main problem the
way the memory address is interpreted and used.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/13519] [avr][Regression 7.1/7.2/7.3/7.4] disassemble command attempts to disassemble SRAM rather than program memory

[mnl at mnl dot de]

https://sourceware.org/bugzilla/show_bug.cgi?id=13519

--- Comment #22 from Michael N. Lipp <mnl at mnl dot de> ---
(In reply to Mauro Quiroga from comment #21)
> Any news regarding this bug and patch? Years has passed and still nothing
> has changed.
> Also, after reviewing the bug list I found that there is relationship with
> other bugs that were filed before and after, like:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=9511
> https://sourceware.org/bugzilla/show_bug.cgi?id=12105
> https://sourceware.org/bugzilla/show_bug.cgi?id=12299
> https://sourceware.org/bugzilla/show_bug.cgi?id=16606
> 
> The effects reported in all of them are the same, being the main problem the
> way the memory address is interpreted and used.

Kind of incredible, isn't it, how much time the inclusion of a 4 line patch
would have saved...

To be honest, having worked my whole professional live with bug trackers, I've
never understood how this mail-a-patch workflow is supposed to work.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/13519] [avr][Regression 7.1/7.2/7.3/7.4] disassemble command attempts to disassemble SRAM rather than program memory

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=13519

--- Comment #23 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Simon Marchi <simark@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=1218a4bf49919878d41ad37cbe906f412d6fbdad

commit 1218a4bf49919878d41ad37cbe906f412d6fbdad
Author: Cristiano De Alti <cristiano_dealti@hotmail.com>
Date:   Mon May 25 11:55:56 2020 -0400

    gdb: make avr_integer_to_address generate code or data address based on
type

    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.

    Change-Id: Iaea1587d053e86f4ab8aebdcabec8d31a6d262cd

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/13519] [avr][Regression 7.1/7.2/7.3/7.4] disassemble command attempts to disassemble SRAM rather than program memory

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=13519

Simon Marchi <simark at simark dot ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED
                 CC|                            |simark at simark dot ca

--- Comment #24 from Simon Marchi <simark at simark dot ca> ---
Fixed.  Note that there is now a simavr.exp board in the testsuite, so it's
easier to test changes / write tests for the AVR architecture now.

-- 
You are receiving this mail because:
You are on the CC list for the bug.