Re: [Ocaml-lib-devel] a bug in BitSet?

[Janne H. <ja...@hy...>]

Bardur Arantsson wrote:
> Generally I would say one should always compile without
> -unsafe and use unsafe_get and companions in those
> instances where it can be *guaranteed* to stay within the
> bounds *AND* where it matters for performance.
> 
> Even in cases where it is easy to prove that the index is
> always within bounds, I wouldn't use unsafe_get/set unless
> it actually matters a great deal for performance. It
> reduces readability and makes code harder to modify later.

unsafe_get/set have one problem w.r.t. testing: when they're used, it is 
impossible to turn *on* bounds checking.  When using .()/.[] and 
compiling with -unsafe, it is at least possible to turn on bounds 
checking when testing.

Having integrated more tests into the extlib testing suite, I have come 
to the conclusion that bounds checking would be really nice during 
testing.  The current situation is that ExtLib seems to be buffer 
overflowing in a few places, causing a segfault somewhere when testing 
ExtString module.

This happens with just a handful of test cases, so I'm guessing that 
there are more problems like this waiting to be uncovered.

As most parts of ExtLib are using unsafe_get/set, it is impossible to 
catch buffer under/overflows in places where they actually happen.  As 
an example, a buffer overflow somewhere can cause an infinite loop 
inside O'Caml's garbage collector (happens currently in native code). 
It would be really nice to be able to catch cases like these *before* 
they cause any damage.  Bounds checking does exactly this.

Since I guess people are not willing to stop using unsafe_get/put (at 
least not completely), we should think of some other way to achieve 
bounds checking so that it doesn't hurt performance.

I'm proposing the use of asserts to check bounds when accessing arrays 
and other data with unsafe methods.  We could turn asserts off when 
building the release library, but we could always have asserts turned on 
when running the testing suite.

This way we would catch possible buffer overruns immediately and not 
when their damage shows up elsewhere.

How would people feel about having release and debug builds of ExtLib? 
The release would have assertions turned off.  This is basically the 
same as the current release version.  The debug build would always have 
assertions on and thus be slower.  However, it would only be used for 
testing purposes.

Merry Christmas,
Janne