Undefined behaviour on "out of range" values of ID layer. about commsdsl HOT 4 CLOSED

On Sat, Oct 24, 2020, 02:21 Melg Eight ***@***.***> wrote: Setup 1. add compiler flags: -fno-omit-frame-pointer -fno-sanitize-recover=undefined -fsanitize=undefined 2. generate code using schema with fixed values for ID frame: ... <fields> <enum name="CommandType" type="uint8" hexAssign="true"> <validValue name="TestConnection1" val="27" displayName="Test connection to 1" /> <validValue name="TestConnection2" val="38" displayName="Test connection to 2" /> </enum> </fields> ... <frame name="OurProtocolFrame"> <id name="Id"> <field> <ref field="CommandType" /> </field> </id> ... 1. Setup test where comms::processAllWithDispatch is used to dispatch messages, and incoming input has value of ID is way off, for example 68. 2. Build and run test. Expected behavior: Test should pass, without crash, and should notify, that ID value was invalid. Current behavior: Test crash with ubsanitizer reporting: Load of value 68, which is not a valid value for type 'const McuProtocol::MsgId' on line ... Reporting on line MsgIdLayer.h#L689 <https://github.com/commschamp/comms_champion/blob/a92c710ad0b5e84e5a085cba6f54cc0f214870f8/comms/include/comms/protocol/MsgIdLayer.h#L689> BaseImpl::setMsgId(id, extraValues...) Thoughts Problem occurs because of assignment of value to the field with enumeration type, with value 68, where enum only happy with 27 and 38. Current implementation calls BaseImpl::setMsgId(id, extraValues...); Before validation of this value, which trigger ub. But, even removing that call will not help by itself, because inside of call msg = createMsgInternal(id, idx, &failureReason); assignment is done before check, which correctly detects invalid value inside of DispatchMsgStaticBinSearchHelper.h <https://github.com/commschamp/comms_champion/blob/a92c710ad0b5e84e5a085cba6f54cc0f214870f8/comms/include/comms/details/DispatchMsgStaticBinSearchHelper.h#L80> I've tried some ad hoc solutions, but it's not easy to grasp all template meta-programming elements, involved in this. Would really appreciate any temporal bugfixes, which will eliminates this problem. PS. In preparing of this issue I've tried to add same flags into root cmake file, and run ctest after it. Your tests reproduce the same kind of problem, but in other place comms_champion/comms/include/comms/field/adapter/FixedBitLength.h:127:69: runtime error: load of value 4, which is not a valid value for type 'const comms::field::basic::EnumValue<comms::Field<comms::option::def::Endian<comms::util::traits::endian::Big> >, FieldsTestSuite::Enum1>::ValueType' (aka 'const FieldsTestSuite::Enum1') 2: 2: CMake Error at comms_champion/cmake/CC_RunWithValgrindScript.cmake:30 (message): 2: Test failed with exit code 1 If you want I can open another issue about sanitizers (address santizier give even more errors) and tests. But our main focus is on dealing with ID field problem. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <https://github.com/commschamp/comms_champion/issues/12>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASJKGVSJX73HCRLMGQGEI3SMGUP5ANCNFSM4S4YNWDA> .