Parse Error on Out of Range Binary Integers

- currently we don't check the minimum lengths for signed/unsigned binary ints and this causes issues when you try to set a binary int to 0 bits (RSDE), so per the DFDL workgroup we add in these checks.
- add width check to unparse code and associated test
- fix bug where we weren't returning after UnparseError
- add SDEs for checks for non-expression fixed lengths
- currently we pass in isSigned boolean from ElementBaseGrammarMixin, but we can use the new PrimType.PrimNumeric.isSigned member instead for simplicity. We also added the minWidth member to simplify checking the minimum width of PrimNumeric types
- add tunable allowSignedIntegerLength1Bit and update tests to use
- add tests for parse and unparse
- update failing tests
- make it a PE if nBits == 0 for PackedBinaryIntegerBaseParser
- add checkMinWidth and checkMaxWidth into BinaryNumberCheckWidth trait
- add tests for 0 bit length and multiple of 4 check
- add runtime multiple of 4 length check

Deprecation/Compatibility
Although still supported via the tunable allowSignedIntegerLength1Bit, it is recommended that schemas be updated to not depend on 1-bit signed binary integers

DAFFODIL-2297

daffodil-codegen-c/src/main/scala/org/apache/daffodil/codegen/c/DaffodilCCodeGenerator.scala

@@ -278,7 +278,7 @@ object DaffodilCCodeGenerator
       case g: BinaryDouble => binaryFloatGenerateCode(g.e, lengthInBits = 64, cgState)
       case g: BinaryFloat => binaryFloatGenerateCode(g.e, lengthInBits = 32, cgState)
       case g: BinaryIntegerKnownLength =>
-        binaryIntegerKnownLengthGenerateCode(g.e, g.lengthInBits, g.signed, cgState)
+        binaryIntegerKnownLengthGenerateCode(g.e, g.lengthInBits, cgState)
       case g: CaptureContentLengthEnd => noop(g)
       case g: CaptureContentLengthStart => noop(g)
       case g: CaptureValueLengthEnd => noop(g)

daffodil-codegen-c/src/main/scala/org/apache/daffodil/codegen/c/generators/BinaryIntegerKnownLengthCodeGenerator.scala

@@ -18,14 +18,14 @@
 package org.apache.daffodil.codegen.c.generators
 
 import org.apache.daffodil.core.dsom.ElementBase
+import org.apache.daffodil.runtime1.dpath.NodeInfo.PrimType.PrimNumeric
 
 trait BinaryIntegerKnownLengthCodeGenerator extends BinaryValueCodeGenerator {
 
   // Generate C code to parse and unparse an integer element
   def binaryIntegerKnownLengthGenerateCode(
     e: ElementBase,
     lengthInBits: Long,
-    signed: Boolean,
     cgState: CodeGeneratorState
   ): Unit = {
     val cLengthInBits = lengthInBits match {
@@ -35,6 +35,7 @@ trait BinaryIntegerKnownLengthCodeGenerator extends BinaryValueCodeGenerator {
       case n if n <= 64 => 64
       case _ => e.SDE("Binary integer lengths longer than 64 bits are not supported.")
     }
+    val signed = e.primType.asInstanceOf[PrimNumeric].isSigned
     val primType = if (signed) s"int$cLengthInBits" else s"uint$cLengthInBits"
     val addField = valueAddField(e, lengthInBits, primType, _, cgState)
     val validateFixed = valueValidateFixed(e, _, cgState)

daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/ElementBase.scala

@@ -774,11 +774,37 @@ trait ElementBase
     if (
       result.isDefined && repElement.isSimpleType && representation == Representation.Binary
     ) {
+      val nBits = result.get
       primType match {
         case primNumeric: NodeInfo.PrimType.PrimNumeric =>
-          if (primNumeric.width.isDefined) {
-            val nBits = result.get
-            val width = primNumeric.width.get
+          if (primNumeric.minWidth.isDefined) {
+            val minWidth = primNumeric.minWidth.get
+            if (nBits < minWidth) {
+              val isSigned = primNumeric.isSigned
+              val signedStr = if (isSigned) "a signed" else "an unsigned"
+              val outOfRangeFmtStr =
+                "Minimum length for %s binary integer is %d bit(s), number of bits %d out of range. " +
+                  "An unsigned integer with length 1 bit could be used instead."
+              if (isSigned && tunable.allowSignedIntegerLength1Bit && nBits == 1) {
+                SDW(
+                  WarnID.SignedBinaryIntegerLength1Bit,
+                  outOfRangeFmtStr,
+                  signedStr,
+                  minWidth,
+                  nBits
+                )
+              } else {
+                SDE(
+                  outOfRangeFmtStr,
+                  signedStr,
+                  minWidth,
+                  nBits
+                )
+              }
+            }
+          }
+          if (primNumeric.maxWidth.isDefined) {
+            val width = primNumeric.maxWidth.get
             if (nBits > width) {
               SDE(
                 "Number of bits %d out of range for binary %s, must be between 1 and %d bits.",

daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/ElementBaseGrammarMixin.scala

@@ -786,7 +786,7 @@ trait ElementBaseGrammarMixin
   ) {
     ConvertZonedCombinator(
       this,
-      new IBM4690PackedIntegerKnownLength(this, false, binaryNumberKnownLengthInBits),
+      new IBM4690PackedIntegerKnownLength(this, binaryNumberKnownLengthInBits),
       textConverter
     )
   }
@@ -796,7 +796,7 @@ trait ElementBaseGrammarMixin
   ) {
     ConvertZonedCombinator(
       this,
-      new IBM4690PackedIntegerRuntimeLength(this, false),
+      new IBM4690PackedIntegerRuntimeLength(this),
       textConverter
     )
   }
@@ -806,7 +806,7 @@ trait ElementBaseGrammarMixin
   ) {
     ConvertZonedCombinator(
       this,
-      new IBM4690PackedIntegerDelimitedEndOfData(this, false),
+      new IBM4690PackedIntegerDelimitedEndOfData(this),
       textConverter
     )
   }
@@ -816,7 +816,7 @@ trait ElementBaseGrammarMixin
   ) {
     ConvertZonedCombinator(
       this,
-      new IBM4690PackedIntegerPrefixedLength(this, false),
+      new IBM4690PackedIntegerPrefixedLength(this),
       textConverter
     )
   }
@@ -827,7 +827,6 @@ trait ElementBaseGrammarMixin
         this,
         new PackedIntegerKnownLength(
           this,
-          false,
           packedSignCodes,
           binaryNumberKnownLengthInBits
         ),
@@ -838,23 +837,23 @@ trait ElementBaseGrammarMixin
     prod("packedRuntimeLengthCalendar", binaryCalendarRep == BinaryCalendarRep.Packed) {
       ConvertZonedCombinator(
         this,
-        new PackedIntegerRuntimeLength(this, false, packedSignCodes),
+        new PackedIntegerRuntimeLength(this, packedSignCodes),
         textConverter
       )
     }
   private lazy val packedDelimitedLengthCalendar =
     prod("packedDelimitedLengthCalendar", binaryCalendarRep == BinaryCalendarRep.Packed) {
       ConvertZonedCombinator(
         this,
-        new PackedIntegerDelimitedEndOfData(this, false, packedSignCodes),
+        new PackedIntegerDelimitedEndOfData(this, packedSignCodes),
         textConverter
       )
     }
   private lazy val packedPrefixedLengthCalendar =
     prod("packedPrefixedLengthCalendar", binaryCalendarRep == BinaryCalendarRep.Packed) {
       ConvertZonedCombinator(
         this,
-        new PackedIntegerPrefixedLength(this, false, packedSignCodes),
+        new PackedIntegerPrefixedLength(this, packedSignCodes),
         textConverter
       )
     }
@@ -897,7 +896,7 @@ trait ElementBaseGrammarMixin
   private lazy val packedSignCodes =
     PackedSignCodes(binaryPackedSignCodes, binaryNumberCheckPolicy)
 
-  private def binaryIntegerValue(isSigned: Boolean) = {
+  private lazy val binaryIntegerValue = {
     //
     // Is it a single byte or smaller
     //
@@ -910,10 +909,10 @@ trait ElementBaseGrammarMixin
     }
     (binaryNumberRep, lengthKind, binaryNumberKnownLengthInBits) match {
       case (BinaryNumberRep.Binary, LengthKind.Prefixed, _) =>
-        new BinaryIntegerPrefixedLength(this, isSigned)
-      case (BinaryNumberRep.Binary, _, -1) => new BinaryIntegerRuntimeLength(this, isSigned)
+        new BinaryIntegerPrefixedLength(this)
+      case (BinaryNumberRep.Binary, _, -1) => new BinaryIntegerRuntimeLength(this)
       case (BinaryNumberRep.Binary, _, _) =>
-        new BinaryIntegerKnownLength(this, isSigned, binaryNumberKnownLengthInBits)
+        new BinaryIntegerKnownLength(this, binaryNumberKnownLengthInBits)
       case (_, LengthKind.Implicit, _) =>
         SDE("lengthKind='implicit' is not allowed with packed binary formats")
       case (_, _, _)
@@ -923,26 +922,25 @@ trait ElementBaseGrammarMixin
           binaryNumberKnownLengthInBits
         )
       case (BinaryNumberRep.Packed, LengthKind.Delimited, -1) =>
-        new PackedIntegerDelimitedEndOfData(this, isSigned, packedSignCodes)
+        new PackedIntegerDelimitedEndOfData(this, packedSignCodes)
       case (BinaryNumberRep.Packed, LengthKind.Prefixed, -1) =>
-        new PackedIntegerPrefixedLength(this, isSigned, packedSignCodes)
+        new PackedIntegerPrefixedLength(this, packedSignCodes)
       case (BinaryNumberRep.Packed, _, -1) =>
-        new PackedIntegerRuntimeLength(this, isSigned, packedSignCodes)
+        new PackedIntegerRuntimeLength(this, packedSignCodes)
       case (BinaryNumberRep.Packed, _, _) =>
         new PackedIntegerKnownLength(
           this,
-          isSigned,
           packedSignCodes,
           binaryNumberKnownLengthInBits
         )
       case (BinaryNumberRep.Ibm4690Packed, LengthKind.Delimited, -1) =>
-        new IBM4690PackedIntegerDelimitedEndOfData(this, isSigned)
+        new IBM4690PackedIntegerDelimitedEndOfData(this)
       case (BinaryNumberRep.Ibm4690Packed, LengthKind.Prefixed, -1) =>
-        new IBM4690PackedIntegerPrefixedLength(this, isSigned)
+        new IBM4690PackedIntegerPrefixedLength(this)
       case (BinaryNumberRep.Ibm4690Packed, _, -1) =>
-        new IBM4690PackedIntegerRuntimeLength(this, isSigned)
+        new IBM4690PackedIntegerRuntimeLength(this)
       case (BinaryNumberRep.Ibm4690Packed, _, _) =>
-        new IBM4690PackedIntegerKnownLength(this, isSigned, binaryNumberKnownLengthInBits)
+        new IBM4690PackedIntegerKnownLength(this, binaryNumberKnownLengthInBits)
       case (BinaryNumberRep.Bcd, _, _) =>
         primType match {
           case PrimType.Long | PrimType.Int | PrimType.Short | PrimType.Byte =>
@@ -974,13 +972,8 @@ trait ElementBaseGrammarMixin
     // This is in the spirit of that section.
     val res: Gram = primType match {
 
-      case PrimType.Byte | PrimType.Short | PrimType.Int | PrimType.Long | PrimType.Integer => {
-        binaryIntegerValue(true)
-      }
-
-      case PrimType.UnsignedByte | PrimType.UnsignedShort | PrimType.UnsignedInt |
-          PrimType.UnsignedLong | PrimType.NonNegativeInteger => {
-        binaryIntegerValue(false)
+      case n: PrimType.PrimNumeric if n.isInteger => {
+        binaryIntegerValue
       }
 
       case PrimType.Double | PrimType.Float => {

daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/primitives/PrimitivesBinaryNumber.scala

@@ -40,37 +40,33 @@ import org.apache.daffodil.unparsers.runtime1.BinaryIntegerKnownLengthUnparser
 import org.apache.daffodil.unparsers.runtime1.BinaryIntegerPrefixedLengthUnparser
 import org.apache.daffodil.unparsers.runtime1.BinaryIntegerRuntimeLengthUnparser
 
-class BinaryIntegerRuntimeLength(val e: ElementBase, signed: Boolean)
-  extends Terminal(e, true) {
+class BinaryIntegerRuntimeLength(val e: ElementBase) extends Terminal(e, true) {
 
   override lazy val parser = new BinaryIntegerRuntimeLengthParser(
     e.elementRuntimeData,
-    signed,
     e.lengthEv,
     e.lengthUnits
   )
 
   override lazy val unparser: Unparser = new BinaryIntegerRuntimeLengthUnparser(
     e.elementRuntimeData,
-    signed,
     e.lengthEv,
     e.lengthUnits
   )
 }
 
-class BinaryIntegerKnownLength(val e: ElementBase, val signed: Boolean, val lengthInBits: Long)
+class BinaryIntegerKnownLength(val e: ElementBase, val lengthInBits: Long)
   extends Terminal(e, true) {
 
   override lazy val parser = {
-    new BinaryIntegerKnownLengthParser(e.elementRuntimeData, signed, lengthInBits.toInt)
+    new BinaryIntegerKnownLengthParser(e.elementRuntimeData, lengthInBits.toInt)
   }
 
   override lazy val unparser: Unparser =
-    new BinaryIntegerKnownLengthUnparser(e.elementRuntimeData, signed, lengthInBits.toInt)
+    new BinaryIntegerKnownLengthUnparser(e.elementRuntimeData, lengthInBits.toInt)
 }
 
-class BinaryIntegerPrefixedLength(val e: ElementBase, signed: Boolean)
-  extends Terminal(e, true) {
+class BinaryIntegerPrefixedLength(val e: ElementBase) extends Terminal(e, true) {
 
   private lazy val erd = e.elementRuntimeData
   private lazy val plerd = e.prefixedLengthElementDecl.elementRuntimeData
@@ -81,7 +77,6 @@ class BinaryIntegerPrefixedLength(val e: ElementBase, signed: Boolean)
       erd,
       e.prefixedLengthBody.parser,
       plerd,
-      signed,
       e.lengthUnits,
       pladj
     )
@@ -101,7 +96,6 @@ class BinaryIntegerPrefixedLength(val e: ElementBase, signed: Boolean)
       e.prefixedLengthBody.unparser,
       plerd,
       maybeNBits,
-      signed,
       e.lengthUnits,
       pladj
     )

daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/primitives/PrimitivesIBM4690Packed.scala

@@ -33,11 +33,9 @@ import org.apache.daffodil.unparsers.runtime1.IBM4690PackedIntegerKnownLengthUnp
 import org.apache.daffodil.unparsers.runtime1.IBM4690PackedIntegerPrefixedLengthUnparser
 import org.apache.daffodil.unparsers.runtime1.IBM4690PackedIntegerRuntimeLengthUnparser
 
-class IBM4690PackedIntegerRuntimeLength(val e: ElementBase, signed: Boolean)
-  extends Terminal(e, true) {
+class IBM4690PackedIntegerRuntimeLength(val e: ElementBase) extends Terminal(e, true) {
   override lazy val parser = new IBM4690PackedIntegerRuntimeLengthParser(
     e.elementRuntimeData,
-    signed,
     e.lengthEv,
     e.lengthUnits
   )
@@ -49,23 +47,21 @@ class IBM4690PackedIntegerRuntimeLength(val e: ElementBase, signed: Boolean)
   )
 }
 
-class IBM4690PackedIntegerKnownLength(val e: ElementBase, signed: Boolean, lengthInBits: Long)
+class IBM4690PackedIntegerKnownLength(val e: ElementBase, lengthInBits: Long)
   extends Terminal(e, true) {
 
   override lazy val parser =
-    new IBM4690PackedIntegerKnownLengthParser(e.elementRuntimeData, signed, lengthInBits.toInt)
+    new IBM4690PackedIntegerKnownLengthParser(e.elementRuntimeData, lengthInBits.toInt)
 
   override lazy val unparser: Unparser =
     new IBM4690PackedIntegerKnownLengthUnparser(e.elementRuntimeData, lengthInBits.toInt)
 }
 
-class IBM4690PackedIntegerPrefixedLength(val e: ElementBase, signed: Boolean)
-  extends Terminal(e, true) {
+class IBM4690PackedIntegerPrefixedLength(val e: ElementBase) extends Terminal(e, true) {
   override lazy val parser = new IBM4690PackedIntegerPrefixedLengthParser(
     e.elementRuntimeData,
     e.prefixedLengthBody.parser,
     e.prefixedLengthElementDecl.elementRuntimeData,
-    signed,
     e.lengthUnits,
     e.prefixedLengthAdjustmentInUnits
   )

daffodil-core/src/main/scala/org/apache/daffodil/core/grammar/primitives/PrimitivesLengthKind.scala

@@ -205,7 +205,6 @@ case class HexBinaryLengthPrefixed(e: ElementBase) extends Terminal(e, true) {
 
 abstract class PackedIntegerDelimited(
   e: ElementBase,
-  signed: Boolean,
   packedSignCodes: PackedSignCodes
 ) extends StringDelimited(e) {
 
@@ -223,9 +222,8 @@ abstract class PackedIntegerDelimited(
 
 case class PackedIntegerDelimitedEndOfData(
   e: ElementBase,
-  signed: Boolean,
   packedSignCodes: PackedSignCodes
-) extends PackedIntegerDelimited(e, signed, packedSignCodes) {
+) extends PackedIntegerDelimited(e, packedSignCodes) {
   val isDelimRequired: Boolean = false
 }
 
@@ -289,8 +287,7 @@ case class BCDDecimalDelimitedEndOfData(e: ElementBase) extends BCDDecimalDelimi
   val isDelimRequired: Boolean = false
 }
 
-abstract class IBM4690PackedIntegerDelimited(e: ElementBase, signed: Boolean)
-  extends StringDelimited(e) {
+abstract class IBM4690PackedIntegerDelimited(e: ElementBase) extends StringDelimited(e) {
 
   override lazy val parser: DaffodilParser = new IBM4690PackedIntegerDelimitedParser(
     e.elementRuntimeData,
@@ -304,8 +301,8 @@ abstract class IBM4690PackedIntegerDelimited(e: ElementBase, signed: Boolean)
   )
 }
 
-case class IBM4690PackedIntegerDelimitedEndOfData(e: ElementBase, signed: Boolean)
-  extends IBM4690PackedIntegerDelimited(e, signed) {
+case class IBM4690PackedIntegerDelimitedEndOfData(e: ElementBase)
+  extends IBM4690PackedIntegerDelimited(e) {
   val isDelimRequired: Boolean = false
 }