Question 38538 in Ubuntu Question posted in
The official Ubuntu documentation can be found here.

const static char assignment with PROGMEM causes error in avr-g++ 5.4.0 – Ubuntu

I have a piece of code that was shipped as part of the XLR8 development platform that formerly used a bundled version (4.8.1) of the avr-gcc/g++ compiler. I tried to use the latest version of avr-g++ included with by my linux distribution (Ubuntu 22.04) which is 5.4.0

When running that compiler, I am getting the following error that seems to make sense to me. Here is the error and the chunk of related code below. In the bundled version of avr-g++ that was provided with the XLR8 board, this was not an error. I’m not sure why because it appears that the code below is attempting to place 16 bit words into an array of chars.

A couple questions,

  1. Can anyone explain the reason this worked with previous avr-gcc releases and was not considered an error?
  2. Because of the use of sizeof in the snippet below to control the for loop terminal count, I think the 16 bit size was supposed to be the data type per element of the array. Is that accurate?
  3. If the size of the element was 16 bits, then is the correct fix simply to make that array of type unsigned int rather than char?
/home/rich/.arduino15/packages/alorium/hardware/avr/2.3.0/libraries/XLR8Info/src/XLR8Info.cpp:157:12: error: narrowing conversion of ‘51343u’ from ‘unsigned int’ to ‘char’ inside { } [-Wnarrowing]
      0x38BF};

bool     XLR8Info::hasICSPVccGndSwap(void)  {      
// List of chip IDs from boards that have Vcc and Gnd swapped on the ICSP header
// Chip ID of affected parts are 0x????6E00. Store the ???? part
const static char cidTable[] PROGMEM =
  {0xC88F,  0x08B7,  0xA877,  0xF437,
   0x94BF,  0x88D8,  0xB437,  0x94D7,  0x38BF,  0x145F,  0x288F,  0x28CF,
   0x543F,  0x0837,  0xA8B7,  0x748F,  0x8477,  0xACAF,  0x14A4,  0x0C50,
   0x084F,  0x0810,  0x0CC0,  0x540F,  0x1897,  0x48BF,  0x285F,  0x8C77,
   0xE877,  0xE49F,  0x2837,  0xA82F,  0x043F,  0x88BF,  0xF48F,  0x88F7,
   0x1410,  0xCC8F,  0xA84F,  0xB808,  0x8437,  0xF4C0,  0xD48F,  0x5478,
   0x080F,  0x54D7,  0x1490,  0x88AF,  0x2877,  0xA8CF,  0xB83F,  0x1860,
   0x38BF};
uint32_t chipId = getChipId();
for (int i=0;i< sizeof(cidTable)/sizeof(cidTable[0]);i++) {
   uint32_t cidtoTest = (cidTable[i] << 16) + 0x6E00;
   if (chipId == cidtoTest) {return true;}
} 
return false;
}
Tags:

2

Answers


  1. 0

    As you already pointed out, the array type char definitely looks wrong. My guess is, that this is bug that may have never surfaced in the field. hasICSPVccGndSwap should always return false, so maybe they never used a chip type that had its pins swapped and got away with it.

    Can anyone explain the reason this worked with previous avr-gcc releases and was not considered an error?

    Yes, the error/warning behavior was changed with version 5.

    As of G++ 5, the behavior is the following: When a later standard is in effect, e.g. when using -std=c++11, narrowing conversions are diagnosed by default, as required by the standard. A narrowing conversion from a constant produces an error, and a narrowing conversion from a non-constant produces a warning, but -Wno-narrowing suppresses the diagnostic.

    I would’ve expected v4.8.1 to throw a warning at least, but maybe that has been ignored.

    Because of the use of sizeof in the snippet below to control the for loop terminal count, I think the 16 bit size was supposed to be the data type per element of the array. Is that accurate?

    Yes, this further supports that the array type should’ve been uint16 in the first place.

    If the size of the element was 16 bits, then is the correct fix simply to make that array of type int rather than char?

    Yes.

    Login or Signup to reply.
  2. 0

    Several bugs here. I am not familiar with that software, but there are at least the following, obvious bugs:

    1. The element type of cidTable should be a 16-bit, integral type like uint16_t. This follows from the code and also from the comments.

    2. You cannot read from PROGMEM like that. The code will read from RAM, where it uses a flash address to access RAM. Currently, there is only one way to read from flash in avr-g++, which is inline assembly. To make life easier, you can use macros from avr/pgmspace.h like pgm_read_word.

    3. cidTable[i] << 16 is Undefined Behaviour because a 16-bit type is shifted left by 16 bits. The type is an 8-bit type, then it is promoted to int which has 16 bits only. Same problem if the element type is already 16 bits wide.

    Taking it all together, in order to make sense in avr-g++, the code would be something like:

    #include <avr/pgmspace.h>

bool XLR8Info::hasICSPVccGndSwap()
{
    // List of chip IDs from boards that have Vcc and Gnd swapped on
    // the ICSP header.  Chip ID of affected parts are 0x????6E00.
    // Store the ???? part.
    static const uint16_t cidTable[] PROGMEM =
    {
        0xC88F,  0x08B7,  0xA877,  0xF437, ...
    };

    uint32_t chipId = getChipId();
    for (size_t i = 0; i < sizeof(cidTable) / sizeof (*cidTable); ++i)
    {
        uint16_t cid = pgm_read_word (&cidTable[i]);
        uint32_t cidtoTest = ((uint32_t) cid << 16) + 0x6E00;
        if (chipId == cidtoTest)
            return true;
    } 
    return false;
}
    Login or Signup to reply.
Please signup or login to give your own answer.

Back To Top
Search