View Issue Details

IDProjectCategoryView StatusLast Update
0035332FPCRTLpublic2019-06-02 17:17
ReporterChristo Crause Assigned ToJeppe Johansen  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version3.3.1 
Fixed in Version3.3.1 
Summary0035332: AVR - incorrect stack error checking
DescriptionEnabling stack error checking for a simple project results in spurious stack errors (runtime error 202). The attached demo works correctly without -Ct but fails on entry to the first call to recursiveSum (as can be seen by the stack pointer printed) with -Ct.

The user ErrorProc prints out the error code and stack pointer which indicates very little stack space is actually used when the stack error is generated.

Output of this case:
Err: 0x00CA
SP: 0x08D3
Steps To ReproduceCompile RTL with -SfSTACKCHECK added to #ifdef CPUAVR in embedded/system.cfg.

Comppile attached simple.lpr:
ppcrossavr -Tembedded -Wpatmega328p -Ct -O3 -Mobjfpc simple.lpr
TagsNo tags attached.
Fixed in Revision42157
FPCOldBugId
FPCTarget3.2.0
Attached Files

Relationships

related to 0029962 new Stack checking functionality isn't properly initialized 

Activities

Christo Crause

2019-04-06 17:36

reporter  

simple.lpr (1,652 bytes)

Christo Crause

2019-04-06 18:04

reporter  

stackcheck.patch (1,620 bytes)   
diff --git a/../../3.3.1/rtl/avr/avr.inc b/avr/avr.inc
index 4844844672..95e3a99ce9 100644
--- a/../../3.3.1/rtl/avr/avr.inc
+++ b/avr/avr.inc
@@ -96,8 +96,10 @@ function get_caller_frame(framebp:pointer;addr:pointer=nil):pointer;assembler;
 
 
 {$define FPC_SYSTEM_HAS_SPTR}
-Function Sptr : pointer;assembler;
+Function Sptr : pointer;assembler; nostackframe;
   asm
+    in r24, 0x3d
+    in r25, 0x3e
   end;

diff --git a/../../3.3.1/rtl/inc/system.inc b/inc/system.inc
index c677e0bcfd..269a3e0f6b 100644
--- a/../../3.3.1/rtl/inc/system.inc
+++ b/inc/system.inc
@@ -44,7 +44,7 @@ type
 {$endif FPC_HAS_FEATURE_TEXTIO}
 
 const
-  STACK_MARGIN = 16384;    { Stack size margin for stack checking }
+  STACK_MARGIN = {$ifdef CPUAVR}64{$else}16384{$endif};    { Stack size margin for stack checking }
 { Random / Randomize constants }
   OldRandSeed : Cardinal = 0;

diff --git a/../../3.3.1/rtl/embedded/system.pp b/embedded/system.pp
index d62c9ef159..f2afae2b11 100644
--- a/../../3.3.1/rtl/embedded/system.pp
+++ b/embedded/system.pp
@@ -277,7 +277,10 @@ begin
 end;
 
 var
-  initialstkptr : Pointer; // external name '__stkptr';
+  initialstkptr : Pointer
+    {$ifdef CPUAVR} external name '_stack_top'
+    {$else} Pointer// __stkptr'
+    {$endif};
 {$endif FPC_HAS_FEATURE_STACKCHECK}
 
 begin
@@ -294,7 +297,7 @@ begin
 
 {$ifdef FPC_HAS_FEATURE_STACKCHECK}
   StackLength := CheckInitialStkLen(initialStkLen);
-  StackBottom := initialstkptr - StackLength;
+  StackBottom := {$ifdef CPUAVR}@{$endif}initialstkptr - StackLength;
 {$endif FPC_HAS_FEATURE_STACKCHECK}
 
 {$ifdef FPC_HAS_FEATURE_EXCEPTIONS}

stackcheck.patch (1,620 bytes)   

Christo Crause

2019-04-06 18:09

reporter   ~0115285

The attached patch fixes the stack check behaviour. Using this patched RTL allows the simple.lpr code to call and complete recursiveSum(237) correctly, while giving as expected a stack error when executing recursiveSum(238).

A write-up of the analysis of stack checking for AVR: https://github.com/ccrause/freepascal/wiki/Investigating-stack-and-heap-management-(avr-embedded)

Dimitrios Chr. Ioannidis

2019-04-18 09:45

reporter   ~0115641

Last edited: 2019-04-18 09:55

View 3 revisions

IMHO, this patch should be accepted.

Also, can I ask who is/are the current AVR maintainer/s ?

@Christo Crause, are you familiar with this ticket 0029962 ?

Christo Crause

2019-04-18 18:06

reporter   ~0115659

I missed 0029962. The problems Marcin identified is exactly what I also found. My fix is slightly different but in general both patches address the same issues. AVR has a further problem located in avr.inc which isn't addressed by Marcin because Marcin probably focused on ARM.

Christo Crause

2019-06-02 17:17

reporter   ~0116538

Thanks Jeppe

Issue History

Date Modified Username Field Change
2019-04-06 17:36 Christo Crause New Issue
2019-04-06 17:36 Christo Crause File Added: simple.lpr
2019-04-06 18:04 Christo Crause File Added: stackcheck.patch
2019-04-06 18:09 Christo Crause Note Added: 0115285
2019-04-18 09:45 Dimitrios Chr. Ioannidis Note Added: 0115641
2019-04-18 09:55 Dimitrios Chr. Ioannidis Note Edited: 0115641 View Revisions
2019-04-18 09:55 Dimitrios Chr. Ioannidis Note Edited: 0115641 View Revisions
2019-04-18 18:06 Christo Crause Note Added: 0115659
2019-04-26 17:19 Florian Relationship added related to 0029962
2019-06-01 19:20 Jeppe Johansen Assigned To => Jeppe Johansen
2019-06-01 19:20 Jeppe Johansen Status new => resolved
2019-06-01 19:20 Jeppe Johansen Resolution open => fixed
2019-06-01 19:20 Jeppe Johansen Fixed in Version => 3.3.1
2019-06-01 19:20 Jeppe Johansen Fixed in Revision => 42157
2019-06-01 19:20 Jeppe Johansen FPCTarget => 3.2.0
2019-06-02 17:17 Christo Crause Status resolved => closed
2019-06-02 17:17 Christo Crause Note Added: 0116538