pgm_read_word can cause exception if return type doesn't match size of pgm_read type

[Makuna]

Basic Infos

Description

Problem description
If the type size that pgm_read_word() result does not match, an exception is thrown.
In the sample below, it demonstrates the issue; but if the result is stored in a uint16_t type; which matches the pgm_read_word(), then it works correctly.

Sketch

const uint32_t  pwmTable[] PROGMEM  = {42,43};
unsigned int i = 0;

void setup() {
 Serial.begin(115200);
 while (!Serial);
 Serial.println("Startup...");
}

void loop() {
  delay(5000);
  Serial.print(i);
  Serial.print(" = ");

  // uint16_t readvalue = pgm_read_word(pwmTable + i);  // this line won't cause exception
  uint32_t readvalue = pgm_read_word(pwmTable + i); // exception here
  i = (i+1) % (sizeof(pwmTable) / sizeof(pwmTable[0]));

  Serial.println(readvalue);
  Serial.flush();
}

Debug Messages

Exception (3):
epc1=0x40201c3d epc2=0x00000000 epc3=0x00000000 excvaddr=0x4022dfc4 depc=0x00000000

ctx: cont 
sp: 3ffef850 end: 3ffefa20 offset: 01a0

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[igrr]

Maybe we can replace pgm_read_word with an ordinary function instead of a macro? Then the compiler will be able to perform an implicit conversion.
Also, did you mean const uint32_t pwmTable or const uint16_t pwmTable? I.e. are you pointing out a mismatch between types of pwmTable and pgm_read_, or mismatch between types of pgm_read_ and the variable which captures the result?

With const uint32_t pwmTable, neither uint32_t readvalue nor uint16_t readvalue worked for me.
With const uint16_t pwmTable, both worked.

[Makuna]

I am running the latest release 2.2.0.

I was pointing out that changing the result type (the line that is commented out), caused the issue. I just confirmed it is still a problem with my build. So are you then running a newer compiler than the released?

This is a corner case. It's unusual to store the data in a "raw" format (in this case uint32_t) and access it with 8/16 bits at a time but it is not restricted to do so. I believe its a bug in the generated code.

The work around is to convert these two macros into calling functions. Note that you can't just convert them to functions as the argument was never typed with macros and will cause compile issues for user sketches.

This worked for me...

pgmspace.h

uint8_t _pgm_read_byte(PGM_P addr);
uint16_t _pgm_read_word(PGM_P addr);

#define pgm_read_byte(addr) _pgm_read_byte((PGM_P)(addr))
#define pgm_read_word(addr) _pgm_read_word((PGM_P)(addr))   

pgmspace.c

uint8_t _pgm_read_byte(PGM_P addr) {
    ptrdiff_t offset = ((uint32_t)addr & 0x00000003); /* byte aligned mask */
    const uint32_t* addr32 = (const uint32_t*)((uint32_t)addr & 0xfffffffC);
    uint8_t result = ((*addr32) >> (offset * 8));
    return result;
}

uint16_t _pgm_read_word(PGM_P addr) {
    ptrdiff_t offset = ((uint32_t)addr & 0x00000002);   /* word aligned mask */
    const uint32_t* addr32 = (const uint32_t*)((uint32_t)addr & 0xfffffffC);
    uint16_t result = ((*addr32) >> (offset * 8));
    return result;
}

[devyte]

@igrr this is marked for 2.3.0 milestone. What is the status? I seem to remember some changes to pgmspace,.h, but I have no idea if they're relevant, and the current code looks very different from what is proposed here.

[igrr]

This issue should be tested again with 2.4.0-rc2. I think it was resolved when pgm_read_bytes was implemented as an inline function.

[igrr]

Verified that this is fixed in git version.