Require unsafe_wrap and reinterpret to have proper alignment

JuliaLang · May 16, 2017 · 4bd5f81 · 4bd5f81
1 parent 99768bf
commit 4bd5f81
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 9 deletions.
diff --git a/base/docs/helpdb/Base.jl b/base/docs/helpdb/Base.jl
@@ -1063,6 +1063,14 @@ For example,
 `reinterpret(Float32, UInt32(7))` interprets the 4 bytes corresponding to `UInt32(7)` as a
 `Float32`.
 
+!!! warning
+
+    It is not allowed to `reinterpret` an array to an element type with a larger alignment then
+    the alignment of the array. For a normal `Array`, this is the alignment of its element type.
+    For a reinterpreted array, this is the alignment of the `Array` it was reinterpreted from.
+    For example, `reinterpret(UInt32, UInt8[0, 0, 0, 0])` is not allowed but
+    `reinterpret(UInt32, reinterpret(UInt8, Float32[1.0]))` is allowed.
+
 ```jldoctest
 julia> reinterpret(Float32, UInt32(7))
 1.0f-44

diff --git a/src/array.c b/src/array.c
@@ -188,10 +188,19 @@ JL_DLLEXPORT jl_array_t *jl_reshape_array(jl_value_t *atype, jl_array_t *data,
     a->offset = 0;
     a->data = NULL;
     a->flags.isaligned = data->flags.isaligned;
+    jl_array_t *owner = (jl_array_t*)jl_array_owner(data);
     jl_value_t *el_type = jl_tparam0(atype);
     assert(store_unboxed(el_type) == !data->flags.ptrarray);
     if (!data->flags.ptrarray) {
         a->elsize = jl_datatype_size(el_type);
+        unsigned align = ((jl_datatype_t*)el_type)->layout->alignment;
+        jl_value_t *ownerty = jl_typeof(owner);
+        unsigned oldalign = (ownerty == (jl_value_t*)jl_string_type ? 1 :
+                             ((jl_datatype_t*)jl_tparam0(ownerty))->layout->alignment);
+        if (oldalign < align)
+            jl_exceptionf(jl_argumenterror_type,
+                          "reinterpret from alignment %u bytes to alignment %u bytes not allowed",
+                          oldalign, align);
         a->flags.ptrarray = 0;
     }
     else {
@@ -201,7 +210,7 @@ JL_DLLEXPORT jl_array_t *jl_reshape_array(jl_value_t *atype, jl_array_t *data,
 
     // if data is itself a shared wrapper,
     // owner should point back to the original array
-    jl_array_data_owner(a) = jl_array_owner(data);
+    jl_array_data_owner(a) = (jl_value_t*)owner;
 
     a->flags.how = 3;
     a->data = data->data;
@@ -266,15 +275,22 @@ JL_DLLEXPORT jl_array_t *jl_ptr_to_array_1d(jl_value_t *atype, void *data,
                                             size_t nel, int own_buffer)
 {
     jl_ptls_t ptls = jl_get_ptls_states();
-    size_t elsz;
     jl_array_t *a;
     jl_value_t *el_type = jl_tparam0(atype);
 
     int isunboxed = store_unboxed(el_type);
-    if (isunboxed)
+    size_t elsz;
+    unsigned align;
+    if (isunboxed) {
         elsz = jl_datatype_size(el_type);
-    else
-        elsz = sizeof(void*);
+        align = ((jl_datatype_t*)el_type)->layout->alignment;
+    }
+    else {
+        align = elsz = sizeof(void*);
+    }
+    if (((uintptr_t)data) & (align - 1))
+        jl_exceptionf(jl_argumenterror_type,
+                      "unsafe_wrap: pointer %p is not properly aligned to %u bytes", data, align);
 
     int ndimwords = jl_array_ndimwords(1);
     int tsz = JL_ARRAY_ALIGN(sizeof(jl_array_t) + ndimwords*sizeof(size_t), JL_CACHE_BYTE_ALIGNMENT);
@@ -309,7 +325,7 @@ JL_DLLEXPORT jl_array_t *jl_ptr_to_array(jl_value_t *atype, void *data,
                                          jl_value_t *_dims, int own_buffer)
 {
     jl_ptls_t ptls = jl_get_ptls_states();
-    size_t elsz, nel = 1;
+    size_t nel = 1;
     jl_array_t *a;
     size_t ndims = jl_nfields(_dims);
     wideint_t prod;
@@ -326,10 +342,18 @@ JL_DLLEXPORT jl_array_t *jl_ptr_to_array(jl_value_t *atype, void *data,
     jl_value_t *el_type = jl_tparam0(atype);
 
     int isunboxed = store_unboxed(el_type);
-    if (isunboxed)
+    size_t elsz;
+    unsigned align;
+    if (isunboxed) {
         elsz = jl_datatype_size(el_type);
-    else
-        elsz = sizeof(void*);
+        align = ((jl_datatype_t*)el_type)->layout->alignment;
+    }
+    else {
+        align = elsz = sizeof(void*);
+    }
+    if (((uintptr_t)data) & (align - 1))
+        jl_exceptionf(jl_argumenterror_type,
+                      "unsafe_wrap: pointer %p is not properly aligned to %u bytes", data, align);
 
     int ndimwords = jl_array_ndimwords(ndims);
     int tsz = JL_ARRAY_ALIGN(sizeof(jl_array_t) + ndimwords*sizeof(size_t), JL_CACHE_BYTE_ALIGNMENT);

diff --git a/test/core.jl b/test/core.jl
@@ -952,7 +952,15 @@ let
     @test aa == a
     aa = unsafe_wrap(Array, pointer(a), UInt16(length(a)))
     @test aa == a
+    aaa = unsafe_wrap(Array, pointer(a), (1, 1))
+    @test size(aaa) == (1, 1)
+    @test aaa[1] == a[1]
     @test_throws InexactError unsafe_wrap(Array, pointer(a), -3)
+    # Misaligned pointer
+    res = @test_throws ArgumentError unsafe_wrap(Array, pointer(a) + 1, length(a))
+    @test contains(res.value.msg, "is not properly aligned to $(sizeof(Int)) bytes")
+    res = @test_throws ArgumentError unsafe_wrap(Array, pointer(a) + 1, (1, 1))
+    @test contains(res.value.msg, "is not properly aligned to $(sizeof(Int)) bytes")
 end
 
 struct FooBar2515
@@ -4906,3 +4914,20 @@ type T21719{V}
 end
 g21719(f, goal; tol = 1e-6) = T21719(f, tol, goal)
 @test isa(g21719(identity, 1.0; tol=0.1), T21719)
+
+# reinterpret alignment requirement
+let arr8 = zeros(UInt8, 16),
+    arr64 = zeros(UInt64, 2),
+    arr64_8 = reinterpret(UInt8, arr64),
+    arr64_i
+
+    # Not allowed to reinterpret arrays allocated as UInt8 array to a Int32 array
+    res = @test_throws ArgumentError reinterpret(Int32, arr8)
+    @test res.value.msg == "reinterpret from alignment 1 bytes to alignment 4 bytes not allowed"
+    # OK to reinterpret arrays allocated as UInt64 array to a Int64 array even though
+    # it is passed as a UInt8 array
+    arr64_i = reinterpret(Int64, arr64_8)
+    @test arr8 == arr64_8
+    arr64_i[2] = 1234
+    @test arr64[2] == 1234
+end