String indexing bound check now includes the terminating 0 character. Also fix negative indexes on strings.

fixes #190

Author: irmen

compiler/src/prog8/compiler/astprocessing/AstChecker.kt

@@ -1962,7 +1962,7 @@ internal class AstChecker(private val program: Program,
             } else if (target.datatype.isString) {
                 if (target.value is StringLiteral) {
                     // check string lengths for non-memory mapped strings
-                    val stringLen = (target.value as StringLiteral).value.length
+                    val stringLen = (target.value as StringLiteral).value.length + 1    // include the 0-byte terminator
                     if (index != null && index !in 0..<stringLen)
                         errors.err("index out of bounds", arrayIndexedExpression.indexer.position)
                 }

compiler/src/prog8/compiler/astprocessing/VariousCleanups.kt

@@ -432,6 +432,16 @@ internal class VariousCleanups(val program: Program, val errors: IErrorReporter,
                     val newIndex = NumericLiteral.optimalNumeric(arraysize+index, arrayIndexedExpression.indexer.position)
                     arrayIndexedExpression.indexer.indexExpr = newIndex
                     newIndex.linkParents(arrayIndexedExpression.indexer)
+                } else if(target?.datatype?.isString==true) {
+                    val stringsize = (target.value as StringLiteral).value.length
+                    if(stringsize+index < 0) {
+                        errors.err("index out of bounds", arrayIndexedExpression.position)
+                        return noModifications
+                    }
+                    // replace the negative index by the normal index
+                    val newIndex = NumericLiteral.optimalNumeric(stringsize+index, arrayIndexedExpression.indexer.position)
+                    arrayIndexedExpression.indexer.indexExpr = newIndex
+                    newIndex.linkParents(arrayIndexedExpression.indexer)
                 }
             } else if(arrayIndexedExpression.pointerderef!=null) {
                 TODO("cleanup pointer indexing ${arrayIndexedExpression.position}")

compiler/test/ast/TestVariousCompilerAst.kt

@@ -260,6 +260,48 @@ main {
             compileText(VMTarget(), optimize=false, src, outputDir, writeAssembly=false, errors = errors) shouldBe null
             errors.errors.single() shouldContain  "cannot use byte value"
         }
+
+        test("string indexing bounds checks") {
+            val src="""
+main {
+    sub start() {
+        ubyte[] array = ['h', 'e', 'l', 'l', 'o', 0]
+        str name = "hello"
+
+        name[5] = '!'       ; don't do this in real code...
+        name[5] = 0
+        name[6] = 99        ; out of bounds
+        name[-1] = 99       ; ok
+        name[-5] = 99       ; ok
+
+        cx16.r0L = name[5]
+        cx16.r1L = name[6]   ; out of bounds
+        cx16.r1L = name[-1]  ; ok
+        cx16.r1L = name[-5]  ; ok
+
+        array[5] = '!'
+        array[5] = 0
+        array[6] = 99        ; out of bounds
+        array[-1] = 99       ; ok
+        array[-5] = 99       ; ok
+        array[-6] = 99       ; ok
+
+        cx16.r0L = array[5]
+        cx16.r1L = array[6]  ; out of bounds
+        cx16.r1L = array[-1] ; ok
+        cx16.r1L = array[-5] ; ok
+        cx16.r1L = array[-6] ; ok
+    }
+}"""
+
+            val errors = ErrorReporterForTests()
+            compileText(C64Target(), optimize=false, src, outputDir, writeAssembly=false, errors = errors) shouldBe null
+            errors.errors.size shouldBe 4
+            errors.errors[0] shouldContain ":9:13: index out of bounds"
+            errors.errors[1] shouldContain ":14:24: index out of bounds"
+            errors.errors[2] shouldContain ":20:14: index out of bounds"
+            errors.errors[3] shouldContain ":26:25: index out of bounds"
+        }
     }
 
     context("return value") {

compiler/test/codegeneration/TestArrayThings.kt

@@ -251,32 +251,26 @@ main {
         val src="""
 main {
     sub start() {
-        ubyte[10] array
-        array[-11] = 0
-    }
-}"""
-        val errors = ErrorReporterForTests()
-        compileText(VMTarget(), false, src, outputDir, writeAssembly = false, errors = errors) shouldBe null
-        errors.errors.size shouldBe 1
-        errors.errors[0] shouldContain "out of bounds"
-    }
+        ubyte[] array = ['h', 'e', 'l', 'l', 'o', 0]
+        str name = "hello"
 
-    test("bounds checking on strings invalid cases") {
-        val src="""
-main {
-    sub start() {
-        str name = "1234567890"
-        name[10] = 0
-        name[-1] = 0
-        name[-11] = 0
+        name[-5] = 99       ; ok
+        name[-6] = 99       ; out of bounds
+        cx16.r1L = name[-5]  ; ok
+        cx16.r1L = name[-6]  ; out of bounds
+        array[-6] = 99       ; ok
+        array[-7] = 99       ; out of bounds
+        cx16.r1L = array[-6] ; ok
+        cx16.r1L = array[-7] ; out of bounds
     }
 }"""
         val errors = ErrorReporterForTests()
         compileText(VMTarget(), false, src, outputDir, writeAssembly = false, errors = errors) shouldBe null
-        errors.errors.size shouldBe 3
-        errors.errors[0] shouldContain "out of bounds"
-        errors.errors[1] shouldContain "out of bounds"
-        errors.errors[2] shouldContain "out of bounds"
+        errors.errors.size shouldBe 4
+        errors.errors[0] shouldContain ":8:9: index out of bounds"
+        errors.errors[1] shouldContain ":10:20: index out of bounds"
+        errors.errors[2] shouldContain ":12:9: index out of bounds"
+        errors.errors[3] shouldContain ":14:20: index out of bounds"
     }
 
     test("array and string initializer with multiplication") {

docs/source/todo.rst

@@ -1,8 +1,6 @@
 TODO
 ====
 
-- fix/check github issues.
-
 
 Future Things and Ideas
 ^^^^^^^^^^^^^^^^^^^^^^^

examples/test.p8

@@ -1,7 +1,33 @@
+%import textio
+
 main {
-     sub start() {
-        cx16.r0L = cx16.r1 as ubyte
-        cx16.r1sL = cx16.r2s as byte
-     }
 
+    sub start() {
+        ubyte[] array = ['h', 'e', 'l', 'l', 'o', 0]
+        str name = "hello"
+
+        name[5] = '!'       ; don't do this in real code...
+        name[5] = 0
+        name[6] = 99        ; out of bounds
+        name[-1] = 99       ; ok
+        name[-5] = 99       ; ok
+
+        cx16.r0L = name[5]
+        cx16.r1L = name[6]   ; out of bounds
+        cx16.r1L = name[-1]  ; ok
+        cx16.r1L = name[-5]  ; ok
+
+        array[5] = '!'
+        array[5] = 0
+        array[6] = 99        ; out of bounds
+        array[-1] = 99       ; ok
+        array[-5] = 99       ; ok
+        array[-6] = 99       ; ok
+
+        cx16.r0L = array[5]
+        cx16.r1L = array[6]  ; out of bounds
+        cx16.r1L = array[-1] ; ok
+        cx16.r1L = array[-5] ; ok
+        cx16.r1L = array[-6] ; ok
+    }
 }