Stephan Bergmann
Mostly for historical reasons, LibreOffice has its own `SvMemoryStream` class (for, you guessed it, writing in-memory streams). For quite a while now, code in the PDF filter used that `SvMemoryStream` in a quite peculiar way: In various places, it would take a pointer to some bytes that had already been written into the `SvMemoryStream`’s buffer, and then call `SvMemoryStream::WriteBytes` to append those bytes a second time. Which indeed works, as long as the call to `WriteBytes` does not need to enlarge the `SvMemoryStream`’s buffer first—at which point the pointer to the bytes-to-be-written-a-second-time would point to freed memory.
Interestingly, this never seemed to cause issues in practice. Until, recently, some tests changed and the address-sanitizer builds started to complain about heap-user-after-free issues. Now, one way to fix this would have been to inspect all those places that use that dubious pattern, and change them to first copy the bytes to the side, and pass that copy to `SvMemoryStream::WriteBytes`.
But Noel had the nice idea of instead changing the implementation of `SvMemoryStream`: When `WriteBytes` needs to enlarge the buffer, it now checks whether the pointer to the to-be-written data was coming from within its original buffer. And if yes, it fixes that pointer up to instead point into the enlarged buffer (where the contents of those bytes is guaranteed to be available just fine).
So, how do you check in C++ that a pointer actually points into a range of bytes?
pData >= pBuf && pData < (pBuf + nSize)
looks innocent enough and should get the job done? Except it doesn’t. Because C++ has all that fine print. When comparing pointers with relational operators, if they don’t both point into the same array, the results are unspecified. Which an aggressively optimizing compiler can take advantage of. Either `pData` is within the `pBuf`…`pBuf + nSize` array, and the above check is true. Or `pData` is pointing at some completely different array, in which case… Well, the results are unspecified, so why not declare that the check is true in that case as well? Which means the compiler can pretend that the check is always true and isn’t even needed, and `WriteBytes` will always fix up the pointer that gets passed in to point into the enlarged buffer, even if it originally didn’t point into the original buffer. Ouch.
There’s <https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2024/p3234r1.html> “Utility to check if a pointer is in a given range” that might bring some guaranteed-to-be-working `std::pointer_in_range` to some future version of C++. Until then, we can make use of `boost::pointer_in_range`.
