Unknown problem with a small code

[Phoenix]

Hello,

I found a problem with the source (zip attached). It is a stupid code but if executed in debug mode it gives error on the creation of the file. If you comment on the line "fPathSd: = ''" in Clear () it works (even other trivial changes avoid the appearance of the error). There is probably a violation of memory that I can not understand. It can be rewritten in various ways so as not to make the problem appear but my request and understand why it is written so does not work  .

Every help is welcome

Windows 10
Lazarus 1.8.0 64bit (2017-12-03) FPC 3.0.4

[howardpc]

The problem arises in this procedure

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

1. procedure TObjTest.NewSd(const path: String);
2. begin
3.  Clear;
4.  fPathSd:= path;
5.  if fPathSd = '' then
6.   sd:= TMemoryStream.Create
7.  else
8.   sd:= TFileStream.Create(fPathSd, fmCreate);
9. end;

in which you declare path as a const parameter.
"const" informs the compiler that you (the programmer) will not change path. However, you do change path in the first line of the routine by calling Clear, which sets fPathSd (which the path parameter points to) to become an empty string. As a consequence TFileStream.Create is then called with nonsense data, not with the string you are expecting.
Remove the "const" parameter modifier, and the problem goes away (although it is convoluted programming, not good design IMHO).

Generally when making binary choices it is better to have a FCreateMemoryStream boolean variable (or aCreateAsFileStream: Boolean parameter) which you set explicitly, than to choose what kind of stream is created based on the value of a string. AnsiStrings already have one level of indirection since they are pointers, and since they are compiler-managed you may not always realise when they have been nilled (as in this case). Whereas a private boolean, visible across the entire scope of the object is a simple type you can rely on for the lifetime of the object.

[ASerge]

A shorter example of the same error (do SIGSEGV)

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

1. program Project1;
2. {$APPTYPE CONSOLE}
3.  
4. var
5.   Something: string = 'Something';
6.  
7. function WithSideEffects(const S: string): string;
8. begin
9.   Something := '';
10.   Result := S;
11. end;
12.  
13. begin
14.   UniqueString(Something);
15.   Writeln(WithSideEffects(Something));
16.   Readln;
17. end.

[taazz]

The problem arises in this procedure

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

1. procedure TObjTest.NewSd(const path: String);
2. begin
3.  Clear;
4.  fPathSd:= path;
5.  if fPathSd = '' then
6.   sd:= TMemoryStream.Create
7.  else
8.   sd:= TFileStream.Create(fPathSd, fmCreate);
9. end;

in which you declare path as a const parameter.
"const" informs the compiler that you (the programmer) will not change path. However, you do change path in the first line of the routine by calling Clear, which sets fPathSd (which the path parameter points to) to become an empty string. As a consequence TFileStream.Create is then called with nonsense data, not with the string you are expecting.

erm what did I miss? as far as I can see (didn't look at the sample too close so I might miss something)
1) the const is applied to the path variable only and that is not changed at all
2) fpathSd is set after the clear is called so it will have a proper value
3) even in case of an empty value a memory stream is created no reason for a sigsegv.

Sorry I have no advice for the problem at the moment I'll come back after a closer look.

[ASerge]

1) the const is applied to the path variable only and that is not changed at all
2) fpathSd is set after the clear is called so it will have a proper value
3) even in case of an empty value a memory stream is created no reason for a sigsegv.

After calling Clear, FPathSD is freed, but the pointer to the previously allocated place remains in the Path variable, which then assigned to the FPathSD. Those, the variable already points to garbage.

[balazsszekely]

The error message is clear. You're trying to create a directory not a file.

[taazz]

1) the const is applied to the path variable only and that is not changed at all
2) fpathSd is set after the clear is called so it will have a proper value
3) even in case of an empty value a memory stream is created no reason for a sigsegv.

After calling Clear, FPathSD is freed, but the pointer to the previously allocated place remains in the Path variable, which then assigned to the FPathSD. Those, the variable already points to garbage.

1) what makes you think that the path and fpathsd are the same string?
2) strings are reference counted copy on write even if they where the same string the second that fpathsd is written it should only decrease the reference counter and path should still be alive.
3) if the problem is that the path variable does not keep its own reference (which makes things adsorb) then use constref that will probably solve the problem.

[howardpc]

1) the const is applied to the path variable only and that is not changed at all

You have not appreciated that the const modifier in no way protects the path parameter from being changed. It is merely a promise from the programmer to the compiler that he/she won't change path. The compiler then produces code optimised on the assumption that path will remain unchanged.
However, the programmer does change path via the call to Clear. The compiler then unwittingly turns path into garbage.

[taazz]

1) the const is applied to the path variable only and that is not changed at all

You have not appreciated that the const modifier in no way protects the path parameter from being changed. It is merely a promise from the programmer to the compiler that he/she won't change path. The compiler then produces code optimised on the assumption that path will remain unchanged.
However, the programmer does change path via the call to Clear. The compiler then unwittingly turns path into garbage.

no its a constrain from the compiler that any and all attempts to change the path parameter data (and only the path parameter data) inside the procedure will fail. That does not include smart hacks or access from different variables or parameters. The only thing I want is to stop any and all accidental change inside the procedure/method.

Stop propagating a simpleton view that is false to begin with.

[eric]

There's a difference between the way 'const' is treated when it's a parameter, and the way it's treated when it's a section heading. See http://wiki.freepascal.org/Const . As taazz says, when it's a parameter the compiler does protect it.

[molly]

erm what did I miss? as far as I can see (didn't look at the sample too close so I might miss something)

No idea but for me it is failing miserably.

2) fpathSd is set after the clear is called so it will have a proper value

I beg to differ.

Code: [Select]

procedure TObjTest.NewSd(const path: String = '');
begin
 Clear;
 fPathSd:= path;
 writeLn('newsd with path = "', fPathSd, '"');
 if fPathSd = '' then
  sd:= TMemoryStream.Create
 else
  sd:= TFileStream.Create(fPathSd, fmCreate);
end;
with using the test-code as presented it does print the correct path the first time around, but not when it is changed using execute2;

I had to force execution break, and it outputted more than 20 mb of space characters for me (that smells like reading/writing some random memory).

3) even in case of an empty value a memory stream is created no reason for a sigsegv.

that _is_ what the exception is telling us, yes.

edit:
The result i got above is confusing. I understand the differences in results between using no modifier or using constref, but using const bails out for me for no apparent reason. Is there a differrence between using constref or the compiler doing that for us when it decides to pass a const by reference when using the const modifier ?

[mangakissa]

1) the const is applied to the path variable only and that is not changed at all
2) fpathSd is set after the clear is called so it will have a proper value
3) even in case of an empty value a memory stream is created no reason for a sigsegv.

After calling Clear, FPathSD is freed, but the pointer to the previously allocated place remains in the Path variable, which then assigned to the FPathSD. Those, the variable already points to garbage.

First of all, why calling clear? I've never heard of the function and as far I can see it has no purpose at all.
Second, why creating a new variable with the parameter of the procedure if nothing changed.
Third: Follow the debugger and read the created error. Getmem is right.

[molly]

First of all, why calling clear? I've never heard of the function and as far I can see it has no purpose at all.

TS is allowed to do as TS pleases and implement a clear method (as shown). Freeing the stream and resetting the fpathsd variable actually makes sense (although i would personally do it differently).

Third: Follow the debugger and read the created error. Getmem is right.

For me it simply creates the file.

It might be it is an actual path on TS setup though (which causes the error as shown by Getmem). Don't let yourself fooled by the name of the variable. FileStream.Create couldn't care less (unless ofc it is an actual existing path).

[Phoenix]

Hello,
thank you all for your interest in the problem!!! 
The question of using "const" is very interesting.

Although it is not clear to me why with a change so the problem does not recur

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

1. procedure TMyTest.Execute;
2. Var
3.  path: String;
4. begin
5.  path:= BuildPath;
6.  obj.NewSd(path);
7.  obj.ReNewSd;
8. end;
9.  
10. procedure TMyTest.Execute2;
11. begin
12.  
13. end;
14.  

in theory this should not affect the behavior of "const"

First of all, why calling clear? I've never heard of the function and as far I can see it has no purpose at all.

As indicated is a "stupid" code. I extracted it from a very large source trying to "keep" the bug.
So the code may not make much sense but it certainly shows the same problem.

For me it simply creates the file.

Yes, on Windows10 the file is created. In any case, even with an extension, the problem remains.

[BrunoK]

The problem is that when you call ReNewSd in Execute2 the const path, which in that particular case is an pointer to TObjTest.fPathSd, will have its contents indirectly reinitialized in in the TObjTest.NewSd.

The compiler does not know than both path and fPathSd are pointing to the same location, thus cannot detect an indirect clearing of the const path.

You should remove the const specifier so a local copy of path is created during the call.