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

Use of Variable Length Arrays in generated code #8558

Closed
mdamle opened this issue Jul 22, 2021 · 5 comments
Closed

Use of Variable Length Arrays in generated code #8558

mdamle opened this issue Jul 22, 2021 · 5 comments

Comments

@mdamle
Copy link

mdamle commented Jul 22, 2021

Problem

variable length arrays are used in generated code: CHIPClientCallbacks.cpp. For example: https://github.com/project-chip/connectedhomeip/blob/master/src/controller/data_model/gen/CHIPClientCallbacks.cpp#L854
uint16_t count = chip::Encoding::LittleEndian::Read16(message);
_AudioOutputInfodata[count];
There are about 24 such instances. There are 2 issues with this usage:

  1. Variable Length Arrays are not universally supported by all compilers since they are not part of c++ standard
  2. The generated code uses over the wire value to allocate data on stack which is dangerous.
  • expected behavior
  1. Check size before allocation
  2. Allocate on heap instead of stack
  • actual behavior
    As explained in the problem section.
  • steps to reproduce
    Compile code on top of tree and inspect generated CHIPClientCallbacks.cpp file.
@mdamle
Copy link
Author

mdamle commented Jul 22, 2021

cc @tcarmelveilleux for visibility

@bzbarsky-apple
Copy link
Contributor

@tcarmelveilleux
Copy link
Contributor

The root cause of this issue is wanted to pass "flat arrays" to callbacks when lists are used. I think these lists should be handled by iteration over the payload with an abstracted list iterator backed by a TLV reader, rather than copy to flat arrays. That would avoid alloc entirely, and avoid requiring to trust any sizes from the wire for an alloc. A temporary reader could be used to schema-validate, but access to the list elements should be abstracted with an iterator.

@mlepage-google
Copy link
Contributor

+1 for iterator access.

@bzbarsky-apple
Copy link
Contributor

Fixed in #10681

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants