View Issue Details

IDProjectCategoryView StatusLast Update
0033323FPCCompilerpublic2018-12-05 20:10
ReporterBernt levinsonAssigned ToFlorian 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
PlatformarmOSembeddedOS Versioncortex-m0
Product Version3.1.1Product Build 
Target VersionFixed in Version3.3.1 
Summary0033323: arm - embedded : error in string concatenation on cortex-m0
DescriptionHello,
I encountered a problem with the use of strings variable on Cortex-M0.
This is my simple code to illustrate the problem:

var
  S: string;
  I: Integer;
begin
  S := '';
  I := 10000;
  Str(I, S);
  S := 'fpc ';
  S := S + 'cortex-m0 test';
  sendStringToUART(S);

Theoretically, it must send me back : 'fpc cortex-m0 test'
I receive : '10000co'


It works perfectly on Cortex-M3
Thanks for your help
Regards

Steps To ReproduceSee my small example...
Additional InformationCurrently, I use short string.
TagsNo tags attached.
Fixed in Revision40448
FPCOldBugId
FPCTarget
Attached Files
  • test.zip (1,229 bytes)
  • test2.zip (1,672 bytes)
  • test_cm0.zip (62,374 bytes)
  • test_cm0a.zip (2,249 bytes)
  • test_move.patch (865 bytes)
    Index: rtl/arm/thumb.inc
    ===================================================================
    --- rtl/arm/thumb.inc	(Revision 38833)
    +++ rtl/arm/thumb.inc	(Arbeitskopie)
    @@ -15,6 +15,25 @@
     
      **********************************************************************}
     
    +{$ifndef FPC_SYSTEM_HAS_MOVE}
    +{$define FPC_SYSTEM_HAS_MOVE}
    +
    +procedure Move(const source;var dest;count:longint);[public, alias: 'FPC_MOVE'];
    +var
    +   p1, p2: ^Byte;
    +begin
    +   p1:= @source;
    +   p2:= @dest;
    +   while count > 0 do begin
    +      p2^:= p1^;
    +      inc(p2);
    +      inc(p1);
    +      dec(count);
    +   end;
    +end;
    +{$endif FPC_SYSTEM_HAS_MOVE}
    +
    +
     {$define FPC_SYSTEM_HAS_SYSINITFPU}
     Procedure SysInitFPU;{$ifdef SYSTEMINLINE}inline;{$endif}
     begin
    @@ -99,4 +118,4 @@
       begin
         Result:=Target;
         inc(Target,Source);
    -  end;
    +  end;
    \ Kein Zeilenvorschub am Ende der Datei
    
    test_move.patch (865 bytes)
  • move_align.patch (514 bytes)
    Index: rtl/inc/generic.inc
    ===================================================================
    --- rtl/inc/generic.inc	(revision 38836)
    +++ rtl/inc/generic.inc	(working copy)
    @@ -59,7 +59,7 @@
             then
             begin
               { Align on native pointer size }
    -          aligncount:=(PtrUInt(pdest) and (sizeof(PtrUInt)-1));
    +          aligncount:=(sizeof(PtrUInt)-PtrInt(pdest)) and (sizeof(PtrUInt)-1);
               dec(count,aligncount);
               pend:=psrc+aligncount;
               while psrc<pend do
    
    move_align.patch (514 bytes)
  • finalTest.zip (120,331 bytes)

Activities

Jeppe Johansen

2018-03-05 14:05

developer   ~0106889

Is it affected by different optimization levels?

Bernt levinson

2018-03-05 16:12

reporter   ~0106899

Here is the command line i use to compile my code, without optimization:

ppcrossarm.exe -B -Cparmv6m -Sm -vi -vh -vw -XParm-embedded- -Parm -Tembedded -WpLPC812M101FDH20 test.pp

Florian

2018-03-10 11:41

administrator   ~0107055

I tried to reproduce it by running it on a Raspi by compiling with -Cparmv6m but it worked there. Do you have any further info which could help to identify the problem? Did you use a recent trunk version?

Jeppe Johansen

2018-03-20 12:56

developer   ~0107289

Do you have interrupts enabled during this?

Also can you provide a complete code example?

Bernt levinson

2018-03-31 10:17

reporter   ~0107473

Hi,
Sorry for my late reply.
You will find in attachement a small program to reproduce the problem. Now with the latest trunk version, there is strange issues with "Length" fonction and string in general...

Thanks

Bernt levinson

2018-03-31 10:17

reporter  

test.zip (1,229 bytes)

Jeppe Johansen

2018-03-31 17:39

developer   ~0107485

Can you attach uInit as well?

I'm not immediately seeing any problems in the disassembly when compiling the project without that.

Bernt levinson

2018-04-10 14:04

reporter  

test2.zip (1,672 bytes)

Bernt levinson

2018-04-10 14:05

reporter   ~0107734

Now you can find the entire project here. Sorry for the delay...

Bernt levinson

2018-04-16 16:59

reporter   ~0107820

Is there a difference between Cortex-M0 and Cortex-M0+ ? LPC812 is a cortex-M0+, i do not have a cortex-M0 microcontroller...

Florian

2018-04-16 22:47

administrator   ~0107823

Can you please just compile with -al and attach the resulting .s files?

Bernd

2018-04-22 12:26

reporter  

test_cm0.zip (62,374 bytes)

Bernd

2018-04-22 12:26

reporter   ~0107914

I can confirm the problem on a STM32L072RZ Cortex M0+ controller. The string concatenation seems to produce a hard fault. I attached the assembler listing and an “objdump” of the elf file. I used compiler revision 38313 with the following parameters to compile the program:

-Tembedded -Parm -MObjFPC -Scghi -O1 -g -l -vewnhibq -Filib\arm-embedded -Fu. -FUlib\arm-embedded -Cparmv6m -Wpstm32l073rz -al

Jeppe Johansen

2018-04-22 16:04

developer   ~0107918

On an old binutils toolchain I'm getting a stray veneer in front of the vector table because SYSTEM_Exit isn't marked as a thumb_func. That doesn't seem to be what's causing this problem though

Jeppe Johansen

2018-04-22 17:22

developer   ~0107925

Using a recent binutils it doesn't show up though.

I tried compiling for STM32F051R8 and running it with Qemu, and it worked just fine.

How did you confirm that it was stuck in the Hardfault handler? If you have a debugger you will need to find out what instruction it is faulting at.

Also what board are you using to test with?

Bernd

2018-04-23 00:38

reporter  

test_cm0a.zip (2,249 bytes)

Bernd

2018-04-23 00:39

reporter   ~0107942

I have a NUCLEO-L073RZ. I modified the hard fault handler, so that I could retrieve the fault address. It is located in fpc_move at address 800020c.

800020c: 683a ldr r2, [r7, #0] // r7 contains 0xffffffff

I could try to run your elf file in my hardware. It could run, because the F051 has lesser RAM than the L073. The hard fault handler would not work in that way, since the Cortex M0 seems to have no VTOR register in the SCB, but I could see if the string concatenation fails too.

Bernt levinson

2018-04-24 11:47

reporter   ~0107975

I still have the same issue using the latest binutils (2.29) and my actual FreePascal compiler, revision 38399 (3/3/2018).

Bernd

2018-04-24 17:53

reporter   ~0107979

Last edited: 2018-04-24 18:06

View 2 revisions

There seems to be a problem in fpc_move. May be an unaligned memory access, which Cortex M0 can not handle. If I replace fpc_move with a basic “byte copy” variant, the program runs fine. I attached the patch, which I applied for testing purposes.

Bernd

2018-04-24 18:07

reporter  

test_move.patch (865 bytes)
Index: rtl/arm/thumb.inc
===================================================================
--- rtl/arm/thumb.inc	(Revision 38833)
+++ rtl/arm/thumb.inc	(Arbeitskopie)
@@ -15,6 +15,25 @@
 
  **********************************************************************}
 
+{$ifndef FPC_SYSTEM_HAS_MOVE}
+{$define FPC_SYSTEM_HAS_MOVE}
+
+procedure Move(const source;var dest;count:longint);[public, alias: 'FPC_MOVE'];
+var
+   p1, p2: ^Byte;
+begin
+   p1:= @source;
+   p2:= @dest;
+   while count > 0 do begin
+      p2^:= p1^;
+      inc(p2);
+      inc(p1);
+      dec(count);
+   end;
+end;
+{$endif FPC_SYSTEM_HAS_MOVE}
+
+
 {$define FPC_SYSTEM_HAS_SYSINITFPU}
 Procedure SysInitFPU;{$ifdef SYSTEMINLINE}inline;{$endif}
 begin
@@ -99,4 +118,4 @@
   begin
     Result:=Target;
     inc(Target,Source);
-  end;
+  end;
\ Kein Zeilenvorschub am Ende der Datei
test_move.patch (865 bytes)

Jeppe Johansen

2018-04-24 21:48

developer   ~0107983

Ah yes. Looks like there's a bug in the generic move implementation that doesn't align it correctly when doing forward moves.

Try the attached patch

Jeppe Johansen

2018-04-24 21:48

developer  

move_align.patch (514 bytes)
Index: rtl/inc/generic.inc
===================================================================
--- rtl/inc/generic.inc	(revision 38836)
+++ rtl/inc/generic.inc	(working copy)
@@ -59,7 +59,7 @@
         then
         begin
           { Align on native pointer size }
-          aligncount:=(PtrUInt(pdest) and (sizeof(PtrUInt)-1));
+          aligncount:=(sizeof(PtrUInt)-PtrInt(pdest)) and (sizeof(PtrUInt)-1);
           dec(count,aligncount);
           pend:=psrc+aligncount;
           while psrc<pend do
move_align.patch (514 bytes)

Bernd

2018-04-24 23:19

reporter   ~0107986

It works. Thank you.

Bernt levinson

2018-04-29 16:39

reporter   ~0108065

Hi,
after applying the patch, I have very random results on differents operations:
- firmware doesn't start without -Sh option
- In Example1, the function "uart0SendText" is completely ignored, but firmware works
- In few cases, firmware crash

I applied the patch on the fpc version 3.1.1 38399 and i use the latest binutils version 2.29 (from launchpad)

try the attached examples.
Thanks

Bernt levinson

2018-04-29 16:39

reporter  

finalTest.zip (120,331 bytes)

Jeppe Johansen

2018-05-04 19:04

developer   ~0108141

The firmware shouldn't start at all with -Sh because you don't have a heap manager installed.

I ran your binaries through an emulator and as expected the two with -Sh fail when trying to allocate memory. They simply branch to 0 causing exceptions.

The ones without -Sh worked perfectly and both wrote "eHello worldehello world". However that was tested just ignoring all writes to the system registers.

So I'm guessing that it's the way that you are initializing the clock that is bad. The documentation looks terrible but many places set the xCLKUEN to a sequence of 1->0->1 and then wait for

while (!(LPC_SYSCON->SYSPLLCLKUEN & 1));

which should be:

while (not (SYSCON.SYSPLLCLKUEN and 1))<>0 do;

So you should probably recheck your initialization code

Bernt levinson

2018-05-05 00:14

reporter   ~0108142

Hello Jeppe,
thank you for your answer.
I will make some tests as soon as possible and I will keep you informed.
Regards

Bernt levinson

2018-05-07 14:37

reporter   ~0108174

Hi,
After a lot of various tests, i think the issue is not fixed.

- I change my initialization procedure by replacing "while (not (SYSCON.SYSPLLCLKUEN and 1) = 0) do;" by "while (not (SYSCON.SYSPLLCLKUEN and 1))<>0 do;", but with this change, the firmware does not start.

- the function "uart0SendChar" works great, but String initialization and/or memory allocation may be failed.

So, with my original initialization procedure, the code below works:

  uart0SendChar('e');


The code below also works:

var
  S: string;
  j: Integer;

....
  
  S[1] := 'H';
  S[2] := 'e';
  S[3] := 'l';
  S[4] := 'l';
  S[5] := 'o';
  S[6] := ' ';
  S[7] := 'W';
  S[8] := 'o';
  S[9] := 'r';
  S[10] := 'l';
  S[11] := 'd';

  for j := 1 to 11 do
    uart0SendChar(S[j]);


But, the code below doesn't works:

  S := 'Hello World';
  for j := 1 to 11 do
    uart0SendChar(S[j]);


Finally, the code below works:


  S := 'Hello';
  S := S + ' World';

  for j := 1 to 11 do
    uart0SendChar(S[j]);



I do not know how to proceed with these strings...

Thanks for your help
Regards

Jeppe Johansen

2018-05-07 16:52

developer   ~0108175

If you simply skip setting up the oscillator and run with the inbuilt oscillator at default settings, does it then work?

Bernt levinson

2018-05-07 17:13

reporter   ~0108176

In my code I put "initSystem" in comment and adapted the UART peripheral clock. I get exactly the same effect...

Benito van der Zander

2018-05-10 18:04

reporter   ~0108229

>sizeof(PtrUInt)-PtrInt(pdest)

Is that safe to have an underflow there? Perhaps it is better to use

aligncount:=(sizeof(PtrUInt)-(PtrUInt(pdest) and (sizeof(PtrUInt)-1)) ) and (sizeof(PtrUInt)-1);

Also all that -1 makes it hard to read. Could add

const PtrAligmentMask = sizeof(PtrUInt)-1

and replace it everywhere

aligncount:=(sizeof(PtrUInt)-(PtrUInt(pdest) and PtrAligmentMask)) and PtrAligmentMask;

Bernt levinson

2018-05-14 11:32

reporter   ~0108286

Hi Benito,
there is no improvement with your correction.

Bernt levinson

2018-05-28 01:54

reporter   ~0108552

Hi,
In my despair, I did some tests with my old FPC (2.7.1), that had already the same problem, and Binutils 2.23. I applied the patch of Jeppe and tested this simple example :


  uart0SendChar('e');

  S := 'Hello World';
  for j := 1 to 11 do
    uart0SendChar(S[j]);

And i got this on Tera Term:

-> eHe

Someone would have any idea ?
Thanks

Regards

Bernt levinson

2018-06-04 13:53

reporter   ~0108690

Hi all,
Despite the many fixes (Jeppe, Bernd, Benito), I still can not run my code. This code works perfectly :

...

  uart0Init(9600);

  uart0SendChar('e');
  wait(1000000);

  S := 'Hello World';
  uart0SendText(S);
  
  for j := 1 to 11 do
    uart0SendChar(S[j]);

  S := '';
  I := 10000;
  Str(I, S);
  uart0SendText(S);
  S := 'fpc ';
  uart0SendText(S);
  S := S + 'cortex-m0 test';
  uart0SendText(S);



but if i delete "S := S + 'cortex-m0 test'", I get an hardfault.


... // Hardfault .....

  uart0Init(9600);

  uart0SendChar('e');
  wait(1000000);

  S := 'Hello World';
  uart0SendText(S);
  
  for j := 1 to 11 do
    uart0SendChar(S[j]);

  S := '';
  I := 10000;
  Str(I, S);
  uart0SendText(S);
  S := 'fpc ';
  uart0SendText(S);
  // without this two lines, i get an hardfault
  //S := S + 'cortex-m0 test';
  //uart0SendText(S);


Can someone help me?

jamie philbrook

2018-06-04 16:20

reporter   ~0108697

Just question:

 Does this target support the managed strings?

 If so, do you think you should be manipulating the strings while
data is being sent/Received ?

 I notice you do a long WAIT after sending 'E' and I would bet that works
just fine?
 
 But you are also using a send String functions which could be returning immediately but being processed in the background ?

 But I would try using ShortStrings witch is 255 in length if you don't specify the length in the declaration.
 Short strings are not managed.

Just a thought.

Bernt levinson

2018-06-04 17:32

reporter   ~0108703

Exactly same result with these different options:

- var S: string;
- var S: ShortString;
- var S: String[255];
- {$H-}/{$H+}
- With or without {$mode objfpc}
- With or without the "Wait" function
- With many version of "FPC_MOVE" FreePascal function (Jeppe and Bernd patches)
- FPC 3.1.1 38399/binutils 2.29

There is no data received during program execution.

Thanks Jamie

Bernt levinson

2018-07-10 18:08

reporter   ~0109349

It seems that the problem is also on LPC1100, which is a Cortex-M0, not an M0+. Can someone confirm this problem ?

Jeppe Johansen

2018-07-12 21:32

developer   ~0109417

I identified one problem on a STM32F0Discovery board.

Initialization of the .data segment was loading from an unaligned address. That could probably very easily happen as it did for me. In that case the firmware doesn't start at all since it gets stuck in a hardfault handler at the 4th instruction of the startup.

I committed a fix for that

Michael Ring

2018-11-28 21:53

reporter   ~0112251

Jeppe, where did you commit this fix to?
I today ran in the same issue and fixed it myself because on trunk I still see the not working original version.

my fix is a little different to yours but seems to work well:
Index: generic.inc
===================================================================
--- generic.inc (revision 40381)
+++ generic.inc (working copy)
@@ -77,6 +77,8 @@
         begin
           { Align on native pointer size }
           aligncount:=(PtrUInt(pdest) and (sizeof(PtrUInt)-1));
+ if aligncount > 0 then
+ aligncount := sizeof(PtrUInt)-aligncount;
           dec(count,aligncount);
           pend:=psrc+aligncount;
           while psrc<pend do


I tested both on Cortex-M0 and on mipsr32, for me the crashes are gone and I see no issues in the output of this small test:

    DEBUG_UART.WriteString('Hello World!'+CRLF);
    DEBUG_UART.WriteString(' Hello World!'+CRLF);
    DEBUG_UART.WriteString(' Hello World!'+CRLF);
    DEBUG_UART.WriteString(' Hello World!'+CRLF);
which should cover all 4 possible alignments for const strings

Florian

2018-12-02 15:53

administrator   ~0112323

I committed Jeppe's fix in r40448, please check.

Michael Ring

2018-12-04 23:36

reporter   ~0112364

For me the fix works just fine on both cortex-m0 and mipsel , thank you!

Issue History

Date Modified Username Field Change
2018-03-05 00:19 Bernt levinson New Issue
2018-03-05 14:05 Jeppe Johansen Note Added: 0106889
2018-03-05 16:12 Bernt levinson Note Added: 0106899
2018-03-10 11:41 Florian Note Added: 0107055
2018-03-20 12:56 Jeppe Johansen Note Added: 0107289
2018-03-31 10:17 Bernt levinson Note Added: 0107473
2018-03-31 10:17 Bernt levinson File Added: test.zip
2018-03-31 17:39 Jeppe Johansen Note Added: 0107485
2018-04-10 14:04 Bernt levinson File Added: test2.zip
2018-04-10 14:05 Bernt levinson Note Added: 0107734
2018-04-16 16:59 Bernt levinson Note Added: 0107820
2018-04-16 22:47 Florian Note Added: 0107823
2018-04-22 12:26 Bernd File Added: test_cm0.zip
2018-04-22 12:26 Bernd Note Added: 0107914
2018-04-22 16:04 Jeppe Johansen Note Added: 0107918
2018-04-22 17:22 Jeppe Johansen Note Added: 0107925
2018-04-23 00:38 Bernd File Added: test_cm0a.zip
2018-04-23 00:39 Bernd Note Added: 0107942
2018-04-24 11:47 Bernt levinson Note Added: 0107975
2018-04-24 17:53 Bernd Note Added: 0107979
2018-04-24 18:06 Bernd Note Edited: 0107979 View Revisions
2018-04-24 18:07 Bernd File Added: test_move.patch
2018-04-24 21:48 Jeppe Johansen Note Added: 0107983
2018-04-24 21:48 Jeppe Johansen File Added: move_align.patch
2018-04-24 23:19 Bernd Note Added: 0107986
2018-04-29 16:39 Bernt levinson Note Added: 0108065
2018-04-29 16:39 Bernt levinson File Added: finalTest.zip
2018-05-04 19:04 Jeppe Johansen Note Added: 0108141
2018-05-05 00:14 Bernt levinson Note Added: 0108142
2018-05-07 14:37 Bernt levinson Note Added: 0108174
2018-05-07 16:52 Jeppe Johansen Note Added: 0108175
2018-05-07 17:13 Bernt levinson Note Added: 0108176
2018-05-10 18:04 Benito van der Zander Note Added: 0108229
2018-05-14 11:32 Bernt levinson Note Added: 0108286
2018-05-28 01:54 Bernt levinson Note Added: 0108552
2018-06-04 13:53 Bernt levinson Note Added: 0108690
2018-06-04 16:20 jamie philbrook Note Added: 0108697
2018-06-04 17:32 Bernt levinson Note Added: 0108703
2018-07-10 18:08 Bernt levinson Note Added: 0109349
2018-07-12 21:32 Jeppe Johansen Note Added: 0109417
2018-11-28 21:53 Michael Ring Note Added: 0112251
2018-12-02 15:53 Florian Note Added: 0112323
2018-12-04 23:36 Michael Ring Note Added: 0112364
2018-12-05 20:10 Florian Fixed in Revision => 40448
2018-12-05 20:10 Florian Status new => resolved
2018-12-05 20:10 Florian Fixed in Version => 3.3.1
2018-12-05 20:10 Florian Resolution open => fixed
2018-12-05 20:10 Florian Assigned To => Florian