Delphi XE8 bug in TList<T>, need workaround

Question

After upgrading to XE8 some of our projects start to break data. Looks like a bug in TList realization.

program XE8Bug1;
{$APPTYPE CONSOLE}

uses
  System.SysUtils, Generics.Collections;

type
  TRecord = record
    A: Integer;
    B: Int64;
  end;

var
  FRecord: TRecord;
  FList: TList<TRecord>;

begin
  FList := TList<TRecord>.Create;
  FRecord.A := 1;
  FList.Insert(0, FRecord);
  FRecord.A := 3;
  FList.Insert(1, FRecord);
  FRecord.A := 2;
  FList.Insert(1, FRecord);
  Writeln(IntToStr(FList[0].A) + IntToStr(FList[1].A) + IntToStr(FList[2].A));

end.

This code prints "123" in XE7 and before (as it should be), but in XE8 it prints "120". Maybe someone know a quickfix for this?

Update: unofficial fix is here

Jarvis

The generic collections have been re-implemented in XE8. Perhaps they don't have any unit tests at Emba. If this is as you describe, and it seems likely, your solution is to remain on XE7. You do need to submit a bug report. – Scrawny

Reported as Regression: TList<T>.Insert not working. – Dashpot

So it seems that Embarcadero don't have an effective testing regime. How on earth could they have got this wrong? Such a fundamental class. A well run dev team would have unit tested this comprehensively. Such a fault should never get past that testing. Dismal. – Scrawny

And I'd have to pay extra to get this (4-week old) product bug-fixed. – Thom

Related: #30851411 I guess they figured that nobody needs to use Insert so they never tested it. – Barium

@JerryDodge, fmx.TlistView seems to use TList<T> internally with sizes of T > 8. I did not bother to see if any of them uses the Insert method though. You could trace with the debugger with a breakpoint at TListHelper.InternalInsertN if you like. – Dashpot

This is fixed in XE8 upd 1, including some errors in TListHelper. – Dashpot

Answer 1

I found that now the TList<T>.Insert method call TListHelper.InternalInsertX depends on the data size, in my case:

procedure TListHelper.InternalInsertN(AIndex: Integer; const Value);
var
  ElemSize: Integer;
begin
  CheckInsertRange(AIndex);

  InternalGrowCheck(FCount + 1);
  ElemSize := ElSize;
  if AIndex <> FCount then
    Move(PByte(FItems^)[AIndex * ElemSize], PByte(FItems^)[(AIndex * ElemSize) + 1], (FCount - AIndex) * ElemSize);
  Move(Value, PByte(FItems^)[AIndex * ElemSize], ElemSize);
  Inc(FCount);
  FNotify(Value, cnAdded);
end;

I see the problem in the first Move call. Destination should be:

PByte(FItems^)[(AIndex + 1) * ElemSize]

not

PByte(FItems^)[(AIndex * ElemSize) + 1]

Aaargh!

Finally, I've used the System.Generics.Defaults.pas and System.Generics.Collections.pas units from Delphi XE7 in my projects, and now all works as expected.

Update: as I see, RTL not affected, as it isn't use TList<T>.Insert for T with SizeOf > 8 (or maybe I miss something?)

Jarvis

Fixing by restoring the XE7 implementation is not such a bad idea. You have more trust in it. Yes you can patch this one method, but don't you wonder what else they got wrong? – Scrawny

Although if you replace the entire units, it would be wise to do so in a way that means the RTL/VCL/FMX etc. also use the fixed units. Otherwise you may be hurt by other manifestations of this defect. – Scrawny

Mantra of the inexperienced developers: "why care to look at the old code, when you can write new code in a 'much better' way" ;-) (I have had too many of that kind of developers, and it is disappointing that Embarcadero have them at all) – Oyer

There are many many places in the RTL where TList<T>.Insert is used. How on earth could this have passed undetected? – Dashpot

This bug persists only when SizeOf(T) > 8, so there's only a few places in RTL affected, most of them rounded with {$IFDEF POSIX} :) – Jarvis

@Jarvis SizeOf(T)>8 and the type is unmanaged. Perhaps Emba get away with it by way of preferring dyn allocated classes to automatically allocated records. – Scrawny