search

avrdudes / avr-libc

The AVR-LibC package provides a subset of the standard C library for AVR 8-bit RISC microcontrollers.
https://avrdudes.github.io/avr-libc/
Other
261 stars 56 forks source link

EEPROM tests for ATtiny2313 are failing #954

Open sprintersb opened 8 months ago

sprintersb commented 8 months ago

I am getting fails from running the EEPROM tests for ATtiny2313, for example execute in tests/simulate:

> MCU_LIST_FULL="attiny2313" ./runtest.sh -a $build  -s avr/eeprom-*.c
Simulate: avr/eeprom-1.c attiny2313 ... *** simulate failed: 70
Simulate: avr/eeprom-2.c attiny2313 ... *** simulate failed: 46
Simulate: avr/eeprom-3.c attiny2313 ... *** simulate failed: 64
Simulate: avr/eeprom-4.c attiny2313 ... *** simulate failed: 13

The fails are independent of compiler version, and they can be fixed by

diff --git a/libc/misc/eerd_word.S b/libc/misc/eerd_word.S
index 445f5ab1..7dc0c84c 100644
--- a/libc/misc/eerd_word.S
+++ b/libc/misc/eerd_word.S
@@ -53,9 +53,7 @@
 #else

        ldi     XL, 24          ; start address to write: r24
-# if  RAMEND > 0xFF
        ldi     XH, 0
-# endif

        ldi     n_lo, lo8(2)
 # if  E2END > 0xFF

etc. This means the eeprom_read_blraw routine malfunctions when XH contains garbage. Maybe it is just a simulavr issue?

Using SimulAVR v1.1.0.

sciprosk commented 1 month ago

Looks like being an already reported SimulAVR issue: https://savannah.nongnu.org/bugs/?66044.

I spent some time with GDB today. The problem is indeed with st X+, r0 instruction in this block:

Dump of assembler code for function eeprom_read_block:
   0x000002e8 <+0>:     mov     r26, r24
   0x000002ea <+2>:     mov     r24, r22
   0x000002ec <+4>:     mov     r30, r24
   0x000002ee <+6>:     sbic    0x1c, 1 ; 28
   0x000002f0 <+8>:     rjmp    .-4             ;  0x2ee <eeprom_read_block+6>
   0x000002f2 <+10>:    rjmp    .+10            ;  0x2fe <eeprom_read_block+22>
   0x000002f4 <+12>:    out     0x1e, r30       ; 30
   0x000002f6 <+14>:    sbi     0x1c, 0 ; 28
   0x000002f8 <+16>:    inc     r30
   0x000002fa <+18>:    in      r0, 0x1d        ; 29
   0x000002fc <+20>:    st      X+, r0
=> 0x000002fe <+22>:    subi    r20, 0x01       ; 1
   0x00000300 <+24>:    brcc    .-14            ;  0x2f4 <eeprom_read_block+12>
   0x00000302 <+26>:    ret

In <+20>: XH is not masked out, which generates the following warnings (here in 0xfc16: 0x16 means the content of XL (correctly pointing to r22) and fc is whatever garbage is in XH):

$ simulavr -g -d attiny2313 -T __stop_program -f eeprom-1.elf -C core_avr_dump.core -m 60000000000
Waiting on port 1212 for gdb client to connect...
Connection opened by host 127.0.0.1, port 54832.
WARNING: file /home/buildbudy/simulavr/libsim/rwmem.cpp: line 235: Invalid write access to IO[0xfc16]=0xfb, PC=0x2fc
WARNING: file /home/buildbudy/simulavr/libsim/rwmem.cpp: line 235: Invalid write access to IO[0xfc17]=0xfa, PC=0x2fc
WARNING: file /home/buildbudy/simulavr/libsim/rwmem.cpp: line 235: Invalid write access to IO[0xfc18]=0xf9, PC=0x2fc
WARNING: file /home/buildbudy/simulavr/libsim/rwmem.cpp: line 235: Invalid write access to IO[0xfc19]=0xf8, PC=0x2fc

I did not notice any other problems. All registers are properly set, and __DATA_REGION_LENGTH__ is correct in ELF.

sciprosk commented 1 month ago

I looked into the instruction decoder in the SimulAVR Git repo. Masking XYZ registers in LD/ST instructions is not supported. The following logic required by the "AVR Instruction Set Manual" is not implemented:

Note that only the low byte of the X-pointer is updated in devices with no more than 256 bytes of data space. For such devices, the high byte of the pointer is not used by this instruction and can be used for other purposes.

For illustration, this is a simple patch for SimulAVR that allows all eeprom tests to pass. So these failing tests is not an issue with AVR-LibC code.

diff --git a/libsim/decoder.cpp b/libsim/decoder.cpp
index 3ea13a6..128e994 100644
--- a/libsim/decoder.cpp
+++ b/libsim/decoder.cpp
@@ -773,7 +773,7 @@ avr_op_LDD_Y::avr_op_LDD_Y(word opcode, AvrDevice *c):

 int avr_op_LDD_Y::operator()() {
     /* Y is R29:R28 */
-    word Y = core->GetRegY();
+    word Y = core->dataAddressMask & core->GetRegY();

     core->SetCoreReg(Rd, core->GetRWMem(Y + K));

@@ -787,7 +787,7 @@ avr_op_LDD_Z::avr_op_LDD_Z(word opcode, AvrDevice *c):

 int avr_op_LDD_Z::operator()() {
     /* Z is R31:R30 */
-    word Z = core->GetRegZ();
+    word Z = core->dataAddressMask & core->GetRegZ();

     core->SetCoreReg(Rd, core->GetRWMem(Z + K));

@@ -830,7 +830,7 @@ int avr_op_LD_X::operator()() {
     /* X is R27:R26 */
     word X = core->GetRegX();

-    core->SetCoreReg(Rd, core->GetRWMem(X));
+    core->SetCoreReg(Rd, core->GetRWMem(X & core->dataAddressMask));

     return (core->flagXMega || core->flagTiny10) ? 1 : 2;
 }
@@ -847,9 +847,11 @@ int avr_op_LD_X_decr::operator()() {

     /* Perform pre-decrement */
     X--;
-    core->SetCoreReg(Rd, core->GetRWMem(X));
+    core->SetCoreReg(Rd, core->GetRWMem(X & core->dataAddressMask));
     core->SetCoreReg(26, X & 0xff);
-    core->SetCoreReg(27, (X >> 8) & 0xff);
+    if (core->dataAddressMask > 0xff) {
+        core->SetCoreReg(27, (X >> 8) & 0xff);
+    }

     return core->flagTiny10 ? 3 : 2;
 }
@@ -865,10 +867,12 @@ int avr_op_LD_X_incr::operator()() {
        avr_error( "Result of operation is undefined" );

     /* Perform post-increment */
-    core->SetCoreReg(Rd, core->GetRWMem(X));
+    core->SetCoreReg(Rd, core->GetRWMem(X & core->dataAddressMask));
     X++;
     core->SetCoreReg(26, X & 0xff);
-    core->SetCoreReg(27, (X >> 8) & 0xff);
+    if (core->dataAddressMask > 0xff) {
+        core->SetCoreReg(27, (X >> 8) & 0xff);
+    }

     return core->flagXMega ? 1 : 2;
 }
@@ -885,9 +889,11 @@ int avr_op_LD_Y_decr::operator()() {

     /* Perform pre-decrement */
     Y--;
-    core->SetCoreReg(Rd, core->GetRWMem(Y));
+    core->SetCoreReg(Rd, core->GetRWMem(Y & core->dataAddressMask));
     core->SetCoreReg(28, Y & 0xff);
-    core->SetCoreReg(29, (Y >> 8) & 0xff);
+    if (core->dataAddressMask > 0xff) {
+        core->SetCoreReg(29, (Y >> 8) & 0xff);
+    }

     return core->flagTiny10 ? 3 : 2;
 }
@@ -903,10 +909,12 @@ int avr_op_LD_Y_incr::operator()() {
         avr_error( "Result of operation is undefined" );

     /* Perform post-increment */
-    core->SetCoreReg(Rd, core->GetRWMem(Y));
+    core->SetCoreReg(Rd, core->GetRWMem(Y & core->dataAddressMask));
     Y++;
     core->SetCoreReg(28, Y & 0xff);
-    core->SetCoreReg(29, (Y >> 8) & 0xff);
+    if (core->dataAddressMask > 0xff) { 
+        core->SetCoreReg(29, (Y >> 8) & 0xff);
+    }

     return core->flagXMega ? 1 : 2;
 }
@@ -922,10 +930,12 @@ int avr_op_LD_Z_incr::operator()() {
         avr_error( "Result of operation is undefined" );

     /* Perform post-increment */
-    core->SetCoreReg(Rd, core->GetRWMem(Z));
+    core->SetCoreReg(Rd, core->GetRWMem(Z & core->dataAddressMask));
     Z++;
     core->SetCoreReg(30, Z & 0xff);
-    core->SetCoreReg(31, (Z >> 8) & 0xff);
+    if (core->dataAddressMask > 0xff) {
+        core->SetCoreReg(31, (Z >> 8) & 0xff);
+    }

     return core->flagXMega ? 1 : 2;
 }
@@ -942,9 +952,11 @@ int avr_op_LD_Z_decr::operator()() {

     /* Perform pre-decrement */
     Z--;
-    core->SetCoreReg(Rd, core->GetRWMem(Z));
+    core->SetCoreReg(Rd, core->GetRWMem(Z & core->dataAddressMask));
     core->SetCoreReg(30, Z & 0xff);
-    core->SetCoreReg(31, (Z >> 8) & 0xff);
+    if (core->dataAddressMask > 0xff) {
+        core->SetCoreReg(31, (Z >> 8) & 0xff);
+    }

     return core->flagTiny10 ? 3 : 2;
 }
@@ -1522,7 +1534,7 @@ avr_op_STD_Y::avr_op_STD_Y(word opcode, AvrDevice *c):

 int avr_op_STD_Y::operator()() {
     /* Y is R29:R28 */
-    unsigned int Y = core->GetRegY();
+    unsigned int Y = core->dataAddressMask & core->GetRegY();

     core->SetRWMem(Y + K, core->GetCoreReg(R1));

@@ -1536,7 +1548,7 @@ avr_op_STD_Z::avr_op_STD_Z(word opcode, AvrDevice *c):

 int avr_op_STD_Z::operator()() {
     /* Z is R31:R30 */
-    int Z = core->GetRegZ();
+    int Z = core->dataAddressMask & core->GetRegZ();

     core->SetRWMem(Z + K, core->GetCoreReg(R1));

@@ -1565,7 +1577,7 @@ int avr_op_ST_X::operator()() {
     /* X is R27:R26 */
     word X = core->GetRegX();

-    core->SetRWMem(X, core->GetCoreReg(R1));
+    core->SetRWMem(X & core->dataAddressMask, core->GetCoreReg(R1));

     return (core->flagXMega || core->flagTiny10) ? 1 : 2;
 }
@@ -1583,8 +1595,10 @@ int avr_op_ST_X_decr::operator()() {
     /* Perform pre-decrement */
     X--;
     core->SetCoreReg(26, X & 0xff);
-    core->SetCoreReg(27, (X >> 8) & 0xff);
-    core->SetRWMem(X, core->GetCoreReg(R1));
+    if (core->dataAddressMask > 0xff) {
+        core->SetCoreReg(27, (X >> 8) & 0xff);
+    }
+    core->SetRWMem(X & core->dataAddressMask, core->GetCoreReg(R1));

     return 2;
 }
@@ -1599,12 +1613,14 @@ int avr_op_ST_X_incr::operator()() {
     if (R1 == 26 || R1 == 27)
         avr_error( "Result of operation is undefined" );

-    core->SetRWMem(X, core->GetCoreReg(R1));
+    core->SetRWMem(X & core->dataAddressMask, core->GetCoreReg(R1));

     /* Perform post-increment */
     X++;
     core->SetCoreReg(26, X & 0xff);
-    core->SetCoreReg(27, (X >> 8) & 0xff);
+    if (core->dataAddressMask > 0xff) {
+        core->SetCoreReg(27, (X >> 8) & 0xff);
+    }

     return (core->flagXMega || core->flagTiny10) ? 1 : 2;
 }
@@ -1622,8 +1638,10 @@ int avr_op_ST_Y_decr::operator()() {
     /* Perform pre-decrement */
     Y--;
     core->SetCoreReg(28, Y & 0xff);
-    core->SetCoreReg(29, (Y >> 8) & 0xff);
-    core->SetRWMem(Y, core->GetCoreReg(R1));
+    if (core->dataAddressMask > 0xff) {
+        core->SetCoreReg(29, (Y >> 8) & 0xff);
+    }
+    core->SetRWMem(Y & core->dataAddressMask, core->GetCoreReg(R1));

     return 2;
 }
@@ -1638,12 +1656,14 @@ int avr_op_ST_Y_incr::operator()() {
     if (R1 == 28 || R1 == 29)
         avr_error( "Result of operation is undefined" );

-    core->SetRWMem(Y, core->GetCoreReg(R1));
+    core->SetRWMem(Y & core->dataAddressMask, core->GetCoreReg(R1));

     /* Perform post-increment */
     Y++;
     core->SetCoreReg(28, Y & 0xff);
-    core->SetCoreReg(29, (Y >> 8) & 0xff);
+    if (core->dataAddressMask > 0xff) {
+        core->SetCoreReg(29, (Y >> 8) & 0xff);
+    }

     return (core->flagXMega || core->flagTiny10) ? 1 : 2;
 }
@@ -1661,8 +1681,10 @@ int avr_op_ST_Z_decr::operator()() {
     /* Perform pre-decrement */
     Z--;
     core->SetCoreReg(30, Z & 0xff);
-    core->SetCoreReg(31, (Z >> 8) & 0xff);
-    core->SetRWMem(Z, core->GetCoreReg(R1));
+    if (core->dataAddressMask > 0xff) {
+        core->SetCoreReg(31, (Z >> 8) & 0xff);
+    }
+    core->SetRWMem(Z & core->dataAddressMask, core->GetCoreReg(R1));

     return 2;
 }
@@ -1677,12 +1699,14 @@ int avr_op_ST_Z_incr::operator()() {
     if (R1 == 30 || R1 == 31)
         avr_error( "Result of operation is undefined" );

-    core->SetRWMem(Z, core->GetCoreReg(R1));
+    core->SetRWMem(Z & core->dataAddressMask, core->GetCoreReg(R1));

     /* Perform post-increment */
     Z++;
     core->SetCoreReg(30, Z & 0xff);
-    core->SetCoreReg(31, (Z >> 8) & 0xff);
+    if (core->dataAddressMask > 0xff) {
+        core->SetCoreReg(31, (Z >> 8) & 0xff);
+    }

     return (core->flagXMega || core->flagTiny10) ? 1 : 2;
 }