View Issue Details

IDProjectCategoryView StatusLast Update
0036630FPCCompilerpublic2020-02-03 15:38
ReporterDo-wan KimAssigned ToFlorian 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version3.3.1Product Build44034 
Target VersionFixed in Version3.3.1 
Summary0036630: Win32 lazarus IDE raise Range check error exception on runtime.
DescriptionAfter r44030, win32 Lazarus IDE raise exception "Range check error" on runtime.
Reverting to r44029, it works fine.

IDE compile options : "-g -gl -O2 -Xs -CX -XX -ddisableUTF8RTL"

TagsNo tags attached.
Fixed in Revision44101
FPCOldBugId
FPCTarget-
Attached Files
  • range-error-fix.patch (1,073 bytes)
    Index: compiler/x86/aoptx86.pas
    ===================================================================
    --- compiler/x86/aoptx86.pas	(revision 44087)
    +++ compiler/x86/aoptx86.pas	(working copy)
    @@ -3900,10 +3900,16 @@
                   To:
                     leal/q x(%reg1),%reg2   leal/q -x(%reg1),%reg2
                 }
    +            TransferUsedRegs(TmpUsedRegs);
    +            UpdateUsedRegs(TmpUsedRegs, tai(p.Next));
    +            UpdateUsedRegs(TmpUsedRegs, tai(hp1.Next));
                 if not GetNextInstruction(hp1, hp2) or
    +              (
                   { The FLAGS register isn't always tracked properly, so do not
                     perform this optimisation if a conditional statement follows }
    -              not MatchInstruction(hp2, [A_Jcc, A_SETcc, A_CMOVcc], []) then
    +                not RegReadByInstruction(NR_DEFAULTFLAGS, hp2) and
    +                not RegUsedAfterInstruction(NR_DEFAULTFLAGS, hp2, TmpUsedRegs)
    +              ) then
                   begin
                     reference_reset(NewRef, 1, []);
                     NewRef.base := taicpu(p).oper[0]^.reg;
    
    range-error-fix.patch (1,073 bytes)

Relationships

child of 0036622 resolvedFlorian [Patch] x86 SUB and LEA optimisations 

Activities

Mattias Gaertner

2020-01-26 08:55

manager   ~0120754

1. This should be moved to project Lazarus.

2. I doubt that -ddisableUTF8RTL is still supported by the IDE under windows. Maybe it should raise an error when doing so.

Jonas Maebe

2020-01-26 12:35

manager   ~0120756

I think they mean r44030 of FPC. That changed something in the peephole optimizer, so it may have resulted in a bug that is triggered by Lazarus.

Do-wan Kim

2020-01-28 01:37

reporter   ~0120780

It works ok with 0036632 patch.

J. Gareth Moreton

2020-02-02 16:57

developer   ~0120848

Last edited: 2020-02-02 17:12

View 2 revisions

Can you confirm if this patch works? Basically, for the MovAdd2Lea and MovSub2Lea optimisation, I wasn't properly covering situations where the FLAGS register was being used, so I've developed a more general-purpose approach that I hope is still relatively quick (delays calling RegUsedAfterInstruction until it absolutely needs to).

On i386, sitations that involved arithmetic with Int64 being typecast into an Integer were being incorrectly optimised:

call fpc_div_int64
movl %eax,%ebx
subl $1,%ebx
sbbl $0,%edx

After the optimisation, the following was produced:

call fpc_div_int64
leal -1(%eax),%ebx
sbbl $0,%edx

Since LEA doesn't set any flags, "sbbl $0,%edx" produced incorrect results because the carry flag was not reflective of %ebx underflowing and is effectively undefined. The patch fixes this issue.



range-error-fix.patch (1,073 bytes)
Index: compiler/x86/aoptx86.pas
===================================================================
--- compiler/x86/aoptx86.pas	(revision 44087)
+++ compiler/x86/aoptx86.pas	(working copy)
@@ -3900,10 +3900,16 @@
               To:
                 leal/q x(%reg1),%reg2   leal/q -x(%reg1),%reg2
             }
+            TransferUsedRegs(TmpUsedRegs);
+            UpdateUsedRegs(TmpUsedRegs, tai(p.Next));
+            UpdateUsedRegs(TmpUsedRegs, tai(hp1.Next));
             if not GetNextInstruction(hp1, hp2) or
+              (
               { The FLAGS register isn't always tracked properly, so do not
                 perform this optimisation if a conditional statement follows }
-              not MatchInstruction(hp2, [A_Jcc, A_SETcc, A_CMOVcc], []) then
+                not RegReadByInstruction(NR_DEFAULTFLAGS, hp2) and
+                not RegUsedAfterInstruction(NR_DEFAULTFLAGS, hp2, TmpUsedRegs)
+              ) then
               begin
                 reference_reset(NewRef, 1, []);
                 NewRef.base := taicpu(p).oper[0]^.reg;
range-error-fix.patch (1,073 bytes)

Florian

2020-02-02 20:50

administrator   ~0120852

Committed and thus hopefully fixed.

Do-wan Kim

2020-02-03 08:03

reporter   ~0120856

It works. Thank you :)

Issue History

Date Modified Username Field Change
2020-01-26 03:11 Do-wan Kim New Issue
2020-01-26 08:55 Mattias Gaertner Note Added: 0120754
2020-01-26 12:35 Jonas Maebe Note Added: 0120756
2020-01-26 12:35 Jonas Maebe Assigned To => J. Gareth Moreton
2020-01-26 12:35 Jonas Maebe Status new => assigned
2020-01-28 01:37 Do-wan Kim Note Added: 0120780
2020-02-02 16:57 J. Gareth Moreton File Added: range-error-fix.patch
2020-02-02 16:57 J. Gareth Moreton Note Added: 0120848
2020-02-02 17:11 J. Gareth Moreton Assigned To J. Gareth Moreton =>
2020-02-02 17:11 J. Gareth Moreton Status assigned => feedback
2020-02-02 17:11 J. Gareth Moreton FPCTarget => -
2020-02-02 17:12 J. Gareth Moreton Note Edited: 0120848 View Revisions
2020-02-02 20:50 Florian Assigned To => Florian
2020-02-02 20:50 Florian Status feedback => resolved
2020-02-02 20:50 Florian Resolution open => fixed
2020-02-02 20:50 Florian Fixed in Version => 3.3.1
2020-02-02 20:50 Florian Fixed in Revision => 44101
2020-02-02 20:50 Florian Note Added: 0120852
2020-02-03 08:03 Do-wan Kim Status resolved => closed
2020-02-03 08:03 Do-wan Kim Note Added: 0120856
2020-02-03 15:38 J. Gareth Moreton Relationship added child of 0036622