Isn't it dangerous to use the Longint count with the Int64 size in Stream.read?

9
votes

I was examining the TMemoryStream class and found the following routine:

procedure TMemoryStream.LoadFromStream(Stream: TStream);
var
  Count: Longint;
begin
  Stream.Position := 0;
  Count := Stream.Size; // <-- assigning Int64 to Longint
  SetSize(Count);
  if Count <> 0 then Stream.ReadBuffer(FMemory^, Count);
end;

I have seen this pattern a lot where an Int64 is assigned to a Longint.

My understanding is that Longint is four bytes and Int64 is eight bytes in both 32-bit and 64-bit Windows, so if my file size is $1 FFFF FFFF == 8589934591 == 8 GB then this routine will simply fail to read because the final count will be $ FFFF FFFF == -1.

I do not understand how this is allowed and maybe not taken into consideration (maybe not many people are trying to read an 8+ GB file).

1
delphidelphi-10-seattle
FWIW: Longint only has a range to + ~2GB when in "4 byte" mode. – user2864740
@user2864740 exactly a plus than 2Gb will fail also by this routine. it is just confusing what I see here. – Nasreddine Galfout
I'd call it a bug. I noticed that "unsafe typecast" warning is set to False by default. But even after setting it to True, I got no warning for the clearly unsafe assignment (even with explicit typecast). In my experience, Delphi has always been dodgy with it's warnings. I know var and out parameters wreaked havoc with "uninitialised warnings" in older versions. And it doesn't warn about with :P – Disillusioned
TMemoryStream doesn't support allocating more than 2GB, not even in a 64bit application. So, even though TStream.Size is an Int64, TMemoryStream limits it to 2GB. So you can't load a >2GB file into a TMemoryStream. If you need that (why?), you will have to write your own stream class that supports >2GB, or find a 3rd party stream class that does. – Remy Lebeau
I wouldn't do it anyway. I would use file mapping. – Victoria

1 Answers

1
votes

I logged a ticket for this and it has apparently been fixed in Tokyo 10.2. This is an issue for 64 bit compilation.

https://quality.embarcadero.com/browse/RSP-19094

There are problems with large (>2GB) files in both TCustomMemoryStream and TMemoryStream. In TMemoryStream the issues are simple as the local variables need to be declared as NativeInt instead of LongInt's and Capacity needs to be changed to an NativeInt. In TCustomMemoryStream they are more subtle because both TCustomMemoryStream.Read methods assign the result of an Int64 - Int64 calculation directly to a LongInt. This will overflow even when the result of this calculation isn't larger than a LongInt.

If you want to fix this in Seattle then you will need to either do a code hook, replace the System.Classes unit or roll out your own replacement class for TMemoryStream. Bear in mind that for the last option, you will need to also replace TBytesStream and TStringStream because these descend from TMemoryStream.

The other problem with the the last option is that third party components won't have your "fixes". For us, we only had a couple of places that needed to work with files larger than 2GB so we switched those across.

The fixes for TCustomMemoryStream.Read (must be to both methods) will look something like this:

function TCustomMemoryStream.Read(var Buffer; Count: Longint): Longint;
{ These 2 lines are new }
var
  remaining: Int64;
begin
  if (FPosition >= 0) and (Count >= 0) then
  begin
    remaining{Result} := FSize - FPosition;
    if remaining{Result} > 0 then
    begin
      if remaining{Result} > Count then 
        Result := Count
      else
        Result := remaining;
      Move((PByte(FMemory) + FPosition)^, Buffer, Result);
      Inc(FPosition, Result);
      Exit;
    end;
  end;
  Result := 0;
end;