Taking this discussion to the dev@daffodil list. So whenever there is a suspension, we do create a new buffered data output stream, and a UState (actually a sub-class called UStateForSuspension, there are two subclasses of abstract UState, the other is UStateMain) that provides the additional things needed to say, compute expressions, should they be needed to complete the unparsing later. The UStateForSuspension object also caches the bitOrder, encoders/decoders, etc. - but those caches should be empty given that it is a new object.
But that's not sufficient. Consider that we are unparsing LSBF data, and we end in the middle of a byte. So there is a fragment of a byte. Then next suppose we're on an element or model group (a Term generally) that has alignment 1 byte, and bit order MSBF. So we go to move over to the byte boundary, filling the unused bits with fill byte. Ooops. which bits. Not the MSBF bits the current term would tell us is the proper bit order. It should be the LSBF bits where the byte was left off. So the bit order of the fragment byte must be tracked. Looks like it is. There's a maybeFragmentBitOrder var, and a setter setFragmentBitOrder on the DirectOrBufferedDataOutputStream class. But, it is never called. Oops. Looks like I put the mechanism in place to do this, but didn't finish the implementation. ...mikeb ________________________________ From: Stephen Lawrence Sent: Monday, February 5, 2018 10:29 AM To: Mike Beckerle Subject: Re: bitOrder Regression Ok. Let's talk/email through this and make sure we fully understand the problem/what the solution would entail before decided if the fix can be punted or not. This just popped into my head: Perhaps the solution is everytime there is a suspension, we just need to create a new buffered DOS. This is because that suspension could potentially change the bitorder. This buffered DOS could then have a prior bitOrder of "unknown", or has a flag about what its initial bit order is, or something along those lines. We write to that DOS assuming the prior bit order is acceptable (i.e. the same as the first thing written to the buffered DOS). Then anytime we collapse a buffered DOS into the direct DOS, we just need to ensure that that either 1) the direct DOS ends on a byte boundary in which case we don't care about the bit order of the buffered DOS or 2) the final bitOrder of the direct DOS must match the initial bitOrder of the buffered DOS. I feel like we already have that logic or somethign close, so maybe the current logic is just slightly off. - Steve On 02/05/2018 10:20 AM, Mike Beckerle wrote: > I did not make any progress. I spend enough time to determine I couldn't > easily > fix it. > > > The problem originated in Link16 because it uses bigEndian MSBF envelopes > (NACT) > surrounding little endian LSBF data (Link16), however, that is working > currently. The bug fix is perhaps something we can live without for now. VMF > doesn't run into this because the envelopes (MIL-STD-2045) are also little > endian LSBF. > > > Fixing it, I'd really like to spend some time talking through the design > issues > with you because it is subtle. it's not just a matter of finding the one spot > we > forgot to put something in. (Well, perhaps it is, but I'd like to capture the > design intent and articulate it in comments in the code.) > > > At this point I am not sure we are keeping track of bitOrder in the right way > in > the unparser. We depend on all unparsers running so that the underlying bit > stream is either marked with one bitOrder or the other because the boundary is > known, or it is split at the transition boundary, and checked later once the > actual boundary position is known. There are situations where we're not > recording the bit order, due to IVC elements, and perhaps due to other things. > > -------------------------------------------------------------------------------- > *From:* Stephen Lawrence > *Sent:* Monday, February 5, 2018 8:51:36 AM > *To:* Mike Beckerle > *Subject:* Re: bitOrder Regression > Did you make any progress with this regression? I assume this test came > from VMF so I suspect we want it fixed for 2.1.0, but maybe it can be > punted to the next release? I'm not sure of its severity and what this > bug affects. > > - Steve > > > On 01/31/2018 01:14 PM, Steve Lawrence wrote: >> I think there might be another bug somewhere. I added a hack so that >> when a suspension was created it would check the bitOrder and cause a >> new buffered DOS if it changed (basically, I just call state.bitOrder at >> the end of the suspension code and that handles all the logic). >> >> However, with this hack, I found that when the test switches back to BE >> MSBF for the last element, the bitOrder is never checked. This means >> this element ends up written to a buffered DOS that had LE LSBF and >> things get go off the rails. I confirmed this but adding a >> state.bitOrder in the unparse() method to ensure that bitOrder is >> checked and that fixed it. However, I don't think this is the right >> place for it. And places that feel right to me end up with invariants. >> Not sure the right place for this logic. >> >> So I think there are two issues here: >> >> 1) Suspensions aren't setting/checking prior bit order >> 2) Bit order isn't checked when switching from LE LSBF to BE MSBF >> >> You're more familiar with this code so probably have a better idea of >> the right fix. >> >> - Steve >> >> On 01/31/2018 08:14 AM, Mike Beckerle wrote: >>> My quick tests last night concur. >>> >>> We need to insure that priorBitOrder is set and propagated correctly in the >>> unparser even when suspensions are created. >>> >>> >>> -------- Original message -------- >>> From: Stephen Lawrence <slawre...@tresys.com> >>> Date: 1/30/18 9:10 PM (GMT-05:00) >>> To: Mike Beckerle <mbecke...@tresys.com> >>> Subject: Re: bitOrder Regression >>> >>> Had some time, took at alookt at the test a little bit. Here's what I >>> think is going on: >>> >>> There is an element "a" that is BE MSBF. The bitOrder is correctly set >>> to MSBF when this is unparsed. >>> >>> The following siblings of this element are LE LSBF. However, the first >>> couple of siblings are OVC. The OVC must be suspended, but ends up >>> moving the bit position even though nothing is written. Since nothing is >>> written, bitOrder is never accessed and priorBitOrder is never set. So >>> it is still MSBF. >>> >>> Eventually we get to a sibling "padBit" that doesn't need to suspend. It >>> tries to write a padding bit at bitPosition 28 (1b). The priorBitOrder >>> is still MSBF but padBit is LSBF, so it complains about changing >>> bitOrrder on a non-byte boundary. >>> >>> The issue here is that had the first sibling not suspended it would have >>> changed the bitOrder to LSBF on a byte boundary and by the time we go to >>> "padBit" it's not on a byte boundary, but it wouldn't be changing the >>> bitOrder. >>> >>> So I think the solution is somehow related to having suspensions ensure >>> that the priorBitOrder is set correctly, even though they haven't >>> written anything yet. I'm not sure exactly how to do this, but this >>> should help narrow things down a bit. >>> >>> - Steve >>> >>> On 01/30/2018 06:36 PM, Mike Beckerle wrote: >>>> Grrg. Probably a regression. I will take a look Wednesday afternoon. >>>> >>>> >>>> -------- Original message -------- >>>> From: Stephen Lawrence <slawre...@tresys.com> >>>> Date: 1/30/18 5:13 PM (GMT-05:00) >>>> To: Mike Beckerle <mbecke...@tresys.com> >>>> Subject: bitOrder Regression >>>> >>>> Mike, >>>> >>>> I rebased the Apache changes and noticed there was a test still in >>>> scala-new, which are currently ignored. The test now fails when I move >>>> it to scala, I suspect due to some of the recent bit order changes. Not >>>> sure if this is a known issue and needs to be moved to scala-debug or if >>>> it's a regression. The test is: >>>> >>>> edu.illinois.ncsa.daffodil.section00.general.TestUnparserGeneral2.test_bitOrderOVC1 >>>> >>>> - Steve >>>> >>> >> >
