mORMot Open Source

Sharky

Member

Registered: 2015-06-18

Posts: 5

Bug in TDynArray.AddArray (SynCommons.pas)

The TDynArray.AddArray method is not working at all. I have tried several versions.

The main problem starts here.

Stable:
procedure TDynArray.AddArray(const DynArray; aStartIndex: integer=0; aCount: integer=-1);
var n: integer;
    D: PPointer;
    PS,PD: pointer;
begin
...
    n := PInteger(PtrUInt(D^)-sizeof(Integer))^;

Nightly:
procedure TDynArray.AddArray(const DynArray; aStartIndex: integer=0; aCount: integer=-1);
var DynArrayCount, n: integer;
    D: PPointer;
    PS,PD: pointer;
begin
...
    DynArrayCount := DynArrayLength(D^);

const DynArray is the source Array. The code tries to determinate the length of the source array.

I've tried copying an array with 2 items to another with 3 items, so the source array has 2 records. They are of the same type. I'm using Delphi 2009.

DestArray.AddArray(SourceArray);

Both the stable code (n :=) and the nightly code (DynArrayCount :=) calculate the value as something extremely high (3625175) in my case, which brings the code to fail for memory issues.

edit:

I see this in 1.18 release notes:

  - fixed TDynArray.AddArray() method when Count parameter is not specified

So either it was broken again, or the fix didn't really work. I've tried to manually specify the count parameter. No memory error, but empty records are copied to the array.

Last edited by Sharky (2015-06-18 21:22:32)

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,666

Website

Re: Bug in TDynArray.AddArray (SynCommons.pas)

I'm not able to reproduce the problem.

The TTestLowLevelCommon._TDynArray tests in SynSelfTests.pas do pass with no problem for TDynArray.AddArray().

Have you code to reproduce the issue?
I suspect what you call an "array" (i.e. SourceArray) may not be a dynamic array.
Also note that both source and destination arrays should be of the very same type.

Sharky

Member

Registered: 2015-06-18

Posts: 5

Re: Bug in TDynArray.AddArray (SynCommons.pas)

// Delphi 2009

unit Unit1;

interface

uses
  Windows, Messages, SysUtils, Variants, Classes, Graphics, Controls, Forms,
  Dialogs, StdCtrls, SynCommons;

type
  TForm1 = class(TForm)
    Button1: TButton;
    procedure Button1Click(Sender: TObject);
  private
    { Private declarations }
  public
    { Public declarations }
  end;

  rDataItem = record
    Modified: TDateTime;
    Data: string;
  end;

  TDataItems = array of rDataItem;

var
  Form1: TForm1;
  dyn1: TDynArray;
  dyn1Array: TDataItems;
  dyn2: TDynArray;
  dyn2Array: TDataItems;

implementation

{$R *.dfm}

procedure TForm1.Button1Click(Sender: TObject);
var rec: rDataItem;
begin

  rec.Modified := Now;
  rec.Data := '1';
  dyn1.Init(TypeInfo(TDataItems),dyn1Array);
  dyn1.Add(rec);

  rec.Modified := Now;
  rec.Data := '2';
  dyn2.Init(TypeInfo(TDataItems),dyn2Array);
  dyn2.Add(rec);

  dyn2.AddArray(dyn1);
  Caption := inttostr(dyn2.Count); // gives 1 instead of 2
  //Button1.Caption := dyn2Array[1].Data;
end;

end.

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,666

Website

Re: Bug in TDynArray.AddArray (SynCommons.pas)

You should write

dyn2.AddArray(dyn1Array);

As I expected, you did not pass a dynamic array, but a TDynArray instance!

Sharky

Member

Registered: 2015-06-18

Posts: 5

Re: Bug in TDynArray.AddArray (SynCommons.pas)

I have tried that of course before, but it gives an access violation, so I thought I was wrong:

http://i.imgur.com/NahLXzg.png

The crash happens here:

procedure TDynArray.AddArray(const DynArray; aStartIndex: integer=0; aCount: integer=-1);
var n: integer;
    D: PPointer;
    PS,PD: pointer;
begin
...
   CopyArray(PD,PS,ArrayType,aCount);

edit:

And it gives access violation at call System.@CopyArray

I've also tried the nightly from today, no changes.

Last edited by Sharky (2015-06-19 09:25:20)

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,666

Website

Re: Bug in TDynArray.AddArray (SynCommons.pas)

There was indeed an issue.
I've fixed TDynArray.Slice/AddArray/AddDynArray methods with managed types and added associated regression tests.
See http://synopse.info/fossil/info/72f4149170

I've also introduced a new TDynArray.AddDynArray() method, and updated the documentation, to be clear about the expected parameter types.

Thanks for the feedback!

Sharky

Member

Registered: 2015-06-18

Posts: 5

Re: Bug in TDynArray.AddArray (SynCommons.pas)

Thanks for the fast fix!!

Sharky

Member

Registered: 2015-06-18

Posts: 5

Re: Bug in TDynArray.AddArray (SynCommons.pas)

I have a small suggestion.

DynArray.Copy should check if the array to be copied is empty (initialized, but 0 items in array) instead of giving access violation.

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,666

Website

Re: Bug in TDynArray.AddArray (SynCommons.pas)

Should be fixed by http://synopse.info/fossil/info/ac0bcd56f2