Undefined behaviour: Copying struct/class with uninitialized members
- Dominant language
- C++
- Stars
- 25.7k
- Forks
- 6.2k
- Avg merge
- 2d 19h
- Merged PRs (30d)
- 29
Description
Copying struct/class with uninitialized members is consider as undefined behaviour. (Some References: [[1]](https://stackoverflow.com/questions/60112841/copying-structs-with-uninitialized-members/), [[2]](https://stackoverflow.com/questions/15231527/is-it-ok-to-copy-uninitialized-data-if-it-will-be-unused-set-later)).
And I found this kind of UB in some files.
# struct Literal
Function `createAsmNode(_node)` create a `Literal` object `r`, when this function `return r;`, struct `Literal`'s implicit constructor is called, and `r`'s uninitialized members `kind` is copied to `retval`.
https://github.com/ethereum/solidity/blob/f369cdd05d71f383f1ec4a62981b08720d99796d/libyul/AsmJsonImporter.cpp#L167-L169
https://github.com/ethereum/solidity/blob/f369cdd05d71f383f1ec4a62981b08720d99796d/libyul/AsmJsonImporter.cpp#L57-L68
`kind` is not unsigned narrow character type or `std::byte`, and `LiteralKind` object does not have default-initalization.
https://github.com/ethereum/solidity/blob/f369cdd05d71f383f1ec4a62981b08720d99796d/libyul/AST.h#L69
# class AssemblyItem
## move constructor
Same as struct `Literal`, `AssemblyItem`'s uninitialzed member `m_instruction` may be copied to `retval` while returning from function `appendSubroutine`, if `m_type != Operation`.
https://github.com/ethereum/solidity/blob/f369cdd05d71f383f1ec4a62981b08720d99796d/libevmasm/Assembly.h#L107
https://github.com/ethereum/solidity/blob/26d5b3f88790183f5b30dc0067db629338846428/libevmasm/Assembly.h#L74
https://github.com/ethereum/solidity/blob/f369cdd05d71f383f1ec4a62981b08720d99796d/libevmasm/AssemblyItem.h#L75-L83
`m_instruction` is not unsigned narrow character type or `std::byte`, and will not be default initalized.
https://github.com/ethereum/solidity/blob/26d5b3f88790183f5b30dc0067db629338846428/libevmasm/AssemblyItem.h#L217-L229
## copy constructor
And also found some use of copy constructor before initializing `AssemblyItem`'s member `m_instruction`.
https://github.com/ethereum/solidity/blob/26d5b3f88790183f5b30dc0067db629338846428/libsolidity/codegen/CompilerContext.cpp#L593-L599
# Summary
May be these uninitialized members will not be used later (or will be set before using them), it's better to eliminate these UBs by initializing these members with a specified value :)
Contributor guide
Research direction
Review the cited locations in libyul/AsmJsonImporter.cpp, libyul/AST.h, libevmasm/Assembly.h, libevmasm/AssemblyItem.h, and libsolidity/codegen/CompilerContext.cpp. Trace the return and copy-construction paths for Literal and AssemblyItem, then run the project's existing C++ build and test checks. Done means the listed paths no longer copy uninitialized members and the checks pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 38/100