Delphi record assignment bug
Asked Answered
U

1

7

I've encountered some strange behaviour of delphi XE3 compiler (i compile for x86 architecture).

Imagine i have class with one field - custom record with several field of simple types:

  TPage = class
  type
    TParagraph = record
    public
      FOwner:  TPage;

      FFirst:  Integer;
      FSecond: Integer;

      procedure Select;
    end;
  public
    FSelected: TParagraph;
  end;

procedure TPage.TParagraph.Select;
begin
  FOwner.FSelected:=Self;
end;

The logic is my page can contain several paragraphs and in some moments i want one of these paragraphs become selected (to be able to do some actions with it in other parts of my programm):

procedure TMainForm.Button1Click(Sender: TObject);
var
  lcPage:      TPage;
  lcParagraph: TPage.TParagraph;
begin
  lcPage:=TPage.Create;
  try
    <...>

    lcParagraph.FOwner:=lcPage;
    lcParagraph.FFirst:=1;
    lcParagraph.FSecond:=2;

    lcParagraph.Select;

    <...>
  finally
    lcPage.Free;
  end;

Everything is ok when my record do not exceed some size. One reference and two integers is ok, in this case i get assembler instructions like this:

MainUnit.pas.350: FOwner.FSelected:=Self;
00C117B3 8B45FC           mov eax,[ebp-$04]
00C117B6 8B00             mov eax,[eax]
00C117B8 8B55FC           mov edx,[ebp-$04]
00C117BB 8B0A             mov ecx,[edx]
00C117BD 894804           mov [eax+$04],ecx
00C117C0 8B4A04           mov ecx,[edx+$04]
00C117C3 894808           mov [eax+$08],ecx
00C117C6 8B4A08           mov ecx,[edx+$08]
00C117C9 89480C           mov [eax+$0c],ecx

I can see three correct movs which copies memory from local record to class`.

But! If i add more fields to my record, the generated asm code changes and the record assignment no longer performs correctly.

    TParagraph = record
    public
      FOwner:  TPage;

      FFirst:  Integer;
      FSecond: Integer;
      FThird:  Integer;

      procedure Select;
    end;
MainUnit.pas.350: FOwner.FSelected:=Self;
00C117C9 8B45FC           mov eax,[ebp-$04]
00C117CC 8B55FC           mov edx,[ebp-$04]
00C117CF 8B12             mov edx,[edx]
00C117D1 8BF2             mov esi,edx
00C117D3 8D7A04           lea edi,[edx+$04]
00C117D6 A5               movsd 
00C117D7 A5               movsd 
00C117D8 A5               movsd 
00C117D9 A5               movsd 

And then i get garbage in class' record FSelected: enter image description here

After the lea instuction the CPU state is such:

enter image description here

In this example 02D37280 is the address of my lcPage class, so 02D37284 should contain start of its field - FSelected record. But movsd instruction copies memory from ESI to EDI, from 02D37280 to 02D37284, which make absolutly no sense! If i change ESI register to value of EAX (19F308), which is start of my local lcParagraph variable, the copy is performed properply: enter image description here

Is what i described is known bug? Or am i missing something fundamental about delphi? Is this a good way to assign records? I can easily workaround the problem, for example, by changing FOwner.FSelected:=Self; to CopyMemory(@FOwner.FSelected, @Self, SizeOf(Self)); in procedure TPage.TParagraph.Select;. But i want to figure out what is wrong.

Minimal reproducible example:

program RecordAssignmentIssue;

{$APPTYPE CONSOLE}

{$R *.res}

uses
  System.SysUtils;

type
  TPage = class
  type
    TParagraph = record
    public
      FOwner:  TPage;

      FFirst:  Integer;
      FSecond: Integer;
      FThird:  Integer;

      procedure Select;
    end;
  public
    FSelected: TParagraph;
  end;

procedure TPage.TParagraph.Select;
begin
  FOwner.FSelected:=Self;
end;

var
  lcPage:      TPage;
  lcParagraph: TPage.TParagraph;
begin
  try
    lcPage:=TPage.Create;
    try
      lcParagraph.FOwner:=lcPage;
      lcParagraph.FFirst:=1;
      lcParagraph.FSecond:=2;
      lcParagraph.FThird:=3;

      lcParagraph.Select;

      Assert(CompareMem(@lcPage.FSelected, @lcParagraph, SizeOf(lcParagraph)));
      // get rid of FThird and assertion will pass
    finally
      lcPage.Free;
    end;
  except
    on E: Exception do
      Writeln(E.ClassName, ': ', E.Message);
  end;
end.
Ultraviolet answered 8/11, 2021 at 11:43 Comment(11)
Can you provide a minimal reproducible exampleYuriyuria
Yes, i've edited original post.Ultraviolet
I can reproduce this in XE7 and yes I think that it is a bug. I would comment that the design of the code looks a bit off to me. I'm not sure that the method belongs on the record. I'd make it a method of TPage that is passed a TParagraph. Doing that will workaround the bug. You can also workaround it by putting the record type outside the class. So this looks like an issue with nested type declarations. I wouldn't be surprised if the bug was present even in the latest versions of Delphi. It's also 32 bit only, no problem in 64 bit, even with a much longer record type.Yuriyuria
Same error in Delphi 11.Latoya
Thanks for testing it in other versions of delphi. Now i know it is not my misunderstanding. This example with TPage and TParagraph is not from real production code, but is simplified and abstracted for illustration purpose. Maybe the real production code logic would look more reasonable (in part of whom should belong Select method). But it is not nested record issue. If you change declarations so TParagraph became global record, assertion still wont pass.Ultraviolet
"If you change declarations so TParagraph became global record, assertion still wont pass." It does pass for me in XE7. Note that I don't have easy access to XE3. Note also that I was not suggesting it becomes a global variable. Just that the type declaration is moved to be outside TParagraph. I'd be astounded if that doesn't also resolve the issue even in older versions of Delphi.Yuriyuria
"Just that the type declaration is moved to be outside TParagraph." - yes, i meant exactly this. "Global" record declaration in comparsion to "local" (as for class), nested declaration. If in that case the assertion is passed, then some progress between delphi versions definitely was made :-)Ultraviolet
@DavidHeffernan Moving the TParagraph declaration outside TPage fails in Delphi 11.Latoya
I'm a little surprised by that, but I'm sure you are right. What happens if you add a private method named select to TPage like this: procedure TPage.Select(const Paragraph: TParagraph); begin FSelected := Paragraph; end; and then implement TParagraph.Select like this: procedure TPage.TParagraph.Select; begin FOwner.Select(Self); end; Yuriyuria
@LURD I retested, and it was my mistake. Moving the TParagraph declaration outside TPage fails in XE7 too.Yuriyuria
does the record has to be packed? because the bug report on Emba shows a packed record. quality.embarcadero.com/browse/RSP-36156Rambow
Y
5

This is a bug that is still present in Delphi 11 (thanks to LU RD for confirming that). You should submit a bug report to Quality Portal.

In the meantime, I think that you can work around it by making the assignment in TPage rather than TParagraph. Like this:

program RecordAssignmentIssue;

{$APPTYPE CONSOLE}

uses
  System.SysUtils;

type
  TPage = class
  type
    TParagraph = record
    public
      FOwner:  TPage;

      FFirst:  Integer;
      FSecond: Integer;
      FThird:  Integer;

      procedure Select;
    end;
  private
    procedure Select(const Paragraph: TParagraph);
  public
    FSelected: TParagraph;
  end;

procedure TPage.TParagraph.Select;
begin
  FOwner.Select(Self);
end;

{ TPage }

procedure TPage.Select(const Paragraph: TParagraph);
begin
  FSelected:=Paragraph;
end;

var
  lcPage:      TPage;
  lcParagraph: TPage.TParagraph;

begin
  try
    lcPage:=TPage.Create;
    try
      lcParagraph.FOwner:=lcPage;
      lcParagraph.FFirst:=1;
      lcParagraph.FSecond:=2;
      lcParagraph.FThird:=3;

      lcParagraph.Select;

      Assert(CompareMem(@lcPage.FSelected, @lcParagraph, SizeOf(lcParagraph)));
      // get rid of FThird and assertion will pass
    finally
      lcPage.Free;
    end;
  except
    on E: Exception do
      Writeln(E.ClassName, ': ', E.Message);
  end;
end.

Or another very simple workaround is just to introduce an extra local pointer variable to hold a pointer to Self:

procedure TPage.TParagraph.Select;
var
  P: ^TParagraph;
begin
  P := @Self;
  FOwner.FSelected := P^;
end;
Yuriyuria answered 8/11, 2021 at 14:37 Comment(1)
Reported RSP-36156 Record assignment failsLatoya

© 2022 - 2024 — McMap. All rights reserved.