I'd like to report my findings during the investigation of MDEV-7912, ask for a review on my patch and see your opinion on the status of the related code. The patch and all the issues that I am raising are for the 5.5 branch.

With the attached patch, the server crash is eliminated. The problem we had was caused by casting a 64bit unsigned int to a 32bit unsigned int, representing a number of bytes to allocate. When the ranges were too big, it caused an overflow and we allocated too little space.

On a related note, while looking at the code in the uniques.cc file, I've discovered a lot of potential issues, that are almost guaranteed to fail, if we actually do use more than 4 GB of memory. This holds true especially for Windows, because ulong is 32 bits for a 64 bit compilation, while on Linux, it is 64 bit.

I've tried to convert most of the code that made use of uint or ulong or ulonglong for address space indexing (arrays, indices in arrays, memory sizes) to size_t, so that we escape the portability problem when compiling for 32 bit or 64 bit. Unfortunately, the task branches out into various other files that make use of those values. An example of a such a problem is in Unique::Unique constructor, line 90, where init_tree will get passed an overflown parameter (max_in_memory_size). It just so happens that there is a guard within the init_tree function that sets a minimum value, in case we do happen to pass an overflown value. This is however a lucky occurrence.

Unique::get_use_cost can output a bad cost value, because get_merge_many_buffs_cost casts the same ulonglong memory size parameter to a ulong.

There are many examples like this.

I'd like to get your input on this issue, is this something we plan to fix in the near future? Should I embark on the journey of slowly fixing at least parts of it? Also, this issue seems to plague much of the code base, as I get a huge wall of similar size_t -> uint32 warnings across most files.

Regards,

Vicențiu