kaitai_struct_compiler icon indicating copy to clipboard operation
kaitai_struct_compiler copied to clipboard

Fix problems with ownership in validation code

Open Mingun opened this issue 2 months ago • 1 comments

The actual fix contained in the first commit. The next commits just a minor style nits, which makes generated code looks better and gives more meaningful name to couple of constants.

The problem with validation code can be demonstrated with the following addition to debug_array_user.ksy:

# --debug (or actually --no-auto-read) with arrays of user types requires
# special handling to avoid spoiling whole object due to exception handler.
meta:
  id: debug_array_user
  ks-debug: true
seq:
  - id: one_cat
    type: cat
    valid:
      expr: _._sizeof == 1
  - id: array_of_cats
    type: cat
    repeat: expr
    repeat-expr: 3
  - id: foo
    type: u1
    valid: # should pass
      expr: _ == 1
  - id: bar
    type: s2le
    valid: # there's actually -190 in the file
      expr: _ < -190 or _ > -190
types:
  cat:
    seq:
      - id: meow
        type: u1

I just copy some checks from the valid_expr_fail.ksy test to have them in one file so it easy to check what's changed while working on another issue. I noticed, that check for one_cat generates incorrect code for C++11: one_cat() getter returns cat_t*, but the result is stored in std::unique_ptr<cat_t>. This

  • will not compile, because constructor is explicit
  • even if compiled will lead to double-free problem

The incorrect code that was generated before the fix:

void debug_array_user_t::_read() {
    m_one_cat = std::unique_ptr<cat_t>(new cat_t(m__io, this, m__root));
    m_one_cat->_read();
    {
        std::unique_ptr<cat_t> _ = one_cat();
        if (!(1 == 1)) {
            throw kaitai::validation_expr_error<std::unique_ptr<debug_array_user_t::cat_t>>(one_cat(), _io(), std::string("/seq/0"));
        }
    }
    m_array_of_cats = std::unique_ptr<std::vector<std::unique_ptr<cat_t>>>(new std::vector<std::unique_ptr<cat_t>>());
    const int l_array_of_cats = 3;
    for (int i = 0; i < l_array_of_cats; i++) {
        std::unique_ptr<cat_t> _t_array_of_cats = std::unique_ptr<cat_t>(new cat_t(m__io, this, m__root));
        _t_array_of_cats->_read();
        m_array_of_cats->push_back(std::move(_t_array_of_cats));
    }
    m_foo = m__io->read_u1();
    {
        uint8_t _ = foo();
        if (!(_ == 1)) {
            throw kaitai::validation_expr_error<uint8_t>(foo(), _io(), std::string("/seq/2"));
        }
    }
    m_bar = m__io->read_s2le();
    {
        int16_t _ = bar();
        if (!( ((_ < -190) || (_ > -190)) )) {
            throw kaitai::validation_expr_error<int16_t>(bar(), _io(), std::string("/seq/3"));
        }
    }
}

Mingun avatar Apr 14 '24 11:04 Mingun