Fix incorrect data consumption for &max-size.

We would previously handle `&size` and `&max-size` almost identical
with the only difference that `&max-size` sets up a slightly larger view
to accommodate a sentinel. In particular, we also used identical code to
set up the position where parsing should resume after such a field.

This was incorrect as it is in general impossible to tell where parsing
continues after a field with `&max-size` since it does not signify a
fixed view like `&size`. In this patch we now compute the next position
for a `&max-size` field by inspecting the limited view to detect how
much data was extracted.

Closes #1668.

spicy/toolchain/src/compiler/codegen/parser-builder.cc

@@ -764,6 +764,10 @@ struct ProductionVisitor
         auto ncur = state().ncur;
         state().ncur = {};
 
+        // Epxression tracking `ncur` in case we operate on a limited view from `&max-size` parsing.
+        // This differs from `&size` parsing in that we do not need to consume the full limited view.
+        std::optional<Expression> ncur_max_size;
+
         if ( auto a = AttributeSet::find(field->attributes(), "&max-size") ) {
             // Check that we did not read into the sentinel byte.
             auto cond = builder::greaterEqual(builder::memberCall(state().cur, "offset", {}),
@@ -778,6 +782,10 @@ struct ProductionVisitor
 
                 pb->parseError("parsing not done within &max-size bytes", a->meta());
             });
+
+            // For `&max-size` store away the position into the limited view we ended up parsing to.
+            // This is used below to compute how much data we consumed from the original view.
+            ncur_max_size = state().cur;
         }
 
         else if ( auto a = AttributeSet::find(field->attributes(), "&size") ) {
@@ -814,6 +822,15 @@ struct ProductionVisitor
             pb->saveParsePosition();
         }
 
+        else if ( ncur_max_size )
+            // Compute how far to advance for `&max-size` parsing where we operate on a limited view, but do not
+            // necessarily consume it fully. Since `cur` and `ncur_max_size` point to different views we need compute
+            // the difference in offset; this is safe since the limited view is into the original stream `cur` points
+            // to.
+            ncur = builder::memberCall(state().cur, "advance",
+                                       {builder::difference(builder::memberCall(*ncur_max_size, "offset", {}),
+                                                            builder::memberCall(state().cur, "offset", {}))});
+
         if ( ncur )
             builder()->addAssign(state().cur, *ncur);
 

tests/Baseline/spicy.types.unit.field-max-size/output

@@ -2,6 +2,6 @@
 done, [$xs=b"\x00"]
 done, [$xs=b"\x01\x00"]
 error, [$xs=(not set)]
-[error] terminating with uncaught exception of type spicy::rt::ParseError: parsing not done within &max-size bytes (<...>/field-max-size.spicy:15:40-15:56)
+[error] terminating with uncaught exception of type spicy::rt::ParseError: parsing not done within &max-size bytes (<...>/field-max-size.spicy:16:40-16:56)
 error, [$xs=b"\x01\x01\x01"]
-[error] terminating with uncaught exception of type spicy::rt::ParseError: end-of-data reached before &until-including expression found (<...>/field-max-size.spicy:15:32-15:38)
+[error] terminating with uncaught exception of type spicy::rt::ParseError: end-of-data reached before &until-including expression found (<...>/field-max-size.spicy:16:32-16:38)

tests/Baseline/spicy.types.unit.field-max-size/output2

@@ -0,0 +1,11 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+Mini::X {
+  use_max_size: True
+  xs: 
+  rest: BC
+}
+Mini::X {
+  use_max_size: False
+  xs: 
+  rest: BC
+}

tests/spicy/types/unit/field-max-size.spicy

@@ -1,14 +1,15 @@
-# @TEST-EXEC: spicyc -j -d %INPUT -o test.hlto
-# @TEST-EXEC: printf '\000' | spicy-driver -d test.hlto >>output 2>&1
-# @TEST-EXEC: printf '\001\000' | spicy-driver -d test.hlto >>output 2>&1
-# @TEST-EXEC-FAIL: printf '\001\001\000' | spicy-driver -d test.hlto >>output 2>&1
-# @TEST-EXEC-FAIL: printf '\001\001\001\000' | spicy-driver -d test.hlto >>output 2>&1
-# @TEST-EXEC: btest-diff output
-#
 # @TEST-DOC: Validate effects of field-level `&max-size` attribute
+#
+# @TEST-EXEC: spicyc -j -d %INPUT -o test.hlto
 
 module Mini;
 
+# @TEST-EXEC: printf '\000' | spicy-driver -d test.hlto -p Mini::Test >>output 2>&1
+# @TEST-EXEC: printf '\001\000' | spicy-driver -d test.hlto -p Mini::Test >>output 2>&1
+# @TEST-EXEC-FAIL: printf '\001\001\000' | spicy-driver -d test.hlto -p Mini::Test >>output 2>&1
+# @TEST-EXEC-FAIL: printf '\001\001\001\000' | spicy-driver -d test.hlto -p Mini::Test >>output 2>&1
+# @TEST-EXEC: btest-diff output
+
 const MaxSize = 2;
 
 public type Test = unit {
@@ -17,3 +18,19 @@ public type Test = unit {
     on %done { print "done", self; }
     on %error { print "error", self; }
 };
+
+# Check that `&max-size` has no effect on how much data is consumed. This is a regression test for #1668.
+# @TEST-EXEC: printf '\001_BC' | spicy-dump -d test.hlto -p Mini::X >>output2 2>&1
+# @TEST-EXEC: printf '\000_BC' | spicy-dump -d test.hlto -p Mini::X >>output2 2>&1
+# @TEST-EXEC: btest-diff output2
+
+public type X = unit {
+    use_max_size: uint8 &convert=cast<bool>($$);
+
+    switch (self.use_max_size) {
+        True -> xs: bytes &until=b"_" &max-size=1;
+        False -> xs: bytes &until=b"_";
+    };
+
+    rest: bytes &eod;
+};