Skip to content

Commit

Permalink
Fix unparser interactions of bitOrder with suspensions/splits.
Browse files Browse the repository at this point in the history
Call ustate.bitOrder after each term unparser.
setPriorBitOrder in a few places where it was missing.

This corrects an oversight where the unparser was not modified to handle
bit order changes properly. Because few data formats change bit orders
in the middle of data, this error went unrecognized until it was
discovered in NACT Link16 data where the NACT envelope is MSBF, but the
Link16 payload is LSBF.

Test bitOrderOVC1 exercises changing bit orders where a bit order change
occurs, on a byte boundary, but alignment fill is inserted before to
fill a partial byte. The alignment fill for the fragment byte must be
LSBF even though the element having that alignment fill is MSBF.

The tests test_ep1 and test_ep2 are also envelope-payload tests with BE MSBF
envelop and LE LSBF payloads.

DAFFODIL-1884
  • Loading branch information
mbeckerle committed Feb 8, 2018
1 parent 11d36b2 commit 6654c6b
Show file tree
Hide file tree
Showing 8 changed files with 233 additions and 151 deletions.
Expand Up @@ -247,15 +247,6 @@ trait DataOutputStreamImplMixin extends DataStreamCommonState
private var fragmentLastByteLimit_ : Int = 0
def fragmentLastByteLimit = fragmentLastByteLimit_

private var maybeFragmentBitOrder: Maybe[BitOrder] = Nope
def fragmentBitOrder = {
Assert.usage(maybeFragmentBitOrder.isDefined)
this.maybeFragmentBitOrder.value
}
def setFragmentBitOrder(bo: BitOrder) {
maybeFragmentBitOrder = One(bo)
}

def setFragmentLastByte(newFragmentByte: Int, nBitsInUse: Int) {
Assert.usage(nBitsInUse >= 0 && nBitsInUse <= 7)
Assert.usage(newFragmentByte >= 0 && newFragmentByte <= 255) // no bits above first byte are in use.
Expand Down
Expand Up @@ -179,7 +179,7 @@ final class DirectOrBufferedDataOutputStream private[io] (var splitFrom: DirectO
*/
private var _javaOutputStream: java.io.OutputStream = bufferingJOS

private[io] final def isBuffering: Boolean = {
final def isBuffering: Boolean = {
val res = getJavaOutputStream() _eq_ bufferingJOS
res
}
Expand Down
Expand Up @@ -71,9 +71,6 @@ import edu.illinois.ncsa.daffodil.schema.annotation.props.gen.ByteOrder
import edu.illinois.ncsa.daffodil.processors.charset.BitsCharsetDecoder
import edu.illinois.ncsa.daffodil.processors.charset.BitsCharsetEncoder
import edu.illinois.ncsa.daffodil.processors.unparsers.UState
import edu.illinois.ncsa.daffodil.processors.parsers.ParserBitOrderChecks
import edu.illinois.ncsa.daffodil.processors.parsers.PState
import edu.illinois.ncsa.daffodil.processors.unparsers.UnparserBitOrderChecks

/**
* Trait mixed into the PState.Mark object class and the ParseOrUnparseState
Expand Down Expand Up @@ -200,6 +197,23 @@ abstract class ParseOrUnparseState protected (
private def runtimeData = processor.context
private def termRuntimeData = runtimeData.asInstanceOf[TermRuntimeData]

/**
* Checks if the bit order change is legal.
*
* For parsing we know the bitPos, so we can determine if we're at a byte boundary.
*
* For unparsing we may not know the absolute bitPos, so we cannot necessarily
* determine if the boundary is legal or not.
*
* If we know the absoluteBitPos we do the check (as for parsing).
*
* If we do not know the absoluteBitPos, then in that case, we split the
* DataOutputStream into original and buffered. The "check" then occurs
* when these DataOutputStreams are collapsed back together.
*
* So this "check" call, can have an important side effect when unparsing that
* queues up the check to be done in the future.
*/
protected def checkBitOrder(): Unit

/**
Expand Down Expand Up @@ -306,7 +320,7 @@ abstract class ParseOrUnparseState protected (
if (this.processor.isPrimitive)
if (decoderCacheEntry_.encodingMandatoryAlignmentInBitsArg != 1)
if (this.bitPos1b % 8 != 1)
ParserBitOrderChecks.checkParseBitOrder(this.asInstanceOf[PState])
checkBitOrder()
}
decoderCacheEntry_
}
Expand All @@ -318,7 +332,7 @@ abstract class ParseOrUnparseState protected (
if (this.processor.isPrimitive)
if (encoderCacheEntry_.encodingMandatoryAlignmentInBitsArg != 1)
if (this.bitPos1b % 8 != 1)
UnparserBitOrderChecks.checkUnparseBitOrder(this.asInstanceOf[UState])
checkBitOrder()
}
encoderCacheEntry_
}
Expand Down
Expand Up @@ -167,6 +167,7 @@ trait Suspension

val mkl = maybeKnownLengthInBits.getULong
buffered.setAbsStartingBitPos0b(originalAbsBitPos0b + mkl)
buffered.setPriorBitOrder(ustate.bitOrder)

}
} else {
Expand Down
Expand Up @@ -284,8 +284,53 @@ final class PState private (
dataInputStream.setDebugging(flag)
}

/**
* Checks for bit order change. If the bit order is changing, checks if we're
* on a proper byte boundary.
*/
final override protected def checkBitOrder(): Unit = {
ParserBitOrderChecks.checkParseBitOrder(this)

//
// TODO: This looks like a lot of overhead for every single parse call.
//
// We need to check for bitOrder change. If it is changing, we
// need to know if it is on a proper byte boundary.
//
val dis = this.dataInputStream
val isChanging = isParseBitOrderChanging(dis)
if (isChanging) {
//
// the bit order is changing. Let's be sure
// that it's legal to do so w.r.t. other properties
// These checks will have been evaluated at compile time if
// all the properties are static, so this is really just
// in case the charset or byteOrder are runtime-valued.
//
this.processor.context match {
case trd: TermRuntimeData => {
val mcboc = trd.maybeCheckBitOrderAndCharsetEv
val mcbbo = trd.maybeCheckByteAndBitOrderEv
if (mcboc.isDefined) mcboc.get.evaluate(this) // Expressions must be evaluated on the element, not before it is created.
if (mcbbo.isDefined) mcbbo.get.evaluate(this)
}
case _ => // ok
}

dis.st.setPriorBitOrder(this.bitOrder)
if (!dis.isAligned(8))
SDE("Can only change dfdl:bitOrder on a byte boundary. Bit pos (1b) was %s.", dis.bitPos1b)
}
}

private def isParseBitOrderChanging(dis: ByteBufferDataInputStream): Boolean = {
this.processor.context match {
case ntrd: NonTermRuntimeData => false
case _ => {
val priorBitOrder = dis.st.priorBitOrder
val newBitOrder = this.bitOrder
priorBitOrder ne newBitOrder
}
}
}
}

Expand Down Expand Up @@ -426,53 +471,3 @@ object PState {
createInitialPState(root, dis, output, dataProc)
}
}

object ParserBitOrderChecks {
/**
* Checks for bit order change. If the bit order is changing, checks if we're
* on a proper byte boundary.
*/
final def checkParseBitOrder(pstate: PState) = {
//
// TODO: This looks like a lot of overhead for every single parse call.
//
// We need to check for bitOrder change. If it is changing, we
// need to know if it is on a proper byte boundary.
//
val dis = pstate.dataInputStream
val isChanging = isParseBitOrderChanging(dis, pstate)
if (isChanging) {
//
// the bit order is changing. Let's be sure
// that it's legal to do so w.r.t. other properties
// These checks will have been evaluated at compile time if
// all the properties are static, so this is really just
// in case the charset or byteOrder are runtime-valued.
//
pstate.processor.context match {
case trd: TermRuntimeData => {
val mcboc = trd.maybeCheckBitOrderAndCharsetEv
val mcbbo = trd.maybeCheckByteAndBitOrderEv
if (mcboc.isDefined) mcboc.get.evaluate(pstate) // Expressions must be evaluated on the element, not before it is created.
if (mcbbo.isDefined) mcbbo.get.evaluate(pstate)
}
case _ => // ok
}

dis.st.setPriorBitOrder(pstate.bitOrder)
if (!dis.isAligned(8))
pstate.SDE("Can only change dfdl:bitOrder on a byte boundary. Bit pos (1b) was %s.", dis.bitPos1b)
}
}

private def isParseBitOrderChanging(dis: ByteBufferDataInputStream, pstate: PState): Boolean = {
pstate.processor.context match {
case ntrd: NonTermRuntimeData => false
case _ => {
val priorBitOrder = dis.st.priorBitOrder
val newBitOrder = pstate.bitOrder
priorBitOrder ne newBitOrder
}
}
}
}

0 comments on commit 6654c6b

Please sign in to comment.