Merge: Nitg-g new NativeArray fix
authorJean Privat <jean@pryen.org>
Fri, 29 May 2015 01:40:09 +0000 (21:40 -0400)
committerJean Privat <jean@pryen.org>
Fri, 29 May 2015 01:40:09 +0000 (21:40 -0400)
This is a bug that was discovered during the push of #1403.

What happens is that since NativeArrays work only with boxed values, the copy_to operation could write data beyond its intended boundaries, hence corrupting random memory.

If you execute the test bundled with the PR, on my machine, with `--hardening` on, you get this error:
`BTD BUG: Dynamic type is Sys, static type is Array[Byte]`

What happens here is that the `dest` of the `memmove` was in a emplacement before the `Array[Byte]` itself, due to its length and because the `memmove` used val* as sizeof value, it rewrote the classid of `self`, hence changing its dynamic type effectively from `Array[Byte]` to `Sys`, which produces the typing bug.

If left too long to execute, or in an unlucky memory layout, it simply segfaulted.

The behaviour of NEW_NativeArray henceforth is that it will reserve space for n `val*` instead of the `ctype` of the values.

Pull-Request: #1417
Reviewed-by: Jean Privat <jean@pryen.org>
Reviewed-by: Romain Chanoir <chanoir.romain@courrier.uqam.ca>

src/compiler/global_compiler.nit
tests/base_native_array.nit [new file with mode: 0644]
tests/sav/base_native_array.res [new file with mode: 0644]

index b2cf479..49671f9 100644 (file)
@@ -247,7 +247,7 @@ class GlobalCompiler
                res.is_exact = true
                if is_native_array then
                        var mtype_elt = mtype.arguments.first
-                       v.add("{res} = nit_alloc(sizeof(struct {mtype.c_name}) + length*sizeof({mtype_elt.ctype}));")
+                       v.add("{res} = nit_alloc(sizeof(struct {mtype.c_name}) + length*sizeof(val*));")
                        v.add("((struct {mtype.c_name}*){res})->length = length;")
                else
                        v.add("{res} = nit_alloc(sizeof(struct {mtype.c_name}));")
diff --git a/tests/base_native_array.nit b/tests/base_native_array.nit
new file mode 100644 (file)
index 0000000..e59c93e
--- /dev/null
@@ -0,0 +1,24 @@
+# This file is part of NIT ( http://www.nitlanguage.org ).
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+intrude import standard::collection::array
+
+var a = new Array[Bool]
+var cpt = 0
+while cpt < 20 do
+       a.push(false)
+       cpt += 1
+end
+
+for i in a do i.output
diff --git a/tests/sav/base_native_array.res b/tests/sav/base_native_array.res
new file mode 100644 (file)
index 0000000..9486349
--- /dev/null
@@ -0,0 +1,20 @@
+false
+false
+false
+false
+false
+false
+false
+false
+false
+false
+false
+false
+false
+false
+false
+false
+false
+false
+false
+false