EEPROM Example2_AllFunctions stalls on both the Artemis Thing Plus and ATP - possible memory alignment problem?

[PaulZC]

I am loving the Artemis. It is my new favorite thing!

Subject of the issue

When running the EEPROM Example2_AllFunctions on the Artemis Thing Plus and ATP it stalls (crashes?) part way through the double (64) sequential test on both boards

Your workbench

Artemis Thing Plus and ATP
Windows Arduino IDE 1.8.10
Apollo3 Boards 1.0.15
Connected and powered via USB-C

Steps to reproduce

Run the example.

Actual behaviour

Example stalls at the EEPROM.get for the double test

32 bit tests
Size of int: 4
Location 349 should be -245000: -245000
Location 353 should be 400123: 400123
Location 298 should be -341002: -341002
Location 302 should be 241544: 241544
Size of float: 4
Location 786 should be -7.350000: -7.350000
Location 790 should be 5.220000: 5.220000

64 bit tests
Size of double: 8

Expected behaviour

This:

32 bit tests
Size of int: 4
Location 349 should be -245000: -245000
Location 353 should be 400123: 400123
Location 298 should be -341002: -341002
Location 302 should be 241544: 241544
Size of float: 4
Location 786 should be -7.350000: -7.350000
Location 790 should be 5.220000: 5.220000

64 bit tests
Size of double: 8
Location 727 should be -290.348572: -290.348572
Location 735 should be 384.957336: 384.957336

Flash Contents:
0xFFFFFFFF 0xFFFFFFFF 0xFFFFFFFF 0xFFFFFFFF 0xFFFFFFFF 0xFFFFFFFF 0xFFFFFFFF 0xFFFFFFFF

Cause and Fix

It looks like it is a memory alignment problem caused by the union of the double and the two uint32_t's

Adding attribute packed to the union seems to fix it:

void ap3_EEPROM::get(uint16_t eepromLocation, double &dataToGet)
{
  union __attribute__((packed)) { // PaulZC added __attribute__((packed))
    double lf;
    uint32_t b[2];
  } temp;
  temp.b[1] = *(uint32_t *)(AP3_FLASH_EEPROM_START + eepromLocation);     //LSB;
  temp.b[0] = *(uint32_t *)(AP3_FLASH_EEPROM_START + eepromLocation + 4); //MSB;
  dataToGet = temp.lf;
}

But here's the weird thing. This only affects the get. The update/put seems immune and yet uses the same union. It's got me confused!

Enjoy!
Paul

[PaulZC]

I'm not sure if this helps?
From section 3.3.5 of the Cortex-M4 User Guide:

An aligned access is an operation where a word-aligned address is used for a word, dual word,
or multiple word access, or where a halfword-aligned address is used for a halfword access. Byte
accesses are always aligned.
The Cortex-M4 processor supports unaligned access only for the following instructions:
• LDR, LDRT
• LDRH, LDRHT
• LDRSH, LDRSHT
• STR, STRT
• STRH, STRHT
All other load and store instructions generate a UsageFault exception if they perform an
unaligned access, and therefore their accesses must be address aligned.

[nseidle]

I probably wrote it wrong :)

Thanks for digging into the problem. Please do checkout the new EEPROM rewrite branch: #75

It's a lot more stable and better written (leverages the Arduino original) than this custom attempt I did originally.

[PaulZC]

Agreed. The rewrite branch #75 fixes it.

Wow. The code in EEPROM.cpp is HEAVILY object-oriented. It's going to take me a while to get my head around that lot. (Yes, I know, I don't really need to. But even so...)

Looking again at the quote from the Cortex-M4 User Guide: "Byte accesses are always aligned." So, basing everything on uint8_t's is a very sensible approach!

[PaulZC]

Closing this - the merge fixed it good and proper!