View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0034140||FPC||Compiler||public||2018-08-17 08:36||2018-08-17 21:45|
|Status||closed||Resolution||no change required|
|Platform||OS||Win 7||OS Version||64bit|
|Product Version||3.0.4||Product Build|
|Target Version||Fixed in Version|
|Summary||0034140: compiler removes code it wrongly believes to be unreachable|
|Description||The compiler wrongly assumes that some code is unreachable and removes it. This causes the code to produce wrong results.|
|Steps To Reproduce||It seems the problem is associated with subranges and types dependent on previously defined subranges. The compiler seems to believe that the subrange cannot be violated and removes any code that ensures values are in the proper range.|
|Additional Information||The compiler emits warning stating that the code is unreachable when it is apparent that such warnings are incorrect.|
Attached is a test program - including an executable - that exhibits the problem.
|Tags||No tags attached.|
|Fixed in Revision|
RangeChecking.7z (56,151 bytes)
||isn't this duplicate of 0016006|
In the bug report you referred to, the reason for the incorrect behavior was attributed to the typecast and, there is no typecast in the example I provided.
There are many ways a variable can end up with an out of bounds value at run time, the compiler should not remove the programmer written range checking code. Plus, the conclusion that the code is unreachable is incorrect.
If you enable range checking, you will get a run time error at the time the conversion error occurs (from an integer with value 256 to TINDEXES).
Regardless of how you manage to get an invalid value into a variable (explicit typecast, implicit conversion, disk I/O, memory corruption, side channel attack, ...), the compiler will always assume that a variable of a given type will contain a value that is valid for this type whenever you read it. A variable that contains a value that is not valid for its type results in undefined behaviour, regardless of whether it's an integer that's out of bounds or an ansistring that points to an invalid memory location.
If you wish to perform a manual range check, you have to tell the compiler to ignore what it knows about the TINDEXES type by typecasting the value back to integer.
I doubt there is any sense in proving your argument to be "faulty" (to put it very kindly) but, since compatibility with Delphi seems to be of at least some importance, then:
The way FPC handles the example is INCOMPATIBLE with Delphi.
Delphi executes the test (as a correctly written compiler should) and returns the correct value.
Hopefully, the fact that FPC's behavior is incompatible with Delphi will be enough for someone to correct the code.
FPC's conclusion that, the if statement is unreachable is INCORRECT.
The Delphi argument only matters if it is documented (which it may be). If it is an implementation detail, then we are back at the "undefined behaviour" point.
If you wish to read more about a related discussion, there is the thread that starts with http://lists.freepascal.org/pipermail/fpc-devel/2017-July/038009.html and that contains a lot of interesting arguments.
I still think it makes no sense that a high level compiler should not exploit the available type information to the fullest extent under certain conditions.
I'm afraid that is not an implementation detail. Whenever a _variable_ is being tested, the compiler cannot simply _assume_ that the variable is within the bounds of the type. That is incorrect.
The problem isn't in typecasting, it is in the dead code determination algorithm. It _mistakenly_ concludes that the code is unreachable.
In addition to reaching an incorrect conclusion about the code being unreachable, the compiler allowed a variable declared as being an integer to be used as being of type TINDEXES (and there is no typecast telling it to do that.)
I compiled your program in Delphi with range checking.
It errors out, the same as in FPC.
And it should because you are feeding an invalid value to GetMessage which you explicitly declare to accept values between 0..2 .
So, the range check error tells you your code contains a flaw.
What happens in case of invalid values (i.e. wrong program logic)
is undefined, if you switch off range checking.
Unlike Delphi, the FPC compiler at least warns you what is wrong with your code:
RangeChecking.dpr(103,8) Warning: Comparison might be always false due to range of constant and expression
RangeChecking.dpr(104,3) Warning: unreachable code
There are other cases where Delphi and FPC differ in case of wrong values being supplied. But for correct code/values, FPC and Delphi will behave the same.
For wrong code (as in your case), there simply is no way to guarantee that FPC and Delphi will always behave the same.
It's obvious that there is something wrong with the code, it's done on purpose, to ensure that the sanity check performed by the if I > 2 is working properly. In a more complex piece of code, the error rarely is as obvious.
The compiler cannot accept an integer as being the same as a type TINDEXES (without a typecast forcing it to accept it) and then _arbitrarily_ and cavalierly _assume_ that an integer has the same range constraints as a type TINDEXES.
If the compiler accepts an integer instead of a TINDEXES then it cannot make any assumptions about the range, much less, get rid of sanity code to ensure the index is in the valid range and return a value that exposes the error.
What the compiler is doing in that case is semantically wrong.
While I have you here, is there any interest in solving the problem mentioned in "0034098: wrong address/pointer being assigned to a pchar variable" ?
||The rationale behind the behaviour has been explained and it still holds. For further discussions the mailing lists can be used.|
Types that are assignment-compatible (such as integer and TINDEXES) do not necessarily have the same range. If you do not wish to manually insert range checking code when these assignments happen, the compiler can do it for you if you enable range checking.
Once you have assigned an invalid value to a variable, anything else you do with variable is undefined. The reason is that after assigning an invalid value, that variable could contain any value.
Case in point, in the following program it will contain 0 and hence your program would still print the wrong result for the given input (256) even if the compiler did not optimise anything away:
TINDEXES = 0..2;
tr = bitpacked record
The reason is that the compiler has reduced the number of bits reserved for the storage of "i" in the bitpacked record because the programmer has told the compiler that it will never contain any value outside the range 0..2. Again, enabling range checking will catch this error. Alternatively, insert manual range checking before trying to store a value in the target variable that it cannot hold/represent.
|2018-08-17 08:36||440bx||New Issue|
|2018-08-17 08:36||440bx||File Added: RangeChecking.7z|
|2018-08-17 10:49||Marģers||Note Added: 0110076|
|2018-08-17 16:52||440bx||Note Added: 0110077|
|2018-08-17 17:53||Jonas Maebe||Note Added: 0110080|
|2018-08-17 17:53||Jonas Maebe||Status||new => resolved|
|2018-08-17 17:53||Jonas Maebe||Resolution||open => no change required|
|2018-08-17 17:53||Jonas Maebe||Assigned To||=> Jonas Maebe|
|2018-08-17 17:53||Jonas Maebe||Relationship added||related to 0016006|
|2018-08-17 18:27||440bx||Note Added: 0110083|
|2018-08-17 18:27||440bx||Status||resolved => feedback|
|2018-08-17 18:27||440bx||Resolution||no change required => reopened|
|2018-08-17 18:29||440bx||Note Edited: 0110083||View Revisions|
|2018-08-17 18:31||Jonas Maebe||Assigned To||Jonas Maebe =>|
|2018-08-17 18:32||Jonas Maebe||Status||feedback => new|
|2018-08-17 18:48||Jonas Maebe||Note Added: 0110084|
|2018-08-17 19:51||440bx||Note Added: 0110085|
|2018-08-17 20:27||Michael Van Canneyt||Note Added: 0110087|
|2018-08-17 20:27||Michael Van Canneyt||Status||new => resolved|
|2018-08-17 20:27||Michael Van Canneyt||Resolution||reopened => no change required|
|2018-08-17 20:27||Michael Van Canneyt||Assigned To||=> Michael Van Canneyt|
|2018-08-17 20:28||Michael Van Canneyt||Note Edited: 0110087||View Revisions|
|2018-08-17 21:11||440bx||Note Added: 0110088|
|2018-08-17 21:11||440bx||Status||resolved => feedback|
|2018-08-17 21:11||440bx||Resolution||no change required => reopened|
|2018-08-17 21:11||440bx||Note Edited: 0110088||View Revisions|
|2018-08-17 21:13||440bx||Note Edited: 0110088||View Revisions|
|2018-08-17 21:39||Florian||Note Added: 0110090|
|2018-08-17 21:39||Florian||Status||feedback => closed|
|2018-08-17 21:39||Florian||Assigned To||Michael Van Canneyt =>|
|2018-08-17 21:39||Florian||Resolution||reopened => no change required|
|2018-08-17 21:45||Jonas Maebe||Note Added: 0110091|
|2018-08-17 21:45||Jonas Maebe||Note Edited: 0110091||View Revisions|