C compiler bug or program error?
Asked Answered
M

3

8

I am using the IAR C compiler to build an application for an embedded micro (specifically a Renesas uPD78F0537). In this application I am using two nested for loops to initialize some data, as shown in the following MCVE:

#include <stdio.h>

#define NUM_OF_OBJS 254
#define MAX_OBJ_SIZE 4

unsigned char objs[NUM_OF_OBJS][MAX_OBJ_SIZE];
unsigned char srcData[NUM_OF_OBJS][MAX_OBJ_SIZE];

void main(void)
{
    srcData[161][2] = 10;

    int x, y;
    for (x = 0; x < NUM_OF_OBJS; x++)
    {
        for (y = 0; y < MAX_OBJ_SIZE; y++)
        {
            objs[x][y] = srcData[x][y];
        }
    }

    printf("%d\n", (int) objs[161][2]);
}

The output value is 0, not 10.

The compiler is generating the following code for the for loop:

     13              int x, y;
     14              for (x = 0; x < NUM_OF_OBJS; x++)
   \   0006   14....         MOVW      DE,#objs
   \   0009   16....         MOVW      HL,#srcData
     15              {
     16                  for (y = 0; y < MAX_OBJ_SIZE; y++)
   \   000C   A0F8           MOV       X,#248
     17                  {
     18                      objs[x][y] = srcData[x][y];
   \                     ??main_0:
   \   000E   87             MOV       A,[HL]
   \   000F   95             MOV       [DE],A
     19                  }
   \   0010   86             INCW      HL
   \   0011   84             INCW      DE
   \   0012   50             DEC       X
   \   0013   BDF9           BNZ       ??main_0
     20              }  

The above does not work: The compiler is apparently precalculating NUM_OF_OBJS x MAX_OBJ_SIZE = 1016 (0x3f8). This value is used as a counter, however it is being truncated to 8 bits (0xf8 == 248) and stored in 8-bit register 'X'. As a result, only the first 248 bytes of data are initialised, instead of the full 1016 bytes.

I can work around this, however my question is: Is this a compiler bug? Or am I overlooking something?


Update

  • This microcontroller has 7KB of RAM, and sizeof(int) == 2
  • Copying the data over using pointers (e.g. something along the lines of while (len-- > 0) *dst++ = *src++;) works fine.
Monosymmetric answered 1/9, 2016 at 9:40 Comment(10)
Show the initialization of such arraysSoloman
that looks like Z80 assemblyTrella
Does the system have more than 256 bytes of RAMTrella
To establish a compiler bug, make a MCVE. It's possible that this is an optimization side-effect of some other bug.Trella
Perhaps it has a limitation e.g. not allowed static objects greater than 256bTrella
@Trella There you go. I have added a MCVE.Monosymmetric
You don't show where srcData is initialized. If it isn't initialized explicitly, then what is the point of this code?Resoluble
Contact IAR. They provide support for their customers, and if this code snippet reproduces the error, it should be pretty easy to send to them for them to determine if they have a bug.Moten
@Moten Apparently I cannot file a bug report or a tech support request withtout a license number, and I am using an evaluation version of IAR C, so I guess I will need to look elsewhere.Monosymmetric
Note: don't use unnecessary casts! They keep the compilerr from reporting problems if you change something lateron. There is no need to cast char to int parameter.Leslielesly
M
1

I am pretty much convinced that this is a compiler bug based on the following:

  • Copying the data over using pointers (e.g. something along the lines of while (len-- > 0) *dst++ = *src++;) works fine. Thus this does not look like a problem of RAM size, pointer size, etc.

  • More relevant perhaps: If I just replace one of the two constants (NUM_OF_OBJS or MAX_OBJ_SIZE) with a static variable (thus preventing the compiler from precalculating the total count) it works fine.

Unfortunately I contacted IAR (providing a link to this SO question) and this is their answer:

Sorry to read that you don't have a license/support-agreement (SUA). An examination such as needed for this case can take time, and we (post-sales-support) prioritize to put time and effort to users with valid SUA.

Along with some generic comments which are not particularly useful.

So I guess I'll just assume this is a bug and work around it in my code (there are many possible workarounds, including the two described above).

Monosymmetric answered 7/9, 2016 at 11:43 Comment(2)
Another workaround: memcpy(objs, srcData, sizeof objs);. As a bonus, if the compiler is decent, this might produce the most compact code.Georgiannegeorgic
Yes, that's one option tooMonosymmetric
O
0

Although technically one may assume that those arrays are in .bss and zeroed, compilers are now issuing warnings when you actually make that assumption and read something from bss before writing it. Being global an interrupt, etc could come in and modify those arrays, so is it really a safe assumption (by a compiler) that they are zeros. Gcc does not make this assumption it copies the whole array over. Clang/llvm as well copies the whole array, does not take a shortcut. A shortcut would have looked like writing the one value to both arrays and printing that value, instead of copying the whole array.

Interesting even if the arrays were declared static, or made local, gcc is not able to figure out the shortcut, which is strange for gcc, which isnt the best but usually figures out much more complicated dead code loops.

What is the size of int defined as for this compiler target, maybe int is 8 bits in size and the compiler performed the action you asked. If not then perhaps yes this is a compiler bug. Try making the 254 a little bit smaller does it change the 0xF8 by a proportional amount, does it always appear as if it is carving off the lower 8 bits or is this some magic number that 0xF8 has some relationship to 1016 or 254 or 161, etc.

Ornate answered 1/9, 2016 at 13:58 Comment(8)
0xf8 = 1016 & 0xffMonosymmetric
exactly my point, and if you change 1016 to 1020 do you get 0xFC or something else. Just because 0xF8 is the lower bits of 1016 doesnt mean that is why the compiler generated them, until you prove that. Something you would need to do when filing a report to the compiler folks. likewise does 1012 give 0xF4. what if instead of changing the 254 you change the 4 to be an 8 or a 6 or a 5 or 3, etc...Ornate
my point is I dont see that you demonstrated that the 0xF8 comes from 0x3F8, you just assume that is the case.Ornate
If I #define NUM_OF_OBJS to 253, the 0xf8 in the generated code changes to 0xf4.Monosymmetric
so either int is 8 bits or the compiler is clipping this for some reason and you need to contact them.Ornate
if you print sizeof(int) what do you get? or if the printf was for stackoverflow, then init one item in the array with sizeof(int)Ornate
sizeof(int) is 2.Monosymmetric
contact the IAR folks.Ornate
R
-1

Given that this is the full code, the compiler isn't obliged to do anything, since the code is nonsense. The compiler is free to optimize away the whole loop since it does nothing.

A fully standard compliant compiler must initialize both objs and srcData to all-zeroes, since they have static storage duration.

Therefore the nested loop does nothing but shovelling zeroes from one array to another. If the compiler notices that this loop is pointless, it is free to remove the loop entirely.

Of course it doesn't make much sense to reduce the number of iterations in the loop as a way of optimization, so one may wonder what weird decisions the optimizer took to come up with that machine code. Odd and dumb as it may seem, it is fully standard compliant.

You could declare the loop iterators as volatile to enforce side-effects. In that case the loop has to be executed even though it is pointless, since reading/writing to a volatile variable is a side-effect, which the compiler isn't allowed to optimize away.

Please keep in mind that embedded systems compilers often have a non-standard, "minimal start-up" option where they skip the initialization of static storage duration variables to achieve faster system boot-up. If such a non-standard option is enabled, the variables will contain garbage values instead of zeroes.

Resoluble answered 1/9, 2016 at 11:31 Comment(7)
The reported problem was detected in a real application where the objs and srcData arrays contain real, useful information which is filled in with data received from a serial link. I trimmed down the code to this minimal example for which the compiler generates exactly the same code. So this is not due to the fact that objs/srcData are uninitialised. That's not the case in the full application.Monosymmetric
@Monosymmetric Then you didn't post a MCVE, because what you mention now in this comment is something entirely different from your question...Resoluble
I did post a MCVE. It compiles and reproduces the problematic compiler behaviour -- the compiler generates exactly the same code as in the original application. The fact that the example code itself is useless doesn't change anything.Monosymmetric
@Monosymmetric Except for this specific case, the compiler is standard-compliant. But in your real case, there might be some bug present either in your application or in the compiler. For example you could have some bug tricking the compiler that the buffers aren't modified. Like the classic bug of forgetting to declare the buffer volatile and then share it with an ISR.Resoluble
I have modified the MCVE. Have a look at the updated question.Monosymmetric
@Monosymmetric Seems like a compiler bug then... What happens if you replace the loops with memcpy?Resoluble
If I just copy NUM_OF_OBJS x MAX_OBJ_SIZE bytes using pointers (think while (nb-- > 0) *dst++ = *src++;) it works perfectly.Monosymmetric

© 2022 - 2024 — McMap. All rights reserved.