View Issue Details

IDProjectCategoryView StatusLast Update
0038551FPCCompilerpublic2021-02-28 23:23
Reporterjamie philbrook Assigned ToJonas Maebe  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionwon't fix 
Product Version3.2.0 
Summary0038551: Inline is broken for referenencing the return object for 3.2.x, 3.0.4 and down plus Delphi works as expected.
DescriptionIt has been a common practice at least for me to use the INLINE option in Delphi so that a pointer is generated to the return class instance or pointer and it has been working the same way in 3.0.4 until 3.2.0 which has now broke it because it admits the first line of ASM code needed for the reference to the return object, now..

  I have stopped converting a large C++ app that is full of reference returns and has been working with 3.0.4 and Delphi, it no longer works due to this error which apparently seems to be ok to some of the DEV's.

 I spent a few hours moving the partial converted project over to Delphi now where this still works as expected.
Steps To ReproduceFunction Test(A:integer):Tform1; inline;
Begin
  Result.Caption := A.Tostring;
end;

procedure TForm1.Button1Click(Sender: TObject);
begin
  form1 := Test(1);
end;

The above is just a simplified example of the problem.
3.0.4 works, Delphi works
3.2.0 does not.
you can examine the difference in ASM code, one line Is admitted, the first one that is needed.
TagsNo tags attached.
Fixed in Revision
FPCOldBugId
FPCTarget-
Attached Files

Activities

Jonas Maebe

2021-02-27 16:18

manager   ~0129202

Please (always) provide a complete, compilable program.

Martok

2021-02-27 16:33

reporter   ~0129203

You know I rarely agree with Core, but in this case, that really should not work. The semantics of returning pointers are weird as hell and sometimes buggy anyway, and I'm pretty sure it works in Delphi only because they need this to be able to return managed types without having to allocate 2 tempvars as FPC does.

The proper way to do that in FPC with the same register allocation would be something explicit like this, even if it looks more like Fortran... or COM in C++ without Delphis sugar, now that I think about it.

procedure Test(var Result: TForm1; A:integer); inline;
Begin
  Result.Caption := A.Tostring;
end;

procedure TForm1.Button1Click(Sender: TObject);
begin
  Test(Form1, 1);
end;

runewalsh

2021-02-27 16:37

reporter   ~0129204

Last edited: 2021-02-27 16:41

View 2 revisions

Even when 'result' is a record or managed type, it should never be considered to have the value it had at caller's side, nor 'inline' change its semantics (as 'inline' can be ignored, or procedure not marked as 'inline' still inlined).

And that was about a more safe case when you assume complex 'result' to behave as var-parameter. Assumption that pointer-sized 'result' will be equal to the target of the assignment (or even to caller's 'Self'?) is crazy and you don't have the right to blame anyone when (not even 'if', but 'when'!) it stops working.

Jonas Maebe

2021-02-27 16:40

manager   ~0129205

Looking at the code rather than whether or not it can compile, that does not make any sense at all. The result of "Test" is not initialised anywhere, so this is undefined behaviour. Inlining, optimisation settings, different platforms, and indeed switching to a new compiler version can hence all result in different behaviour.

jamie philbrook

2021-02-27 17:03

reporter   ~0129207

Last edited: 2021-02-27 17:04

View 2 revisions

I see, well Delphi handles this fine..

Please check closer ! The INLINE supplies the pointer to the return object

if you don't want to fix this fine.. I'll just migrate my moon lighting work all over to Delphi because 3.0.4 is where it stops for me..

Thank you for the ride, it was great while it lasted.

Jonas Maebe

2021-02-27 17:27

manager   ~0129209

> The INLINE supplies the pointer to the return object
No, it does not. That is not what "inline" means.

> Thank you for the ride, it was great while it lasted.
You're welcome.

jamie philbrook

2021-02-27 23:19

reporter   ~0129218

Really,, then how do you explain the miles of code I have that uses this technique ? Does the reference pointer just come out of thin air ? I don't think so..

I've had a good look at the ASM code and it's obvious from my point of view, I guess you don't see it..

The Delphi change over is doing well btw. All forms of that code is working as expected and I am removing the parts of fpc code that Delphi does not support.
I've also found the generics seem to be working differently with Delphi, I can't explain that one but if I find it to be some in capability issue I will be sure to report it here in a separate report..
 Thank you.

runewalsh

2021-02-27 23:48

reporter   ~0129219

>how do you explain the miles of code I have that uses this technique?
Easily, you don't know basic ground rules of programming, the main of which is “Everything not defined is undefined.”
https://devblogs.microsoft.com/oldnewthing/20060320-13/?p=31853

You cannot require from ‘inline’ to do something it wasn't (and couldn't ever be) designed for, even if it works by pure chance. Removing ‘inline’ isn't assumed to change semantics of the code, compiler can ignore ‘inline’ if it wants to, or auto-inline procedures not explicitly marked with ‘inline’.

J. Gareth Moreton

2021-02-28 05:43

developer   ~0129220

Last edited: 2021-02-28 05:44

View 2 revisions

It's best to treat "inline" as a hint to the compiler and nothing more - if you ignore its presence for a moment and look at the procedure:

Function Test(A:integer):Tform1; inline;
Begin
  Result.Caption := A.Tostring;
end;

"Result" is undefined and could be anything. If by coincidence it is equal to Form1 when called from an event triggered by Form1, it's not going to equal that if you call Test from some other subroutine that isn't a method of TForm1.

Consider the compiled assembly language. On i386/x86_64, Result is in EAX/RAX by default. What is this register set to when you enter the procedure? If compiled on i386, it will equal 1 because the default calling convention uses EAX for the first integral parameter as well as the return value (which is, internally, a pointer, hence integral). On x86_64, since RAX is only used for the result, it is equal to whatever value it happened to hold before entering the subroutine.

For inline, it merges the subroutine into the caller at the node level, allowing more efficient use of registers and stack space. There is nothing to instruct the compiler what to set Result to beforehand and it is by all accounts an initialised local variable, and it will simply take on the value of the register or stack space assigned to it. If that value happens to equal the calling object, that is just a complete coincidence.

If you want to see how dangerous it is despite it working in FPC 3.0.4, try compiling it on FPC 3.0.4 with the -gt option (trash variables). I'm sure Delphi has something similar.

It would be sad to see you go though over a semantic issue though.

jamie philbrook

2021-02-28 14:41

reporter   ~0129236

Well it kind of puts a fork into my usage of the fpc because as I think about this one I also do lots of reference accessing of large data types on the return because the compiler generates a reference to it so I don't need to use Inline to get the reference needed.

Type
   TmyLargeType = Record
       A:Array[0..10000000] of Byte;
  Endl

Function Test:TmyLargeType;
  Begin
    Result.A[500] := What_Ever;
End;
-----
Var
 A:TMylargeType
begin
  A := Test;
End;
 I do this with Records, Objects and array types . I have a vary large code base using tactics like this, it saves a bundle on code space and speed.

Is this also in danger of going away too ?

maybe we need what I suggested before …

Function name(….):Var TForm1; ///to force a reference to any simple type to make the compiler behave like it does now with large types on return..

In other languages, line C etc this is very common for example..

It would be interesting to see what the response to this is..

runewalsh

2021-02-28 15:15

reporter   ~0129240

Last edited: 2021-02-28 15:22

View 3 revisions

>In other languages, line C etc this is very common for example

No it isn't. Personally I see this for the first (and hope the last) time ever, and use the compiler in wild enough ways to have faced a dozen of code-generation bugs like 0038129. Just use 'var' parameter If it can't be a method outright, now that we have ADVANCEDRECORDS.

jamie philbrook

2021-02-28 17:23

reporter   ~0129243

All I can say is "Unbelievable"
I think I'll just close this report off and spend my time elsewhere.
happy camping everyone.

Florian

2021-02-28 23:23

administrator   ~0129261

As said by other before: exploring undefined is not supported.

Issue History

Date Modified Username Field Change
2021-02-27 16:12 jamie philbrook New Issue
2021-02-27 16:18 Jonas Maebe Note Added: 0129202
2021-02-27 16:33 Martok Note Added: 0129203
2021-02-27 16:37 runewalsh Note Added: 0129204
2021-02-27 16:40 Jonas Maebe Assigned To => Jonas Maebe
2021-02-27 16:40 Jonas Maebe Status new => resolved
2021-02-27 16:40 Jonas Maebe Resolution open => won't fix
2021-02-27 16:40 Jonas Maebe FPCTarget => -
2021-02-27 16:40 Jonas Maebe Note Added: 0129205
2021-02-27 16:41 runewalsh Note Edited: 0129204 View Revisions
2021-02-27 17:03 jamie philbrook Note Added: 0129207
2021-02-27 17:04 jamie philbrook Note Edited: 0129207 View Revisions
2021-02-27 17:27 Jonas Maebe Note Added: 0129209
2021-02-27 23:19 jamie philbrook Note Added: 0129218
2021-02-27 23:48 runewalsh Note Added: 0129219
2021-02-28 05:43 J. Gareth Moreton Note Added: 0129220
2021-02-28 05:44 J. Gareth Moreton Note Edited: 0129220 View Revisions
2021-02-28 14:41 jamie philbrook Note Added: 0129236
2021-02-28 15:15 runewalsh Note Added: 0129240
2021-02-28 15:22 runewalsh Note Edited: 0129240 View Revisions
2021-02-28 15:22 runewalsh Note Edited: 0129240 View Revisions
2021-02-28 17:23 jamie philbrook Note Added: 0129243
2021-02-28 17:24 jamie philbrook Status resolved => closed
2021-02-28 23:23 Florian Note Added: 0129261