Stack checking functionality isn't properly initialized
Original Reporter info from Mantis: Marcin Wiazowski
-
Reporter name:
Original Reporter info from Mantis: Marcin Wiazowski
- Reporter name:
Description:
There are some problems in the way, how the code for embedded targets initializes stack checking functionality. The stack checking feature can be enabled by using -Ct compiler command line parameter. Also fpc.cfg turns stack checking on, when -dDEBUG command line parameter is used.
When stack checking is used, compiler inserts a call to the "fpc_stackcheck" internal function at the beginning of each procedure/function code. "fpc_stackcheck" needs a "StackBottom" global variable (defined in "fpcsrc\rtl\inc\systemh.inc") to be set properly. The problem is that - due to three independent reasons - this variable is not set properly. Below I describe these problems and finally a patch, that works perfectly for me.
PROBLEM 1:
==========
Apart from very few exceptions, twenty seven of "system.pp"/"system.pas" files in Free Pascal (i.e. for all platforms) look similar to the code below; please note, that the only purpose of FPC_HAS_FEATURE_STACKCHECK is to enable "CheckInitialStkLen" forward declaration in "system.inc", if "CheckInitialStkLen" call is needed inside the included "thread.inc":
--------------------------------
unit System;
...
implementation
...
{$I system.inc} // Inside system.inc we have:
//
// If FPC_HAS_FEATURE_STACKCHECK is defined, thread.inc -
// which is included below - uses "CheckInitialStkLen"
// function, so a forward declaration for "CheckInitialStkLen"
// must be defined here:
//
{$ifdef FPC_HAS_FEATURE_STACKCHECK}
function CheckInitialStkLen(stklen : SizeUInt) : SizeUInt; forward;
{$endif FPC_HAS_FEATURE_STACKCHECK}
...
{$i thread.inc} // Inside thread.inc we have:
//
// If FPC_HAS_FEATURE_STACKCHECK is defined,
// "CheckInitialStkLen" function is used:
//
{$ifdef FPC_HAS_FEATURE_STACKCHECK}
StackLength:=CheckInitialStkLen(stkLen);
StackBottom:=Sptr - StackLength;
{$endif FPC_HAS_FEATURE_STACKCHECK}
...
//
// Now "CheckInitialStkLen" function is implemented:
//
function CheckInitialStkLen(stklen : SizeUInt) : SizeUInt;
begin
...
end;
begin
...
///
// System unit initialization - "CheckInitialStkLen" function is
// used to initialize some variables, that are required for proper
// stack overflow checking by a "fpc_stackcheck" function
//
StackLength := CheckInitialStkLen(initialStkLen);
StackBottom := initialstkptr - StackLength;
...
end.
--------------------------------
Important: "CheckInitialStkLen" function is defined and called unconditionally in the System unit's initialization section - so even if FPC_HAS_FEATURE_STACKCHECK is not defined, and the forward declaration for "CheckInitialStkLen" is not declared, the "CheckInitialStkLen" function is always implemented and called during the System unit's initialization - so "StackLength" and "StackBottom" are initialized.
But "fpcsrc\rtl\embedded\system.pp" unit behaves differently:
--------------------------------
unit System;
...
implementation
...
{$I system.inc}
...
{$ifdef FPC_HAS_FEATURE_STACKCHECK}
function CheckInitialStkLen(stklen : SizeUInt) : SizeUInt;inline;
begin
result := stklen;
end;
var
initialstkptr : Pointer; // external name '__stkptr';
{$endif FPC_HAS_FEATURE_STACKCHECK}
begin
...
{$ifdef FPC_HAS_FEATURE_STACKCHECK}
StackLength := CheckInitialStkLen(initialStkLen);
StackBottom := initialstkptr - StackLength;
{$endif FPC_HAS_FEATURE_STACKCHECK}
...
end.
--------------------------------
If FPC_HAS_FEATURE_STACKCHECK is not defined (it's not defined by default), "StackLength" and "StackBottom" variables are not initialized. So "{ifdef FPC_HAS_FEATURE_STACKCHECK}" and "{
endif FPC_HAS_FEATURE_STACKCHECK}" statements (but not the code between them) should be removed, similarly to System units for other platforms.
PROBLEM 2:
==========
As can be seen above, the "initialstkptr" variable, used for "StackBottom" initialization, is declared as:
var
initialstkptr : Pointer; // external name '__stkptr';
Reference to external symbol '__stkptr' has been commented out, so "initialstkptr" is not initialized and "StackBottom" gets a wrong value. But simple uncommenting is not a solution, because embedded targets are specific and '__stkptr' symbol is not defined for them. But a '_stack_top' symbol is defined by the linker instead, so "initialstkptr" can be declared as follows:
var
_stack_top : record end; external name '_stack_top';
initialstkptr : Pointer = @_stack_top;
Since '_stack_top' symbol is defined by the linker (not by the compiler, as most of symbols), it must be accessed by its address, not by its value; the linker assigns absolute addresses for symbols instead of values for them, so an "@" operator must be used to read the address of the '_stack_top' symbol, not its value.
PROBLEM 3:
==========
Also "initialstklen" variable must be properly initialized for proper "StackLength" and "StackBottom" initialization. It is declared in "fpcsrc\rtl\inc\system.inc" as:
var
initialstklen : SizeUint;external name '__stklen';
The '__stklen' global symbol is created, when the compiler generates an assembler file; the generated assembler file also assigns a value to '__stklen'.
A value for '_stklen' is generated as follows: the compiler has a built-in set of different parameters for each compilation target. These parameters are defined in compiler sources, in "fpcsrc\compiler\systems\i*.pas" files, as "tsysteminfo" records. Each "tsysteminfo" record, among others, defines a "stacksize" value; "stacksize" values may differ between compilation targets, but the same target will always get the same "stacksize" value. The compiler ("fpcsrc\compiler\ngenutil.pas") generates a '__stklen' global symbol in the generated assembler file and assigns the predefined "stacksize" value to it. The linker writes this value into the header of the generated executable file (PE, ELF). The "stacksize" value in the header is a request for the operating system when launching the executable, so operating system allocates the requested amount of stack or fails to launch the executable and returns an error. So if the application has been launched, it may blindly assume, that the stack of requested size has been allocated.
But embedded targets are different. There is no operating system to allocate the requested stack amount or to fail and return an error. There are no PE or ELF files to be launched, but binary files stored in FLASH memory instead. So the binary code itself must be responsible for proper stack handling. But the FPC assigns a predefined, constant value to the '__stklen' global symbol when building a binary file and this '__stklen' value is trustingly used by the compiled program code; as mentioned above, "initialstklen" (= '__stklen') variable is defined in "fpcsrc\rtl\inc\system.inc" - and "fpcsrc\rtl\embedded\system.pp" uses this value without any verification. What's more, every programmer may also access the '__stklen' global symbol independently, so proper value of the '__stklen' is vital to the proper application behavior.
Due to FPC internals, the RAM structure for embedded targets is organized as follows (from lower to higher addresses): initialized data area (initialization is performed by the program code, by copying from FLASH), uninitialized data area and unused area. This unused area becomes a stack area, so the stack size must be determined as "address of first byte after RAM end" minus "address of first byte after data end".
The proper stack size for embedded targets is not known at the time when the compiler itself is compiled, so "stacksize" value from the "tsysteminfo" record in "fpcsrc\compiler\systems\i_embed.pas" can't be used. Next, the compiler generates an assembly file and the proper stack size is still not known, because stack location is not determined yet. Next, the linker calculates (by using a script created by FPC) some global symbols like '_stack_top' or '_end', so the stack location and size can be now determined. But the linker, in opposite to the assembler, is not able to assign a value to already defined '__stklen' symbol - it is only able to overwrite an address of that symbol (take note of "@" operator when accessing "_stack_top" in description of PROBLEM 2). So the only chance to assign a proper value to '__stklen' global variable is during the compiled program's runtime - in the System unit's initialization code. Since '__stklen' global variable is represented by "initialstklen" variable in the System unit, this may look like:
var
_stack_bottom : record end; external name '_end';
_stack_top : record end; external name '_stack_top';
begin
...
initialstklen := @_stack_top - @_stack_bottom;
...
end.
SUMMARY:
========
To solve all the problems described above, the following changes should be made in "fpcsrc\rtl\embedded\system.pp":
--------------------------------
{$ifdef FPC_HAS_FEATURE_STACKCHECK}
function CheckInitialStkLen(stklen : SizeUInt) : SizeUInt;inline;
begin
result := stklen;
end;
var
initialstkptr : Pointer; // external name '__stkptr';
{$endif FPC_HAS_FEATURE_STACKCHECK}
--------------------------------
should be changed to:
--------------------------------
function CheckInitialStkLen(stklen : SizeUInt) : SizeUInt;inline;
begin
result := stklen;
end;
var
_stack_bottom : record end; external name '_end';
_stack_top : record end; external name '_stack_top';
initialstkptr : Pointer = @_stack_top;
--------------------------------
and
-------------------------------- {$ifdef FPC_HAS_FEATURE_STACKCHECK} StackLength := CheckInitialStkLen(initialStkLen); StackBottom := initialstkptr - StackLength; {$endif FPC_HAS_FEATURE_STACKCHECK} -------------------------------- should be changed to: -------------------------------- initialstklen := @_stack_top - @_stack_bottom; StackLength := CheckInitialStkLen(initialstklen); StackBottom := initialstkptr - StackLength; --------------------------------
Although not necessary, also "stacksize" values in "fpcsrc\compiler\systems\i_embed.pas" can be set to 0 to clearly show, that they are no longer in use:
--------------------------------
const
system_arm_embedded_info : tsysteminfo =
(
...
stacksize : 0;
...
);
system_avr_embedded_info : tsysteminfo =
(
...
stacksize : 0;
...
);
system_mipsel_embedded_info : tsysteminfo =
(
...
stacksize : 0;
...
);
system_i386_embedded_info : tsysteminfo =
(
...
stacksize : 0;
...
);
system_x86_64_embedded_info : tsysteminfo =
(
...
stacksize : 0;
...
);
--------------------------------
Steps to reproduce:
All tests have been performed with STM32F107VC microcontroller (ARM Embedded compiler target).
test1.dpr (compilation: fpc -Ct -Tembedded -Parm -XParm-none-eabi- -Cparmv7m -Wpstm32f107xc test1.dpr):
--------------------------------
program Test1;
procedure TestProc;
begin // compiler inserts a call to "fpc_stackcheck" here (if -Ct option has been used)
if StackBottom = nil then
Halt;
end;
begin
TestProc;
end.
--------------------------------
Before patching: "StackBottom" is not set, so "Halt" will be called.
After patching: "StackBottom" is set properly, so "Halt" won't be called.
test2.dpr (compilation: fpc -Ct -Tembedded -Parm -XParm-none-eabi- -Cparmv7m -Wpstm32f107xc test2.dpr):
--------------------------------
program Test2;
{$PUSH}
{$S-}
procedure Hardfault_interrupt; public name 'Hardfault_interrupt';
begin
end;
{$POP}
procedure TestProc;
begin // compiler inserts a call to "fpc_stackcheck" here (if -Ct option has been used)
TestProc; // exhaust stack recursively
end;
begin
TestProc;
end.
--------------------------------
Before patching: "fpc_stackcheck" will not detect the stack overflow (because "StackBottom" variable has not been initialized and is set to 0), so stack grows down until stack pointer points below beginning of the RAM area ($20000000 for ARM STM32F1). This causes a hard fault interrupt, but the interrupt handler also must use a bit of stack, which is impossible - this causes a nested hard fault interrupt, and so on, and so on.
After patching: "fpc_stackcheck" will detect the stack overflow in advance and won't allow to overwrite RAM variables below the stack bottom. "fpc_stackcheck" will call "fpc_handleerror" internally, which leads to an internal call to "Halt(202)".
Mantis conversion info:
- Mantis ID: 29962
- Build: 33415
- Platform: Embedded
- Version: 3.1.1
- Monitored by: » Marcin Wiazowski (Marcin Wiazowski), » dioannidis (Dimitrios Chr. Ioannidis), » @d.ioannidis (Dimitrios Chr. Ioannidis)