[RefC] [Test] Valgrind support & Fix invalid memory read of strSubstr (…

…#3196)

CHANGELOG_NEXT.md

@@ -28,6 +28,13 @@ This CHANGELOG describes the merged but unreleased changes. Please see [CHANGELO
 
 ### Compiler changes
 
+#### RefC Backend
+
+* Fix invalid memory read onf strSubStr.
+
+* Fix memory leaks of IORef. Now that IORef holds values by itself,
+  global_IORef_Storage is no longer needed.
+
 #### NodeJS Backend
 
 * The NodeJS executable output to `build/exec/` now has its executable bit set.

src/Compiler/RefC/RefC.idr

@@ -974,10 +974,6 @@ header = do
       #include <runtime.h>
       /* \{ generatedString "RefC" } */
 
-      /* a global storage for IO References */
-      IORef_Storage * global_IORef_Storage;
-
-
       """
     let headerFiles = Libraries.Data.SortedSet.toList !(get HeaderFiles)
     let headerLines = map (\h => "#include <" ++ h ++ ">\n") headerFiles
@@ -998,7 +994,6 @@ footer = do
                         "idris2_setArgs(argc, argv);"
                         ""
           }
-          global_IORef_Storage = NULL;
           Value *mainExprVal = __mainExpression_0();
           trampoline(mainExprVal);
           return 0; // bye bye

support/refc/_datatypes.h

@@ -135,7 +135,7 @@ typedef struct {
 
 typedef struct {
   Value_header header;
-  int32_t index;
+  Value *v;
 } Value_IORef;
 
 typedef struct {
@@ -169,9 +169,3 @@ typedef struct {
   Value_header header;
   pthread_cond_t *cond;
 } Value_Condition;
-
-typedef struct {
-  Value **refs;
-  int filled;
-  int total;
-} IORef_Storage;

support/refc/cBackend.h

@@ -16,5 +16,3 @@
 #include "runtime.h"
 #include "stringOps.h"
 #include "threads.h"
-
-extern IORef_Storage *global_IORef_Storage;

support/refc/memoryManagement.c

@@ -253,7 +253,7 @@ void removeReference(Value *elem) {
       break;
     }
     case IOREF_TAG:
-      /* nothing to delete, added for sake of completeness */
+      removeReference(((Value_IORef *)elem)->v);
       break;
 
     case BUFFER_TAG: {

support/refc/prim.c

@@ -3,56 +3,22 @@
 #include <string.h>
 #include <unistd.h>
 
-// This is NOT THREAD SAFE in the current implementation
-
-IORef_Storage *newIORef_Storage(int capacity) {
-  IORef_Storage *retVal = (IORef_Storage *)malloc(sizeof(IORef_Storage));
-  IDRIS2_REFC_VERIFY(retVal, "malloc failed");
-  retVal->filled = 0;
-  retVal->total = capacity;
-  retVal->refs = (Value **)malloc(sizeof(Value *) * retVal->total);
-  return retVal;
-}
-
-void doubleIORef_Storage(IORef_Storage *ior) {
-  Value **values = (Value **)malloc(sizeof(Value *) * ior->total * 2);
-  IDRIS2_REFC_VERIFY(values, "malloc failed");
-  ior->total *= 2;
-  for (int i = 0; i < ior->filled; i++) {
-    values[i] = ior->refs[i];
-  }
-  free(ior->refs);
-  ior->refs = values;
-}
-
 Value *idris2_Data_IORef_prim__newIORef(Value *erased, Value *input_value,
                                         Value *_world) {
-  // if no ioRef Storag exist, start with one
-  if (!global_IORef_Storage) {
-    global_IORef_Storage = newIORef_Storage(128);
-  }
-  // expand size of needed
-  if (global_IORef_Storage->filled >= global_IORef_Storage->total) {
-    doubleIORef_Storage(global_IORef_Storage);
-  }
-
-  // store value
   Value_IORef *ioRef = IDRIS2_NEW_VALUE(Value_IORef);
   ioRef->header.tag = IOREF_TAG;
-  ioRef->index = global_IORef_Storage->filled;
-  global_IORef_Storage->refs[global_IORef_Storage->filled] =
-      newReference(input_value);
-  global_IORef_Storage->filled++;
-
+  ioRef->v = newReference(input_value);
   return (Value *)ioRef;
 }
 
-Value *idris2_Data_IORef_prim__writeIORef(Value *erased, Value *_index,
+Value *idris2_Data_IORef_prim__writeIORef(Value *erased, Value *_ioref,
                                           Value *new_value, Value *_world) {
-  Value_IORef *index = (Value_IORef *)_index;
-  removeReference(global_IORef_Storage->refs[index->index]);
-  global_IORef_Storage->refs[index->index] = newReference(new_value);
-  return newReference(_index);
+  Value_IORef *ioref = (Value_IORef *)_ioref;
+  newReference(new_value);
+  Value *old = ioref->v;
+  ioref->v = new_value;
+  removeReference(old);
+  return NULL;
 }
 
 // -----------------------------------

support/refc/prim.h

@@ -5,8 +5,8 @@
 // IORef
 
 Value *idris2_Data_IORef_prim__newIORef(Value *, Value *, Value *);
-#define idris2_Data_IORef_prim__readIORef(erased, ix, world)                   \
-  (newReference(global_IORef_Storage->refs[((Value_IORef *)ix)->index]))
+#define idris2_Data_IORef_prim__readIORef(erased, ioref, world)                \
+  (newReference(((Value_IORef *)ioref)->v))
 
 Value *idris2_Data_IORef_prim__writeIORef(Value *, Value *, Value *, Value *);
 

support/refc/stringOps.c

@@ -73,10 +73,10 @@ Value *strAppend(Value *a, Value *b) {
 
 Value *strSubstr(Value *start, Value *len, Value *s) {
   char *input = ((Value_String *)s)->str;
-  int offset = extractInt(start);
+  int offset = extractInt(start); /* start and len is Nat. */
   int l = extractInt(len);
 
-  int tailLen = strlen(input);
+  int tailLen = strlen(input) - offset;
   if (tailLen < l) {
     l = tailLen;
   }

tests/refc/args/run

@@ -1,5 +1,5 @@
 . ../../testutils.sh
 
-idris2 --cg refc -o refc_args TestArgs.idr > /dev/null
-./build/exec/refc_args a b
+idris2 --cg refc -o refc_args TestArgs.idr
+$VALGRIND ./build/exec/refc_args a b
 ./build/exec/refc_args c

tests/refc/buffer/run

@@ -1,7 +1,7 @@
 . ../../testutils.sh
 
-idris2 --cg refc -o refc_buffer TestBuffer.idr > /dev/null
-./build/exec/refc_buffer
+idris2 --cg refc -o refc_buffer TestBuffer.idr
+$VALGRIND ./build/exec/refc_buffer
 base64 -i testWrite.buf
 
 rm testWrite.buf

tests/refc/ccompilerArgs/run

@@ -26,6 +26,6 @@ export LDLIBS="-lexternalc"
 export LD_LIBRARY_PATH="$LD_LIBRARY_PATH:./library/"
 export DYLD_LIBRARY_PATH="$DYLD_LIBRARY_PATH:./library/"
 
-idris2 --cg refc -o cffi Main.idr > /dev/null
+idris2 --cg refc -o cffi Main.idr
 
-./build/exec/cffi
+$VALGRIND ./build/exec/cffi

tests/refc/doubles/run

@@ -1,4 +1,4 @@
 . ../../testutils.sh
 
-idris2 --cg refc -o refc_doubles TestDoubles.idr > /dev/null
-./build/exec/refc_doubles
+idris2 --cg refc -o refc_doubles TestDoubles.idr
+$VALGRIND ./build/exec/refc_doubles

tests/refc/garbageCollect/run

@@ -1,5 +1,5 @@
 . ../../testutils.sh
 
-idris2 --cg refc -o testGC TestGarbageCollect.idr > /dev/null
+idris2 --cg refc -o testGC TestGarbageCollect.idr
 
-./build/exec/testGC
+$VALGRIND ./build/exec/testGC

tests/refc/integers/run

@@ -1,4 +1,4 @@
 . ../../testutils.sh
 
-idris2 --cg refc -o refc_integers TestIntegers.idr > /dev/null
-./build/exec/refc_integers
+idris2 --cg refc -o refc_integers TestIntegers.idr
+$VALGRIND ./build/exec/refc_integers

tests/refc/issue1778/run

@@ -1,4 +1,4 @@
 . ../../testutils.sh
 
-idris2 --cg refc -o reverse Reverse.idr > /dev/null
-./build/exec/reverse
+idris2 --cg refc -o reverse Reverse.idr
+$VALGRIND ./build/exec/reverse

tests/refc/issue2424/run

@@ -1,5 +1,4 @@
 . ../../testutils.sh
 
-idris2 --cg refc -o test ControlAppMonadTest.idr > /dev/null
-
-./build/exec/test
+idris2 --cg refc -o test ControlAppMonadTest.idr
+$VALGRIND ./build/exec/test

tests/refc/issue2452/run

@@ -1,4 +1,4 @@
 . ../../testutils.sh
 
-idris2 --cg refc -o bits_case BitsCase.idr > /dev/null
-./build/exec/bits_case
+idris2 --cg refc -o bits_case BitsCase.idr
+$VALGRIND ./build/exec/bits_case

tests/refc/prims/run

@@ -1,4 +1,4 @@
 . ../../testutils.sh
 
-idris2 --cg refc -o test Test.idr > /dev/null
-./build/exec/test
+idris2 --cg refc -o test Test.idr
+$VALGRIND ./build/exec/test

tests/refc/refc001/run

@@ -1,4 +1,4 @@
 . ../../testutils.sh
 
-idris2 --cg refc -o refc001 Tail.idr > /dev/null
-./build/exec/refc001
+idris2 --cg refc -o refc001 Tail.idr
+$VALGRIND ./build/exec/refc001

tests/refc/refc002/run

@@ -1,4 +1,4 @@
 . ../../testutils.sh
 
-idris2 --cg refc -o refc002 RecordProjection.idr > /dev/null
-./build/exec/refc002
+idris2 --cg refc -o refc002 RecordProjection.idr
+$VALGRIND ./build/exec/refc002

tests/refc/refc003/run

@@ -1,4 +1,4 @@
 . ../../testutils.sh
 
-idris2 --cg refc -o refc003 Issue1191.idr > /dev/null
-./build/exec/refc003
+idris2 --cg refc -o refc003 Issue1191.idr
+$VALGRIND ./build/exec/refc003

tests/refc/reg001/run

@@ -1,5 +1,4 @@
 . ../../testutils.sh
 
 idris2 --cg refc Issue1843.idr -o test
-
-./build/exec/test
+$VALGRIND ./build/exec/test

tests/refc/strings/TestStrings.idr

@@ -29,8 +29,17 @@ main = do
     putStrLn $ reverse helloWorld
     putStrLn $ reverse ""
     putStrLn $ substr 1 2 helloWorld
-    putStrLn $ substr 10 10 helloWorld
-    putStrLn $ substr 1 2 ""
+
+    let overtake0 = substr 10 10 helloWorld
+    putStrLn $ "\{show $ length overtake0} : \{overtake0}"
+    let overtake1 = substr 1 2 ""
+    putStrLn $ "\{show $ length overtake1} : \{overtake1}"
+    putStrLn $ substr (-10) (1) helloWorld
+    putStrLn $ substr 2 (-2) helloWorld
+    putStrLn $ substr (-5) (-2) helloWorld
+    putStrLn $ substr 1000000 1 helloWorld
+    putStrLn $ substr 5 100000 helloWorld
+
     putStrLn $ show $ assert_total $ strIndex helloWorld 1
 
     putStrLn $ strCons 'a' "bc"

tests/refc/strings/expected

@@ -4,8 +4,13 @@ Hello, world
 dlrow ,olleH
 
 el
-ld
+2 : ld
+0 : 
+H
 
+
+
+, world
 'e'
 abc
 a

tests/refc/strings/run

@@ -1,4 +1,4 @@
 . ../../testutils.sh
 
-idris2 --cg refc -p contrib -o refc_strings TestStrings.idr > /dev/null
-./build/exec/refc_strings
+idris2 --cg refc -p contrib -o refc_strings TestStrings.idr
+$VALGRIND ./build/exec/refc_strings

tests/testutils.sh

@@ -11,6 +11,12 @@ idris2="$1"
 rm -rf build
 rm -rf prefix
 
+if type valgrind >/dev/null 2>&1; then
+    export VALGRIND="valgrind --leak-check=full -s --log-file=output.valgrind.refc.log"
+else
+    unset VALGRIND
+fi
+
 idris2() {
     $idris2 --no-banner --console-width 0 --no-color "$@"
 }