Add tests for sizeof of repeated attributes

- Forbid negative counts of repetitions in `repeat-expr` - Warn about zero counts in `repeat-expr` - Forbid falsy values in `repeat-until` - Warn about only one iteration in `repeat-until`
kaitai-io · Sep 12, 2023 · 630c012 · 630c012
1 parent cc217c3
commit 630c012
Show file tree

Hide file tree

Showing 2 changed files with 122 additions and 15 deletions.
diff --git a/jvm/src/test/scala/io/kaitai/struct/CalculateSeqSizes$Test.scala b/jvm/src/test/scala/io/kaitai/struct/CalculateSeqSizes$Test.scala
@@ -5,7 +5,9 @@ import io.kaitai.struct.datatype.DataType
 import io.kaitai.struct.datatype.DataType._
 import io.kaitai.struct.exprlang.{Ast, Expressions}
 import io.kaitai.struct.format.{DynamicSized, FixedSized, MetaSpec, Sized, YamlAttrArgs}
+import io.kaitai.struct.format.{RepeatSpec, NoRepeat, RepeatExpr, RepeatUntil, RepeatEos}
 import io.kaitai.struct.precompile.CalculateSeqSizes
+import io.kaitai.struct.problems.CompilationProblemException
 
 import org.scalatest.FunSpec
 import org.scalatest.Matchers._
@@ -82,6 +84,10 @@ class CalculateSeqSizes$Test extends FunSpec {
     ))
   }
 
+  private def repeat(size: Sized, repeat: RepeatSpec): Sized = {
+    CalculateSeqSizes.sizeMultiply(size, repeat, List())
+  }
+
   describe("CalculateSeqSizes") {
     it("built-in types has correct size") {
       sizeof("s1") should be (FixedSized( 8))
@@ -209,5 +215,81 @@ class CalculateSeqSizes$Test extends FunSpec {
         )) should be (DynamicSized)
       }
     }
+
+    describe("repeat") {
+      it("no repeat does not change calculated size") {
+        repeat(FixedSized(42), NoRepeat) should be (FixedSized(42))
+        repeat(DynamicSized,   NoRepeat) should be (DynamicSized)
+      }
+
+      it("`repeat-expr: <negative count>` produces compilation error") {
+        intercept[CompilationProblemException] {
+          repeat(FixedSized(0), RepeatExpr(Ast.expr.IntNum(-1)))
+        }.getMessage() should be ("(main): /: error: negative count of repetitions: -1")
+
+        intercept[CompilationProblemException] {
+          repeat(FixedSized(42), RepeatExpr(Ast.expr.IntNum(-1)))
+        }.getMessage() should be ("(main): /: error: negative count of repetitions: -1")
+
+        intercept[CompilationProblemException] {
+          repeat(DynamicSized, RepeatExpr(Ast.expr.IntNum(-1)))
+        }.getMessage() should be ("(main): /: error: negative count of repetitions: -1")
+      }
+
+      it("`repeat-expr: <zero count>` produces FixedSized(0)") {
+        repeat(FixedSized(42), RepeatExpr(Ast.expr.IntNum(0))) should be (FixedSized(0))
+        repeat(DynamicSized,   RepeatExpr(Ast.expr.IntNum(0))) should be (FixedSized(0))
+      }
+
+      it("`repeat-expr: <positive count>` multiplies size") {
+        repeat(FixedSized(42), RepeatExpr(Ast.expr.IntNum(1))) should be (FixedSized(42))
+        repeat(DynamicSized,   RepeatExpr(Ast.expr.IntNum(1))) should be (DynamicSized)
+
+        repeat(FixedSized(42), RepeatExpr(Ast.expr.IntNum(2))) should be (FixedSized(84))
+        repeat(DynamicSized,   RepeatExpr(Ast.expr.IntNum(2))) should be (DynamicSized)
+      }
+
+      it("`repeat-until: <falsy expression>` produces compilation error") {
+        intercept[CompilationProblemException] {
+          repeat(FixedSized(0), RepeatUntil(Ast.expr.Bool(false)))
+        }.getMessage() should be ("(main): /: error: infinity cycle: stop condition is always `false`")
+
+        intercept[CompilationProblemException] {
+          repeat(FixedSized(42), RepeatUntil(Ast.expr.Bool(false)))
+        }.getMessage() should be ("(main): /: error: infinity cycle: stop condition is always `false`")
+
+        intercept[CompilationProblemException] {
+          repeat(DynamicSized, RepeatUntil(Ast.expr.Bool(false)))
+        }.getMessage() should be ("(main): /: error: infinity cycle: stop condition is always `false`")
+      }
+
+      it("`repeat-until: <truly expression>` returns the same size") {
+        repeat(FixedSized(42), RepeatUntil(Ast.expr.Bool(true))) should be (FixedSized(42))
+        repeat(DynamicSized,   RepeatUntil(Ast.expr.Bool(true))) should be (DynamicSized)
+      }
+
+      it("`repeat-until: <any expression>` produces DynamicSized") {
+        val x = Ast.expr.Name(Ast.identifier("x"))
+
+        repeat(FixedSized(42), RepeatUntil(x)) should be (DynamicSized)
+        repeat(DynamicSized,   RepeatUntil(x)) should be (DynamicSized)
+      }
+
+      it("FixedSize(0) always produce FixedSize(0)") {
+        val x = Ast.expr.Name(Ast.identifier("x"))
+
+        repeat(FixedSized(0), NoRepeat) should be (FixedSized(0))
+
+        repeat(FixedSized(0), RepeatExpr(Ast.expr.IntNum(0))) should be (FixedSized(0))
+        repeat(FixedSized(0), RepeatExpr(Ast.expr.IntNum(1))) should be (FixedSized(0))
+        repeat(FixedSized(0), RepeatExpr(Ast.expr.IntNum(2))) should be (FixedSized(0))
+        repeat(FixedSized(0), RepeatExpr(x)) should be (FixedSized(0))
+
+        repeat(FixedSized(0), RepeatUntil(Ast.expr.Bool(true))) should be (FixedSized(0))
+        repeat(FixedSized(0), RepeatUntil(x)) should be (FixedSized(0))
+
+        repeat(FixedSized(0), RepeatEos) should be (FixedSized(0))
+      }
+    }
   }
 }
diff --git a/shared/src/main/scala/io/kaitai/struct/precompile/CalculateSeqSizes.scala b/shared/src/main/scala/io/kaitai/struct/precompile/CalculateSeqSizes.scala
@@ -5,6 +5,7 @@ import io.kaitai.struct.datatype.DataType
 import io.kaitai.struct.datatype.DataType._
 import io.kaitai.struct.exprlang.Ast
 import io.kaitai.struct.format._
+import io.kaitai.struct.problems.KSYParseError
 
 class CalculateSeqSizes(specs: ClassSpecs) {
   def run(): Unit = {
@@ -13,21 +14,45 @@ class CalculateSeqSizes(specs: ClassSpecs) {
 }
 
 object CalculateSeqSizes {
-  def sizeMultiply(sizeElement: Sized, repeat: RepeatSpec) = {
-    sizeElement match {
-      case FixedSized(elementSize) =>
-        repeat match {
-          case NoRepeat =>
-            sizeElement
-          case RepeatExpr(expr) =>
-            expr.evaluateIntConst match {
-              case Some(count) => FixedSized(elementSize * count.toInt)
-              case None => DynamicSized
-            }
-          case _: RepeatUntil | RepeatEos =>
-            DynamicSized
+  def sizeMultiply(sizeElement: Sized, repeat: RepeatSpec, path: List[String]): Sized = {
+    repeat match {
+      case NoRepeat => sizeElement
+
+      case RepeatExpr(expr) => (expr.evaluateIntConst, sizeElement) match {
+        case (Some(count), _) if count < 0 =>
+          throw KSYParseError.withText(s"negative count of repetitions: ${count}", path)
+
+        case (Some(count), _) if count == 0 => {
+          //TODO: add user visible warning
+          Log.seqSizes.warn(() => s"repetition count expression ${expr} is always `0`, no iterations will be performed")
+          FixedSized(0)
         }
-      case _ => sizeElement
+        case (Some(count), FixedSized(size)) => FixedSized(size * count.toInt)//FIXME: toInt is a potential footgun
+
+        case (_, FixedSized(0)) => FixedSized(0)
+        case (_, _) => DynamicSized
+      }
+
+      case RepeatUntil(expr) => expr.evaluateBoolConst match {
+        case Some(false) =>
+          throw KSYParseError.withText("infinity cycle: stop condition is always `false`", path)
+
+        case Some(true) => {
+          //TODO: add user visible warning
+          Log.seqSizes.warn(() => s"expression ${expr} is always `true`, cycle will be stopped after first iteration")
+          sizeElement
+        }
+
+        case None => sizeElement match {
+          case FixedSized(0) => FixedSized(0)
+          case _ => DynamicSized
+        }
+      }
+
+      case RepeatEos => sizeElement match {
+        case FixedSized(0) => FixedSized(0)
+        case _ => DynamicSized
+      }
     }
   }
 
@@ -70,7 +95,7 @@ object CalculateSeqSizes {
     var seqPos: Option[Int] = Some(0)
     curClass.seq.foreach { attr =>
       val sizeElement = dataTypeBitsSize(attr.dataType)
-      val sizeContainer = sizeMultiply(sizeElement, attr.cond.repeat)
+      val sizeContainer = sizeMultiply(sizeElement, attr.cond.repeat, attr.path)
 
       op(attr, seqPos, sizeElement, sizeContainer)