View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0034409 | FPC | Compiler | public | 2018-10-12 13:56 | 2019-12-02 12:24 |
| Reporter | Marģers | Assigned To | Florian | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | closed | Resolution | reopened | ||
| Platform | x86_64 | OS | linux | OS Version | |
| Product Version | 3.2.0 | Product Build | |||
| Target Version | 3.3.1 | Fixed in Version | 3.3.1 | ||
| Summary | 0034409: [regresion] mov ax, w - generate invalid assembler code | ||||
| Description | w : word; version 3.2.0 mov ax, w - generate: movw %edi,%ax Error: Error while assembling exitcode 1 version 3.0.0 mov ax, w - generate: movw %di,%ax | ||||
| Steps To Reproduce | compile following code with parameters: -Rintel -O4 -a function foo ( w : word):byte; assembler; asm mov ax, w end; begin foo(3); end. | ||||
| Tags | No tags attached. | ||||
| Fixed in Revision | 43617 | ||||
| FPCOldBugId | |||||
| FPCTarget | - | ||||
| Attached Files |
| ||||
|
|
version 3.3.1 Error while assembling version 3.1.1 (2018/3/13) Error while assembling version 3.0.4 Ok look like this error is around for some time |
|
|
I should be able to fix this one, given Intel assembly is my forte. This bug does not seem to occur under win64. Compiling the code produces the expected "mov %cx,%ax" output. |
|
|
So the problem DOES actually occur under win64 when using the latest source: .section .text.n_p$testfile_$$_foo$word$$byte,"x" .balign 16,0x90 .globl P$TESTFILE_$$_FOO$WORD$$BYTE P$TESTFILE_$$_FOO$WORD$$BYTE: .Lc1: .seh_proc P$TESTFILE_$$_FOO$WORD$$BYTE pushq %rbp .seh_pushreg %rbp .Lc3: .Lc4: movq %rsp,%rbp .Lc5: leaq -16(%rsp),%rsp .seh_stackalloc 16 .seh_endprologue # CPU ATHLON64 movw %ecx,%ax <--- ERROR! # CPU ATHLON64 leaq (%rbp),%rsp popq %rbp ret .seh_endproc .Lc2: Watch this space! |
|
|
i34409.patch (5,149 bytes)
Index: compiler/llvm/nllvmbas.pas
===================================================================
--- compiler/llvm/nllvmbas.pas (revision 40012)
+++ compiler/llvm/nllvmbas.pas (working copy)
@@ -41,7 +41,7 @@
fsymboldata: tfplist;
function getllvmasmopindexforsym(sym: tabstractnormalvarsym): longint;
function getllvmasmparasym(sym: tabstractnormalvarsym): tasmsymbol;
- procedure ResolveRef(const filepos: tfileposinfo; var op: toper); override;
+ procedure ResolveRef(const filepos: tfileposinfo; var op: toper; OpSize: TOpSize = S_NO); override;
public
constructor create(p : TAsmList); override;
destructor destroy; override;
@@ -117,7 +117,7 @@
end;
- procedure tllvmasmnode.ResolveRef(const filepos: tfileposinfo; var op: toper);
+ procedure tllvmasmnode.ResolveRef(const filepos: tfileposinfo; var op: toper; OpSize: TOpSize);
var
sym: tabstractnormalvarsym;
ref: treference;
Index: compiler/ncgbas.pas
===================================================================
--- compiler/ncgbas.pas (revision 40012)
+++ compiler/ncgbas.pas (working copy)
@@ -38,7 +38,7 @@
tcgasmnode = class(tasmnode)
protected
- procedure ResolveRef(const filepos: tfileposinfo; var op:toper); virtual;
+ procedure ResolveRef(const filepos: tfileposinfo; var op:toper; OpSize: TOpSize = S_NO); virtual;
public
procedure pass_generate_code;override;
end;
@@ -123,7 +123,7 @@
*****************************************************************************}
- procedure tcgasmnode.ResolveRef(const filepos: tfileposinfo; var op:toper);
+ procedure tcgasmnode.ResolveRef(const filepos: tfileposinfo; var op:toper; OpSize: TOpSize);
var
sym : tabstractnormalvarsym;
{$ifdef x86}
@@ -133,6 +133,7 @@
forceref,
getoffset : boolean;
indexreg : tregister;
+ subreg : tsubregister;
sofs : longint;
begin
if (op.typ=top_local) then
@@ -210,8 +211,44 @@
else
begin
op.typ:=top_reg;
- op.reg:=sym.localloc.register;
+ { i34409 - make sure the right sub-register is used in,
+ for example, raw "MOV AX, param" assembler instructions. [Kit] }
+
+ if (OpSize in [S_NO, S_B, S_W, S_L, S_Q]) then
+ begin
+ case OpSize of
+ S_NO:
+ { Try to determine the correct register to use
+ (selecting one that's too large could cause a
+ buffer overflow when writing to a reference). [Kit] }
+ begin
+ case tcgsize2size[sym.localloc.size] of
+ 1: subreg := R_SUBL;
+ 2: subreg := R_SUBW;
+ 4: subreg := R_SUBD;
+ 8: subreg := R_SUBQ;
+ else
+ subreg := getsubreg(sym.localloc.register);
+ end;
+ end;
+ S_B: subreg := R_SUBL;
+ S_W: subreg := R_SUBW;
+ S_L: subreg := R_SUBD;
+ S_Q: subreg := R_SUBQ;
+ else
+ { Should never reach here }
+ InternalError(2018102400);
+ end;
+
+ op.reg := newreg(
+ R_INTREGISTER,
+ getsupreg(sym.localloc.register),
+ subreg
+ )
+ end
+ else
+ op.reg := sym.localloc.register;
{$ifdef avr}
case sofs of
1: op.reg:=cg.GetNextReg(op.reg);
@@ -320,7 +357,7 @@
{ fixup the references }
for i:=1 to taicpu(hp2).ops do
begin
- ResolveRef(taicpu(hp2).fileinfo,taicpu(hp2).oper[i-1]^);
+ ResolveRef(taicpu(hp2).fileinfo,taicpu(hp2).oper[i-1]^, taicpu(hp2).opsize);
with taicpu(hp2).oper[i-1]^ do
begin
case typ of
@@ -369,7 +406,7 @@
{ fixup the references }
for i:=1 to taicpu(hp).ops do
begin
- ResolveRef(taicpu(hp).fileinfo,taicpu(hp).oper[i-1]^);
+ ResolveRef(taicpu(hp).fileinfo,taicpu(hp).oper[i-1]^, taicpu(hp).opsize);
{$ifdef x86}
with taicpu(hp).oper[i-1]^ do
begin
|
|
|
This is probably not the cleanest way to do it, and I'm not sure if it doesn't introduce unforeseen side-effects, but I have added an extra parameter to the "ResolveRef" routine, namely the operation size. I'm guessing the compiler sets the reference register to 32 bits for the sake of performance reasons, so setting it to a smaller sub-register might cause problems elsewhere. In the meantime, find attached the patch with the proposed fix. It fixes the issue under Windows and Linux, and the compiler builds and seems to perform as it should. Some extensive testing might be required though. |
|
|
Set to "feedback" since the patch hasn't actually been applied yet. |
|
|
I have chosen a shorter approach. |
|
|
Thanks |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2018-10-12 13:56 | Marģers | New Issue | |
| 2018-10-12 16:06 | Marģers | Note Added: 0111383 | |
| 2018-10-13 13:27 | J. Gareth Moreton | Note Added: 0111388 | |
| 2018-10-13 13:28 | J. Gareth Moreton | Assigned To | => J. Gareth Moreton |
| 2018-10-13 13:28 | J. Gareth Moreton | Status | new => assigned |
| 2018-10-13 13:28 | J. Gareth Moreton | Note Edited: 0111388 | View Revisions |
| 2018-10-13 13:32 | J. Gareth Moreton | Note Edited: 0111388 | View Revisions |
| 2018-10-24 06:33 | J. Gareth Moreton | Note Added: 0111532 | |
| 2018-10-25 05:03 | J. Gareth Moreton | File Added: i34409.patch | |
| 2018-10-25 05:06 | J. Gareth Moreton | Note Added: 0111544 | |
| 2018-10-25 05:06 | J. Gareth Moreton | Status | assigned => resolved |
| 2018-10-25 05:06 | J. Gareth Moreton | Fixed in Version | => 3.3.1 |
| 2018-10-25 05:06 | J. Gareth Moreton | Resolution | open => fixed |
| 2018-10-25 05:06 | J. Gareth Moreton | Assigned To | J. Gareth Moreton => Florian |
| 2018-10-25 10:12 | J. Gareth Moreton | Note Added: 0111552 | |
| 2018-10-25 10:12 | J. Gareth Moreton | Status | resolved => feedback |
| 2018-10-25 10:12 | J. Gareth Moreton | Resolution | fixed => reopened |
| 2018-10-25 10:12 | J. Gareth Moreton | Target Version | => 3.3.1 |
| 2019-11-30 21:40 | Florian | Status | feedback => resolved |
| 2019-11-30 21:40 | Florian | Fixed in Revision | => 43617 |
| 2019-11-30 21:40 | Florian | FPCTarget | => - |
| 2019-11-30 21:40 | Florian | Note Added: 0119562 | |
| 2019-12-02 12:24 | Marģers | Status | resolved => closed |
| 2019-12-02 12:24 | Marģers | Note Added: 0119580 |
