Possible bug in bitmatch when looping?

[Hans Ole Rafaelsen]

(Sorry if this list is not the proper place to report bugs)

Hi,

I have a problem when looping over a bitstring representation of a list of integers and turning this bitstring into a list of integers. For some reason the resulting list have the value of the last integer in the bitstring. I have attached code that demonstrates this, a simple workaround and the output when running in the toplevel.

Is this a bug, or is this not the proper way of doing this kind of operation?

Thanks for a great tool! :-)

Regards,

Hans Ole Rafaelsen

[Richard Jones]

On Wed, Oct 15, 2008 at 12:51:27PM +0200, Hans Ole Rafaelsen wrote:
> (Sorry if this list is not the proper place to report bugs)

It is the right place. You might want to CC me for urgent bugs
though.

> I have a problem when looping over a bitstring representation of a list of
> integers and turning this bitstring into a list of integers. For some reason
> the resulting list have the value of the last integer in the bitstring. I
> have attached code that demonstrates this, a simple workaround and the
> output when running in the toplevel.

Have you got a shorter example? -- brain hurts!

I suspect this may be related to the fastpath hack here:

http://code.google.com/p/bitstring/source/browse/trunk/pa_bitstring.ml#669
(and the same on line 671)

In the fastpath, in order to avoid doing an allocation in the C code
and thus being able to make a direct C call[1], we allocate the return
value in OCaml. That is the purpose of the '0l' and '0L' constants in
those two lines.

Unfortunately what I think may be happening is that those constants
aren't being allocated anew each time, but are the same constant being
passed into the function. So it ends up overwriting the value.

It should be relatively easy to test this by trying to replace that
code with something like Int32.of_int 0 / Int64.of_int 0

Rich.

[1] http://camltastic.blogspot.com/2008/08/tip-calling-c-functions-directly-with.html

--
Richard Jones
Red Hat

[Hans Ole Rafaelsen]

Hello Richard,

On Fri, Oct 17, 2008 at 12:43 AM, Richard Jones <ri...@annexia.org> wrote:

On Wed, Oct 15, 2008 at 12:51:27PM +0200, Hans Ole Rafaelsen wrote:
> (Sorry if this list is not the proper place to report bugs)

It is the right place.  You might want to CC me for urgent bugs
though.

> I have a problem when looping over a bitstring representation of a list of
> integers and turning this bitstring into a list of integers. For some reason
> the resulting list have the value of the last integer in the bitstring. I
> have attached code that demonstrates this, a simple workaround and the
> output when running in the toplevel.

Have you got a shorter example? -- brain hurts!

Here is a shorter example that demonstrates it:
 let int32_to_bitstring i =
  BITSTRING {
     i : 32
   }

let int32_of_bitstring bs =  
  bitmatch bs with
    | {
    csrc : 32
      } ->
    csrc

let f () =
  let a = int32_to_bitstring 55l in
  let b = int32_to_bitstring 66l in
  let aa = int32_of_bitstring a in
  let bb = int32_of_bitstring b in
    aa, bb

let _ = f ()

I suspect this may be related to the fastpath hack here:

http://code.google.com/p/bitstring/source/browse/trunk/pa_bitstring.ml#669
(and the same on line 671)

In the fastpath, in order to avoid doing an allocation in the C code
and thus being able to make a direct C call[1], we allocate the return
value in OCaml.  That is the purpose of the '0l' and '0L' constants in
those two lines.

Unfortunately what I think may be happening is that those constants
aren't being allocated anew each time, but are the same constant being
passed into the function.  So it ends up overwriting the value.

It should be relatively easy to test this by trying to replace that
code with something like Int32.of_int 0 / Int64.of_int 0

I inserted your suggested solution, and it fixes the problem :-)
 

Rich.

[1] http://camltastic.blogspot.com/2008/08/tip-calling-c-functions-directly-with.html

--
Richard Jones
Red Hat

Thanks,

Hans Ole

[Richard Jones]

On Fri, Oct 17, 2008 at 10:11:42AM +0200, Hans Ole Rafaelsen wrote:
> I inserted your suggested solution, and it fixes the problem :-)

Thanks for chasing this up. I've fixed it and added a regression
test.

Since this was also a serious bug, I've made a new release. Time for
version 2.0.0.

Rich.