From 630c012d7c3252c755a962cb510bc8c39b878367 Mon Sep 17 00:00:00 2001 From: Mingun Date: Sun, 5 Dec 2021 22:59:43 +0500 Subject: [PATCH] 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` --- .../struct/CalculateSeqSizes$Test.scala | 82 +++++++++++++++++++ .../struct/precompile/CalculateSeqSizes.scala | 55 +++++++++---- 2 files changed, 122 insertions(+), 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 index 76eb90ad2..7c2b04941 100644 --- 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: ` 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: ` 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: ` 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: ` 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: ` 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: ` 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 index fb5864759..d3a2b8bbc 100644 --- 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)