Lazarus

Free Pascal => FPC development => Topic started by: ASerge on July 12, 2018, 06:48:30 am

Title: Is there a way to detect "Trash local variables" at compile time?
Post by: ASerge on July 12, 2018, 06:48:30 am
Simple program:
Code: Pascal  [Select]
  1. {$MODE OBJFPC}
  2. {$APPTYPE CONSOLE}
  3.  
  4. type
  5.   TR = record
  6.     F: Integer;
  7.   end;
  8.  
  9. procedure Test(const R: TR; out O: TR);
  10. begin
  11.   Writeln('In Test ', R.F);
  12.   O := R;
  13. end;
  14.  
  15. var
  16.   R: TR;
  17. begin
  18.   R.F := 123;
  19.   Writeln('Before ', R.F);
  20.   Test(R, R);
  21.   Writeln('After ', R.F);
  22.   Readln;
  23. end;
Normal execution is:
Code: [Select]
Before 123
In Test 123
After 123
But if use -gt options (or "Trash local variables" in Lazarus/Options/Debugging), than behavior is changed. On my machine:
Code: [Select]
Before 123
In Test 1431655765
After 1431655765
I want to determine if this option is "on" at compile time to issue a warning that this can cause errors. Ideally, disable this option locally.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: 440bx on July 12, 2018, 09:07:57 am
Hi Serge,

After spending some time looking at the compiler source code, I don't believe there is a compiler directive that corresponds to that compiler command line switch.    If that is correct then the answer to your question is no.    Take that answer as "likely correct", I looked carefully but, I could have missed it.

On a related note, I think it is likely that you've found a bug because if you compile to produce a 32bit executable, you'll notice that it runs fine (it should since there are no locals to trash in your example program).  On the other hand, a 64bit executable gets the wrong results.  I was looking at the code the compiler generated and, in 64bit, it trashes the arguments (which are in registers).  I don't know if it would do that if your program had locals but, it definitely should never trash the arguments/parameters.

That's all I've got at this time.  Hopefully, it you'll find that information useful.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: Martin_fr on July 12, 2018, 09:27:13 am
You are using "const" incorrectly.

https://www.freepascal.org/docs-html/ref/refsu67.html
Quote
Remark: Note that specifying const is a contract between the programmer and the compiler. It is the programmer who tells the compiler that the contents of the const parameter will not be changed when the routine is executed, it is not the compiler who tells the programmer that the parameter will not be changed.

You must make sure,  that while you are in "Test" nothing will change "R". The compiler may give you an error, if it detects that you try to break this promise. But you must obey it even if the compiler does not detect it.
E.g., if you would pass a global var, then you are not allowed to change that global var.

"Out O" also is passed by ref. That means if you pass the same variable to R and O, then you are not allowed to change O (because it would change R.
With -gt you change O.

The error you get is a warning that it is wrong to pass the same variable as R and O.

And again: With any future version of fpc it may be possible that your code breaks, even if you do not use -gt.

-----
In your simple (simplified?) example changing R (indirectly by changing O) at the end of the end of Test does no damage (with the current version of fpc / it may well crash even without -gt in future versions of fpc).

For reference, with most versions of fpc the below will print 12.
But with little changes, it can get worse, maybe even crash.
Code: Pascal  [Select]
  1. program Project1;
  2. {$mode objfpc}{$H+}
  3. var
  4.   Bar: String;
  5.  
  6. procedure Foo(const s: string);
  7. begin
  8.   Bar:= copy('123', 1,2); // changes s, not allowed
  9.   writeln(s)
  10. end;
  11. begin
  12.   Bar:=copy('abc', 1, 2);
  13.   Foo(Bar); // pass "ab"
  14.   readln;
  15. end.
  16.  
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: 440bx on July 12, 2018, 10:33:01 am
"Out O" also is passed by ref. That means if you pass the same variable to R and O, then you are not allowed to change O (because it would change R.
Nice observation.  I totally missed that he was passing the same variable for both parameters.

With -gt you change O.
That I don't understand.  -gt is supposed to trash locals not arguments.  I don't see how or why the -gt switch would affect O (or any of the arguments for that matter, since they are not local variables, unless FPC considers arguments local variables, which would not make any sense.)

The error you get is a warning that it is wrong to pass the same variable as R and O.
But, when his example is compiled in 32bit, everything works fine (as I believe it should, in spite of his breaking the implied contract with the const parameter, since there are no locals in his function.)

Am I missing something ?

Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: marcov on July 12, 2018, 10:38:26 am
That I don't understand.  -gt is supposed to trash locals not arguments. 

It trashes the out parameter before it reads the const value. Then the const value is read corrupted because they are the same.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: Thaddy on July 12, 2018, 10:53:13 am
But, when his example is compiled in 32bit, everything works fine (as I believe it should, in spite of his breaking the implied contract with the const parameter, since there are no locals in his function.)
No it does not! If it worked for you that is by accident.
32 bit output here:
Code: Bash  [Select]
  1. pi@raspberrypi:~ $ fpc -gt trash.pas
  2. Free Pascal Compiler version 3.1.1-r39423 [2018/07/10] for arm
  3. Copyright (c) 1993-2018 by Florian Klaempfl and others
  4. Target OS: Linux for ARMHF
  5. Compiling trash.pas
  6. Linking trash
  7. 23 lines compiled, 0.8 sec
  8. pi@raspberrypi:~ $ ./trash
  9. Before 123
  10. In Test 1431655765
  11. After 1431655765
  12.  

Also note that out implies local.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: 440bx on July 12, 2018, 01:22:45 pm
No it does not! If it worked for you that is by accident.
Your "example" only shows that the compiler generates different code for a different CPU.  That's what you're calling an accident. 

Also note that out implies local.
That statement implies something very different.   Locals are variables for which space is reserved when setting up the stack frame.  All parameters, whether var, out, or value, unless they are being passed in registers, are already on the stack  _before_ the stack frame is set up, which means they are most definitely not locals (they could be locals in the caller's frame but certainly not in the callee's.)

Parameters/arguments are local in the sense of their visibility NOT their allocation.  Here is a stack frame for you:
Code: [Select]
trashlocalsdelphi.dpr.29: begin
00405A7C 55               push ebp
00405A7D 8BEC             mov ebp,esp
00405A7F 51               push ecx

trashlocalsdelphi.dpr.30: AVar := $ABCDEF12;
00405A80 C745FC12EFCDAB   mov [ebp-$04],$abcdef12  <- THAT is a local
                                                      negative offset from ebp. 

trashlocalsdelphi.dpr.34: Writeln('In Test ', R.F);
00405A87 A118784000       mov eax,[$00407818]
00405A8C BAC45A4000       mov edx,$00405ac4
00405A91 E802E5FFFF       call @Write0UString
00405A96 8B5508           mov edx,[ebp+$08]
00405A99 E81AE3FFFF       call @Write0Long
00405A9E E8F5E5FFFF       call @WriteLn
00405AA3 E860D3FFFF       call @_IOTest
trashlocalsdelphi.dpr.37: O := R;
00405AA8 8B450C           mov eax,[ebp+$0c]        <- parameters, positive
00405AAB 8B5508           mov edx,[ebp+$08]        <- offset from ebp (unless in a register)
00405AAE 8910             mov [eax],edx
trashlocalsdelphi.dpr.40: end;
00405AB0 59               pop ecx
00405AB1 5D               pop ebp                  <- tear down the frame
00405AB2 C20800           ret $0008                <- get rid of arguments

From the Free Pascal Reference Guide, page 191:
The difference of out parameters and parameters by reference is very small (however, see below
for managed types): the former gives the compiler more information about what happens to the
arguments when passed to the procedure: it knows that the variable does not have to be initialized
prior to the call.


Your statement implies that var parameters are also locals, which is downright absurd since it would imply that space for them is allocated on the callee's stack before the procedure has even been called.  That would be quite an accident.   Locally visible and locally allocated are very different things.  Arguments/parameters are locally visible but _not_ locally allocated, as such they can _never_ be the target of a routine designed to trash locals.

Here is something that would be very good reading for you:

https://software.intel.com/en-us/articles/intel-sdm
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: marcov on July 12, 2018, 01:50:11 pm
Your statement implies that var parameters are also locals,

Var parameters have a defined value on entry. OUT parameters not, hence that the fuzzer also randomizes them.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: Martin_fr on July 12, 2018, 02:09:36 pm
Also note that out implies local.
No they are not. But as Marcov wrote, they (like locals) have no defined values when the procedure is entered.

-gt is to help detecting when an app uses such undefined/uninitialized values.
So it is only right that out param are affected.

If anything, the doc may need an update to mention this.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: 440bx on July 12, 2018, 02:52:30 pm
Var parameters have a defined value on entry. OUT parameters not, hence that the fuzzer also randomizes them.

That's reasonable and it is a nice feature to ensure that the caller gets a "return" value but, an out parameter isn't a variable allocated in the callee's stack.   The -gt switch, as documented, is used to ensure that local variables (by definition, allocated on the callee's stack) are initialized before they are used by the callee, or to make it obvious that one or more of them weren't before they were used.  It's nice that it does that for out parameters as well but, it is beyond its document purpose.

It's also a bit problematic to implement the feature that way because its proper (say, consistent) operation depends on how the compiler has chosen to pass the parameters and the sequence in which the "fuzzer" is going to scramble the values.  The 32bit version of the program is a good example, the const is passed by value, the out by reference, the out gets scrambled but it doesn't scramble the parameter passed by value, that's why the 32bit version works, because a unique location has effectively been split into two separate locations.  The 64bit version passes the parameters in registers which ends "working" in the sense that the unique value does get scrambled.

Anyway, as a result of looking into this problem, I noticed that the fuzzer uses 1 of 3 values (if I recall correctly) none of which is particularly interesting.  A bunch of 5s sometimes, a bunch of As in other occasions.  Microsoft chose to use $CC, which is the opcode for int 3.  If the stack gets corrupted, when the ret is executed, it's likely, though, not guaranteed of course, that the code will end up executing a breakpoint, which is good because, it's consistent.  A bunch of 5_s or a bunch of A_s could end up doing god knows what.  For that reason, I think the FPC team should consider using $CC to scramble the values.  Ends up being simpler too.  Always use one value instead of picking one out of three.

Peace.

ETA:

If anything, the doc may need an update to mention this.
I was typing my reply while you posted yours.  I wholeheartedly agree with your suggestion.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: ASerge on July 12, 2018, 02:58:29 pm
You are using "const" incorrectly.
...
You must make sure,  that while you are in "Test" nothing will change "R".
The const parameter in the procedure is not changed.
Quote
"Out O" also is passed by ref. That means if you pass the same variable to R and O, then you are not allowed to change O (because it would change R.
With -gt you change O.
I don't agree. It's just a side effect. If the procedure is written correctly, there is no problem. I have never seen such a restriction.
But the fact that the procedure erases the contents of the memory by the out parameter pointer looks bad. But even so, the impossibility of excluding it is even worse.
Moreover, such "incorrect" use is of course the problem of the caller, but it occurs inside the correct code.
And besides, it's inconsistent. This is the code that runs the same regardless of option -gt
Code: Pascal  [Select]
  1. {$APPTYPE CONSOLE}
  2.  
  3. var
  4.   Vglb: Integer = 1;
  5.  
  6. procedure Test(const Vin: Integer; out Vout: Integer);
  7. begin
  8.   Vout := 2;
  9.   Writeln('In test: ', Vin);
  10. end;
  11.  
  12. begin
  13.   Test(Vglb, Vglb);
  14.   Writeln('After: ', Vglb);
  15.   Readln;
  16. end.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: 440bx on July 12, 2018, 03:10:28 pm
And besides, it's inconsistent.
Consistency, as you pointed out, that's the real problem with having -gt fuzz out parameters. 


This is the code that runs the same regardless of option -gt

Code: [Select]
{$APPTYPE CONSOLE}

var
  Vglb: Integer = 1;

procedure Test(const Vin: Integer; out Vout: Integer);
begin
  Vout := 2;
  Writeln('In test: ', Vin);
end;

begin
  Test(Vglb, Vglb);
  Writeln('After: ', Vglb);
  Readln;
end.
That always works because you're not assigning one parameter to the other.  You are returning a value, which is not dependent on the other parameter, that should make -gt happy and, as expected, it does.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: Thaddy on July 12, 2018, 03:28:18 pm
Trashing out is the only thing that is consistent. That's GOOD not bad.
How else can you determine if an out parameter's behavior is correct?

If you happen to feed out from a const parameter I would expect a warning, though.
Hence I use the term "implied" local, not "inferred" local.
The former means "treat it as locally initialized".
The latter means "needs to be locally initialized".

Those are two different things, but in both cases -gt should trash out.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: marcov on July 12, 2018, 03:45:57 pm
Microsoft chose to use $CC, which is the opcode for int 3.  If the stack gets corrupted, when the ret is executed, it's likely, though, not guaranteed of course, that the code will end up executing a breakpoint, which is good because, it's consistent. 

Stack corruption with FPC is rarely because of buffer overflows, since dynamic arrays, strings and classes are all on the heap. 5 and A are both 101 bit patterns, which are easy to spot.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: Thaddy on July 12, 2018, 04:12:21 pm
int3 is an intel implementation, not microsoft's suggestion. It is cross-platform for Intel x86 families.
I used it -from Delphi 2!!! - to use in a memory-manager to detect heap corruption, so not stack as is the case in point.
https://en.wikipedia.org/wiki/INT_%28x86_instruction%29

It also works with GDB and linux or FreeBSD, likely all others for x86 families.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: Thaddy on July 12, 2018, 04:24:22 pm
No it does not! If it worked for you that is by accident.
Your "example" only shows that the compiler generates different code for a different CPU.  That's what you're calling an accident. 
It only shows you did not provide full information.....Be careful... Rubbish in, rubbish out.. Popper's falsification hits again... 8-) 8-) :P
BTW: on a 64 bit windows (10.2 developer preview, running updates) , it gets also trashed for a 32 bit executable, so test more carefully. (I suspect you forgot -gt.....you actually deserve a grumpy)
It is not an accident.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: ASerge on July 12, 2018, 04:39:03 pm
And besides, it's inconsistent.
Consistency, as you pointed out, that's the real problem with having -gt fuzz out parameters. 
That always works because you're not assigning one parameter to the other.  You are returning a value, which is not dependent on the other parameter, that should make -gt happy and, as expected, it does.
OK, lets so:
Code: Pascal  [Select]
  1. {$APPTYPE CONSOLE}
  2.  
  3. type
  4.   TR = record
  5.     F: Integer;
  6.   end;
  7.  
  8. var
  9.   Vgbl: Integer = 1;
  10.   Rgbl: TR = (F:1);
  11.  
  12. procedure TestInt(const Vin: Integer; out Vout: Integer);
  13. begin
  14.   Vout := Vin + 1;
  15.   Writeln('In test int: ', Vin);
  16. end;
  17.  
  18. procedure TestR(const Vin: TR; out Vout: TR);
  19. begin
  20.   Vout.F := Vin.F + 1;
  21.   Writeln('In test R: ', Vin.F);
  22. end;
  23.  
  24.  
  25. begin
  26.   TestInt(Vgbl, Vgbl);
  27.   TestR(Rgbl, Rgbl);
  28.   Writeln('Int after: ', Vgbl);
  29.   Writeln('Rec after: ', Rgbl.F);
  30.   Readln;
  31. end.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: ASerge on July 12, 2018, 04:42:39 pm
And as I said, I'm not the one who called Test(R, R), but the one who writes the Test() procedure. She looks correct. What to do to make it work correctly?
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: 440bx on July 12, 2018, 04:47:46 pm
No it does not! If it worked for you that is by accident.
Your "example" only shows that the compiler generates different code for a different CPU.  That's what you're calling an accident. 
It only shows you did not provide full information.....Be careful... Rubbish in, rubbish out.. Popper's falsification hits again... 8-) 8-) :P
BTW: on a 64 bit windows (10.2 developer preview, running updates) , it gets also trashed for a 32 bit executable, so test more carefully. (I suspect you forgot -gt.....you actually deserve a grumpy)
It is not an accident.

It's quite unfortunate that you don't have something better to do than whine and expose your ignorance in public. 
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: Martin_fr on July 12, 2018, 04:48:05 pm
And besides, it's inconsistent.
Consistency, as you pointed out, that's the real problem with having -gt fuzz out parameters. 

The "inconsistency" is in the "const" param (and afaik documented).
Const can pass by ref or value. It is entirely up to the compiler which to choose.

use "constref" if you always want by reference.

Quote
Quote
    "Out O" also is passed by ref. That means if you pass the same variable to R and O, then you are not allowed to change O (because it would change R.
    With -gt you change O.
I don't agree. It's just a side effect.
It may be a side-effect, but that side effect changed the const param (if it is passed by ref). And (according to the doc) you promised to the compiler, that you would write the code in such way that this would not happen (neither directly nor by side effect).


Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: 440bx on July 12, 2018, 05:04:45 pm
And as I said, I'm not the one who called Test(R, R), but the one who writes the Test() procedure. She looks correct. What to do to make it work correctly?
I don't see a clean way of reliably getting around the stack and parameter fuzzer.  I can suggest a horrible hack in assembler but, "horrible" actually falls short of describing it accurately.  Are you even allowed to use assembler in your test routine ?
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: Thaddy on July 12, 2018, 05:07:37 pm
It's quite unfortunate that you don't have something better to do than whine and expose your ignorance in public.
Oh, well...< people who do understand this is a valid one:  >:D >:D >:D >:D >:D >
If you lack the knowledge don't mix in a discussion.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: Thaddy on July 12, 2018, 05:35:13 pm
I don't see a clean way of reliably getting around the stack and parameter fuzzer.  I can suggest a horrible hack in assembler but, "horrible" actually falls short of describing it accurately.  Are you even allowed to use assembler in your test routine ?
Well, that is called a Stack Canary... (google for it I am fed up with you (at the moment)).
Really, you lack basic knowledge. Now take some classes in compiler engineering....
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: ASerge on July 12, 2018, 05:37:12 pm
use "constref" if you always want by reference.
...
It may be a side-effect, but that side effect changed the const param (if it is passed by ref). And (according to the doc) you promised to the compiler, that you would write the code in such way that this would not happen (neither directly nor by side effect).
Once again, the problem is that in code that does not know how it will be used (with the same parameter or not) no protection (warn at compile time) that it cannot be used when using the -gt option.
Constref does not solve the problem, and there are no compiler warnings.

As I understand now there is no such way, and if someone gets an obvious error using my code, then I will explain to him that so use the procedure is not very good, because it is not reflected in the language and documentation as a bad side effect.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: Thaddy on July 12, 2018, 05:40:06 pm
Serge,

I respect your knowledge, but it is not a side-effect. It is the consequence of using out. And that is also basic.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: ASerge on July 12, 2018, 05:59:16 pm
I respect your knowledge, but it is not a side-effect. It is the consequence of using out. And that is also basic.
Rather the unexpected behavior of the -gt option for out parameters. More precisely, the impossibility to avoid this behavior. After all, this does not happen for var parameters.
In general the out parameter has the following advantages (if you do not care about its original value):
1. The compiler warns you if you try to use it before initialization.
2. No warning if you pass a non-initialized parameter to a procedure.
3. Clear visual description of its use.
But the unexpected behavior pushes away from using these advantages.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: Thaddy on July 12, 2018, 06:03:54 pm
Hence my suggestion for a warning.
The behavior itself is sane.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: Martin_fr on July 12, 2018, 06:10:00 pm
use "constref" if you always want by reference.
...
It may be a side-effect, but that side effect changed the const param (if it is passed by ref). And (according to the doc) you promised to the compiler, that you would write the code in such way that this would not happen (neither directly nor by side effect).
Once again, the problem is that in code that does not know how it will be used (with the same parameter or not) no protection (warn at compile time) that it cannot be used when using the -gt option.
Constref does not solve the problem, and there are no compiler warnings.
Constref  was about the inconsistency (32 vs 64 bit results).

As for solving your "Test", do not use any const at all.

Again: Currently your code only fails with -gt.
But there is no saying that the exact same code, without -gt will work in future versions of fpc. Maybe some optimization that future fpc will apply makes this exact same code crash.

The problem is in the code (unless you stick with current fpc forever), with or without -gt.  -gt just makes you aware of the problem.


Quote
As I understand now there is no such way, and if someone gets an obvious error using my code, then I will explain to him that so use the procedure is not very good, because it is not reflected in the language and documentation as a bad side effect.
The problem that may not be documented (well it may, but I am not aware off) is how the "out" param works.

That is, that out can (in some cases) pass in the callers variable. So the result of the out param is assigned to the receiving variable while still inside the called procedure; rather than after/at the return from that procedure. In that it acts like a var param.

Yes, -gt doc may also need to be corrected.

======
Maybe there generally is room to lobby for a note/hint from the compiler for any case where the same variable is passed twice or more by reference to the same procedure.
Code: Pascal  [Select]
  1. function Foo(var a: TBaz; b: TBaz);
  2. ...
  3. Foo(x,x);
  4.  
Probably will also not be "as expected", because inside of Foo any assignment to "a" will also change "b", and vice versa.

In case of out params, it may even be possible to ask, that fpc should generate code passing in a temp variable, whenever it detects that  the out-receiver is passed in at least one more by ref param.
Though passing a temp will change behaviour... (not sure if the documented behaviour would allow for this, did not check that part of the docs)

Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: ASerge on July 12, 2018, 06:23:39 pm
As for solving your "Test", do not use any const at all.
Structure may be big, and input parameter not changed.
So I have to do it not const only because of this behavior?
Quote
But there is no saying that the exact same code, without -gt will work in future versions of fpc. Maybe some optimization that future fpc will apply makes this exact same code crash.
The problem is in the code (unless you stick with current fpc forever), with or without -gt.  -gt just makes you aware of the problem.
Do you think this code is a problematic code?
Code: Pascal  [Select]
  1. procedure Test(const R: TR; out O: TR);
  2. begin
  3.   O := R;
  4. end;
In fact, the procedure can be quite correctly handle the situation when the const and out parameter is the same (the same memory area), but to report this to the options -gt fails.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: Thaddy on July 12, 2018, 06:25:33 pm
As I understand now there is no such way, and if someone gets an obvious error using my code, then I will explain to him that so use the procedure is not very good, because it is not reflected in the language and documentation as a bad side effect.
My tests shows he has a rather liberate opinion of truth... He did not test. I did.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: Thaddy on July 12, 2018, 06:29:31 pm
Do you think this code is a problematic code?
Code: Pascal  [Select]
  1. procedure Test(const R: TR; out O: TR);
  2. begin
  3.   O := R;
  4. end;
In fact, the procedure can be quite correctly handle the situation when the const and out parameter is the same (the same memory area), but to report this to the options -gt fails.
No. It is not correct. The compiler should make a copy in this case. (or warn).
<sigh, sorry> This is about deep copy and shallow copy after all. That's why I hate C++ and still teach it.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: Martin_fr on July 12, 2018, 07:19:16 pm
As I understand now there is no such way, and if someone gets an obvious error using my code, then I will explain to him that so use the procedure is not very good, because it is not reflected in the language and documentation as a bad side effect.
My tests shows he has a rather liberate opinion of truth... He did not test. I did.
You misquoted. I did not say, what you attributed to me. I myself quoted it (in order to reply to it), and it was marked as such.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: Martin_fr on July 12, 2018, 07:19:21 pm
Code: Pascal  [Select]
  1. procedure Test(const R: TR; out O: TR);
  2. begin
  3.   O := R;
  4. end;
In fact, the procedure can be quite correctly handle the situation when the const and out parameter is the same (the same memory area), but to report this to the options -gt fails.
In current fpc, or in all future?

But start with todays fpc. If "TR" include ref counted types (or ever will in future):
- const will not increase the ref count
- out may clear refcounted data before it passes it in (it may not always do, not sure, but afaik it can)

So if  R.SomeAnsiString has a refcount of 1, then const will leave it at 1, but out will reduce it. The string will be released before being passed in, and that is for both parameters.

The const param (if passed by value), may still have the pointer to where the string was. So this could even crash. Not necessarily in Test, but at some time later.


Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: 440bx on July 12, 2018, 08:24:48 pm
And (according to the doc) you promised to the compiler, that you would write the code in such way that this would not happen (neither directly nor by side effect).

Martin,

What you point out in that statement is absolutely correct BUT, it's not only the user that has to respect that promise, the compiler is the first one that is expected to honor that promise.  The -gt switch causes the compiler to emit code that overwrites the const parameter, thereby causing all the problems.

-gt causes the compiler to break the promise it expects the user to uphold.   What that switch is doing, admittedly with good intentions, is clearly incorrect.


Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: Martin_fr on July 12, 2018, 08:44:13 pm
And (according to the doc) you promised to the compiler, that you would write the code in such way that this would not happen (neither directly nor by side effect).

Martin,

What you point out in that statement is absolutely correct BUT, it's not only the user that has to respect that promise, the compiler is the first one that is expected to honor that promise.  The -gt switch causes the compiler to emit code that overwrites the const parameter, thereby causing all the problems.

-gt causes the compiler to break the promise it expects the user to uphold.   What that switch is doing, admittedly with good intentions, is clearly incorrect.

Well, that only comes into play, if the the "out param" is otherwise never written too.

So yes, if I declare an out param, to which I never write, then it is this switch that brakes my promise -> alternative reading: I broke the promise by using the switch.

But if I declare an out param, I am likely to write to it, am I not? (Yeah, other cases can be constructed, like check that the const param is different before writing the out param / But even that breaks, see my prev post).
If indeed I am writing to the out param, the all -gt does is, it pulls the broken promise to the top of the procedure, so it is easier to spot.

-------------
But yes it brings up the OP request.

There is $R- to disable range checking (because indeed there is valid code, that gives false positives on range checking).

Now if valid code can be found that relies on -gt off, then a flag for this would be needed.
If I am not mistaken, we did not yet have such valid code in this thread. (But I did not double check)
valid means: not relying on the implementation detail of the current compiler, but on any code that any future fpc may generate within the bounds of documentation. (and where the fpc team does not declare that the doc itself is buggy)
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: 440bx on July 12, 2018, 09:36:07 pm
Well, that only comes into play, if the the "out param" is otherwise never written too.

No, because the code that implements the -gt switch will have corrupted the parameters before the user/programmer gets a chance to do it.

So yes, if I declare an out param, to which I never write, then it is this switch that brakes my promise -> alternative reading: I broke the promise by using the switch.
No, the switch breaks the promise whenever the function/procedure is called with both arguments referring to the same variable.  In that case, which is the case Serge has demonstrated, that switch breaks the promise before the user has a chance to. 

What I'm going to say next is admittedly arguable but, it can be argued that Serge didn't break the promise at all since he is simply assigning the variable to itself.   He doesn't change the value, the value remains constant.  That said, I will be the first to point out and admit that just writing to what is supposed to be a constant is questionable but, that's what -gt does and it does it without respecting the value of what is supposed to be constant.

But if I declare an out param, I am likely to write to it, am I not? (Yeah, other cases can be constructed, like check that the const param is different before writing the out param / But even that breaks, see my prev post).
If indeed I am writing to the out param, the all -gt does is, it pulls the broken promise to the top of the procedure, so it is easier to spot.
No because the programmer isn't creating a problem by writing the same value onto itself (the promise is kept that way).  -gt on the other hand, tramples on the constant by overwriting it with an arbitrary value of its choice.


This is a typical pointer aliasing problem and -gt a priory assumed that there was no aliasing.  What it does _depends_ on the parameters not being aliased, whereas what Serge is doing does not.  He is writing to a const (which is admittedly questionable) but, unlike the -gt switch he preserves the value which meets the essence of the promise made to the compiler.

A compiler should not assume that a pointer and a variable or two pointers are not aliases of each other unless it can deterministically determine they are not.    Serge's procedure call is perfectly valid, the assumption made by the -gt switch isn't, thus leading to an incorrect result.

That behavior of the -gt switch is, and should be considered a bug.  It unwarrantedly assumed that the parameters are not aliases of each other thus causing a problem.

Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: ASerge on July 12, 2018, 09:57:48 pm
If "TR" include ref counted types (or ever will in future):
- const will not increase the ref count
- out may clear refcounted data before it passes it in (it may not always do, not sure, but afaik it can)
1. If (as Spartans  :))
2. With counted types option -gt don't fill out parameters.
Code: Pascal  [Select]
  1. {$MODE OBJFPC}
  2. {$LONGSTRINGS ON}
  3. {$APPTYPE CONSOLE}
  4.  
  5. type
  6.   TR = record
  7.     X: Integer;
  8.     S: string;
  9.   end;
  10.  
  11. procedure Test(const Rin: TR; out Rout: TR);
  12. begin
  13.   Writeln(StringRefCount(Rin.S));
  14.   Rout := Rin;
  15.   Writeln(StringRefCount(Rin.S));
  16. end;
  17.  
  18. var
  19.   R: TR;
  20. begin
  21.   R.S := 'Some string';
  22.   R.X := 1;
  23.   Test(R, R);
  24.   Writeln(R.S, ' ', R.X);
  25.   Readln;
  26. end.
R.X will be output as 1 (of course, S is empty as you mentioned above). But, if we comment out the field S, the garbage will be printed again.
I think:
1. Using const and out parameters - it's normal. In this case, if the types are the same, the developer should take into account that they can pass the same value (pointer).
2. Option -gt poorly documented. I.e. it is not clear what and in what cases it does.
3. If it makes the worked code non-functional, then there must be a possibility of disabling it locally or at least defining it to issue a warning.
4. Perhaps the compiler should issue a warning if the same variable is used more than once when calling procedure, provided that one of the parameters is out. And make a temporary copy of the variable to eliminate the side effect.
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: Martin_fr on July 12, 2018, 10:02:33 pm
Well, that only comes into play, if the the "out param" is otherwise never written too.
No, because the code that implements the -gt switch will have corrupted the parameters before the user/programmer gets a chance to do it.
That is the entire intent of -gt.
If you have code, that may crash or misbehave (sometimes, or even rarely), -gt attempts to increase the change that this code will crash or misbehave.
The exact line is not guaranteed, but that is similar (at least in some cases) when it is a local var that was trashed.
Though I grant you, wish a trashed local var, the error usually happens either at or (far) after the access to that variable. And here it happen earlier. But still the goal is archived, you get an indication that something is wrong with your code.

Quote
So yes, if I declare an out param, to which I never write, then it is this switch that brakes my promise -> alternative reading: I broke the promise by using the switch.
No, the switch breaks the promise whenever the function/procedure is called with both arguments referring to the same variable.  In that case, which is the case Serge has demonstrated, that switch breaks the promise before the user has a chance to. 
Again, so what? Goal archived, the user knows there is an error. Better than shipping a broken app, and learn from your customer that it is broken.

[/quote]
What I'm going to say next is admittedly arguable but, it can be argued that Serge didn't break the promise at all since he is simply assigning the variable to itself.   He doesn't change the value, the value remains constant.  That said, I will be the first to point out and admit that just writing to what is supposed to be a constant is questionable but, that's what -gt does and it does it without respecting the value of what is supposed to be constant.
[/quote]
Ok, that is somewhat valid. (Except for refcounted types, because the assignment may change ref counts)

I am somehow not sure how far that will carry as an argument to get a {$GT-} directive (which is what would be needed in that case)


Quote
But if I declare an out param, I am likely to write to it, am I not? (Yeah, other cases can be constructed, like check that the const param is different before writing the out param / But even that breaks, see my prev post).
If indeed I am writing to the out param, the all -gt does is, it pulls the broken promise to the top of the procedure, so it is easier to spot.
No because the programmer isn't creating a problem by writing the same value onto itself (the promise is kept that way).  -gt on the other hand, tramples on the constant by overwriting it with an arbitrary value of its choice.
<repeated>

Quote
This is a typical pointer aliasing problem and -gt a priory assumed that there was no aliasing.  What it does _depends_ on the parameters not being aliased, whereas what Serge is doing does not.  He is writing to a const (which is admittedly questionable) but, unlike the -gt switch he preserves the value which meets the essence of the promise made to the compiler.
Again not for refcounted types.

But yes the doc currently states "will not be changed". So maybe that should read " will not be written to"
E.g if I have "const A=5;" I can not later in code do "A:=5" even though it would not change the value of the code.

Anyway I can only guess here....
I remember the "const param" docs had to be updated before, because they where not accurate enough. But maybe this time they are... I don't know that.


Quote
A compiler should not assume that a pointer and a variable or two pointers are not aliases of each other unless it can deterministically determine they are not.    Serge's procedure call is perfectly valid, the assumption made by the -gt switch isn't, thus leading to an incorrect result.

That behavior of the -gt switch is, and should be considered a bug.  It unwarrantedly assumed that the parameters are not aliases of each other thus causing a problem.
Well, 50% disagree.
That is, it is a bug, but it might be a bug in the doc. It may be that this behaviour is missing from the doc. But I did not design it, so I can not say what the intend is.

And again, if passing in a ref counted value in this Test function, then it may be that the problem happens even without -gt. Again I dont know what the indent here is.
But previous discussions on the "const param" left me with the impression, that it is meant to leave all burden to the user. (simple stating this, not judging it)
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: Martin_fr on July 12, 2018, 10:38:25 pm
Interesting the string is empty with a constant as string. constant strings have a special refcount value (afaik). That is why I usually use "copy('foo-',1,3);"

R.X will be output as 1 (of course, S is empty as you mentioned above). But, if we comment out the field S, the garbage will be printed again.
I think:
1. Using const and out parameters - it's normal. In this case, if the types are the same, the developer should take into account that they can pass the same value (pointer).
2. Option -gt poorly documented. I.e. it is not clear what and in what cases it does.
3. If it makes the worked code non-functional, then there must be a possibility of disabling it locally or at least defining it to issue a warning.
4. Perhaps the compiler should issue a warning if the same variable is used more than once when calling procedure, provided that one of the parameters is out. And make a temporary copy of the variable to eliminate the side effect.

1) But with ref counts, anything you do in the called function is too late.  See more below
2) yes
3) See issue about "const param" above. If the doc stays, then probably yes: working code. Otherwise working by accident, but not documentation, which breaks by lots of things.
4) +1


Code: Pascal  [Select]
  1. {$MODE OBJFPC}
  2. {$LONGSTRINGS ON}
  3. {$APPTYPE CONSOLE}
  4.  
  5. type
  6.   TR = record
  7.     X: Integer;
  8.     S: string;
  9.   end;
  10.  
  11. procedure Test(constref Rin: TR; out Rout: TR);
  12. begin
  13.   Writeln(StringRefCount(Rin.S),ptrint(pointer(rin.s)));
  14. end;
  15.  
  16. var
  17.   R: TR;
  18. begin
  19.   R.S := 'abcdef';
  20.   R.S[1] := 'a';  // ref count to 1
  21.   R.X := 1;
  22.  
  23.   Writeln(StringRefCount(R.S));
  24.   Test(R, R);
  25.   Writeln(StringRefCount(R.S));  // 0 // even the callers value was damaged
  26.   Readln;
  27. end.

But it gets better (tested with 32 bit)
Code: Pascal  [Select]
  1. {$MODE OBJFPC}
  2. {$LONGSTRINGS ON}
  3. {$APPTYPE CONSOLE}
  4.  
  5. function Test(out Rout: string; const Rin: string): string;
  6. begin
  7.   Writeln(StringRefCount(Rin), ', ',ptrint(pointer(rin)));
  8.   Result := rin;
  9. end;
  10.  
  11. var
  12.   R, x: string;
  13. begin
  14.   R := 'abcdef';
  15.   R[1] := 'a';
  16.  
  17.   Writeln(StringRefCount(R));
  18.   x:=Test(R, R);
  19.   Writeln(StringRefCount(R));
  20.   Writeln(StringRefCount(x), ', ',ptrint(pointer(x)));
  21.   Readln;
  22. end.
Rin points to memory that has been freed. Accessing it could have all sorts of bad effects. E.g

Same for x => crash on app exit.
If you where to do x[1]:= 'k'; then that  may also crash....

(I had the whole const param with ref counts before. So I know quite well how to get it to go wrong....)
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: Martin_fr on July 12, 2018, 10:42:29 pm
But there may be a good argument for being able to disable -gt locally.

Not tested, but
Code: Pascal  [Select]
  1. function Test(out Rout: TC; var Rin: TC);

would not violate any "const" promise. But if it suffers the same -gt, then -gt certainly went wrong. (IMHO)
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: BeniBela on July 13, 2018, 02:00:53 am
Maybe there generally is room to lobby for a note/hint from the compiler for any case where the same variable is passed twice or more by reference to the same procedure.

Definitely

But it should be a warning. Or even an error


This is an awful trap.


Rule of thumb would be to never pass the same variable twice to a function. Not as result either. Even if you check that it is ok, an update could later change a copy parameter to a const parameter
Title: Re: Is there a way to detect "Trash local variables" at compile time?
Post by: PascalDragon on July 16, 2018, 10:56:02 pm
Do you think this code is a problematic code?
Code: Pascal  [Select]
  1. procedure Test(const R: TR; out O: TR);
  2. begin
  3.   O := R;
  4. end;
In fact, the procedure can be quite correctly handle the situation when the const and out parameter is the same (the same memory area), but to report this to the options -gt fails.
No. It is not correct. The compiler should make a copy in this case. (or warn).
<sigh, sorry> This is about deep copy and shallow copy after all. That's why I hate C++ and still teach it.
The function is correct. It's the caller that must pay attention to what it is passing to Test.
As stated in the documentation a const-parameter means a contract, namely that the value passed in isn't changed. Now a out-parameter (or even a var-parameter) is essentially always written to. If one now passes the same variable to both parameters the contract is broken. Simple as that. It would be the same if Test manipulated a global variable that is also passed in as const-parameter.
And the compiler should not need to do a copy in this case, because an idea behind the const is that the compiler can optimize away such copies!