View Issue Details

IDProjectCategoryView StatusLast Update
0029083FPCCompilerpublic2015-12-07 19:41
ReporterLadislav LacinaAssigned ToSergei Gorelkin 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
PlatformallOSallOS Version
Product Version3.0.0Product Build 
Target VersionFixed in Version3.1.1 
Summary0029083: CALL instruction in assembler procedures wrongly preserves EBX registers
DescriptionI have a procedure with "assembler" modifier and call from such procedure another procedure with "assembler". In this situation is wrongly preserved the EBX register.
This situation occurs in OLDFPCCALL calling mode. Not sure about new calling mode.

Steps To Reproduce{$CALLING OLDFPCCALL}

var global_x, global_y:longint;

Procedure clipping;assembler;
{INPUT:
 in EAX expects X coordinate
 in EBX expects Y coordinate

 OUTPUT:
 in EAX modified X coord.
 in EBX modifies Y coord.
 }
asm
add eax,100
add ebx,100
end;


procedure dummy_draw(x,y:longint);assembler;
asm
mov eax,x
mov ebx,y
call clipping {??? Why now in EBX is not the new value but the old one ???}

mov global_x,eax
mov global_y,ebx
end;

{----main----}
begin
dummy_draw(1,2);

writeln('X: ',global_x); {prints "101" which is correct}
writeln('Y: ',global_y); {prints "2" but should print "102" instead}

end.
TagsNo tags attached.
Fixed in Revision32542,32608
FPCOldBugId
FPCTarget
Attached Files

Activities

Jonas Maebe

2015-11-24 23:17

manager   ~0087563

oldfpccall passes all parameters on the stack. The fact that eax contains one of the parameter values on entry is pure coincidence.

Ladislav Lacina

2015-11-24 23:59

reporter   ~0087564

No, I can't agree.
Look how Procedure Clipping was compiled in FPC 2.4.2

====================================================
Dump of assembler code for function CLIPPING:
$000020a0 <CLIPPING+0>: add $0x64,%eax
$000020a3 <CLIPPING+3>: add $0x64,%ebx
$000020a6 <CLIPPING+6>: ret
$000020a7 <CLIPPING+7>: nop
End of assembler dump.
====================================================
And how in FPC 3.0.0
====================================================
Dump of assembler code for function CLIPPING:
   0x000017a0 <+0>: push %ebx
   0x000017a1 <+1>: push %esi
   0x000017a2 <+2>: push %edi
   0x000017a3 <+3>: add $0x64,%eax
   0x000017a6 <+6>: add $0x64,%ebx
   0x000017a9 <+9>: pop %edi
   0x000017aa <+10>: pop %esi
   0x000017ab <+11>: pop %ebx
   0x000017ac <+12>: ret
====================================================

AS far I know the calling mechanism was not changed between 2.4.2 and 3.0.0 So why are now the procedures encapsulated by PUSH EBX,ESI,EDI and POP EDI,ESI,EBX?
It changes behaviour of the compiler.
It really must be even in ;assembler; procedures?
Can be this feature disabled?

Jonas Maebe

2015-11-25 00:14

manager   ~0087565

Sorry, I was looking at the wrong routine. I can't reproduce your problem on OS X though:

.text
        .align 4
.globl _P$PROGRAM_$$_CLIPPING
_P$PROGRAM_$$_CLIPPING:
# [tt2.pp]
# [15] asm
        leal -12(%esp),%esp
# [16] add eax,100
        addl $100,%eax
# [17] add ebx,100
        addl $100,%ebx
# [18] end;
        leal 12(%esp),%esp
        ret

Please never fill in "all" as platform unless you actually tested on all platforms. What is the actual platform you are testing on?

Pierre Muller

2015-11-25 09:00

developer   ~0087576

Hi,

I think the problem is that assembler
modifier now still preserves ABI preserved registers.

EBX is probably supposed to bbe preserved by the go32v2 ABI,
which explains why, on go32v2 target, this register is saved and restored
in the entry/exit code of the assembler routine.

  You will need to add nostackframe modifer to solve your issue.
http://www.freepascal.org/docs-html/3.0.0/ref/refsu79.html#x196-21800014.10.9

Sergei Gorelkin

2015-11-25 16:02

developer   ~0087582

I could not reproduce the issue with trunk and i386-win32 target. Is it go32v2 specific, or does it appear only on 3.0.0 compiler?

Pierre Muller

2015-11-25 16:12

developer   ~0087583

I can confirm that the
push ebx
push esi
push edi

is generated by 3.0.0 ppc386 (for go32v2, win32 and darwin target)
but trunk does not add these instructions!

Jonas Maebe

2015-11-25 16:15

manager   ~0087584

Last edited: 2015-11-25 16:15

View 2 revisions

It's indeed go32v2-specific.

Edit: oops, wrong.

Sergei Gorelkin

2015-11-26 00:55

developer   ~0087599

Ok, I found the reason. It appears that for trunk, a mistake in r25224 is canceled out by another mistake in r30011. The latter revision is not in 3.0.0.

r25224 closely follows functionality of tcg.g_save_registers/g_restore_registers, but misses the fact the mentioned methods are not executed at all for "assembler" and "oldfpccall" procedures, due to checks in ncgutil.gen_save_registers.
At the same time, non-x86 targets basically function the same way as r25224, i.e. they override tcg.g_save_registers/tcg.g_restore_registers to empty methods and don't check for po_assembler in g_proc_entry/g_proc_exit.

r30011, in turn, changes the way in which registers are marked as used in asm block when no explicit register list is present (and that includes pure-assembler procedures): it calls cg.allocallcpuregisters in pass2 instead of querying the paramanager while parsing. However, cg.allocallcpuregisters allocates registers only for default calling convention -- so esi,edi,ebx are no longer considered used and thus not saved/restired.

Still to come up with a nice solution for this.

Sergei Gorelkin

2015-12-07 19:41

developer   ~0087835

Well, I believe that revisions 32542 and 32608 fixes it.

Issue History

Date Modified Username Field Change
2015-11-24 22:11 Ladislav Lacina New Issue
2015-11-24 23:17 Jonas Maebe Note Added: 0087563
2015-11-24 23:17 Jonas Maebe Status new => resolved
2015-11-24 23:17 Jonas Maebe Resolution open => no change required
2015-11-24 23:17 Jonas Maebe Assigned To => Jonas Maebe
2015-11-24 23:59 Ladislav Lacina Note Added: 0087564
2015-11-24 23:59 Ladislav Lacina Status resolved => feedback
2015-11-24 23:59 Ladislav Lacina Resolution no change required => reopened
2015-11-25 00:14 Jonas Maebe Note Added: 0087565
2015-11-25 00:15 Jonas Maebe Assigned To Jonas Maebe =>
2015-11-25 09:00 Pierre Muller Note Added: 0087576
2015-11-25 16:02 Sergei Gorelkin Note Added: 0087582
2015-11-25 16:12 Pierre Muller Note Added: 0087583
2015-11-25 16:15 Jonas Maebe Note Added: 0087584
2015-11-25 16:15 Jonas Maebe Note Edited: 0087584 View Revisions
2015-11-26 00:55 Sergei Gorelkin Note Added: 0087599
2015-12-07 19:41 Sergei Gorelkin Note Added: 0087835
2015-12-07 19:41 Sergei Gorelkin Fixed in Revision => 32542,32608
2015-12-07 19:41 Sergei Gorelkin Status feedback => resolved
2015-12-07 19:41 Sergei Gorelkin Fixed in Version => 3.1.1
2015-12-07 19:41 Sergei Gorelkin Resolution reopened => fixed
2015-12-07 19:41 Sergei Gorelkin Assigned To => Sergei Gorelkin