From 1c8abef3cf8b2ccb2f75b22b9c4509b43c76e11c Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Fri, 10 Aug 2012 11:44:18 +0000 Subject: [PATCH] Fix memory leaks and alignment issues (thanks rixed at happyleptic.org) First one: a huge memory leak when bitmatching on int64. The reason is that on my arch (MIPS), which is a ARCH_ALIGN_INT64 arch, the function to extract int64 calls caml_copy_int64 but you are not supposed to do that since all functions are declared noalloc (should segfault but merely leaks memory on my program). See this message of Xavier for more explanations as to why it's a bug: http://caml.inria.fr/pub/ml-archives/caml-list/2010/03/2e386c384c04f4a424acc44a1 fae3abd.en.html So the easy fix would be to remove the noallocs for the int64 functions, but there is a better way around the issue: instead of assigning a int64 onto a possibly non 8 bytes aligned memory location we can merely memcpy the value in there, thus avoiding the extra allocation and the caml_c_call wrapper. While looking at this I stumbled upon the other bug: bitstring_c.c extract values by assigning a casted rvalue into a properly stack-allocated local variable. You cannot do that, this is undefined behavior since it assumes that unaligned reads are permitted. Again, on my MIPS such unaligned access are (by default) diverted to an exception handler which (slowly!) fix them on the fly (one can see the errorcount in /proc/kernel/debug/mips/unaligned_instructions), but on other archs it could as well result in a sigbus. I believe this would be a problem on ARM as well. Again, memcpy comes to the rescue. The attached patch fixes both issues. --- bitstring_c.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/bitstring_c.c b/bitstring_c.c index 1921cfa..b9ad109 100644 --- a/bitstring_c.c +++ b/bitstring_c.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include @@ -74,7 +75,7 @@ { \ type *ptr = (type *) ((void *) String_val (strv) + Int_val (offv)); \ type r; \ - r = *ptr; \ + memcpy(&r, ptr, sizeof(r)); \ swap_##endian(size,r); \ return Val_int (r); \ } @@ -93,7 +94,7 @@ fastpath1(16,ne,signed,int16_t) { \ type *ptr = (type *) ((void *) String_val (strv) + Int_val (offv)); \ type r; \ - r = *ptr; \ + memcpy(&r, ptr, sizeof(r)); \ swap_##endian(size,r); \ rval(rv) = r; \ return rv; \ @@ -117,12 +118,12 @@ fastpath2(32,ne,signed,int32_t,Int32_val) ocaml_bitstring_extract_fastpath_int##size##_##endian##_##signed \ (value strv, value offv, value rv) \ { \ - CAMLparam3(strv, offv, rv); \ type *ptr = (type *) ((void *) String_val (strv) + Int_val (offv)); \ type r; \ - r = *ptr; \ + memcpy(&r, ptr, sizeof(r)); \ swap_##endian(size,r); \ - CAMLreturn(caml_copy_int64(r)); \ + memcpy(Data_custom_val(rv), &r, sizeof(r)); \ + return rv; \ } #else -- 1.8.3.1