preparing patch release and fixing issue 722

[lemire]

Some of our tests deliberately do unaligned reads.

This can cause crashes under some systems as reported by @brawer
in #722

It is not a bug.

We shall avoid running these tests by default.

[andreigudkov]

Some of our tests deliberately do unaligned reads.

Are you sure? I think that the root cause is that portable serialization generates layouts which do not guarantee proper alignments for its members (unlike non-portable frozen_serialize). That's why portable_deserialize_frozen has to perform unaligned access. If this is correct then the proper solution would be the following:

• Continue to run these tests on systems where unaligned access is permitted (such as x86_64).
• Hide portable frozen serialization functionality entirely on systems which do not allow unaligned access. What is the reason to provide these functions if any attempt to use them will generate an error?

[brawer]

Given that the serialization format can't be changed, could the C implementation perhaps be fixed to read from unaligned memory without bus errors, on all platforms? The Linux kernel defines some trivial helpers to handle this. With similar helpers, the CRoaring implementation would also work on ARMv7 and ARMfp, where it currently crashes. Even on platforms such as PowerPC, where bus errors from unaligned reads get caught and emulated by the kernel, the current implementation would get much faster. I couldn't find current numbers, but this IBM article from 2005 has some interesting benchmarks, claiming a performance loss of up to 4610% when triggering the emulation path. On other RISC processors, the situation is likely similar to PowerPC. Even on x86 there is a performance penalty from unaligned memory access.

Do we know where in the code the unaligned reads actually happen? From the format spec it’s not immediately obvious. — Edit: Looks like runFlagBitset can be an uneven number of bytes, which then mis-aligns everything else in the file. Ouch.

[lemire]

@andreigudkov

Are you sure?

Yes, I am. Please review the PR that introduced these functions...

#421

We even introduced a macro 'ALLOW_UNALIGNED'.

We very much deliberately introduced unaligned reads.

If this is correct then the proper solution would be the following...

Pull request invited.

@brawer

You write:

the CRoaring implementation would also work on ARMv7 and ARMfp, where it currently crashes

Please note that what crashes is a function documented in the following manner...

 * The function is unsafe in the following ways:
 * 1) It may execute unaligned memory accesses.
 * 2) A buffer overflow may occur if buf does not point to a valid serialized
 *    bitmap.

This is very much unsafe.

The frozen functions in general are also quite unsafe in their current state even when you use the versions that do not trigger unaligned reads: if you are pointing the code to bad data, your system is likely to crash.

That's why there is no mention of them in the README file.

Now, as a general comment, there is a lot of work that could be done on the format side of things. We could have a format that it is well suited for safe mapping (ideally with hash values and so forth).

Please consider contributing... See https://github.com/RoaringBitmap/RoaringFormatSpec