DeBesten / opentx

Automatically exported from code.google.com/p/opentx
0 stars 0 forks source link

"Undefined behaviour" compiler warnings #87

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
I'm not sure if this is intentional or not but I just wanted to note it just in 
case it is an issue.  When using gcc 4.8.1 I get several warnings of undefined 
behaviour in a few places.  They all appear to be for-loops that are writing 
outside the bounds of an array (however, still within the structure as far as I 
can tell).

This is my gcc:
Target: arm-none-eabi
Configured with: ../configure --target=arm-none-eabi --prefix=/usr 
--libexecdir=/usr/lib --with-pkgversion='Arch User Repository' 
--with-bugurl=https://aur.archlinux.org/packages/arm-none-eabi-gcc 
--enable-multilib --enable-interwork --enable-languages=c,c++ --with-newlib 
--with-gnu-as --with-gnu-ld --disable-nls --disable-shared --disable-threads 
--with-headers=newlib/libc/include --disable-libssp --disable-libstdcxx-pch 
--disable-libmudflap --disable-libgomp --with-system-zlib 
--disable-newlib-supplied-syscalls
Thread model: single
gcc version 4.8.1 (Arch User Repository) 

I compile like so:
make PCB=TARANIS EXT=FRSKY SDCARD=YES PPM_CENTER_ADJUSTABLE=YES 
FLIGHT_MODES=YES AUTOSWITCH=YES PPM_LIMITS_SYMETRICAL=YES AUTOSOURCE=YES 
DBLKEYS=YES HELI=YES TEMPLATES=YES PPM_UNIT=US GVARS=YES FAI=CHOICHE 
TRANSLATIONS=EN LUA=YES

opentx.cpp: In function 'uint16_t evalChkSum()':
opentx.cpp:465:34: warning: iteration 8u invokes undefined behavior 
[-Waggressive-loop-optimizations]
     sum += g_eeGeneral.calibMid[i];
                                  ^
opentx.cpp:464:3: note: containing loop
   for (int i=0; i<NUM_STICKS+NUM_POTS+5; i++)
   ^
opentx.cpp: In function 'void menuCommonCalib(uint8_t)':
opentx.cpp:465:34: warning: iteration 8u invokes undefined behavior 
[-Waggressive-loop-optimizations]
     sum += g_eeGeneral.calibMid[i];
                                  ^
opentx.cpp:464:3: note: containing loop
   for (int i=0; i<NUM_STICKS+NUM_POTS+5; i++)
   ^
opentx.cpp: In function 'void opentxStart()':
opentx.cpp:465:34: warning: iteration 8u invokes undefined behavior 
[-Waggressive-loop-optimizations]
     sum += g_eeGeneral.calibMid[i];
                                  ^
opentx.cpp:464:3: note: containing loop
   for (int i=0; i<NUM_STICKS+NUM_POTS+5; i++)
   ^

Original issue reported on code.google.com by coderi...@gmail.com on 31 Jul 2013 at 2:17

GoogleCodeExporter commented 8 years ago
which branch are you compiling ??
Your command line works perfectly on trunk branch with yagarto or similar arm 
gcc toolchains

If you are compiling on other branches they are not expected to be in ready 
state.
in any case the issue is invalid.

Original comment by romolo.m...@gmail.com on 31 Jul 2013 at 2:22

GoogleCodeExporter commented 8 years ago
It has been done this way since er9x to save flash. Sure it lacks a cast on gcc 
4.8.1 which is not the compiler we officially use.

Would you tell-me if it remains a warning if you change these lines with 
((int16_t *)g_eeGeneral.calibMid)[i]

Original comment by bson...@gmail.com on 31 Jul 2013 at 2:29

GoogleCodeExporter commented 8 years ago
I tried trunk and the lua branch, both get the warning.  It has actually been 
there for a long time so I imagine the warning shows up for almost any version 
of the code.  It is possible that my gcc and/or the gcc libraries/headers are 
newer or compiled with different options than yagarto (is that a Windows 
toolchain?).  I'm compiling on Linux.

In any case the warning looks legitimate. For example if we look at 
g_eeGeneral.calibMid it is defined as "calibMid[NUM_STICKS+NUM_POTS]" but the 
for-loop is looping though "NUM_STICKS+NUM_POTS+5" entries (ie. larger than the 
array).  This may be intentional but it looks odd.

Original comment by coderi...@gmail.com on 31 Jul 2013 at 2:32

GoogleCodeExporter commented 8 years ago
The warning is still there with ((int16_t *)g_eeGeneral.calibMid)[i].  Smart 
compiler.  :)

Original comment by coderi...@gmail.com on 31 Jul 2013 at 2:35

GoogleCodeExporter commented 8 years ago
If it's intentional (and I see why now) then it's probably not a problem.  The 
only time it would be a problem is on a platform where the structure packing or 
alignment was different than expected.

Original comment by coderi...@gmail.com on 31 Jul 2013 at 2:38

GoogleCodeExporter commented 8 years ago
Nope. Structure packing cannot be different than expected. Everything would be 
broken.

Original comment by bson...@gmail.com on 31 Jul 2013 at 2:41

GoogleCodeExporter commented 8 years ago
Impossible to reproduce here, I don't use this compiler. The array is 
3*(NUM_STICKS+NUM_POTS), we only sum the first 13 values (on Taranis) to have 
the checksum. An intermediate pointer to squeeze the compiler will be ok, 
provided it will be optimized away on 64A.

Original comment by bson...@gmail.com on 31 Jul 2013 at 2:42

GoogleCodeExporter commented 8 years ago
uint16_t evalChkSum()
{
  uint16_t sum = 0;
  const int16_t *calibValues = &g_eeGeneral.calibMid[0];
  for (int i=0; i<NUM_STICKS+NUM_POTS+5; i++)
    sum += calibValues[i];
  return sum;
}

Original comment by bson...@gmail.com on 31 Jul 2013 at 2:47

GoogleCodeExporter commented 8 years ago
That removes the warning.  Also saves 8 bytes of flash.

Original comment by coderi...@gmail.com on 31 Jul 2013 at 2:53