Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0005511OCamlOCaml otherlibspublic2012-02-20 16:582013-08-31 12:48
Reportergerd 
Assigned To 
PrioritynormalSeverityminorReproducibilityhave not tried
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version3.12.1 
Target VersionFixed in Version3.13.0+dev 
Summary0005511: Bigarray reshape wrong for 64 bit systems
DescriptionJust found these lines in bigarray_stubs.c, in caml_ba_reshape:

if (dim[i] < 0 || dim[i] > 0x7FFFFFFFL)
  caml_invalid_argument("Bigarray.reshape: negative dimension");

The second condition, dim[i] > 0x7FFFFFFFL, can never be true on 32 bit platforms (because dimensions are signed), and is apparently wrong on 64 bit platforms. If I'm not totally blind, this test can just be deleted.
Steps To ReproduceOn a 64 bit system:

# open Bigarray;;
# let g = Genarray.create char c_layout [| 0x80000000 |];;
val g :
  (char, Bigarray.int8_unsigned_elt, Bigarray.c_layout) Bigarray.Genarray.t =
  <abstr>
# reshape_1 g 0x80000000;;
Exception: Invalid_argument "Bigarray.reshape: negative dimension".
TagsNo tags attached.
Attached Files

- Relationships
related to 0004457closedxleroy Bigarray dimensions too limited on 64bit architectures 

-  Notes
(0006947)
gerd (reporter)
2012-02-20 17:11
edited on: 2012-02-20 17:12

On the second glance, I think the test "size > 0x7FFFFFFFL" (for 32 bit only) belongs into caml_ba_alloc, after multiplying the dimensions. The real problem is that "size" (and num_elts) is unsigned, but the dimensions are signed. So, when reshaping, it may happen that a positive size is interpreted as a negative dimension (e.g. when a 1000 * 3_000_000 char array is reshaped into a one-dimension 3_000_000_000 array). IMHO, this should better be caught in caml_ba_alloc, by just disallowing that large arrays.

I'm not absolutely sure whether such a case actually exists, because one has to pass the new dimensions in to reshape, and is limited by max_int anyway.

(0006954)
xleroy (administrator)
2012-02-21 18:55

The intent of this test was indeed to guarantee that bigarray dimensions always fit in 32-bit signed, for compatibility between 32- and 64-bit platforms. As discussed in the related PR, this guarantee is useless anyway, and the test in Bigarray.reshape should have been removed. Now it is (SVN commit 12180).

- Issue History
Date Modified Username Field Change
2012-02-20 16:58 gerd New Issue
2012-02-20 17:11 gerd Note Added: 0006947
2012-02-20 17:12 gerd Note Edited: 0006947 View Revisions
2012-02-21 18:51 xleroy Relationship added related to 0004457
2012-02-21 18:55 xleroy Note Added: 0006954
2012-02-21 18:55 xleroy Status new => resolved
2012-02-21 18:55 xleroy Resolution open => fixed
2012-02-21 18:55 xleroy Fixed in Version => 3.13.0+dev
2013-08-31 12:48 xleroy Status resolved => closed


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker