[fpc-devel] Const optimization is a serious bug

[Martin]

On 07/07/2011 15:04, Alexander Klenin wrote:
> On Thu, Jul 7, 2011 at 23:09, Martin<fpc at mfriebe.de>  wrote:
>> I do think Florians, or Mattias  example are relevant.
>> Let's for a moment ignore the const ansistring issue.
>>
>> The 2 examples prove, that even with "const i : SomeRecord", your code can
>> crash for none obvious reasons, IF you break the promise, not to modify the
>> value of I, by modifying the value that you passed in.
> Have I not just written, in the very detailed manner, that
> reference counting is a technique specifically designed to prevent this problem?
> That you code CAN NOT crash, much less for non-obvious reasons,
> if you use ansistring?
> That this is a critical advantage of using it, as compared to, say, shortstring?
> That, finally, the examples with pointers and manual memory management
> are proving just the opposite of what their author wanted -- i.e.
> current implementation makes AnsiString no better than PChar in terms
> of safety?
>
>> That is:
>>   "const x: sometype" for any type (with the exception maybe of those , that
>> are passed by value) can lead to undefined behaviour and/or crashes, if the
>> promise of "const" is broken.
>>
>> If the above statement can be agreed on (and the examples show this), then
>> why should the "const ansi string" be a bigger issue?
> If you look at the point (2) of the mail you responded to,
> you will see that I discuss this exact issue. "Const string" is passed by value,
> and it does not lead to any undefined behaviour and crashes
> except this case (barring, of course, as-yet-unknown compiler bugs).
>
>> Yes, the const ansi string breaks the rules that ought to be provided by
>> ref-counting.
>> But it only breaks this ref-count promise, for code that is already unsave,
>> even if ref-counting was in place.
> How so? If the bug would be fixed, any procedure with "const string" parameter
> will become perfectly safe, just as with, e.g.,  "const integer".
(code below written in my mailer, so my have typos, but should be enough 
to get the point

program Foo;
var Bar: ansistring;

procedure Test(const s: ansistring);
var x: TStringList;
begin
   if s[1] = '<' then x := TStringList.Create;
   Bar := '<<<';
   if s[1] = '<' then x.Add('a'); // crash, because you changed Bar
end;

begin
   Bar := copy('afhdsgfyowahfol',2,4);
   Test(Bar);
end;

However the Example only demonstrates half the problem. The above *HACK* 
could be mad working. Within todays FPC it could even intentionally be 
used....

EXCEPT:
The above hack is unsafe. It exploits undocumented behaviour. With any 
future FPC it's behaviour could totally change.
Any code that relies on an undocumented randomly found out detail about 
how the compiler *currentlly* translates some code, is very bad code: 
destined to crash some day.

Because if "s" is constant, then so is if "s[1] = '<'"=> so once this 
expression has been calculated, the compiler is free to keep the result 
cached. And that would change the flow of the above code.

Of course in the above example that would actually stop it from 
crashing, but in other code, it may start it crashing, or just return 
different results from what they used to be.

-------
So in conclusion. Even if the above code did have ref counting, it would 
not work (code that changes it's behaviour at random due to compiler 
optimizations is not working).

Oh yes of course refcounting would do copy on write? well cast it to a 
pchar, to avoid the copy on write. It's an example, it's consrtructed to 
show the possibility of the issue in the minimum space, Hence it is not 
the most "common world" code. But it means it can happen in common world 
code.