[fpc-devel] Const optimization is a serious bug

[Martin]

On 04/07/2011 21:39, Chad Berchek wrote:
> I've been reading over some of the recent discussion about reference
> counting problems with const string parameters. I've done some
> experiments and I believe that the so-called const optimization is a
> serious flaw, not just a corner case of questionable legitimacy. I have
> some sample code I will show which should be quite scary. Additionally,
> this is a security vulnerability. It is also a quiet bug, because it may
> go undetected for a long time but randomly result in unreproducible 
> crashes.
>
....

I don't think it is a bug.

However It should be more prominently documented, I have found in 
neither of those places:
http://www.freepascal.org/docs-html/ref/refsu10.html#x33-360003.2.4
http://www.freepascal.org/docs-html/ref/refsu58.html#x135-14500011.4.4

A bug, to me is where the actual behaviour and the intended behaviour 
are different. However the behaviour observed is intended. (So yes, per 
definition a crash is only a bug, if the crash was not intended).

You can argue that this feature is not a good design decision. That is a 
different question. But anyway, this feature is already used, and 
therefore will cause lots of complains if removed....
Anyway, if you do not like the feature do not use it. ("const s : 
string", has no other effect but only the missing ref count => so by not 
using "const" in that case, you loose nothing, except that you get the 
refcount back)

In more detail:
  "(const s: string)" is a declaration by the programmer, that the 
variable (the string) will not be modified at all. That is neither by 
the procedure called, nor by any code outside the procedure, via any 
other  reference to the data.
If the programmer sticks to what he declared, then it works, if not it 
crashes.

 From this point of view it is no different to: 
"SomePointerToRecord^.RecordMember". If there is NO "if 
SomePointerToRecord <> nil then " to protect it, then the programmer has 
declared that he has otherwise taken care that the pointer is valid at 
the time. If the pointer is nil (or otherwise invalid), then it will crash.

Sure the pointer issue is much easier to spot. But the point is, the 
programmer makes a statement what is to be expected. If this expectation 
is not fulfilled, it is a problem of the implementation, not the feature.


For all else:
- it is probably good to draw attention to it, because the majority of 
people seem to be unaware. Being unaware of how are feature works, and 
yet using that feature, has a big potential of going wrong. (but that is 
not a problem of the feature, only a problem of the incorrect usage)

- Request  have been made:
  A global switch to disable the optimization, for those who want not to 
use it.
  A debug/sanity check (like range checks) would be good