From 2aae1f5e30e1fee05669af6c151a56eee9bcf181 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Sat, 20 Jul 2024 22:36:19 +0200 Subject: [PATCH] stricter checks for negative array indexing --- .../compiler/astprocessing/AstChecker.kt | 7 ++ .../compiler/astprocessing/VariousCleanups.kt | 4 + compiler/test/ast/TestVariousCompilerAst.kt | 2 +- .../test/codegeneration/TestArrayThings.kt | 100 ++++++++++++++++++ docs/source/programming.rst | 3 +- docs/source/syntaxreference.rst | 3 +- docs/source/todo.rst | 5 - examples/queens.p8 | 8 +- examples/test.p8 | 46 +------- 9 files changed, 124 insertions(+), 54 deletions(-) diff --git a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt index 00864ad81..8ce17cd78 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt @@ -1418,6 +1418,13 @@ internal class AstChecker(private val program: Program, if(target is VarDecl) { if(target.datatype !in IterableDatatypes && target.datatype!=DataType.UWORD) errors.err("indexing requires an iterable or address uword variable", arrayIndexedExpression.position) + val indexVariable = arrayIndexedExpression.indexer.indexExpr as? IdentifierReference + if(indexVariable!=null) { + if(indexVariable.targetVarDecl(program)?.datatype in SignedDatatypes) { + errors.err("variable array indexing can't be performed with signed variables", indexVariable.position) + return + } + } val arraysize = target.arraysize?.constIndex() val index = arrayIndexedExpression.indexer.constIndex() if(arraysize!=null) { diff --git a/compiler/src/prog8/compiler/astprocessing/VariousCleanups.kt b/compiler/src/prog8/compiler/astprocessing/VariousCleanups.kt index 07acfe977..a059b9bbc 100644 --- a/compiler/src/prog8/compiler/astprocessing/VariousCleanups.kt +++ b/compiler/src/prog8/compiler/astprocessing/VariousCleanups.kt @@ -277,6 +277,10 @@ internal class VariousCleanups(val program: Program, val errors: IErrorReporter, val target = arrayIndexedExpression.arrayvar.targetVarDecl(program) val arraysize = target?.arraysize?.constIndex() if(arraysize!=null) { + if(arraysize+index < 0) { + errors.err("index out of bounds", arrayIndexedExpression.position) + return noModifications + } // replace the negative index by the normal index val newIndex = NumericLiteral.optimalNumeric(arraysize+index, arrayIndexedExpression.indexer.position) arrayIndexedExpression.indexer.indexExpr = newIndex diff --git a/compiler/test/ast/TestVariousCompilerAst.kt b/compiler/test/ast/TestVariousCompilerAst.kt index fa1f4149f..03ad9441e 100644 --- a/compiler/test/ast/TestVariousCompilerAst.kt +++ b/compiler/test/ast/TestVariousCompilerAst.kt @@ -535,7 +535,7 @@ main { byte @shared col = 20 col++ ubyte @shared ubb = lsb(col as uword) - uword @shared vaddr = bottom[col] as uword << 8 ; a mkword will get inserted here + uword @shared vaddr = bottom[cx16.r0L] as uword << 8 ; a mkword will get inserted here } }""" val result = compileText(VMTarget(), optimize=true, src, writeAssembly=false)!! diff --git a/compiler/test/codegeneration/TestArrayThings.kt b/compiler/test/codegeneration/TestArrayThings.kt index 5df4db002..03192ab9e 100644 --- a/compiler/test/codegeneration/TestArrayThings.kt +++ b/compiler/test/codegeneration/TestArrayThings.kt @@ -6,6 +6,7 @@ import io.kotest.matchers.shouldNotBe import io.kotest.matchers.string.shouldContain import prog8.code.target.C64Target import prog8.code.target.VMTarget +import prog8tests.helpers.ErrorReporterForTests import prog8tests.helpers.compileText import kotlin.io.path.readText @@ -220,5 +221,104 @@ main { }""" compileText(C64Target(), false, src, writeAssembly = true) shouldNotBe null } + + test("negative array index variables are not allowed, but ptr indexing is allowed") { + val src=""" +main { + sub start() { + uword @shared pointer + ubyte[10] array + str name = "hello" + + byte sindex + + array[sindex] = 10 + cx16.r0L = array[sindex] + name[sindex] = 0 + cx16.r0L = name[sindex] + pointer[sindex] = 10 + cx16.r0L=pointer[sindex] + } +}""" + val errors = ErrorReporterForTests() + compileText(VMTarget(), false, src, writeAssembly = false, errors = errors) shouldBe null + errors.errors.size shouldBe 4 + errors.errors[0] shouldContain "signed variables" + errors.errors[1] shouldContain "signed variables" + errors.errors[2] shouldContain "signed variables" + errors.errors[3] shouldContain "signed variables" + } + + test("bounds checking for both positive and negative indexes, correct cases") { + val src=""" +main { + sub start() { + ubyte[10] array + array[0] = 0 + array[9] = 0 + array[-1] = 0 + array[-10] = 0 + } +}""" + compileText(VMTarget(), false, src, writeAssembly = false) shouldNotBe null + } + + test("bounds checking on strings, correct cases") { + val src=""" +main { + sub start() { + str name = "1234567890" + name[0] = 0 + name[9] = 0 + } +}""" + compileText(VMTarget(), false, src, writeAssembly = false) shouldNotBe null + } + + test("bounds checking for positive indexes, invalid case") { + val src=""" +main { + sub start() { + ubyte[10] array + array[10] = 0 + } +}""" + val errors = ErrorReporterForTests() + compileText(VMTarget(), false, src, writeAssembly = false, errors = errors) shouldBe null + errors.errors.size shouldBe 1 + errors.errors[0] shouldContain "out of bounds" + } + + test("bounds checking for negative indexes, invalid case") { + val src=""" +main { + sub start() { + ubyte[10] array + array[-11] = 0 + } +}""" + val errors = ErrorReporterForTests() + compileText(VMTarget(), false, src, writeAssembly = false, errors = errors) shouldBe null + errors.errors.size shouldBe 1 + errors.errors[0] shouldContain "out of bounds" + } + + test("bounds checking on strings invalid cases") { + val src=""" +main { + sub start() { + str name = "1234567890" + name[10] = 0 + name[-1] = 0 + name[-11] = 0 + } +}""" + val errors = ErrorReporterForTests() + compileText(VMTarget(), false, src, 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" + } }) diff --git a/docs/source/programming.rst b/docs/source/programming.rst index 4f9987272..b5d521b1c 100644 --- a/docs/source/programming.rst +++ b/docs/source/programming.rst @@ -337,7 +337,8 @@ An uword variable can be used in limited scenarios as a 'pointer' to a byte in m dynamic, location. You can use array indexing on a pointer variable to use it as a byte array at a dynamic location in memory: currently this is equivalent to directly referencing the bytes in memory at the given index. In contrast to a real array variable, the index value can be the size of a word. -Unlike array variables, you cannot use a negative index to count from the end, because the size of the array is unknown. +Unlike array variables, negative indexing for pointer variables does *not* mean it will be counting from the end, because the size of the buffer is unknown. +Instead, it simply addresses memory that lies *before* the pointer variable. See also :ref:`pointervars_programming` **LSB/MSB split word arrays:** diff --git a/docs/source/syntaxreference.rst b/docs/source/syntaxreference.rst index df77539c1..b1fe97278 100644 --- a/docs/source/syntaxreference.rst +++ b/docs/source/syntaxreference.rst @@ -525,7 +525,8 @@ Array indexing Strings and arrays are a sequence of values. You can access the individual values by indexing. Negative index means counted from the end of the array rather than the beginning, where -1 means -the last element in the array, -2 the second-to-last, etc. (Python uses this same scheme) +the last element in the array, -2 the second-to-last, etc. (Python uses this same scheme. +Note that this syntax is only valid for arrays, not for strings! Python does allow the latter, but prog8 does not right now.) Use brackets to index into an array: ``arrayvar[x]`` :: array[2] ; the third byte in the array (index is 0-based) diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 08e34b40b..22fdb839d 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -3,11 +3,6 @@ TODO See open issues on github. -Generate proper index out of bounds error for array[-11] if the array is only size 10. (only array[-1]..array[-10] are valid) - -Putting a signed variable as an array index should be a compiler error. (using it for a pointer indexing is fine - but weird). -Add some more explanation about this to the "array indexing" paragraph in the docs. - Re-generate the skeletons doc files. optimize signed byte/word division by powers of 2 (and shift right?), it's now using divmod routine. (also % ?) diff --git a/examples/queens.p8 b/examples/queens.p8 index 6edcb796a..ba1f28d8d 100644 --- a/examples/queens.p8 +++ b/examples/queens.p8 @@ -10,8 +10,10 @@ main { const ubyte NUMQUEENS=8 ubyte[NUMQUEENS] board - sub could_place(byte row, byte col) -> bool { - byte i + sub could_place(ubyte row, ubyte col) -> bool { + if row==0 + return true + ubyte i for i in 0 to row-1 { if board[i]==col or board[i]-i==col-row or board[i]+i==col+row return false @@ -44,7 +46,7 @@ main { } ubyte col for col in 0 to NUMQUEENS-1 { - if could_place(row as byte, col as byte) { + if could_place(row, col) { board[row] = col ; we need to save the local variables row and col. sys.push(row) diff --git a/examples/test.p8 b/examples/test.p8 index 1b284c4f1..5a7faafb3 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -5,49 +5,9 @@ main { sub start() { - uword @shared ptr = $2000 - uword @shared index = 1000 - - @($2000+1000) = 123 - @($2000+1001) = 124 - - cx16.r0L = @(ptr+index) - cx16.r1L = @(ptr+1001) - - txt.print_ub(cx16.r0L) - txt.spc() - txt.print_ub(cx16.r1L) - txt.spc() - - cx16.r2L = ptr[index] - cx16.r3L = ptr[1001] - - txt.print_ub(cx16.r2L) - txt.spc() - txt.print_ub(cx16.r3L) - txt.spc() - - cx16.r0L = 200 - cx16.r1L = 201 - - @(ptr+index) = cx16.r0L - @(ptr+1001) = cx16.r1L - - txt.print_ub(@($2000+1000)) - txt.spc() - txt.print_ub(@($2000+1001)) - txt.spc() - - cx16.r0L = 203 - cx16.r1L = 204 - - ptr[index] = cx16.r0L - ptr[1001] = cx16.r1L - - txt.print_ub(@($2000+1000)) - txt.spc() - txt.print_ub(@($2000+1001)) - txt.spc() + ubyte[10] array + array[10] = 0 + ; array[-11] = 0 ; txt.print_ub(ptr[index]) ; txt.nl()