| Anonymous | Login | Signup for a new account | 2013-06-20 11:56 CEST | ![]() |
| All Projects | FPC | Lazarus: Packages, Patches | Lazarus CCR | Mantis | fpGUI | fpcprojects: fpprofiler |
| Main | My View | View Issues | Change Log | Roadmap |
| View Issue Details [ Jump to Notes ] | [ Issue History ] [ Print ] | ||||||||
| ID | Project | Category | View Status | Date Submitted | Last Update | ||||
| 0018113 | FPC | Compiler | public | 2010-11-29 23:08 | 2012-11-25 19:21 | ||||
| Reporter | Ladislav Lacina | ||||||||
| Assigned To | Sergei Gorelkin | ||||||||
| Priority | normal | Severity | major | Reproducibility | always | ||||
| Status | resolved | Resolution | fixed | ||||||
| Platform | GO32V2 | OS | GO32V2 | OS Version | |||||
| Product Version | 2.4.2 | Product Build | |||||||
| Target Version | Fixed in Version | 2.7.1 | |||||||
| Summary | 0018113: Level 2 optimalization makes wrong code for absolute variables like Mem array (unit system) | ||||||||
| Description | When 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 Information | this bug is associated with bug 0011724 | ||||||||
| Tags | No tags attached. | ||||||||
| FPCOldBugId | 0 | ||||||||
| Fixed in Revision | 16619,22067 | ||||||||
| Attached Files | |||||||||
Relationships |
||||||
|
||||||
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 | |
| Main | My View | View Issues | Change Log | Roadmap |



