Fix VfrCompile bugs #88
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mBitWidth is accessed for non-bitfield structures with bitfield children, when traversing down through nested fields in GetDataFieldInfo. Thus it is read uninitialized in this case, and needs to be zeroed. This can be caught using sanitizers. Signed-off-by: Mike Beaton <[email protected]>
This structure was of size 23, but needs to be a multiple of 4 in order not to cause unaligned access errors in VfrCompile, which can be seen using ASan. Signed-off-by: Mike Beaton <[email protected]>
Member
Author
Member
Author
|
Draft while I streamline the fixed bit logic, as suggested by @mhaeuser. |
The previous logic for accessing bit fields is wrong when the bit field starts beyond the first byte, in a storage unit larger than a byte. This caused unaligned memory accesses which can be caught by sanitizers. We also bump the tool minor version number to reflect a functional bugfix. Signed-off-by: Mike Beaton <[email protected]>
…nal types Internal types such as pFormIdField, pFormSetGuidField have no associated field type, however the Dump command assumes that all types do. This causes null pointer dereferencing which can be caught by sanitizers. De facto, the field type for these types was already dumped as <null>, this achieves the same safely. Signed-off-by: Mike Beaton <[email protected]>
Add minor code cleanups made while tracking down the bugs fixed in the preceding commits. - Fix spelling 'arrary' -> 'array' where it occurs in the tool code and comments. - When renaming ExtractFieldNameAndArrary to ExtractFieldNameAndArrayIdx, we fix the typo and also clarify what the method does: it returns the field name and the array index when present. Since it is always used to populate a local variable named ArrayIdx, we stick to that spelling in the renamed method. - Add comment clarifying that mArrayNum holds array length (but again, don't go for a bigger global rename). - Rename CVfrVarDataTypeDB::GetFieldWidth to GetFieldType. It is only used once, and the new name correctly matches what it does. Signed-off-by: Mike Beaton <[email protected]>
mikebeaton
force-pushed
the
vfrcompile-fixes
branch
from
97238c0 to
7d2bdf8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See individual commit messages.
Prefer to rebase and merge not squash and merge when okay, to preserve commit messages. Can do.