[Java] Permit block field access whenever it is safe (and, unfortunately, in some cases where it is not).

This commit makes the field order checks more-lenient. The aim is to
allow the application code to access fields in the root/top-level block
or blocks inside groups whenever it is safe. However, the approach is
broken and does not work in general.

To (try to) achieve this aim, we rely on the fact that once a block is
positioned correctly, it is safe to access during the remainder of
encoding/decoding. For example, the following code is safe:

```
encoder.wrap(...);
encoder.blockField1(42);
encoder.varData("abc");
encoder.blockField2(43);
```

As soon as `wrap(...)` is called, the top-level block will be positioned
over a valid region for the remainder of encoding.

Groups are slightly trickier. The following code is _safe_:

```
encoder.wrap(...);
Foo.BarEncoder bar = encoder.groupBarCount(2);
bar.next()
    .x(1).y(2)
    .next();
    .x(3).y(4);
```

But the following code is _unsafe_:

```
encoder.wrap(...);
Foo.BarEncoder bar = encoder.groupBarCount(2);
bar.x(1).y(2)
    .next();
    .x(3).y(4);
```

And the following code is _unsafe_ too:

```
encoder.wrap(...)
Foo.BarEncoder bar = encoder.groupBarCount(0);
encoder.baz("abc");
bar.x(1).y(2);
```

The `offset` for the group encoder is only set when `next()` is called.
Therefore, calling `x(1)` and `y(2)` will likely overwrite some data
elsewhere in the buffer (probably near offset 0).

We can still rely on the fact that once `next()` has been called - even
if it is called multiple times, or too many times (causing it to throw)
- the group's block fields will be safe to encode/decode.

Thinking about this in terms of our state machine that models codec
state and field accesses, we can put this another way. There is at least
one transition that represents the `next()` method call. The states
(transitively) reachable from these transitions can safely access the
associated group block.

As already mentioned, despite the passing tests, the approach in this
commit doesn't always work.

I had thought it would be possible to assign a number to each state as
we find them using DFS and use this number as a shortcut to determining
whether a block access was safe. For example, in each block field we
would check:

```
if (currentState.number() >= firstStateAfterNextIsCalled.number())
{
    // ALLOW
}
else
{
    // DENY
}
```

However, (even ignoring the cycles due to repeating groups and the use
of DFS) this solution makes the following assumptions:

1. There is a single state: `firstStateAfterNextIsCalled`, such that all
subsequent transitively reachable states are safe.

2. It is possible to number the states in such a way that
`numberOfState(b) >= numberOfState(a)` implies `b` is (transitively)
reachable from `a`.

Let's consider the following psuedo-schema w.r.t. our assumptions above:

```
group a {
  x: int32
}
group b {
  y: int32
}
```

Can we set `x` if we've called `BEncoder#next`? Sometimes.

The following code is invalid.

```
AEncoder a = encoder.aCount(0);
BEncoder b = encoder.bCount(1).next();
b.y(2);
a.x(1); // Whoops! We overwrote some data somewhere.
```

But this code is valid.

```
AEncoder a = encoder.aCount(1).next();
BEncoder b = encoder.bCount(1).next();
b.y(2);
a.x(1);
```

Therefore, multiple transitions must exist for the `BEncoder#next` call
with different exit states, which breaks Assumption 1.

Assumption 2 is also problematic. As alluded to above, we must have a
bifurcation of states depending on whether a group has elements or is
empty. With arbitrary bifurcations, it is not possible to number the
states in such a way that Assumption 2 holds.

Consider the following state diagram:

```
        state N
         /   \
     empty   non-empty
      /        \
state X        state Y
```

Depending on whether a group is empty, e.g., `aCount(0)`, or not, e.g.,
`aCount(1)`, we transition to either `state X` or `state Y` from
`state N`.

For Assumption 2 to hold, we need
`numberOf(state X) >= numberOf(state N)` (reachable) and
`numberOf(state Y) >= numberOf(state N)` (reachable) and
`numberOf(state X)  < numberOf(state Y)` (unreachable) and
`numberOf(state Y)  < numberOf(state X)` (unreachable).
Clearly, this is impossible.

Prior to this commit, we avoided such problems by enforcing a
more-strict ordering of field accesses. With this more-strict ordering,
setting fields in a group block in later states was not allowed.
Therefore, the state machine didn't have to have to split into mutually
exclusive subgraphs of reachable states depending on whether or not the
group is empty.

For example, let's consider a psuedo-schema with two groups.

```
group a {
  x: int32
}
group b {
  y: int32
}
```

Once the application code calls `encoder.bCount(2).next()`, our state
machine will enter the `V0_B_N_BLOCK` state regardless of whether
previously the application code called `encoder.aCount(0)` or
`encoder.aCount(1).next()`. In `V_B_N_BLOCK` the application code is not
allowed to call `AEncoder#x`; therefore, it is safe, i.e., it has no
false positives. However, it is also too-strict, i.e., it has false
negatives, e.g., the `encoder.aCount(1).next()` case.

How likely are these false negatives to come up in the "wild"?

If we think false negatives will often come up, is it possible to remove
them whilst still using a state machine?

We could consider generating more states to represent the reachable
states under each fork of group emptiness. The number of states would
grow exponentially w.r.t. the number of top-level groups, but let's
ignore that for the moment and just consider whether or not it is
possible.

Continuing the example above with two groups, when the application code
calls `encoder.bCount(2).next()`, the state it enters would depend on
whether it had previously called `encoder.aCount(0)` or
`encoder.aCount(n)...next()` for some value `n > 0`.

To safely model this example, we'd need at least the following states:

1. has no `a` elements
2. has no `a` elements, and has no `b` elements
3. has `a` elements, but has not called `AEncoder#next`
4. has `a` elements, and has called `AEncoder#next`
5. has `a` elements, has called `AEncoder#next`, has `b` elements, but
has not called `BEncoder#next`
6. has `a` elements, has called `AEncoder#next`, has `b` elements, and
has called `BEncoder#next`
7. has no `a` elements, has `b` elements, but has not called
`BEncoder#next`
8. has no `a` elements, has `b` elements, and has called
`BEncoder#next`

A trickier case is with nested groups:

```
group foo {
  x: int32
  group bar {
    y: int32
  }
  r: string
  s: string
}
```

When is `Bar#y` safe to call?

I believe it is safe to call in states where `FooEncoder#next` and
`BarEncoder#next` have been called in-order more recently than
`BarEncoder#count(0)`. Note, as `bar` is a group within `foo`, the
`BarEncoder#next` and `BarEncoder#count` methods may be called
repeatedly during the encoding of a single message.  Implementing a
more-permissive safety check, using this model, would entail checking
that the current state of the encoder/decoder is in a state where this
condition holds.

To safely model this example, we'd need (at least) the following states:

1. has no `foo` elements
2. has `foo` elements, but has not called `FooEncoder#next`
3. has `foo` elements, and has called `FooEncoder#next`
4. has `foo` elements, has called `FooEncoder#next`, and has no `bar`
elements
5. has `foo` elements, has called `FooEncoder#next`, has no `bar`
elements, and has encoded `r`
6. has `foo` elements, has called `FooEncoder#next`, has no `bar`
elements, and has encoded `s`
7. has `foo` elements, has called `FooEncoder#next`, has `bar` elements,
but has not called `BarEncoder#next`
8. has `foo` elements, has called `FooEncoder#next`, has `bar` elements,
has not called `BarEncoder#next`, and has encoded `r`
9. has `foo` elements, has called `FooEncoder#next`, has `bar` elements,
has not called `BarEncoder#next`, and has encoded `s`
10. has `foo` elements, has called `FooEncoder#next`, has `bar` elements,
and has called `BarEncoder#next`
11. has `foo` elements, has called `FooEncoder#next`, has `bar` elements,
and has called `BarEncoder#next`, and has encoded `r`
12. has `foo` elements, has called `FooEncoder#next`, has `bar` elements,
and has called `BarEncoder#next`, and has encoded `s`

```
FooEncoder foo = encoder.fooCount(3); // State 2
foo.next(); // State 3
foo.barCount(0); // State 4
foo.r("abc"); // State 5
foo.s("def"); // State 6
foo.next(); // State 4
BarEncoder bar = foo.barCount(1); // State 7
foo.r("abc"); // State 8
bar.next(); // State 11
bar.y(42); // State 11
foo.s("def"); // State 12
foo.next(); // State X?
bar.y(43); // Weird but allowed.
foo.barCount(0); // State Y? State X cannot be State 10, as State 10 should not allow _re-encoding_ of the bar group count
```

OK. We didn't have enough states. In the case where we "wrap back
around", we need to model whether `bar` block access is allowed (State 13 vs
State 3):

13. has `foo` elements, has called `FooEncoder#next`, has called
`BarEncoder#next` for last iteration, but hasn't set `bar` group size in
this iteration.

An even trickier case is with doubly-nested groups:

```
group foo {
  x: int32
  group bar {
    y: int32
    group baz {
      z: int32
    }
  }
}
```

When is `Baz#z` safe to call?

I believe it is safe to call in states where `FooEncoder#next`,
`BarEncoder#next` and `BazEncoder#next` have been called in-order
more recently than `BarEncoder#count(0)` or `BazEncoder#count(0).
Encoding this in states is likely to result in more cases like State 13
above.

Generalising: a group block field may be accessed only when all outer
groups' and its own `#next` methods have been called in-order more
recently than any outer groups' or its own `#count` method with a `0`
argument.

Can model "more-recently" exactly, i.e., with no false positives and no
false negatives, using a state machine?

Another way of looking at the state machines we generate, is as parsers
for a language. Where our language, based on an SBE schema, comprises
valid "strings" of calls to an encoder/decoder. Is this language
a regular language? If not, representing it using a FSM will not work.
Cue Pumping Lemma?

Next steps:

- Add more tests to show where this approach breaks
- Revert the changes and see if it is acceptable that some valid uses of
codecs will be prevented when enabling field order checking. I.e., it
will give false negatives when testing the hypothesis "this
encoding/decoding is valid".
    - Personally, I think false negatives are better than false
positives. (False positives are possible as of this commit.) Having said
that, it will discourage use of these checks in existing systems where
valid code already exists that does not pass the checks.

Author: ZachBray

sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/FieldOrderModel.java

@@ -35,7 +35,7 @@
 final class FieldOrderModel
 {
     private final Int2ObjectHashMap<State> states = new Int2ObjectHashMap<>();
-    private final Map<Token, TransitionGroup> transitions = new LinkedHashMap<>();
+    private final Map<Token, FieldTransitions> transitions = new LinkedHashMap<>();
     private final Int2ObjectHashMap<State> versionWrappedStates = new Int2ObjectHashMap<>();
     private final Set<String> reservedNames = new HashSet<>();
     private final State notWrappedState = allocateState("NOT_WRAPPED");
@@ -71,13 +71,24 @@ public void forEachState(final Consumer<State> consumer)
 
     public List<Transition> getTransitions(final TransitionContext context, final Token token)
     {
-        final TransitionGroup transitionGroup = transitions.get(token);
-        if (transitionGroup == null)
+        final FieldTransitions fieldTransitions = transitions.get(token);
+        if (fieldTransitions == null)
         {
             return Collections.emptyList();
         }
 
-        return transitionGroup.transitions.get(context);
+        return fieldTransitions.transitions.get(context);
+    }
+
+    public State minimumEntryState(final Token token)
+    {
+        final FieldTransitions fieldTransitions = transitions.get(token);
+        if (fieldTransitions == null)
+        {
+            return null;
+        }
+
+        return fieldTransitions.minimumEntryState();
     }
 
     public static FieldOrderModel findTransitions(
@@ -128,8 +139,8 @@ public void generateGraph(
         final String indent)
     {
         sb.append(indent).append("digraph G {\n");
-        transitions.values().forEach(transitionGroup ->
-            transitionGroup.transitions.forEach((context, transitions1) ->
+        transitions.values().forEach(fieldTransitions ->
+            fieldTransitions.transitions.forEach((context, transitions1) ->
             {
                 transitions1.forEach(transition ->
                 {
@@ -382,9 +393,9 @@ private void allocateTransition(
         final List<State> from,
         final State to)
     {
-        final TransitionGroup transitionGroup = transitions.computeIfAbsent(token, ignored -> new TransitionGroup());
+        final FieldTransitions fieldTransitions = transitions.computeIfAbsent(token, ignored -> new FieldTransitions());
         final Transition transition = new Transition(description, from, to);
-        transitionGroup.add(firingContext, transition);
+        fieldTransitions.add(firingContext, transition);
     }
 
     static final class State
@@ -461,7 +472,7 @@ enum TransitionContext
         LAST_ELEMENT_IN_GROUP
     }
 
-    private static final class TransitionGroup
+    private static final class FieldTransitions
     {
         private final Map<Object, List<Transition>> transitions = new LinkedHashMap<>();
 
@@ -490,5 +501,25 @@ public void add(final Object context, final Transition transition)
 
             transitionsForContext.add(transition);
         }
+
+        public State minimumEntryState()
+        {
+            final MutableReference<State> minimumState = new MutableReference<>();
+            transitions.values().forEach(transitionsForContext ->
+            {
+                transitionsForContext.forEach(transition ->
+                {
+                    transition.from.forEach(state ->
+                    {
+                        if (null == minimumState.get() || state.number < minimumState.get().number)
+                        {
+                            minimumState.set(state);
+                        }
+                    });
+                });
+            });
+
+            return minimumState.get();
+        }
     }
 }

sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaGenerator.java

@@ -23,6 +23,7 @@
 import org.agrona.MutableDirectBuffer;
 import org.agrona.Strings;
 import org.agrona.Verify;
+import org.agrona.collections.MutableBoolean;
 import org.agrona.generation.DynamicPackageOutputManager;
 import org.agrona.sbe.*;
 
@@ -295,10 +296,32 @@ private static CharSequence generateFieldOrderStates(final FieldOrderModel field
         sb.append("     * }</pre>\n");
         sb.append("     */\n");
         sb.append("    private enum CodecState\n")
-            .append("    {\n");
+            .append("    {");
+        final MutableBoolean isFirstState = new MutableBoolean(true);
         fieldOrderModel.forEachState(state ->
-            sb.append("        ").append(unqualifiedStateCase(state)).append(",\n"));
-
+        {
+            if (isFirstState.get())
+            {
+                isFirstState.set(false);
+            }
+            else
+            {
+                sb.append(",");
+            }
+            sb.append("\n        ").append(unqualifiedStateCase(state))
+                .append("(").append(state.number()).append(")");
+        });
+        sb.append(";\n\n");
+
+        sb.append("        private final int stateNumber;\n\n");
+        sb.append("        CodecState(final int stateNumber)\n");
+        sb.append("        {\n");
+        sb.append("            this.stateNumber = stateNumber;\n");
+        sb.append("        }\n\n");
+        sb.append("        int stateNumber()\n");
+        sb.append("        {\n");
+        sb.append("            return stateNumber;\n");
+        sb.append("        }\n");
         sb.append("    }\n\n");
 
         sb.append("    private CodecState codecState = ")
@@ -372,7 +395,7 @@ private static CharSequence generateFieldOrderStateTransitions(
 
         generateFieldOrderStateTransitions(
             sb,
-            indent + "            ",
+            indent + "        ",
             fieldOrderModel,
             FieldOrderModel.TransitionContext.SELECT_MULTI_ELEMENT_GROUP,
             token);
@@ -404,11 +427,26 @@ private static void generateFieldOrderStateTransitions(
                 .append(indent).append("        break;\n");
         });
 
-        sb.append(indent).append("    default:\n")
-            .append(indent).append("        throw new IllegalStateException(")
-            .append("\"Cannot access field \\\"").append(token.name())
-            .append("\\\" in state: \" + codecState());\n")
-            .append(indent).append("}\n");
+        sb.append(indent).append("    default:\n");
+        final FieldOrderModel.State minimumEntryState = fieldOrderModel.minimumEntryState(token);
+        if (token.signal() == Signal.BEGIN_FIELD && minimumEntryState != null)
+        {
+            sb.append(indent).append("        if (codecState().stateNumber() < ")
+                .append(qualifiedStateCase(minimumEntryState)).append(".stateNumber())\n");
+            sb.append(indent).append("        {\n");
+            sb.append(indent).append("            throw new IllegalStateException(")
+                .append("\"Cannot access field \\\"").append(token.name())
+                .append("\\\" in state: \" + codecState());\n");
+            sb.append(indent).append("        }\n");
+        }
+        else
+        {
+            sb.append(indent).append("        throw new IllegalStateException(")
+                .append("\"Cannot access field \\\"").append(token.name())
+                .append("\\\" in state: \" + codecState());\n");
+        }
+
+        sb.append(indent).append("}\n");
     }
 
     private static CharSequence generateFieldOrderStateTransitionsForNextGroupElement(

sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/FrameCodecDecoder.java

@@ -33,11 +33,23 @@ public final class FrameCodecDecoder
      */
     private enum CodecState
     {
-        NOT_WRAPPED,
-        V0_BLOCK,
-        V0_PACKAGENAME_DONE,
-        V0_NAMESPACENAME_DONE,
-        V0_SEMANTICVERSION_DONE,
+        NOT_WRAPPED(0),
+        V0_BLOCK(1),
+        V0_PACKAGENAME_DONE(2),
+        V0_NAMESPACENAME_DONE(3),
+        V0_SEMANTICVERSION_DONE(4);
+
+        private final int stateNumber;
+
+        CodecState(final int stateNumber)
+        {
+            this.stateNumber = stateNumber;
+        }
+
+        int stateNumber()
+        {
+            return stateNumber;
+        }
     }
 
     private CodecState codecState = CodecState.NOT_WRAPPED;
@@ -249,7 +261,10 @@ public int irId()
                     codecState(CodecState.V0_BLOCK);
                     break;
                 default:
-                    throw new IllegalStateException("Cannot access field \"irId\" in state: " + codecState());
+                    if (codecState().stateNumber() < CodecState.V0_BLOCK.stateNumber())
+                    {
+                        throw new IllegalStateException("Cannot access field \"irId\" in state: " + codecState());
+                    }
             }
         }
 
@@ -312,7 +327,10 @@ public int irVersion()
                     codecState(CodecState.V0_BLOCK);
                     break;
                 default:
-                    throw new IllegalStateException("Cannot access field \"irVersion\" in state: " + codecState());
+                    if (codecState().stateNumber() < CodecState.V0_BLOCK.stateNumber())
+                    {
+                        throw new IllegalStateException("Cannot access field \"irVersion\" in state: " + codecState());
+                    }
             }
         }
 
@@ -375,7 +393,10 @@ public int schemaVersion()
                     codecState(CodecState.V0_BLOCK);
                     break;
                 default:
-                    throw new IllegalStateException("Cannot access field \"schemaVersion\" in state: " + codecState());
+                    if (codecState().stateNumber() < CodecState.V0_BLOCK.stateNumber())
+                    {
+                        throw new IllegalStateException("Cannot access field \"schemaVersion\" in state: " + codecState());
+                    }
             }
         }
 

sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/FrameCodecEncoder.java

@@ -33,11 +33,23 @@ public final class FrameCodecEncoder
      */
     private enum CodecState
     {
-        NOT_WRAPPED,
-        V0_BLOCK,
-        V0_PACKAGENAME_DONE,
-        V0_NAMESPACENAME_DONE,
-        V0_SEMANTICVERSION_DONE,
+        NOT_WRAPPED(0),
+        V0_BLOCK(1),
+        V0_PACKAGENAME_DONE(2),
+        V0_NAMESPACENAME_DONE(3),
+        V0_SEMANTICVERSION_DONE(4);
+
+        private final int stateNumber;
+
+        CodecState(final int stateNumber)
+        {
+            this.stateNumber = stateNumber;
+        }
+
+        int stateNumber()
+        {
+            return stateNumber;
+        }
     }
 
     private CodecState codecState = CodecState.NOT_WRAPPED;
@@ -206,7 +218,10 @@ public FrameCodecEncoder irId(final int value)
                     codecState(CodecState.V0_BLOCK);
                     break;
                 default:
-                    throw new IllegalStateException("Cannot access field \"irId\" in state: " + codecState());
+                    if (codecState().stateNumber() < CodecState.V0_BLOCK.stateNumber())
+                    {
+                        throw new IllegalStateException("Cannot access field \"irId\" in state: " + codecState());
+                    }
             }
         }
 
@@ -270,7 +285,10 @@ public FrameCodecEncoder irVersion(final int value)
                     codecState(CodecState.V0_BLOCK);
                     break;
                 default:
-                    throw new IllegalStateException("Cannot access field \"irVersion\" in state: " + codecState());
+                    if (codecState().stateNumber() < CodecState.V0_BLOCK.stateNumber())
+                    {
+                        throw new IllegalStateException("Cannot access field \"irVersion\" in state: " + codecState());
+                    }
             }
         }
 
@@ -334,7 +352,10 @@ public FrameCodecEncoder schemaVersion(final int value)
                     codecState(CodecState.V0_BLOCK);
                     break;
                 default:
-                    throw new IllegalStateException("Cannot access field \"schemaVersion\" in state: " + codecState());
+                    if (codecState().stateNumber() < CodecState.V0_BLOCK.stateNumber())
+                    {
+                        throw new IllegalStateException("Cannot access field \"schemaVersion\" in state: " + codecState());
+                    }
             }
         }