Merge pull request #451 from abergeron/fix_segfault

Fix crash in reshape.

Author: nouiz

src/gpuarray_array.c

@@ -696,41 +696,43 @@ int GpuArray_reshape_inplace(GpuArray *a, unsigned int nd,
   if (newstrides == NULL)
     return error_sys(ctx->err, "calloc");
 
-  while (ni < nd && oi < a->nd) {
-    np = newdims[ni];
-    op = a->dimensions[oi];
+  if (newsize != 0) {
+    while (ni < nd && oi < a->nd) {
+      np = newdims[ni];
+      op = a->dimensions[oi];
+
+      while (np != op) {
+        if (np < op) {
+          np *= newdims[nj++];
+        } else {
+          op *= a->dimensions[oj++];
+        }
+      }
 
-    while (np != op) {
-      if (np < op) {
-        np *= newdims[nj++];
-      } else {
-        op *= a->dimensions[oj++];
+      for (ok = oi; ok < oj - 1; ok++) {
+        if (ord == GA_F_ORDER) {
+          if (a->strides[ok+1] != (ssize_t)a->dimensions[ok]*a->strides[ok])
+            goto need_copy;
+        } else {
+          if (a->strides[ok] != (ssize_t)a->dimensions[ok+1]*a->strides[ok+1])
+            goto need_copy;
+        }
       }
-    }
 
-    for (ok = oi; ok < oj - 1; ok++) {
       if (ord == GA_F_ORDER) {
-        if (a->strides[ok+1] != (ssize_t)a->dimensions[ok]*a->strides[ok])
-          goto need_copy;
+        newstrides[ni] = a->strides[oi];
+        for (nk = ni + 1; nk < nj; nk++) {
+          newstrides[nk] = newstrides[nk - 1]*newdims[nk - 1];
+        }
       } else {
-        if (a->strides[ok] != (ssize_t)a->dimensions[ok+1]*a->strides[ok+1])
-          goto need_copy;
-      }
-    }
-
-    if (ord == GA_F_ORDER) {
-      newstrides[ni] = a->strides[oi];
-      for (nk = ni + 1; nk < nj; nk++) {
-        newstrides[nk] = newstrides[nk - 1]*newdims[nk - 1];
-      }
-    } else {
-      newstrides[nj-1] = a->strides[oj-1];
-      for (nk = nj-1; nk > ni; nk--) {
-        newstrides[nk-1] = newstrides[nk]*newdims[nk];
+        newstrides[nj-1] = a->strides[oj-1];
+        for (nk = nj-1; nk > ni; nk--) {
+          newstrides[nk-1] = newstrides[nk]*newdims[nk];
+        }
       }
+      ni = nj++;
+      oi = oj++;
     }
-    ni = nj++;
-    oi = oj++;
   }
 
   /* Fixup trailing ones */

tests/CMakeLists.txt

@@ -1,4 +1,4 @@
-include(CheckSymbolExists)
+include(CheckCSourceCompiles)
 find_package(PkgConfig)
 
 pkg_search_module(CHECK check)
@@ -17,8 +17,15 @@ if(NOT CHECK_FOUND)
 endif()
 
 if(CHECK_FOUND)
-  set(CMAKE_REQUIRED_INCLUDE ${CHECK_INCLUDE_DIRS})
-  CHECK_SYMBOL_EXISTS(ck_assert_ptr_ne "check.h" CHECK_FUNCS)
+  set(CMAKE_REQUIRED_FLAGS ${CHECK_C_FLAGS} ${CHECK_LDFLAGS_OTHERS})
+  set(CMAKE_REQUIRED_INCLUDES ${CHECK_INCLUDE_DIRS})
+  set(CMAKE_REQUIRED_LIBRARIES ${CHECK_LIBRARIES})
+  CHECK_C_SOURCE_COMPILES(
+    "#include <check.h>
+     int main() {
+       ck_assert_ptr_ne(NULL, NULL);
+     }"
+    CHECK_FUNCS)
   if (NOT CHECK_FUNCS)
     set(CHECK_FOUND 0)
   endif()

tests/check_array.c

@@ -270,13 +270,28 @@ START_TEST(test_take1_offset) {
 }
 END_TEST
 
+START_TEST(test_reshape_0) {
+  /* This tests that we don't segfault when reshaping 0-sized arrays */
+  const size_t odims[3] = {24, 0, 33};
+  const size_t ndims1[3] = {0, 24, 33};
+  const size_t ndims2[3] = {24, 33, 0};
+
+  GpuArray v;
+  ga_assert_ok(GpuArray_empty(&v, ctx, GA_FLOAT, 3, odims, GA_C_ORDER));
+  ga_assert_ok(GpuArray_reshape_inplace(&v, 3, ndims1, GA_ANY_ORDER));
+  ga_assert_ok(GpuArray_reshape_inplace(&v, 3, odims, GA_ANY_ORDER));
+  ga_assert_ok(GpuArray_reshape_inplace(&v, 3, ndims2, GA_ANY_ORDER));
+}
+END_TEST
+
 Suite *get_suite(void) {
   Suite *s = suite_create("array");
   TCase *tc = tcase_create("take1");
   tcase_add_checked_fixture(tc, setup, teardown);
   tcase_set_timeout(tc, 8.0);
   tcase_add_test(tc, test_take1_ok);
   tcase_add_test(tc, test_take1_offset);
+  tcase_add_test(tc, test_reshape_0);
   suite_add_tcase(s, tc);
   return s;
 }