View Issue Details

IDProjectCategoryView StatusLast Update
0015777FPCCompilerpublic2010-03-14 17:52
ReporterAdriaan van Os Assigned ToJonas Maebe  
PrioritynormalSeverityfeatureReproducibilityalways
Status closedResolutionfixed 
PlatformDarwinOSMac OS X 
Product Version2.5.1 
Fixed in Version2.6.0 
Summary0015777: Compatibility of void pointers in procparam parameter lists in macpas mode
DescriptionIn CodeWarrior, universal pointers in the parameter list of a formal procedure declaration, are compatible with any pointer type in the parameter list of the actual procedure that is passed as a procedure parameter. The attached patch implements this for FPC in macpas mode for the void Pointer type.

This adds two files to the testsuite.
TagsNo tags attached.
Fixed in Revision15010
FPCOldBugId0
FPCTarget
Attached Files

Activities

2010-02-15 18:47

 

fpc-procparam.diff.zip (1,382 bytes)

Jonas Maebe

2010-03-09 21:25

manager   ~0035107

I prefer to implement proper support for "univ" instead of hacks like that for special cases that important to one person or another.

But I'm wondering, is the following supposed to work too, and if so, what does it print?

uses
  MacTypes;

procedure test(l: univ SInt32);
begin
  writeln(l);
end;

var
  s: single;
begin
  s:=1.0;
  test(s);
  test(2.0);
end.

(in case only one of the two expressions compiles, please also show what the output is)

Furthermore, how are such parameters passed? According to the regular calling convention for SInt32 (i.e., in an integer register on PPC), or always by reference? (address of "s" resp. a temp containing 2.0 in an integer register on PPC)

Adriaan van Os

2010-03-09 22:49

developer   ~0035114

Extending the program a little to:

program test;

type SInt32 = Longint;

procedure test(l: univ SInt32);
begin
    writeln(l)
end;

procedure testit;
var
    s: single;
    d: double;
    i: SInt32;
begin
    s:=1.0;
    d:=1.0;
    i:= SInt32( s);
    test(s);
    test(d);
    test(i);
    test(1.0);
    test(2.0);
end;

begin
    testit
end.

the CodeWarrior compiled output is

       0
       1
1065353216
1065353216
1073741824

The "0" output seems to be wrong. Anyway, with these types, I think, we have some interference of special type conversion rule for scalar standard types.

Disassembly follows:

Names:
        1: SYSTEM
        2: MWERKS
        3: __MWERKS__
        4: FALSE
        5: TRUE
        6: POWERPC
        7: __POWERPC__
        8: __POWERPC
        9: MAC68K
       10: __MC68K__
       11: __MAC68K__
       12: CFM68K
       13: __CFM68K__
       14: __MC68020__
       15: __MC68881__
       16: INTEL
       17: __INTEL__
       18: __A5__
       19: __FOURBYTEINTS__
       20: __IEEEDOUBLES__
       21: __PROFILE__
       22: __MWBROWSER__
       23: MACINTOSH
       24: TEST
       25: test
       26: INT32
       27: LONGINT
       28: L
       29: UNIV
       30: WRITELN
       31: P300:Users:adriaan:Desktop:Test:UTF8Program.p
       32: .test
      aka: test
       33: M_OUTPUT
       34: .M_WRITELONGINT
      aka: M_WRITELONGINT
       35: .M_WRITELN
      aka: M_WRITELN
       36: l
       37: TESTIT
       38: testit
       39: S
       40: SINGLE
       41: D
       42: DOUBLE
       43: I
       44: .testit
      aka: testit
       45: @6
       46: @7
       47: @8
       48: @9
       49: s
       50: d
       51: i
       52: .main
      aka: main
       53: TOC


Hunk: Kind=HUNK_SOURCE_BREAK Name="P300:Users:adriaan:Desktop:Test:UTF8Program.p"(31) ModDate=C7BC7A23

Hunk: Kind=HUNK_LOCAL_CODE Align=4 Class=PR Name=".test"(32) Size=64
00000000: 7C0802A6 mflr r0
00000004: 90010008 stw r0,8(SP)
00000008: 9421FFC0 stwu SP,-64(SP)
0000000C: 90610058 stw r3,88(SP)
00000010: 80620000 lwz r3,M_OUTPUT(RTOC)
00000014: 80810058 lwz r4,88(SP)
00000018: 38A0FFFF li r5,-1
0000001C: 48000001 bl .M_WRITELONGINT
00000020: 60000000 nop
00000024: 80620000 lwz r3,M_OUTPUT(RTOC)
00000028: 48000001 bl .M_WRITELN
0000002C: 60000000 nop
00000030: 80010048 lwz r0,72(SP)
00000034: 38210040 addi SP,SP,64
00000038: 7C0803A6 mtlr r0
0000003C: 4E800020 blr
XRef: Kind=HUNK_XREF_16BIT_IL Offset=$00000010 Class=TC Name="M_OUTPUT"(33)
XRef: Kind=HUNK_XREF_24BIT Offset=$0000001C Class=PR Name=".M_WRITELONGINT"(34)
XRef: Kind=HUNK_XREF_16BIT_IL Offset=$00000024 Class=TC Name="M_OUTPUT"(33)
XRef: Kind=HUNK_XREF_24BIT Offset=$00000028 Class=PR Name=".M_WRITELN"(35)

Hunk: Kind=HUNK_LOCAL_CODE Align=4 Class=PR Name=".testit"(44) Size=108
00000000: 7C0802A6 mflr r0
00000004: DBE1FFF8 stfd fp31,-8(SP)
00000008: 93E1FFEC stw r31,-20(SP)
0000000C: 90010008 stw r0,8(SP)
00000010: 9421FFA0 stwu SP,-96(SP)
00000014: C0020000 lfs fp0,@6(RTOC)
00000018: D0010038 stfs fp0,56(SP)
0000001C: CBE20000 lfd fp31,@7(RTOC)
00000020: 83E10038 lwz r31,56(SP)
00000024: C0210038 lfs fp1,56(SP)
00000028: 48000001 bl .test
0000002C: FC00F81E fctiwz fp0,fp31
00000030: D801003C stfd fp0,60(SP)
00000034: 80610040 lwz r3,64(SP)
00000038: 48000001 bl .test
0000003C: 7FE3FB78 mr r3,r31
00000040: 48000001 bl .test
00000044: 80620000 lwz r3,@8(RTOC)
00000048: 48000001 bl .test
0000004C: 80620000 lwz r3,@9(RTOC)
00000050: 48000001 bl .test
00000054: 80010068 lwz r0,104(SP)
00000058: 38210060 addi SP,SP,96
0000005C: CBE1FFF8 lfd fp31,-8(SP)
00000060: 7C0803A6 mtlr r0
00000064: 83E1FFEC lwz r31,-20(SP)
00000068: 4E800020 blr
XRef: Kind=HUNK_XREF_16BIT Offset=$00000014 Class=TD Name="@6"(45)
XRef: Kind=HUNK_XREF_16BIT Offset=$0000001C Class=TD Name="@7"(46)
XRef: Kind=HUNK_XREF_24BIT Offset=$00000028 Class=PR Name=".test"(32)
XRef: Kind=HUNK_XREF_24BIT Offset=$00000038 Class=PR Name=".test"(32)
XRef: Kind=HUNK_XREF_24BIT Offset=$00000040 Class=PR Name=".test"(32)
XRef: Kind=HUNK_XREF_16BIT Offset=$00000044 Class=TD Name="@8"(47)
XRef: Kind=HUNK_XREF_24BIT Offset=$00000048 Class=PR Name=".test"(32)
XRef: Kind=HUNK_XREF_16BIT Offset=$0000004C Class=TD Name="@9"(48)
XRef: Kind=HUNK_XREF_24BIT Offset=$00000050 Class=PR Name=".test"(32)

Hunk: Kind=HUNK_GLOBAL_CODE Align=4 Class=PR Name=".main"(52) Size=32
00000000: 7C0802A6 mflr r0
00000004: 90010008 stw r0,8(SP)
00000008: 9421FFC0 stwu SP,-64(SP)
0000000C: 48000001 bl .testit
00000010: 80010048 lwz r0,72(SP)
00000014: 38210040 addi SP,SP,64
00000018: 7C0803A6 mtlr r0
0000001C: 4E800020 blr
XRef: Kind=HUNK_XREF_24BIT Offset=$0000000C Class=PR Name=".testit"(44)

Hunk: Kind=HUNK_LOCAL_IDATA Align=2 Class=TD Name="@9"(48) Size=4
00000000: 40 00 00 00 '@...'

Hunk: Kind=HUNK_LOCAL_IDATA Align=2 Class=TD Name="@8"(47) Size=4
00000000: 3F 80 00 00 '?...'

Hunk: Kind=HUNK_LOCAL_IDATA Align=2 Class=TD Name="@7"(46) Size=8
00000000: 3F F0 00 00 00 00 00 00 '?.......'

Hunk: Kind=HUNK_LOCAL_IDATA Align=2 Class=TD Name="@6"(45) Size=4
00000000: 3F 80 00 00 '?...'

Hunk: Kind=HUNK_LOCAL_IDATA Align=4 Class=TC Name="M_OUTPUT"(33) Size=4
00000000: 00 00 00 00 '....'
XRef: Kind=HUNK_XREF_32BIT Offset=$00000000 Class=RW Name="M_OUTPUT"(33)

Hunk: Kind=HUNK_GLOBAL_IDATA Align=4 Class=TC0 Name="TOC"(53) Size=0


Hunk: Kind=HUNK_END

Jonas Maebe

2010-03-09 23:18

manager   ~0035117

Thanks for checking.

Jonas Maebe

2010-03-09 23:34

manager   ~0035119

Those interferences are very nice though, because it means I can also implement it by just inserting explicit typeconversions (and not have to take care that in all cases the exact bit pattern is preserved).

Jonas Maebe

2010-03-10 22:36

manager   ~0035209

There's one thing I don't understand about your test program though: I thought "univ" only allowed passing any argument of the same size to the parameter, but in your example you are passing a double (size 8) to a longint (size 4) parameter. Shouldn't that normally fail at compile time?

Adriaan van Os

2010-03-11 07:03

developer   ~0035227

I think the double case is a peculiarity in the CodeWarrior compiler that we need not (or should not) reproduce.

Adriaan van Os

2010-03-11 08:13

developer   ~0035228

Wirth regard to your remark that the sizes must match.

If the formal univ parameter is a value parameter then univ acts like a value type cast, where sizes can differ for standard type conversions. If the formal univ parameter is a var parameter then univ acts like a variable type-cast (at the left side of the assigment operator) where sizes have to match always.

Adriaan van Os

2010-03-11 08:21

developer   ~0035229

program test;

type
    Int8 = -128..127;
    Int16 = integer;
    Int32 = longint;
    Rec1 = packed record f1, f2: Int8 end;
    Rec2 = packed record f1, f2: Int16 end;
    Rec3 = packed record f1, f2: Int32 end;

procedure test1(l: univ Int32);
begin
    writeln(l)
end;

procedure test2(l: Int32);
begin
    writeln(l)
end;

procedure test3(var l: univ Int32);
begin
    writeln(l)
end;

procedure test4(const l: univ Int32);
begin
    writeln(l)
end;

procedure testit;
var
    s: single;
    d: double;
    i8: Int8;
    i16: Int16;
    i32: Int32;
    r1: rec1;
    r2: rec2;
    r3: rec3;
begin
    s:=1.0;
    d:=1.0;
    i8:=1;
    i16:=1;
    r2.f1:=1;
    r2.f1:=1;
    i32:= Int32( s);
    test1(s);
    test3(s);
    test4(s);
    test1(d);
    test1(i32);
    test2(i32);
    test3(i32);
    test4(i32);
    test1(1.0);
    test4(1.0);
    test1(2.0);
    test4(2.0);
    test1(r2);
    test3(r2);
    test4(r2);
    test1(i8);
    test4(i8);
    test1(i16);
    test4(i16);
    i8:= Int8(i32);
    i8:= Int8(i16);
    i16:= Int16(i32);
    i32:= Int32(i16);
end;

begin
    testit
end.


produces the following output in CodeWarrior


       0
1065353216
       0
       1
1065353216
1065353216
1065353216
1065353216
1065353216
1065353216
1073741824
1073741824
   65664
   65664
   65664
       1
       1
       1
       1

Adriaan van Os

2010-03-11 08:29

developer   ~0035231

Maybe that makes implementation easy (as you wrote) by just adding "typecast" nodes the callparam nodes ?

Jonas Maebe

2010-03-11 10:58

manager   ~0035238

Does this also compile? (I hope not)

procedure test(procedure p(l: univ longint);
begin
end;

procedure p(d: double);
begin
end;

begin
  test(p);
end.

2010-03-11 11:20

 

Adriaan van Os

2010-03-11 11:22

developer   ~0035239

Everything in the attached test program compiles, but most of it is big nonsense. I suggest that for univ parameters of anonymous procedure parameters and typed procedure parameters, we check that the size is correct (as there are no type conversion nodes).

Jonas Maebe

2010-03-13 23:17

manager   ~0035464

Last edited: 2010-03-14 13:16

As discussed above, FPC only allows procvar compatibility with univ parameters if the sizes match exactly. However, even in that case you can end up with crashing code in case one of the types is passed via the stack and the other isn't (e.g. with single and longint on Darwin/i386: longint is passed in a register and single via the stack), because then the callee will pop the wrong number of bytes off of the stack at the end.

Therefore FPC will issue a warning in *most* cases (in particular, it does not check whether two different records are passed in the same way) where problems *could* occur on one platform or another (i.e., even when it might be safe on the current platform) when (ab)using univ for implicit procvar conversions.

Adriaan van Os

2010-03-14 09:29

developer   ~0035481

Thanks for implementing UNIV. You may want to add the two testsuite files from my original patch.

A side-effect is (of course) that the compiler will now catch a forward-declaration mismatch where univ is missing in one of the declarations.

Do you want to update the mode MacPas Wiki or shall I do that ?

Adriaan van Os

2010-03-14 09:39

developer   ~0035482

I mean, with UNIV added to those tests, of course.

Jonas Maebe

2010-03-14 12:04

manager   ~0035486

> You may want to add the two testsuite files from my original patch.

Done

> Do you want to update the mode MacPas Wiki or shall I do that ?

Please feel free to do that, thanks.

Issue History

Date Modified Username Field Change
2010-02-15 18:47 Adriaan van Os New Issue
2010-02-15 18:47 Adriaan van Os File Added: fpc-procparam.diff.zip
2010-03-09 20:55 Jonas Maebe Status new => assigned
2010-03-09 20:55 Jonas Maebe Assigned To => Jonas Maebe
2010-03-09 21:25 Jonas Maebe Note Added: 0035107
2010-03-09 21:25 Jonas Maebe Status assigned => feedback
2010-03-09 22:49 Adriaan van Os Note Added: 0035114
2010-03-09 23:18 Jonas Maebe Note Added: 0035117
2010-03-09 23:18 Jonas Maebe Status feedback => assigned
2010-03-09 23:34 Jonas Maebe Note Added: 0035119
2010-03-10 22:36 Jonas Maebe Note Added: 0035209
2010-03-11 07:03 Adriaan van Os Note Added: 0035227
2010-03-11 08:13 Adriaan van Os Note Added: 0035228
2010-03-11 08:21 Adriaan van Os Note Added: 0035229
2010-03-11 08:29 Adriaan van Os Note Added: 0035231
2010-03-11 10:58 Jonas Maebe Note Added: 0035238
2010-03-11 11:20 Adriaan van Os File Added: testunivprocparams.p.zip
2010-03-11 11:22 Adriaan van Os Note Added: 0035239
2010-03-13 23:17 Jonas Maebe Fixed in Revision => 15010
2010-03-13 23:17 Jonas Maebe Status assigned => resolved
2010-03-13 23:17 Jonas Maebe Fixed in Version => 2.5.1
2010-03-13 23:17 Jonas Maebe Resolution open => fixed
2010-03-13 23:17 Jonas Maebe Note Added: 0035464
2010-03-14 09:29 Adriaan van Os Status resolved => feedback
2010-03-14 09:29 Adriaan van Os Resolution fixed => reopened
2010-03-14 09:29 Adriaan van Os Note Added: 0035481
2010-03-14 09:39 Adriaan van Os Note Added: 0035482
2010-03-14 12:04 Jonas Maebe Note Added: 0035486
2010-03-14 12:09 Jonas Maebe Status feedback => resolved
2010-03-14 12:09 Jonas Maebe Resolution reopened => fixed
2010-03-14 12:18 Jonas Maebe FPCOldBugId => 0
2010-03-14 12:18 Jonas Maebe Severity minor => feature
2010-03-14 13:16 Jonas Maebe Note Edited: 0035464
2010-03-14 17:52 Adriaan van Os Status resolved => closed