[FBC BUG] 32bit allocate overflow

[Jattenalle]

Issue:
The allocate family takes a uinteger as its param causing overflow and misleading results if passed larger types/values than uinteger

Affects:
-arch 32bit, both GAS and GCC

Description:
Passing a value larger than MAX_UINTEGER causes an overflow, which can cause code that should fail to succeed with no indicator to the user of any issue.
See the code for an example.

Solution:
Add a check to ensure the passed value is less than or equal to MAX_UINTEGER

Code to reproduce, compile for -arch 32:

Code: Select all

'fbcs: -arch 32
'// "Bug" in 32bit allocate accepting values larger than 32bit causing overflow and success
const as uinteger _mb = (1024 * 1024)
const as uinteger _gb = (1024 * _mb)

dim as ulongint size = 4.5 * _gb '// 4.5 GB

print "Allocating "& size \ _mb &" MB"
dim as any ptr p = allocate(size)
if p then
	print "	""Successfully"" allocated "& size \ _mb &" MB on a 32bit system, p is 0x"& hex(p)
	print "  In reality the block is only the lower 32bits (overflow), which is "& (size and &h00000000ffffffffull) \ _mb &" MB"
	print "  This can cause some serious buffer overflow bugs"
	deallocate p
	p = 0
else
	print " Failed as expected!"
end if

sleep
end

[Berkeley]

Annotation(?): the limit of 32 bit is 2^32 = 4,294,967,296 different values. 4294967295 is the highest value of uint32. My FreeBASIC reference says, that "INTEGER" is int32 in the x86 version, only int64 in the x64 version.

[shadow008]

There are at least 3 confounding factors that point to this not actually being a bug:

1) MOST IMPORTANT: Unsigned integer overflow is defined behavior, I'm nearly certain (though I cannot find docs explicitly stating that). Signed integer overflow, however, is undefined. This means that the default case is that there is no warning for unsigned integer overflow.
2) A 32 bit machine cannot handle more than the max uinteger (i.e. platform specific integer size) memory segment anyway, so accepting a value over that would not make sense. This makes alternatives, such as a 64 bit int regardless of platform size, absurd. This isn't the concern you brought up, but it's worth mentioning.
3) Taking into consideration point 1, there is no good reason to carve out an explicit exception just for allocate() parameters. Such an exception would also have to extend to callocate and New and friends. Because it's defined, the programmer is necessarily meant to understand that fact and handle it in their code.

TLDR: unsigned integer overflow is not a bug. Thus, overflowing integer values are to be recognized and handled by the coder, not the compiler.

[Jattenalle]

Thus, overflowing integer values are to be recognized and handled by the coder, not the compiler.

Except we have precedence for adding a compiler check for type overflows where it can cause issues:

Code: Select all

for i as ubyte = 0 to 255
next

[shadow008]

Except we have precedence for adding a compiler check for type overflows where it can cause issues:

Code: Select all

for i as ubyte = 0 to 255
next

hmmm... you're right. I guess that could be an argument in making unsigned overflow always a warning for all types *shrug*

I can only assume this is a consequence of integer promotion. I don't have a compiler to check right now, but I'd assume that ubyte and ushort (and also uinteger<8> and uinteger<16> and friends) will have overflow checking due to the fact that it is the compilers responsibility to do the integer promotion, not the user's.

That said, the compiler does not need to do promotion on 32/64 bit uintegers and overflowing results are deterministic. So my guess is that ubyte/ushort are the exceptions due to integer promotion stuff, not the rule. Someone else with more knowledge will have to jump in.

[paul doe]

...
That said, the compiler does not need to do promotion on 32/64 bit uintegers and overflowing results are deterministic. So my guess is that ubyte/ushort are the exceptions due to integer promotion stuff, not the rule. Someone else with more knowledge will have to jump in.

Actually, in the case of the loop above the check and the ensuing warning was added because the overflow produces a nasty bug that's pretty difficult to track (due to the way for computes the exit condition of the loop). But in other cases (as the one discussed here), overflows have less dramatic, although still incorrect, results.

[adeyblue]

It becomes far more insidious when it's the compiler that's doing the overflowing. 32-bit obvs

Code: Select all

Type Udt

    as ULong a
    as ULong b
    as ULong c
    as ULong d

End Type

Extern "c" '' windows only, msvcrt
    Declare Function _msize(ByVal p as Any Ptr) As UInteger
End Extern

Dim numItems as ULong = &h10000000
Dim p as Udt Ptr = New Udt[numItems]
Print "p = " & Hex(p) & ", size = " & Hex(_msize(p))

Wait, how much did we allocate?

p = 731540, size = 1

Yep, that's a problem.

[cbruce]

shadow008 got the right of it; that unsigned integer overflow is defined behaviour in all C and C++ implementations. And, as paul doe said, FB just checks it in "for loops".

I've been using unsigned integer overflow for years, (as have most programmers), since I've utilized prngs based on LCGs. That's all that an LCG does is overflow!

Here is an example doc for you:
https://www.gnu.org/software/c-intro-an ... rflow.html

6.3.1 Overflow with Unsigned Integers
Unsigned arithmetic in C ignores overflow; it produces the true result modulo the nth power of 2, where n is the number of bits in the data type. We say it “truncates” the true result to the lowest n bits.

A true result that is negative, when taken modulo the nth power of 2, yields a positive number. For instance,

unsigned int x = 1;
unsigned int y;

y = -x;
causes overflow because the negative number -1 can’t be stored in an unsigned type. The actual result, which is -1 modulo the nth power of 2, is one less than the nth power of 2. That is the largest value that the unsigned data type can store. For a 32-bit unsigned int, the value is 4,294,967,295. See Maximum and Minimum Values.

Adding that number to itself, as here,

unsigned int z;

z = y + y;
ought to yield 8,489,934,590; however, that is again too large to fit, so overflow truncates the value to 4,294,967,294. If that were a signed integer, it would mean -2, which (not by coincidence) equals -1 + -1.

[cbruce]

By the way, if you really want to check for unsigned integer overflow in your code - there is a very good stackoverflow thread that has been covering the issue for many years:

c++ - How do I detect unsigned integer overflow? - Stack Overflow
https://stackoverflow.com/questions/199 ... r-overflow