View Issue Details

IDProjectCategoryView StatusLast Update
0035332FPCRTLpublic2019-06-02 19:17
ReporterChristo CrauseAssigned ToJeppe Johansen 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version3.3.1Product Build41844 
Target VersionFixed 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
  • simple.lpr (1,652 bytes)
  • 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)

Relationships

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

Activities

Christo Crause

2019-04-06 19:36

reporter  

simple.lpr (1,652 bytes)

Christo Crause

2019-04-06 20: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 20: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 11:45

reporter   ~0115641

Last edited: 2019-04-18 11: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 20: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 19:17

reporter   ~0116538

Thanks Jeppe

Issue History

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