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.
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.
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
Created attachment 6243 [details] The patch
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.
Sorry, forgot: the patch is against the 7.4 branch as of today.
The best way to get a patch in is to follow the contribution instructions: http://sourceware.org/gdb/contribute/
(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
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.
Someone needs to champion the patch upstream. Please see: http://sourceware.org/gdb/contribute/ and: https://sourceware.org/gdb/wiki/ContributionChecklist
Thanks for the links, Pedro, those are helpful. I'll see what I can put together. It will be a few days.
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?
Created attachment 7692 [details] patch against gdb-7.7.1 Adaptation of the previous patch to gdb 7.7.1
Created attachment 8330 [details] 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.
This bug still exists in the current release (7.11) and is causing problems. Building from source with patch in the comments corrects the problem. Why is this 4+ year old bugfix not yet merged?
See comment #9.
There you see why it is fun to contribute to most github projects and a nuisance to contribute to FSF projects. I've applied that patch to all gdb versions since 7.7 by now and am still very confident that it took me less time than creating that silly patch submission. All the information is in this bug report in this (most upstream afaik) bug tracker. Use it. If we developed software with FSF procedures in our company, instead of using issue tracking we'd be bankrupt for a long time -- not only because of the effort, but also because we'd have lost all customers.
(In reply to Pedro Alves from comment #15) > See comment #9. Should I submit the patch to gdb-patches? In that case should I mention this bug number? Regards, Cristiano
Yes please. Please include a concise and self-contained rationale along with the patch, so it can added to the git log.
(In reply to Pedro Alves from comment #18) > Yes please. Please include a concise and self-contained rationale along > with the patch, so it can added to the git log. OK, done https://sourceware.org/ml/gdb-patches/2016-03/msg00318.html Regards, Cristiano
Hi, the submission of the patch to gdb-patches is more than a year old. Is there any estimate when this patch will become part of the main line?
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.
(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.
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
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.