Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

SetLength / Move - causes memory corruption

Today I've stumbled on a problem which causes my array to be corrupted. Here is a reproducible test case:

unit Unit40;

interface

type
  TVertex = record
    X, Y: Double;
  end;

  TEdge = record
    V1, V2: TVertex;
  end;
  TEdges = array of TEdge;

type
  TBoundryInfo = array of TEdges;

procedure MemoryCorrupt;

implementation

procedure MemoryCorrupt;
var
  BoundryInfo: TBoundryInfo;
  i, PointIndex, BoundryLength: Integer;
begin
  BoundryLength := 57;
  PointIndex := 0;
  SetLength(BoundryInfo, BoundryLength);
  for i := 0 to BoundryLength - 1 do
  begin
    if i <> 17 then
    begin
      SetLength(BoundryInfo[i], 1);
      BoundryInfo[i][0].V1.X := 1;
      BoundryInfo[i][0].V2.X := 1;
      BoundryInfo[i][0].V1.Y := 1;
      BoundryInfo[i][0].V2.Y := 1;
    end else
    begin
      SetLength(BoundryInfo[i], 2);
      BoundryInfo[i][0].V1.X := 1;
      BoundryInfo[i][0].V2.X := 1;
      BoundryInfo[i][0].V1.Y := 1;
      BoundryInfo[i][0].V2.Y := 1;
      BoundryInfo[i][1].V1.X := 1;
      BoundryInfo[i][1].V2.X := 1;
      BoundryInfo[i][1].V1.Y := 1;
      BoundryInfo[i][1].V2.Y := 1;
    end;
  end;
  BoundryLength := 9;
  SetLength(BoundryInfo, BoundryLength);
  Move(BoundryInfo[PointIndex+1], BoundryInfo[PointIndex],
    ((BoundryLength - 1) - PointIndex) * SizeOf(BoundryInfo[PointIndex]));
  Dec(BoundryLength);
  Finalize(BoundryInfo[BoundryLength]);
  SetLength(BoundryInfo, BoundryLength); //After this, arrays contains garbage
  BoundryInfo[0][0].V1.X := 3;
end;

end.

I guess memory corruption after last SetLength is only a symptom of bad usage of Move. Could someone explain to me what am I doing wrong and how to properly use Move in this case?

In original problem I am removing elements from BoundryInfo in a loop, that is why I am calling Finalize(BoundryInfo[BoundryLength])

like image 501
Wodzu Avatar asked Aug 24 '12 14:08

Wodzu


2 Answers

In your code,

Move(BoundryInfo[PointIndex+1], BoundryInfo[PointIndex], 
  ((BoundryLength - 1) - PointIndex) * SizeOf(BoundryInfo[PointIndex]));

Will copy the pointer of BoundryInfo[PointIndex+1] to BoundryInfo[PointIndex]. This pointer is another dynamic array, you have to take care of reference counting.

That is:

SetLength(BoundryInfo[PointIndex],0); // release memory
Move(BoundryInfo[PointIndex+1], BoundryInfo[PointIndex], 
  ((BoundryLength - 1) - PointIndex) * SizeOf(BoundryInfo[PointIndex]));
PPointerArray(BoundryInfo)^[BoundryLength-1] := nil; // avoid GPF

In short:

  • Finalize the item which will be overriden during move();
  • Write nil to the latest item, which is duplicated by the move().
like image 162
Arnaud Bouchez Avatar answered Nov 20 '22 15:11

Arnaud Bouchez


By using Move and subverting the dynamic array reference counting mechanism you are simply setting a trap for yourself. I would strongly recommend that you stick within the standard mechanisms, and let the compiler worry about the details. It will get them right every time.

for i := 0 to high(BoundaryInfo)-1 do
  BoundaryInfo[i] := BoundaryInfo[i+1];
SetLength(BoundaryInfo, Length(BoundaryInfo)-1);
like image 28
David Heffernan Avatar answered Nov 20 '22 15:11

David Heffernan