Unexpected/Inconsistent Overflow checking behavior
lazarus / 67-Windows

----------------------------------------------------------------------
440bx · 2020-01-03 05:41

Hello,

Overflow checking does not behave the same way when compiling for
32bit and 64bit.

In the following code:Code: Pascal  [Select][+][-]window.onload =
function(){var x1 = document.getElementById("main_content_section");
if (x1) { var x = document.getElementsByClassName("geshi");for (var i
= 0; i var r : int32;begin  r := low(r);  writeln(r);   r := -r;    
 // should cause an overflow  writeln(r);end.When compiling for 32bit,
the above will, as expected, report an overflow as a runtime error. 
When compiling for 64bit, no overflow is reported and the value of "r"
remains low(r); that is, it remains negative even though it was
negated.

The inconsistency in 64bit seems related to the automatic promotion of
"r" to a 64bit integer (which is unnecessary and unwarranted in this
case.)

----------------------------------------------------------------------
Leledumbo · 2020-01-04 11:05

I got runtime error on both (3.3.1 svn about a week ago), but
differing in the code. 32-bit gives me EIntOverflow: Arithmetic
overflow (RTE 215), while 64-bit gives ERangeError: Range check error
(RTE 201). That's with explicit -Cr given, otherwise none gives RTE.
To be precise, this is targetting Linux, there's a chance it differs
for other targets.

----------------------------------------------------------------------
440bx · 2020-01-04 12:58

Quote from: Leledumbo on January 04, 2020, 12:05:37 pm<snip> while
64-bit gives ERangeError: Range check error (RTE 201). That's with
explicit -Cr given, otherwise none gives RTE.
IMO, that's a bit of a problem.  In 32bit, the overflow is detected
just by enabling overflow checking whereas in 64bit, enabling overflow
checking isn't enough even though it should be.

That makes the compiler's behavior inconsistent.  In one case (32bit),
the overflow is detected by enabling overflow checking and in the
other case (64bit) the overflow is detected only by enabling a
different check, specifically, range checking.

Overflow checking should be enough in either bitness.  The
inconsistency in 64bit is caused by the compiler unnecessarily
promoting the 32bit integer to 64bit.

Thanks for testing the behavior on the trunk version.

----------------------------------------------------------------------
ASerge · 2020-01-04 20:16

Quote from: 440bx on January 04, 2020, 01:58:53 pmThat makes the
compiler's behavior inconsistent.  In one case (32bit), the overflow
is detected by enabling overflow checking and in the other case
(64bit) the overflow is detected only by enabling a different check,
specifically, range checking.
Overflow checking should be enough in either bitness.
I agree. This is due to the implementation of the overflow check
method.
Int64:
Code: ASM  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i # [12] R :=
-R;      // should cause an overflow        movslq 
U_$P$PROGRAM_$$_R(%rip),%rbx        negq    %rbx        movq   
$-9223372036854775808,%rax        cmpq    %rax,%rbx        jne    
.Lj23        call    fpc_overflowInt32:
Code: ASM  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i # [12] R :=
-R;      // should cause an overflow        movl   
U_$P$PROGRAM_$$_R,%ebx        negl    %ebx        cmpl   
$-2147483648,%ebx        jne     .Lj23        call    fpc_overflowBy
the way, Delphi (32 or 64) always use such code:
Code: ASM  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i
Project15.dpr.12: R := -R;      // should cause an overflow  neg dword
ptr [$004278dc]  jno $0041f5d5  call @IntOverWhich outputs IntOverflow
regardless of bitness.

----------------------------------------------------------------------
440bx · 2020-01-04 20:49

Quote from: ASerge on January 04, 2020, 09:16:57 pmBy the way, Delphi
(32 or 64) always use such code:
Code: ASM  [Select][+][-]window.onload = function(){var x1 =
document.getElementById("main_content_section"); if (x1) { var x =
document.getElementsByClassName("geshi");for (var i = 0; i
Project15.dpr.12: R := -R;      // should cause an overflow  neg dword
ptr [$004278dc]  jno $0041f5d5  call @IntOverWhich outputs IntOverflow
regardless of bitness.
Delphi's method is the inherently correct way of checking for and
handling an overflow.

----------------------------------------------------------------------
Zoran · 2020-01-05 06:02

Please report in bugtracker.

----------------------------------------------------------------------
440bx · 2020-01-05 06:39

Quote from: Zoran on January 05, 2020, 07:02:58 amPlease report in
bugtracker.
The reason I didn't is because I'm not sure the FPC developers will
consider it a bug.  My guess is, they'll simply say to use range
checking in addition to overflow checking.

That said, if one of the developers considers it a bug, I'll gladly
report it as such.

----------------------------------------------------------------------
Zoran · 2020-01-05 13:57

Quote from: 440bx on January 05, 2020, 07:39:24 amQuote from: Zoran on
January 05, 2020, 07:02:58 amPlease report in bugtracker.
The reason I didn't is because I'm not sure the FPC developers will
consider it a bug.  My guess is, they'll simply say to use range
checking in addition to overflow checking.

That said, if one of the developers considers it a bug, I'll gladly
report it as such.

Please, don't be afraid to report a possible bug. If you report it in
the forum only, there is a big chance the developers will never see
this discussion and it will just get lost.

If they have the opinion that it is not a bug, one of them will take
the bug report, explain why they do not consider it a bug and resolve
it as "no change required".

I don't think that they will just reject this.
Anyway, reporting it will certainly do no harm.