Spot the Bug

Time for another brain flexing bug hunt! This one pertains to declares again, only instead of the code sorta kinda working, it simply causes a crash. Oooh!

The code we're looking at is a regular Win32 API for the GDI+ collection of functionality, and looks like this:

GpStatus WINGDIPAPI GdipPathIterNextSubpath(GpPathIterator* iterator, INT *resultCount, INT* startIndex, INT* endIndex, BOOL* isClosed)

You can assume that GpStatus is an integer enumeration (named Status), and GpPathIterator is an opaque data pointer, stored in a regular integer.

The REALbasic code using this API looks like this:[rbcode]
Function NextSubpath(ByRef startIndex as Integer, ByRef endIndex as Integer, ByRef isClosed as Boolean) As Integer
Soft Declare Function GdipPathIterNextSubpath Lib kGdiPlusLib ( handle as Integer, ByRef count as Integer, ByRef startIndex as Integer, _
ByRef endIndex as Integer, ByRef isClosed as Boolean) as Status

dim ret as Integer
if IsGdiPlusSupported then
mLastResult = GdipPathIterNextSubpath( mHandle, ret, startIndex, endIndex, isClosed )
else
mLastResult = Status.UnsupportedGdiplusVersion
end if

return ret
End Function[/rbcode]
You can assume that mHandle was correctly obtained, and non-zero. You can also assume that IsGdiPlusSupported returns true.

So can you spot the bug?

(Spoiler Alert)
The bug is is the last parameter in the REALbasic declare. In REALbasic, a Boolean is only one byte long, but a Win32 BOOL type is four bytes long. When passing a ByVal boolean, this isn't an issue because all ByVal parameters (ignoring structures) are passed as 32-bit values by convention. So the compiler does the right thing in the ByVal case and the two types are compatible. However, in the ByRef case, what happens is that you have (in C parlance) a pointer to a Boolean. In this case, the pointer value (the address being pointed to) is the four byte parameter being passed, but what the pointer references is only one by of memory. However, the API is expecting the pointer's reference to be four bytes long. So what happens is that the callee writes four bytes of data, but the caller has only allocated one byte of data -- resulting in memory corruption. Due to the locality of the corruption, this corrupts the REALbasic class and sometime down the road, causes a crash (usually fairly quickly due to reference counting).

So the proper way to solve that issue is to modify the declare so that the parameter sizes match, like this:[rbcode]
Function NextSubpath(ByRef startIndex as Integer, ByRef endIndex as Integer, ByRef isClosed as Boolean) As Integer
Soft Declare Function GdipPathIterNextSubpath Lib kGdiPlusLib ( handle as Integer, ByRef count as Integer, ByRef startIndex as Integer, _
ByRef endIndex as Integer, ByRef isClosed as Integer ) as Status

dim ret as Integer
if IsGdiPlusSupported then
dim retBool as Integer
mLastResult = GdipPathIterNextSubpath( mHandle, ret, startIndex, endIndex, retBool )
isClosed = retBool <> 0
else
mLastResult = Status.UnsupportedGdiplusVersion
end if

return ret
End Function[/rbcode]
Now you've got four bytes allocated, and there's no memory corruption.