[SOLVED] Variable addresses in SRAM

[PascalDragon]

This is a good question.  Since fpc_shortstr_concat can handle the situation where either of the two sources can also be the destination, the compiler can omit the temp in this case.  However, in the disassembly listing @dseligo showed, the compiler is using a temporary shortstring to pass to fpc_shortstr_concat as the dests parameter, followed by a fpc_shortstr_to_shortstr call to copy the temporary string into the final destination.  Seems like there is an opportunity for optimization here?

Removal of pessimisation more like :-) My suspicion would be that the compiler is doing something odd to that it can get High() of the destination parameter... particularly since it's a var I certainly hope it's not doing spurious copies.

High(SomeShortStringVar) simply returns the number of elements that SomeShortStringVar has available. This is always solved at compile time and in the case of fpc_shortstr_concat since dests is declared as ShortString it will always be 255.

For the adventurous programmer, here is a proposed compiler enhancement that skips creating a temporary variable when assigning a shortstring concatenation expression to a shortstring variable: https://gitlab.com/ccrause/fpc-source/-/commits/shortstrconcat

It might be better to extend fpc_shortstr_concat (and maybe also fpc_shortstr_concat_multi) by a parameter that contains the length of the target variable and use a PShortString instead of a reference to a ShortString. This way the assignment can be done better in the general way.

[MarkMLl]

It might be better to extend fpc_shortstr_concat (and maybe also fpc_shortstr_concat_multi) by a parameter that contains the length of the target variable and use a PShortString instead of a reference to a ShortString. This way the assignment can be done better in the general way.

It's still unclear where shortstring- as distinct from string[something] is coming from.

MarkMLl

[alpine]

@ccrause
Well done.

Without underestimating your effort (that is an improvement on itself) for the embedded it shouldn't change the picture. One can get a false confidence and pretty common statements as:

Code: Pascal  [Select][+][-]

1.  msg := var_name + ': ' + var_value;

will have the same (bad) effects. Again, with the left/right associativity of concat, it can be optimized, but it will require much more effort.
Simply calling a procedures for assignment and concatenation, even with a shortstrings, removes all that burden of the expression evaluation.
The ( + ) operator isn't any good for defensive programming, something that should be practiced from all embedded programmers. I avoid using it even on a PC (+ have also other side effects that I'm not happy with).

[MarkMLl]

The ( + ) operator isn't any good for defensive programming, something that should be practiced from all embedded programmers. I avoid using it even on a PC (+ have also other side effects that I'm not happy with).

Agreed, but if basic text handling can't be done safely this negates the value of using Pascal- particularly now that C/C++ have, generally speaking, taken on board Wirth's ideas about type checking etc. (as far as allowed by their modus operandi and the amount of legacy code).

MarkMLl

[ccrause]

Without underestimating your effort (that is an improvement on itself) for the embedded it shouldn't change the picture. One can get a false confidence and pretty common statements as:

Code: Pascal  [Select][+][-]

1.  msg := var_name + ': ' + var_value;

will have the same (bad) effects. Again, with the left/right associativity of concat, it can be optimized, but it will require much more effort.

Indeed, this patch seems like a quick fix for a specific situation, not a general fix for other similar situations.

Simply calling a procedures for assignment and concatenation, even with a shortstrings, removes all that burden of the expression evaluation.
The ( + ) operator isn't any good for defensive programming, something that should be practiced from all embedded programmers. I avoid using it even on a PC (+ have also other side effects that I'm not happy with).

This is good advice.  I am used to using + for string concatenation, so would like to explore how much one can improve the situation.  I still need to digest PascalDragon's comment about using PShortString, perhaps a general fix is not too difficult ...

[PascalDragon]

It might be better to extend fpc_shortstr_concat (and maybe also fpc_shortstr_concat_multi) by a parameter that contains the length of the target variable and use a PShortString instead of a reference to a ShortString. This way the assignment can be done better in the general way.

It's still unclear where shortstring- as distinct from string[something] is coming from.

I don't get why that's unclear. The function needs to work with any kind of ShortString and the only one that covers that is ShortString aka String[255] itself.

Simply calling a procedures for assignment and concatenation, even with a shortstrings, removes all that burden of the expression evaluation.
The ( + ) operator isn't any good for defensive programming, something that should be practiced from all embedded programmers. I avoid using it even on a PC (+ have also other side effects that I'm not happy with).

This is good advice.  I am used to using + for string concatenation, so would like to explore how much one can improve the situation.  I still need to digest PascalDragon's comment about using PShortString, perhaps a general fix is not too difficult ...

Just checked again and remembered that the compiler allows to pass a String[X] (with X < 255) to a var parameter of type ShortString so you wouldn't need to change that. But you as I had written you'd need to add the maximum length of the destination string as that gets lost then:

Code: Pascal  [Select][+][-]

1. program tshortstr;
2.  
3. procedure Test(var s: ShortString);
4. begin
5.   Writeln(Length(s), ' ', High(s));
6. end;
7.  
8. var
9.   s: String[20];
10. begin
11.   Test(s); // will print “0 255”
12. end.

With the destination length you should be able to always avoid the passing of a temporary variable (except that's a given due to how the expression works, but that shouldn't bother you).

[MarkMLl]

I don't get why that's unclear. The function needs to work with any kind of ShortString and the only one that covers that is ShortString aka String[255] itself.

But the destination /isn't/ a string[255], it's got a known maximum length which has to be respected.

MarkMLl

[440bx]

Just checked again and remembered that the compiler allows to pass a String[X] (with X < 255) to a var parameter of type ShortString so you wouldn't need to change that. But you as I had written you'd need to add the maximum length of the destination string as that gets lost then:

That doesn't sound right.  "var" parameters have to match exactly for good reason and one of them is not allowing passing a string[10] (or whatever other size) for a shortstring type which is supposed to be string[255].

Here is a slightly modified version of your sample that shows the problem(s) with allowing that.

Code: Pascal  [Select][+][-]

1. {$APPTYPE CONSOLE}
2.  
3.  
4. program _ShortStrings;
5.  
6.  
7. procedure Test(var s: ShortString);
8. begin
9.   Writeln(Length(s), ' ', High(s));
10.  
11.   s := 'a very long string just to see what happens when a string longer ' +
12.        'than what fits is used.';
13. end;
14.  
15. var
16.   s1 : string[10];
17.   s2 : string[10];
18.   s3 : string[10];
19.  
20. begin
21.   writeln;
22.  
23.   writeln('s1: ', s1);
24.   writeln('s2: ', s2);
25.   writeln('s3: ', s3);
26.  
27.  
28.   Test(s1);
29.  
30.   writeln('s1: ', s1);
31.   writeln('s2: ', s2);
32.   writeln('s3: ', s3);
33.  
34.   readln;
35. end.                  

Test assumes that shortstring holds 255 characters, therefore it places that long string into s1 but, by doing that, its messing up strings s2 and s3.  That should not be happening and the reason it's happening is because the compiler decided to treat string[10] as if it were string[255].  IOW, the compiler can't have its string and eat it too.

ETA:

changed "vary" (typo) to "very" in the string constant.

[ccrause]

Just checked again and remembered that the compiler allows to pass a String[X] (with X < 255) to a var parameter of type ShortString so you wouldn't need to change that. But you as I had written you'd need to add the maximum length of the destination string as that gets lost then:

That doesn't sound right.  "var" parameters have to match exactly for good reason and one of them is not allowing passing a string[10] (or whatever other size) for a shortstring type which is supposed to be string[255].

Strict parameter enforcement would prevent the problem (bug?), but would also prevent the current ease of mixing different sized shortstrings.  The ease of use is specifically mentioned in the documentation: Whatever the actual type, single byte strings can be used interchangeably. The compiler always takes care of the necessary type conversions.

PascalDragon's comment on adding the shortstring size when a shortstring parameter is declared as var seems like the viable fix.  Of course working with short shortstrings is probably more prevalent on embedded type targets with low memory, so adding an extra hidden parameter to the call overhead is not great - but seems unavoidable.

[440bx]

I just tried it with Delphi 2 and Delphi 2 accepts passing a string[10] for a shortstring BUT, when the program is run, the Test function knows that only 10 characters fit in the string.

IOW, when high(s) is taken, the result is _not_ 255, it is 10, consequently the string constant moved into the parameter is truncated thereby leaving s2 and s3 unaffected (which is how it should be.)

[funlw65]

... but if basic text handling can't be done safely this negates the value of using Pascal- particularly now that C/C++ have, generally speaking, taken on board Wirth's ideas about type checking etc. (as far as allowed by their modus operandi and the amount of legacy code).

MarkMLl

Absolutely! If I go back to using functions for concatenating strings (as in C), then there is no point in using Pascal - better staying in C or JAL. I think FreePascal for embedded must be rewritten. And striped. That means a new project? I don't think it will happen...

BTW, how good is FreePascal at dead code removal? Personally, I think that the Unit system is already a burden for embedded...

[PascalDragon]

I don't get why that's unclear. The function needs to work with any kind of ShortString and the only one that covers that is ShortString aka String[255] itself.

But the destination /isn't/ a string[255], it's got a known maximum length which has to be respected.

That's why the compiler passes a temporary ShortString variable and then assigns that to the shorter string. Yes, that might mean that data gets lost then, but that's how short strings work!

I just tried it with Delphi 2 and Delphi 2 accepts passing a string[10] for a shortstring BUT, when the program is run, the Test function knows that only 10 characters fit in the string.

IOW, when high(s) is taken, the result is _not_ 255, it is 10, consequently the string constant moved into the parameter is truncated thereby leaving s2 and s3 unaffected (which is how it should be.)

It turns out that Delphi (and probably also TP) is a cheeky, little bugger: If you have a function or method with a var parameter of type ShortString then it also passes the maximum size of the provided string variable as a hidden parameter. This is only done if the parameter is declared as ShortString, but not for any other String[<N>] even String[255]! Also this is not done for out parameters. Something like this also isn't done if the result is a ShortString.

(Funny sidenote: this also means that methods that use this don't have any parameters listed in the RTTI )

Essentially this means that we have to extend the compiler to handle this correctly and then the ShortString concatenation would handle this transparently as long as the compiler passes the left side of the assignment directly instead of using a temporary.

I think FreePascal for embedded must be rewritten. And striped. That means a new project? I don't think it will happen...

You are always free to use your own RTL, but as part of the project we'll only use our main RTL with features disabled, because maintaining the embedded targets only makes sense if we don't have unnecessary maintenance burden for them.

BTW, how good is FreePascal at dead code removal? Personally, I think that the Unit system is already a burden for embedded...

The unit system itself is not a “burden” for Embedded.

As long as you compile the units with -CX and the program with -XX the linker can strip rather well because every function will have its own section and dead code removal by the linker is done by section.
Biggest problem are the various managers that we have. E.g. the memory manager: imagine your code only uses GetMem and FreeMem. This would however still mean that the code for ReallocMem is still linked in as well because it's referenced by the memory manager. This is not something that can be easily solved, though I do have a potential optimization for that in mind, but it will take some time till I find the time to implement it.

[MarkMLl]

That's why the compiler passes a temporary ShortString variable and then assigns that to the shorter string. Yes, that might mean that data gets lost then, but that's how short strings work!

I'm sorry, it's working wrong in that case: the size of the destination is known, and even if data loss is expected there's absolutely no need to move more than that number of characters.

And don't remind /me/ that strings get truncated: I've seen a "professional" development environment which truncated every line at 80 chars without warning... and yes, it /was/ written in Pascal.

But the fact that the result is truncated is not relevant here. What /is/ relevant is that the stack has to have the potential of being 256 bytes larger than expected, and that this requirement is multiplied if the overall program uses multiple threads or coroutines. And speaking as a sometime embedded system programmer, that is a grievous fault.

This is fundamental, 1980s stuff. And if it breaks something more recent like RTTI, then it's the more recent stuff that has to be adjusted.

MarkMLl

[PascalDragon]

That's why the compiler passes a temporary ShortString variable and then assigns that to the shorter string. Yes, that might mean that data gets lost then, but that's how short strings work!

I'm sorry, it's working wrong in that case: the size of the destination is known, and even if data loss is expected there's absolutely no need to move more than that number of characters.

If it is a plain ShortString parameter then the maximum size is 255, because that's what a ShortString's maximum length is. However in the bug reported by ccrause FPK highlighted an aspect that neither of us had on the radar: Open Strings (and also Strict String Checking).

Open Strings is exactly what solves this problem in Delphi and which is also supported in FPC: if that switch is enabled ($P+ or {$OpenStrings On}) then if you have a var parameter of type ShortString then the compiler will provide an additional, hidden High parameter which will contain the maximum length of the provided string variable. If that switch is not enabled then there will be no hidden High parameter and High(aArg) will always return 255 for a ShortString variable. The thing is that in Delphi (and presumably also TP) the Open Strings are enabled by default, while in FPC they aren't enabled by default (in any mode). Also the switch is a local one in Delphi, but a global one in FPC (that should be changed).

Also there is the Strict String Checking which is controlled by the $V or $VarStringChecks directive. This controls whether you can pass a String[<N>] to a function declared with a var parameter of type String[<M>] where <N> <> <M>. This is a local switch and by default is is enabled in Delphi and it's also enabled by default in FPC in modes TP and Delphi.

So every passing of a by-reference ShortString here works by design. And yes, that includes potentially doing buffer overflows.

Side note: Delphi applies these switches only to var parameters, but FPC also applies them to out parameters.

But the fact that the result is truncated is not relevant here. What /is/ relevant is that the stack has to have the potential of being 256 bytes larger than expected, and that this requirement is multiplied if the overall program uses multiple threads or coroutines. And speaking as a sometime embedded system programmer, that is a grievous fault.

I'm not saying that the string concatenation shouldn't be improved (after all I did give ccrause feedback to his changes). I'm only saying that everything here works as designed (though one can argue over the default settings of the switches).

This is fundamental, 1980s stuff. And if it breaks something more recent like RTTI, then it's the more recent stuff that has to be adjusted.

That was just some side note I got when testing this in Delphi. FPC correctly generates the RTTI for a method containing an OpenString parameter

[440bx]

If you have a function or method with a var parameter of type ShortString then it also passes the maximum size of the provided string variable as a hidden parameter. This is only done if the parameter is declared as ShortString, but not for any other String[<N>] even String[255]! Also this is not done for out parameters. Something like this also isn't done if the result is a ShortString.

As @ccrause already mentioned above, it really seems that, in some cases, the extra hidden "max size" parameter to manage the string size correctly is a necessity.