View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0018113FPCCompilerpublic2010-11-29 23:082012-11-25 19:21
ReporterLadislav Lacina 
Assigned ToSergei Gorelkin 
PrioritynormalSeveritymajorReproducibilityalways
StatusresolvedResolutionfixed 
PlatformGO32V2OSGO32V2OS Version
Product Version2.4.2Product Build 
Target VersionFixed in Version2.7.1 
Summary0018113: Level 2 optimalization makes wrong code for absolute variables like Mem array (unit system)
DescriptionWhen Level 2 optimalization is on compiler sometimes forget to add segment prefix for access to absolute variables like mem:array[0..something] ob byte absolute 0:0
as defined in SYSTEM.PP for GO32V2 platform. This bug is not in every code with MEM however I found an example in file VESA.INC from unit Graph for GO32V2. (procedure PutPixVESA256)
I wrote two variants of critical piece of code - just look at it.


BUGREPORT:

Compilation of procedure PutPixVESA256 from VESA.INC file from unit Graph
for GO32V2 target.


{winwriteseg:word; offs:longint; color:word}
SetWriteBank(smallint(offs shr 16));
mem[WinWriteSeg : word(offs)] := byte(color);

$00029AC8: call $299d8 <SETWRITEBANK at vesa.inc:387>
$00029ACD: movzwl 0x48642,%edx
$00029AD4: shl $0x4,%edx
$00029AD7: and $0xffff,%esi
$00029ADD: add %esi,%edx
$00029ADF: mov %bl,(%edx) {BUG! Should be "mov %bl,%fs:(%EDX)"}
$00029AE1: mov (%esp),%ebx
$00029AE4: mov 0x4(%esp),%esi
$00029AE8: add $0x8,%esp
$00029AEB: ret
End of assembler dump.



{winwriteseg:word; offs:longint; color:word; col8:byte}
SetWriteBank(smallint(offs shr 16));
col8:=byte(color);
mem[WinWriteSeg : word(offs)] := col8;


$00029AC8: call $299d8 <SETWRITEBANK at vesa.inc:387>
$00029ACD: mov %bl,%al
$00029ACF: movzwl 0x48642,%edx
$00029AD6: shl $0x4,%edx
$00029AD9: and $0xffff,%esi
$00029ADF: add %esi,%edx
$00029AE1: mov %al,%fs:(%edx) {Correct}
$00029AE4: mov (%esp),%ebx
$00029AE7: mov 0x4(%esp),%esi
$00029AEB: add $0x8,%esp
$00029AEE: ret
$00029AEF: nop

Additional Informationthis bug is associated with bug 0011724
TagsNo tags attached.
FPCOldBugId0
Fixed in Revision16619,22067
Attached Files

- Relationships
related to 0011724closedMarco van de Voort Graph unit in GO32V2 - bugs in banking mode 

-  Notes
(0043765)
Jonas Maebe (manager)
2010-11-29 23:36

It's probably the peephole optimizer that doesn't always keep the segment prefix.
(0044076)
Ladislav Lacina (reporter)
2010-12-08 00:12

I made an very simple testing example. I hope it helps:

Procedure Confuse;
begin

end;


Procedure TestBug(chr:word);
begin
Confuse; {if you comment it, everything is fine even in Level 2}
Mem[$B800:0]:=byte(chr);
end;

begin
writeln(0000013#10#13#10);
TestBug(42); {should print '*'}
readln;
end.
-------------------------------------------------------------------
Here is how is it compiled:

{Level 2}
Procedure TestBug(chr:word);
begin
Confuse;
Mem[$B800:0]:=byte(chr); {BUG! - FS selector is omited}
end;

Dump of assembler code for function TESTBUG:
$000020A4: sub $0x4,%esp
$000020A7: mov %ebx,(%esp)
$000020AA: mov %ax,%bx
$000020AD: call $20a0 <CONFUSE at c:/tp/projekty/graph/testbug.pas:6>
$000020B2: mov %bl,0xb8000
$000020B8: mov (%esp),%ebx
$000020BB: add $0x4,%esp
$000020BE: ret
$000020BF: nop
End of assembler dump.

{------------------------------------------------------}

Procedure TestBug(chr:word);
begin
{Confuse;}
Mem[$B800:0]:=byte(chr); {Now OK}
end;

Dump of assembler code for function TESTBUG:
$000020A4: mov %al,%fs:0xb8000
$000020AA: ret
$000020AB: nop
End of assembler dump.


{------------------------------------------------------}
{------------------------------------------------------}

{Level 1}

Procedure TestBug(chr:word);
begin
Confuse; {Without Level 2 optimalizations is all OK}
Mem[$B800:0]:=byte(chr);
end;

Dump of assembler code for function TESTBUG:
$000020A8: push %ebp
$000020A9: mov %esp,%ebp
$000020AB: sub $0xc,%esp
$000020AE: mov %esi,0xfffffff4(%ebp)
$000020B1: mov %edi,0xfffffff8(%ebp)
$000020B4: mov %ax,0xfffffffc(%ebp)
$000020B8: call $20a0 <CONFUSE at c:/tp/projekty/graph/testbug.pas:4>
$000020BD: mov $0xb8000,%edi
$000020C2: push %es
$000020C3: push %fs
$000020C5: pop %es
$000020C6: lea 0xfffffffc(%ebp),%esi
$000020C9: cld
$000020CA: movsb %ds:(%esi),%es:(%edi)
$000020CB: pop %es
$000020CC: mov 0xfffffff4(%ebp),%esi
$000020CF: mov 0xfffffff8(%ebp),%edi
$000020D2: leave
$000020D3: ret
End of assembler dump.
(0044684)
Ladislav Lacina (reporter)
2010-12-29 17:19

Tried snapshots from 29.12. and it is NOT fixed!

Tried both - trunk 2.5.1 and fixes branch 2.4.3 (both GO32V2 versions)
And the behaviour is still buggy without any change.
(0044685)
Jonas Maebe (manager)
2010-12-29 17:24

It fixed the test I created. Apparently it's something different then, and I cannot fix it since I don't have a platform that can run go32v2 binaries.
(0044686)
Jonas Maebe (manager)
2010-12-29 17:33

Correction: it fixed the test you wrote in comment 0044076 when I looked at the generated assembler code.
(0044765)
Ladislav Lacina (reporter)
2010-12-31 17:12

Unfortunately, I have to repeat, this bug is not fixed. I tried again the example from 0044076, with FPC 2.5.1 for DOS, from 29.12. and it produces exactly the same code. Tried both - FP.EXE and FPC.EXE

Maybe your patch was not uploaded? Or it is't merged with GO32V2?
Please, look at it once more.
(0044796)
Jonas Maebe (manager)
2011-01-02 15:15

> Unfortunately, I have to repeat, this bug is not fixed.

I believe you, but as I said I cannot help any further. The only thing I can still do is disable the peephole optimizer for go32v2 altogether.
(0059907)
Sergei Gorelkin (developer)
2012-05-24 14:38

Tested today's trunk, the generated code looks ok (segment prefix is present if compiled with -O2).
(0061453)
Jonas Maebe (manager)
2012-08-06 22:53

No longer reproducible.
(0061595)
Ladislav Lacina (reporter)
2012-08-12 19:57
edited on: 2012-08-12 20:00

No, It is not fixed. It is only more hidden now but the bug is still present.
This compilation prodeces the bug:
FPC testbug.pas -Os -O1 -O2 -Cp80386 -OpPENTIUM -Ratt -g- -p- -b-

BUT !!!
If you add parameter "-a" the code is fine:
FPC testbug.pas -Os -O1 -O2 -Cp80386 -OpPENTIUM -Ratt -g- -p- -b- -a
This line produces correct code.

So, please, don't look at TESTBUG.S file. You have to examine TESTBUG.EXE
Do this:

gdb testbug.exe
disassemble P$PROGRAM_$$_TESTBUG$WORD

Of course, there is interresting quiestin. Why the hell the "-a" parameter "fixes" the compilation?

This was examined with FPC 2.7.1 downloaded 12.8.2012

(0061602)
Sergei Gorelkin (developer)
2012-08-12 21:55

-a makes compiler use the external assembler instead of internal one, and correct results with -a mean that there's a bug in internal assembler causing prefix to be lost.
Probably there were actually two distinct bugs causing loss of prefix, one at code generation stage (fixed by Jonas), another one in the internal assembler.

Anyway, thanks for providing the precise instructions, now I can reproduce it and hope to fix it soon.
(0064028)
Ladislav Lacina (reporter)
2012-11-25 19:01

I reopened this bug.

This issue is fixed in current snapshots of 2.7.1 but NOT in FPC 2.6.2rc1
(0064030)
Jonas Maebe (manager)
2012-11-25 19:21

Indeed, that's why the "Fixed in version" field says "2.7.1". 2.6.2 is frozen, nothing will be merged anymore.

- Issue History
Date Modified Username Field Change
2010-11-29 23:08 Ladislav Lacina New Issue
2010-11-29 23:31 Jonas Maebe Relationship added related to 0011724
2010-11-29 23:36 Jonas Maebe Note Added: 0043765
2010-12-08 00:12 Ladislav Lacina Note Added: 0044076
2010-12-23 16:24 Jonas Maebe Fixed in Revision => 16619
2010-12-23 16:24 Jonas Maebe Status new => resolved
2010-12-23 16:24 Jonas Maebe Fixed in Version => 2.5.1
2010-12-23 16:24 Jonas Maebe Resolution open => fixed
2010-12-23 16:24 Jonas Maebe Assigned To => Jonas Maebe
2010-12-29 17:19 Ladislav Lacina Status resolved => feedback
2010-12-29 17:19 Ladislav Lacina Resolution fixed => reopened
2010-12-29 17:19 Ladislav Lacina Note Added: 0044684
2010-12-29 17:24 Jonas Maebe Note Added: 0044685
2010-12-29 17:24 Jonas Maebe Assigned To Jonas Maebe =>
2010-12-29 17:24 Jonas Maebe Status feedback => new
2010-12-29 17:33 Jonas Maebe Note Added: 0044686
2010-12-31 17:12 Ladislav Lacina Note Added: 0044765
2011-01-02 15:15 Jonas Maebe Note Added: 0044796
2012-05-24 14:38 Sergei Gorelkin Note Added: 0059907
2012-08-06 22:53 Jonas Maebe Status new => resolved
2012-08-06 22:53 Jonas Maebe Resolution reopened => fixed
2012-08-06 22:53 Jonas Maebe Assigned To => Jonas Maebe
2012-08-06 22:53 Jonas Maebe Note Added: 0061453
2012-08-12 19:57 Ladislav Lacina Status resolved => feedback
2012-08-12 19:57 Ladislav Lacina Resolution fixed => reopened
2012-08-12 19:57 Ladislav Lacina Note Added: 0061595
2012-08-12 19:58 Ladislav Lacina Note Edited: 0061595
2012-08-12 20:00 Ladislav Lacina Note Edited: 0061595
2012-08-12 20:09 Jonas Maebe FPCOldBugId => 0
2012-08-12 20:09 Jonas Maebe Assigned To Jonas Maebe =>
2012-08-12 20:09 Jonas Maebe Status feedback => new
2012-08-12 21:55 Sergei Gorelkin Note Added: 0061602
2012-08-13 06:55 Sergei Gorelkin Fixed in Revision 16619 => 16619,22067
2012-08-13 06:55 Sergei Gorelkin Status new => resolved
2012-08-13 06:55 Sergei Gorelkin Fixed in Version 2.6.0 => 2.7.1
2012-08-13 06:55 Sergei Gorelkin Resolution reopened => fixed
2012-08-13 06:55 Sergei Gorelkin Assigned To => Sergei Gorelkin
2012-11-25 19:01 Ladislav Lacina Status resolved => feedback
2012-11-25 19:01 Ladislav Lacina Resolution fixed => reopened
2012-11-25 19:01 Ladislav Lacina Note Added: 0064028
2012-11-25 19:21 Jonas Maebe Status feedback => resolved
2012-11-25 19:21 Jonas Maebe Resolution reopened => fixed
2012-11-25 19:21 Jonas Maebe Note Added: 0064030



MantisBT 1.2.12[^]
Copyright © 2000 - 2012 MantisBT Group
Powered by Mantis Bugtracker