Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent compiler generated code when 'repition index' is used in 'repeat-until' expression #958

Open
transverberate opened this issue Apr 2, 2022 · 1 comment · May be fixed by kaitai-io/kaitai_struct_compiler#245 or kaitai-io/kaitai_struct_compiler#308

Comments

@transverberate
Copy link

transverberate commented Apr 2, 2022

Issue

I have a situation where there is a list of data types.
Before the list-entries (entries) are given, a maximum number of entries is given (max_num_entries) at the head of the list.

Additionally, any list entry may meet a condition which denotes it as the terminal entry - ending the list preemptively (before the max number of entries is reached).

Here is a minimal example of the phenomena
(Note that for this particular example there is a more elegant/proper solution; this is merely illustrating the problem).

seq:
  - id: first_list
    type: list_type
  - id: second_list
    type: list_type
types: 
  list_type:
    seq:
      - id: max_num_entries
        type: u1
      - id: entries
        type: u1  # in reality the entry is more complex than a 'u1'
        repeat: until
        # in reality the terminator is more complex than a value of '0xFF'
        repeat-until: _ == 0xFF or _index >= max_num_entries

Here, a repeat-until expression checks to see if either the
terminal condition _ == 0xFF is met or if the _index exceeds the max number of entries.

The behavior of the javascript compiler (and kaitai IDE) is as expected.
Here is the noteworthy piece of code generated:

ListType.prototype._read = function() {
  this.maxNumEntries = this._io.readU1();
  this.entries = []
  var i = 0;
  do {
    var _ = this._io.readU1();
    this.entries.push(_);
    i++;
  } while (!( ((_ == 255) || (i >= this.maxNumEntries)) ));
}

Compare this to the python output of the compiler

def _read(self):
    self.max_num_entries = self._io.read_u1()
    self.entries = []
    i = 0
    while True:
        _ = self._io.read_u1()
        self.entries.append(_)
        if  ((_ == 255) or (i >= self.max_num_entries)) :
            break
        i += 1

Hopefully the problem here is apparent - the value of i is incremented before the loop check in the javascript code,
while the value of i is incremented after the loop check in the python code.
In order to achieve the same behavior for python, the ksy must be modified to

repeat-until: _ == 0xFF or _index >= (max_num_entries - 1)

Proposed Fix

For each target language, ensure that the 'loop check' generated by a repeat-until expression is
the last thing to execute in its requisite repeat block.

I've searched the issues and haven't found anything similar,
however I may not have been using the appropriate keywords.

@transverberate
Copy link
Author

More serious errors resulting from repetition index i in repeat-until expressions

Further exploration reveals that use of the repetition index i in repeat-until expression yields uncompilable code for Perl and Rust targets.

Perl

Using the aforementioned example ksy

sub _read {
    my ($self) = @_;

    $self->{max_num_entries} = $self->{_io}->read_u1();
    $self->{entries} = ();
    do {
        $_ = $self->{_io}->read_u1();
        push @{$self->{entries}}, $_;
    } until ( (($_ == 255) || ($i >= $self->max_num_entries())) );
}

results in

Global symbol "$i" requires explicit package name (did you forget to declare "my $i"?)

Rust

fn read<S: KaitaiStream>(&mut self,
                         stream: &mut S,
                         _parent: &Option<Box<KaitaiStruct>>,
                         _root: &Option<Box<KaitaiStruct>>)
                         -> Result<()>
    where Self: Sized {
    self.maxNumEntries = self.stream.read_u1()?;
    self.entries = vec!();
    while {
        let tmpa = self.stream.read_u1()?;
        self.entries.append(self.stream.read_u1()?);
        !( ((tmpa == 255) || (i >= self.max_num_entries)) )
    } { }
}

results in

error[[E0425]](https://doc.rust-lang.org/stable/error-index.html#E0425): cannot find value `i` in this scope

Other inconsistencies

In addition to python, the i repetition index being lower than 'excepted' (to be fair this behavior is technically undefined) occurs in

  • Lua
  • Nim

Unique behavior in Go

func (this *MinimalExample_ListType) Read(io *kaitai.Stream, parent *MinimalExample, root *MinimalExample) (err error) {
	this._io = io
	this._parent = parent
	this._root = root

	tmp3, err := this._io.ReadU1()
	if err != nil {
		return err
	}
	this.MaxNumEntries = tmp3
	for i := 1;; i++ {
		tmp4, err := this._io.ReadU1()
		if err != nil {
			return err
		}
		_it := tmp4
		this.Entries = append(this.Entries, _it)
		if  ((_it == 255) || (i >= this.MaxNumEntries))  {
			break
		}
	}
	return err
}

In Go, the repition index is 1 value higher than every other language target throughout the entire scope of each repetition. This means that the behavior of i is consistent with the 'expected behavior' within a repeat-until expression, but inconsistent within all other expressions (most users will expect _index to start at zero when used outside repeat-until).

Mingun added a commit to Mingun/kaitai_struct_compiler that referenced this issue Sep 15, 2024
Previous behavior (everything with 1 in last column was fixed):

|Language|Initial|Incremented|`_index` of first iteration
|--------|-------|-----------|---------------------------
|C++     |0      |before     |1
|C#      |0      |before     |1
|Go      |1      |after      |1
|Java    |0      |before     |1
|JS      |0      |before     |1
|Lua     |0      |after      |0
|Nim     |0?     |after      |0 (variable declared without initializer)
|Perl    |-      |-          |- (variable was not declared)
|PHP     |0      |before     |1
|Python  |0      |after      |0
|Ruby    |0      |before     |1
|Rust    |0      |before     |1

Fixes kaitai-io/kaitai_struct#958
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants