Skip to content

Commit

Permalink
Add tests for sizeof of repeated attributes
Browse files Browse the repository at this point in the history
- 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`
  • Loading branch information
Mingun committed Mar 8, 2024
1 parent 54ff7f0 commit db4a1ad
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 15 deletions.
82 changes: 82 additions & 0 deletions jvm/src/test/scala/io/kaitai/struct/CalculateSeqSizes$Test.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 scala.collection.immutable.SortedMap

Expand Down Expand Up @@ -85,6 +87,10 @@ class CalculateSeqSizes$Test extends AnyFunSpec {
))
}

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))
Expand Down Expand Up @@ -212,5 +218,81 @@ class CalculateSeqSizes$Test extends AnyFunSpec {
)) 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): /:\n\terror: negative count of repetitions: -1\n")

intercept[CompilationProblemException] {
repeat(FixedSized(42), RepeatExpr(Ast.expr.IntNum(-1)))
}.getMessage() should be ("(main): /:\n\terror: negative count of repetitions: -1\n")

intercept[CompilationProblemException] {
repeat(DynamicSized, RepeatExpr(Ast.expr.IntNum(-1)))
}.getMessage() should be ("(main): /:\n\terror: negative count of repetitions: -1\n")
}

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): /:\n\terror: infinity cycle: stop condition is always `false`\n")

intercept[CompilationProblemException] {
repeat(FixedSized(42), RepeatUntil(Ast.expr.Bool(false)))
}.getMessage() should be ("(main): /:\n\terror: infinity cycle: stop condition is always `false`\n")

intercept[CompilationProblemException] {
repeat(DynamicSized, RepeatUntil(Ast.expr.Bool(false)))
}.getMessage() should be ("(main): /:\n\terror: infinity cycle: stop condition is always `false`\n")
}

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))
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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
}
}
}

Expand Down Expand Up @@ -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)

Expand Down

0 comments on commit db4a1ad

Please sign in to comment.