kaitai-io / kaitai_struct

Kaitai Struct: declarative language to generate binary data parsers in C++ / C# / Go / Java / JavaScript / Lua / Nim / Perl / PHP / Python / Ruby
https://kaitai.io
3.99k stars 194 forks source link

C++ codegen: Incorrect code when using "size" with "switch-on", having no explicit default case #249

Open tempelmann opened 7 years ago

tempelmann commented 7 years ago

This code:

seq:
- id: v1
  type: u1
- id: v2
  size: 4
  type:
    switch-on: v1
    cases:
      1: strz

Generates something along these lines (slightly simplified):

void test_t::_read() {
    m_v1 = m__io->read_u1();
    n_v2 = true;
    switch (v1()) {
        case 1: {
            n_v2 = false;
            m_v2 = kaitai::kstream::bytes_to_str(kaitai::kstream::bytes_terminate(m__io->read_bytes((8 + ((1 == 2)?1:0))), 0, false), std::string("UTF-8"));
            break;
        }
        default: {
            m_raw_v2 = m__io->read_bytes((8 + ((1 == 2)?1:0)));
            break;
        }
    }
}

The problem is m_raw_v2: It is never declared in the header file, leading to a compile error.

The CppCompiler never learns of this raw type, and therefore doesn't know that it has to generate the property in the class definition.

The culprit is found in EveryReadIsExpression, inside attrSwitchTypeParse():

  (dataType) => if (switchBytesOnlyAsRaw) {
    dataType match {
      case t: BytesType =>
        attrParse2(RawIdentifier(id), dataType, io, extraAttrs, rep, false, defEndian, Some(assignType))
      case _ =>
        attrParse2(id, dataType, io, extraAttrs, rep, false, defEndian, Some(assignType))
    }

It generates a RawIdentifier without passing that information on or adding it to the list of attributes. It simply gets forgotten right away. This needs fixing so that the raw attribute is passed to the compiler via attributeDeclaration.

tempelmann commented 7 years ago

It seems adding Unit.addUniqueAttr might be the solution. This adds the missing declaration:

  case t: BytesType =>
    val rawId = RawIdentifier(id)
    Utils.addUniqueAttr(extraAttrs, AttrSpec(List(), rawId, dataType))
    attrParse2(rawId, dataType, io, extraAttrs, rep, false, defEndian, Some(assignType))
GreyCat commented 7 years ago

I propose to start with addition of relevant test case into tests. Usually this means:

  1. Addition of formats/name_of_the_test.ksy
  2. Addition of at least one spec in specs/$LANG/... (preferably in Ruby, as they could be semi-automatically converted to other languages by spec-ruby-to-$LANG family of scripts — spec_translate_missing would usually do all the possible conversions)

That would allow to check the situation with all other languages as well and see if they need any fixes or not.