[Bug target/60991] New: [avr] Stack corruption when using 24-bit integers __int24 or __memx pointers in large stack frame
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60991
Bug ID: 60991
Summary: [avr] Stack corruption when using 24-bit integers
__int24 or __memx pointers in large stack frame
Product: gcc
Version: 4.8.1
Status: UNCONFIRMED
Severity: critical
Priority: P3
Component: target
Assignee: unassigned at gcc dot gnu.org
Reporter: johnst...@inn-soft.com
Both avr-gcc 4.8.1 and 4.7.2 corrupt the stack when using 24-bit integers in a
large stack frame (>= approximately 64 bytes) where the integer is stored at
the end of the stack frame such that it is normally out of reach of the "STD
Y+q" instruction. The below example demonstrates the problem. While the
example appears pedantic, it becomes a real problem when a program is
aggressively inlined through multiple layers of complicated functions that are
called only once, or when an array is allocated on the stack.
Note this problem exists with both __int24 data type and __memx pointers.
Steps I used to reproduce:
1. Create new AVR GCC C Executable project in Atmel Studio 6.1 or 6.2 beta.
6.2 beta uses avr-gcc 4.8.1 and 6.1 uses avr-gcc 4.7.2.
2. Target is ATxmega256A3U but I should think any target will work (but
untested). I use default compiler settings which gave command lines of:
"C:\Program Files (x86)\Atmel\Atmel Toolchain\AVR8
GCC\Native\3.4.1051\avr8-gnu-toolchain\bin\avr-gcc.exe" -x c -funsigned-char
-funsigned-bitfields -DDEBUG -O1 -ffunction-sections -fdata-sections
-fpack-struct -fshort-enums -mrelax -g2 -Wall -mmcu=atxmega256a3u -c -std=gnu99
-MD -MP -MF "CrashTest.d" -MT"CrashTest.d" -MT"CrashTest.o" -o "CrashTest.o"
".././CrashTest.c"
"C:\Program Files (x86)\Atmel\Atmel Toolchain\AVR8
GCC\Native\3.4.1051\avr8-gnu-toolchain\bin\avr-gcc.exe" -o CrashTest.elf
CrashTest.o -Wl,-Map="CrashTest.map" -Wl,--start-group -Wl,-lm
-Wl,--end-group -Wl,--gc-sections -mrelax -mmcu=atxmega256a3u
3. Use this code:
#include
__attribute__((__noinline__)) void Blowup(void) {
#define SZ 62
volatile char junk[SZ];
junk[0] = 5;
junk[SZ-1] = 6;
volatile __int24 staticConfig = 0;
} // BUG: Instruction pointer register will change to a bogus value when
returning.
int main(void)
{
Blowup();
// BUG: This loop will never be reached because Blowup() won't return
properly.
while(1) { wdt_reset(); }
}
4. The emitted assembly for Blowup function is problematic as follows, from
the LSS file:
volatile __int24 staticConfig = 0;
22a:22 96 adiwr28, 0x02; 2
22c:1d ae stdY+61, r1; 0x3d
22e:1e ae stdY+62, r1; 0x3e
230:1f ae stdY+63, r1; 0x3f
232:23 97 sbiwr28, 0x03; 3
AVR-GCC appears to hold the frame pointer in Y register pair: r28..r29. This
code corrupts that pointer, which is later adjusted again by the frame size and
stored in SPH/SPL, such that the subsequent "ret" instruction jumps to invalid
location.
The STD instruction is limited to a 6-bit immediate offset ("Y+q" syntax). The
"staticConfig" variable was stored in the frame normally beyond the reach of
this instruction, so AVR-GCC appears to try to use "STD Y+q" anyway by
temporary adding and then subtracting the appropriate offset to the frame
pointer. Except, that AVR-GCC appears to be incorrectly doing this - as we can
see the added value in "ADIW" of 0x02 is not the same as the subtracted value
in "SBIW" of 0x03. Examining the earlier assembly code seems to suggest that
the correct value for "SBIW" would be 0x02 also.
Because the "SBIW" instruction screwed up the "Y" register pair which is later
stored in SPH/SPL, the "RET" call jumps to bad memory. Also, I would imagine
subsequent variable accesses in the "Blowup" function to the stack would not
work properly.
I also suspect that reads from the "staticConfig" variable using "LDD Y+q"
instruction could have this same problem, but I did not test. "LDD Y+q" has
similar 6-bit limitation, so I suspect AVR-GCC would be using the same "ADIW" /
"SBIW" trick to work around that which failed here.
[Bug target/60991] [avr] Stack corruption when using 24-bit integers __int24 or __memx pointers in large stack frame
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60991
--- Comment #1 from johnst...@inn-soft.com ---
I don't really know GCC source code well enough to feel comfortable changing
and testing this myself, but I suspect the problem lies with this code?
gcc/config/avr/avr.c: avr_out_store_psi function: revision 209767:
3992 return avr_asm_len ("adiw r28,%o0-61" CR_TAB
3993 "std Y+61,%A1"CR_TAB
3994 "std Y+62,%B1"CR_TAB
3995 "std Y+63,%C1"CR_TAB
3996 "sbiw r28,%o0-60", op, plen,
-5);
Notice the top line has "%o0-61" and bottom line has "%o0-60", which must be
some kind of offset. I suspect this code was copied and pasted from the
out_movsi_mr_r function, while forgetting to update the last line here since it
is 24-bit integer and not the 32-bit integer it was copied from.
It appears that this code was introduced with the new __int24 support and
nothing changed since then.
[Bug pch/64502] New: Incorrect warning about empty translation units when using pre-compiled headers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64502
Bug ID: 64502
Summary: Incorrect warning about empty translation units when
using pre-compiled headers
Product: gcc
Version: 4.9.2
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: pch
Assignee: unassigned at gcc dot gnu.org
Reporter: johnst...@inn-soft.com
Created attachment 34380
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34380&action=edit
GCC intermediate output of MyProgram.c with GCH
When using a pre-compiled header that contains C code, GCC will complain anyway
that the translation unit is empty - even when it is obviously not. It appears
that the warning does not consider code that is inside the translation unit.
The test case below is rather contrived to create a minimal test case.
(Obviously, we aren't putting "int main" in the pre-compiled header!) However,
in the real world, we use GCC to compile the same source code for several
different platforms. The pre-compiled header always contains code (e.g. some
typedefs used by all platforms), but the C file itself may exclude all code via
"#if PLATFORM..." if it is not applicable to the current platform.
Steps to reproduce:
1. Create the following two files - Hello.h and MyProgram.c:
// Hello.h:
// NOTE: In the real world, we wouldn't be putting "int main" in the
// header, of course. Instead, there might be some typedefs here that
// aren't used, for example.
int main(void) {
return 0;
}
//
// MyProgram.c
#include "Hello.h"
// NOTE: In the real world, we would be excluding platform-specific code
// by using "#ifdef PLATFORM" around the entire body of code in this
// C file.
2. Now, compile it:
$ gcc -pedantic Hello.h -save-temps
$ gcc -pedantic MyProgram.c -o MyProgram -save-temps
MyProgram.c:1:0: warning: ISO C forbids an empty translation unit [-Wpedantic]
#include "Hello.h"
^
3. As illustrated above, GCC is wrongly complaining that the translation unit
is empty. It is, in fact, not empty - the code is in the pre-compiled
header...
4. Removing the pre-compiled header silences the warning:
$ rm Hello.h.gch
$ gcc -pedantic MyProgram.c -o MyProgram -save-temps
The problem is reproduced on both Cygwin 32-bit GCC 4.9.2, and on avr-gcc
distributed by Atmel, version 4.7.2.
Output of gcc -v for the Cygwin version:
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/i686-pc-cygwin/4.9.2/lto-wrapper.exe
Target: i686-pc-cygwin
Configured with:
/cygdrive/i/szsz/tmpp/gcc/gcc-4.9.2-1.i686/src/gcc-4.9.2/configure
--srcdir=/cygdrive/i/szsz/tmpp/gcc/gcc-4.9.2-1.i686/src/gcc-4.9.2 --prefix=/usr
--exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin
--libexecdir=/usr/libexec --datadir=/usr/share --localstatedir=/var
--sysconfdir=/etc --libdir=/usr/lib --datarootdir=/usr/share
--docdir=/usr/share/doc/gcc --htmldir=/usr/share/doc/gcc/html -C
--build=i686-pc-cygwin --host=i686-pc-cygwin --target=i686-pc-cygwin
--without-libiconv-prefix --without-libintl-prefix --libexecdir=/usr/lib
--enable-shared --enable-shared-libgcc --enable-static
--enable-version-specific-runtime-libs --enable-bootstrap --enable-__cxa_atexit
--with-dwarf2 --with-arch=i686 --with-tune=generic --disable-sjlj-exceptions
--enable-languages=ada,c,c++,fortran,java,lto,objc,obj-c++ --enable-graphite
--enable-threads=posix --enable-libatomic --enable-libgomp --disable-libitm
--enable-libquadmath --enable-libquadmath-support --enable-libssp
--enable-libada --enable-libjava --enable-libgcj-sublibs --disable-java-awt
--disable-symvers --with-ecj-jar=/usr/share/java/ecj.jar --with-gnu-ld
--with-gnu-as --with-cloog-include=/usr/include/cloog-isl
--without-libiconv-prefix --without-libintl-prefix --with-system-zlib
--enable-linker-build-id
Thread model: posix
gcc version 4.9.2 (GCC)
Attached are intermediate output files saved with "-save-temps" for cases both
with and without pre-compiled headers. My conclusion is that this warning only
considers the code in "MyProgram.i" and ignores what code might be included via
the proprietary "#pragma GCC pch_preprocess "Hello.h.gch"" that is apparently
an implementation detail of pre-compiled headers. Obviously, this doesn't
really relate to C99 grammar, which doesn't even consider pre-compiled headers.
One idea for fixing the problem might be a flag saved in the GCH file
indicating whether the file contained any real code, or whether it is empty.
The warning would only be raised if the C file AND the GCH file are both empty.
[Bug pch/64502] Incorrect warning about empty translation units when using pre-compiled headers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64502 --- Comment #1 from johnst...@inn-soft.com --- Created attachment 34381 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34381&action=edit GCC pre-compiled intermediate output of Hello.h
[Bug pch/64502] Incorrect warning about empty translation units when using pre-compiled headers
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64502 --- Comment #2 from johnst...@inn-soft.com --- Created attachment 34382 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34382&action=edit GCC intermediate output of MyProgram.c WITHOUT GCH
[Bug target/38549] [avr] eicall not properly set for > 128K program space
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38549 johnst...@inn-soft.com changed: What|Removed |Added CC||johnst...@inn-soft.com --- Comment #5 from johnst...@inn-soft.com 2010-11-02 20:25:05 UTC --- I can confirm this is indeed a problem. I am developing a bootloader for ATxmega128A1 (128 KB app flash + 8 KB bootloader = 136 KB flash total). My code: #define PROG_START 0x (*((void(*)(void))PROG_START))();//jump This emits the following: # Notice on reset, EIND register is written to a 1 as shown here. # I searched the entire emitted disassembly and found no other # reference to the I/O address for EIND. 000202e0 <__ctors_end>: ... 202ec:01 e0 ldir16, 0x01; 1 202ee:0c bf out0x3c, r16; 60 # Notice that Z is set to 0, as expected. However, EIND is not # set to 0 and so the processor attempts to do the jump to # the location specified by EIND == 1 and Z == 0, which isn't a valid # place to jump to. (*((void(*)(void))PROG_START))();//jump 20590:e0 e0 ldir30, 0x00; 0 20592:f0 e0 ldir31, 0x00; 0 20594:19 95 eicall Presumably this will come up much more frequently now that the ATxmega processors are available: all of these have so much flash that I would imagine this will be a frequent problem. I assume the problem happens with EIJMP which also uses EIND. I notice that eicall / eijmp are used in libgcc.s. I wouldn't be surprised if there are bugs there, too - but did not investigate further. My fix is simple; just set EIND = 0 before my jump. However it leaves little faith in my compiler for the application itself, since I don't know if it will work reliably on AVR with large flash space for all jumps and calls, etc.
