simavr icon indicating copy to clipboard operation
simavr copied to clipboard

Expand the I/O table to support the ATmega2560

Open edgar-bonet opened this issue 4 years ago • 4 comments

I had simavr segfault on me by running sts 0x01ff, r0 on a simulated ATmega2560. This turned out to be _avr_set_r() calling an invalid function pointer found past the end of the array avr->io.

This pull request fixes the issue by expanding the avr->io array in order to cover the full I/O space of the ATmega2560.

edgar-bonet avatar Oct 13 '20 10:10 edgar-bonet

Hmm are you sure about that? AVR use 16 bits addresses, and I ran a LOT of code on 256's without that problem showing at all. That code has been there for a LONG time I should think this would have been picked up by now

buserror avatar Oct 13 '20 18:10 buserror

Hmm are you sure about that?

Absolutely. I can reliably segfault simavr with this:

echo -e "sts 0x01ff, r0\nsleep" > crash.s
avr-gcc -mmcu=atmega2560 -nostdlib crash.s -o crash.elf
./run_avr -f 1000000 -m atmega2560 crash.elf

That code has been there for a LONG time I should think this would have been picked up by now

The thing is, 0x01ff is not a valid I/O port: it's one of those “reserved” locations at the top of the I/O space. No legit AVR program will expose this bug. A buggy program, however, could poke into one of these reserved locations. Such program may crash, but it should not crash the simulator itself.

edgar-bonet avatar Oct 13 '20 19:10 edgar-bonet

In that case I'd really like some sort of error message to signal the problem. I'd MUCH, MUCH prefer simavr crashing because of an out of bounds problem than simavr silently working, and then have a random crash on the hardware I can't debug because "it works in simavr"! Ultimately, simavr is suposed to be a tool that let you develop/debug your firmware.

buserror avatar Apr 01 '21 18:04 buserror

In that case I'd really like some sort of error message to signal the problem.

Indeed, it would be really nice if simavr could print a warning whenever the firmware attempts to access a reserved memory location.

I'd MUCH, MUCH prefer simavr crashing because of an out of bounds problem than simavr silently working, and then have a random crash on the hardware I can't debug because "it works in simavr"!

I wouldn't. The error message “Segmentation fault” is a very clear indication that the crash is due to a bug in the simulator, not a bug in the firmware. I lost a lot of time figuring out that the simulator bug was triggered by a firmware bug.

Besides, even if we say segfaulting is an expected behavior (in which case it should be documented), the current behavior of simavr in this respect is inconsistent:

  • Simavr seems to emulate most reserved memory locations as regular R/W memory: writes are harmless, and reads return the last value written.

  • A few of these locations make simavr crash.

I just checked the behavior of an actual AVR chip. It's a mega328P, as I don't have a 2560 handy. I read and wrote all the reserved locations between the last I/O register and RAMSTART: 0xc7 through 0xff. From this experiment it seems that:

  • Writes are harmless and ignored.

  • All reads return 0, save from 0xc7 which returns 0x20.

edgar-bonet avatar Apr 02 '21 13:04 edgar-bonet