Discussion:
Overflow in TMemoryStream?
(too old to reply)
Martin Schreiber
2016-09-11 08:35:37 UTC
Permalink
Raw Message
Hi,

While working on the MSEgui fork of classes unit I saw a suspect piece of code
in streams.inc:
"
function TMemoryStream.Realloc(var NewCapacity: PtrInt): Pointer;

begin
If NewCapacity<0 Then
NewCapacity:=0
else
begin
// if growing, grow at least a quarter
if (NewCapacity>FCapacity) and (NewCapacity < (5*FCapacity) div 4) then
NewCapacity := (5*FCapacity) div 4;
"
Isn't there an overflow if the capacity grows above high(ptrint) div 5 (about
430MB on 32 bit)?
IIRC there was a discussion on the list about memory problems with big
TMemoryStream's.

Martin
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
Martok
2016-09-11 18:07:22 UTC
Permalink
Raw Message
Hi,

yes, I can confirm this as an overflow, but on its own, it should be safe. Above
430MB, the stream doesn't grow by a quarter but just by however much was
requested, luckily the branch fails before the wrong capacity could be set.

Test:
type
TMS2 = class(TMemoryStream) end;
var
ms: TMS2;
ds: Int64;
begin
ds:= 100*1000*1000;
ms:= TMS2.Create;
ms.SetSize(ds);
WriteLn(ds:15,' ', ms.Size:15, ' ', ms.Capacity:15);
inc(ds, ds div 10); // grow by less than 25%
ms.SetSize(ds);
WriteLn(ds:15,' ', ms.Size:15, ' ', ms.Capacity:15);
end.

with ds=100M, prints:
100000000 100000000 100003840
110000000 110000000 125005824 << grew by 1/4*100M

with ds=500M, prints:
500000000 500000000 500002816
550000000 550000000 550002688 << bug, grew by 1/10*500M

However, with ds=869M, prints:
869000000 869000000 869003264
955900000 18666185569013440 955904000
and mostly crashes with Runtime Error 203 except when I'm step-by-step-debugging
it...
That looks like a *separate* overflow to me, probably caused by the wild mix of
Int64 and Longint that our Streams inherited from Delphi...

I don't have RTL built with full symbols right now, maybe someone else can
investigate?


Martok

_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
Michael Van Canneyt
2016-09-12 08:38:27 UTC
Permalink
Raw Message
Post by Martok
Hi,
yes, I can confirm this as an overflow, but on its own, it should be safe. Above
430MB, the stream doesn't grow by a quarter but just by however much was
requested, luckily the branch fails before the wrong capacity could be set.
type
TMS2 = class(TMemoryStream) end;
var
ms: TMS2;
ds: Int64;
begin
ds:= 100*1000*1000;
ms:= TMS2.Create;
ms.SetSize(ds);
WriteLn(ds:15,' ', ms.Size:15, ' ', ms.Capacity:15);
inc(ds, ds div 10); // grow by less than 25%
ms.SetSize(ds);
WriteLn(ds:15,' ', ms.Size:15, ' ', ms.Capacity:15);
end.
100000000 100000000 100003840
110000000 110000000 125005824 << grew by 1/4*100M
500000000 500000000 500002816
550000000 550000000 550002688 << bug, grew by 1/10*500M
Output on linux 64-bit

cadwal: >tms
100000000 100000000 100003840
110000000 110000000 125005824
cadwal: >tms
500000000 500000000 500002816
550000000 550000000 625004544 This is 1/4
cadwal: >tms
869000000 869000000 869003264
955900000 955900000 1086255104

So it looks like a 32 vs. 64 bit issue.

from the method Realloc :

NewCapacity := (5*FCapacity) div 4; // 5*FCapacity can cause overflow

Changing this to
NewCapacity:=FCapacity + (FCapacity div 4)

Will probably fix the issue.

Michael.
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
Michael Van Canneyt
2016-09-12 08:44:38 UTC
Permalink
Raw Message
Post by Michael Van Canneyt
So it looks like a 32 vs. 64 bit issue.
NewCapacity := (5*FCapacity) div 4; // 5*FCapacity can cause overflow
Changing this to
NewCapacity:=FCapacity + (FCapacity div 4)
Will probably fix the issue.
I have committed a patch. Please test and report if it is fixed.
I don't have a 32-bit system available to test on...

Michael.
_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
Martok
2016-09-13 10:23:04 UTC
Permalink
Raw Message
Post by Michael Van Canneyt
I have committed a patch. Please test and report if it is fixed.
I don't have a 32-bit system available to test on...
Tested on win32: the overflow is fixed, 500M gets incremented by 125M.

I think the RunError is caused by the way ReallocMem works: growing from 869M to
1086M seems to be done by allocating the new area and copying, so there's a
period of almost all of the 2G heap used...


_______________________________________________
fpc-devel maillist - fpc-***@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

Loading...