Undefined behaviour: Copying struct/class with uninitialized members

[hribz]

Copying struct/class with uninitialized members is consider as undefined behaviour. (Some References: [1], [2]).
And I found this kind of UB in some files.

struct Literal

Function createAsmNode<Literal>(_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.

solidity/libyul/AsmJsonImporter.cpp

Lines 167 to 169 in f369cdd

	Literal AsmJsonImporter::createLiteral(Json const& _node)
	{
	auto lit = createAsmNode<Literal>(_node);

solidity/libyul/AsmJsonImporter.cpp

Lines 57 to 68 in f369cdd

	template <class T>
	T AsmJsonImporter::createAsmNode(Json const& _node)
	{
	T r;
	SourceLocation nativeLocation = createSourceLocation(_node);
	yulAssert(nativeLocation.hasText(), "Invalid source location in Asm AST");
	// TODO: We should add originLocation to the AST.
	// While it's not included, we'll use nativeLocation for it because we only support importing
	// inline assembly as a part of a Solidity AST and there these locations are always the same.
	r.debugData = DebugData::create(nativeLocation, nativeLocation);
	return r;
	}

kind is not unsigned narrow character type or std::byte, and LiteralKind object does not have default-initalization.

solidity/libyul/AST.h

Line 69 in f369cdd

struct Literal { langutil::DebugData::ConstPtr debugData; LiteralKind kind; LiteralValue value; };

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.

solidity/libevmasm/Assembly.h

Line 107 in f369cdd

AssemblyItem appendSubroutine(AssemblyPointer const& _assembly) { auto sub = newSub(_assembly); append(newPushSubSize(size_t(sub.data()))); return sub; }

solidity/libevmasm/Assembly.h

Line 74 in 26d5b3f

AssemblyItem newSub(AssemblyPointer const& _sub) { m_subs.push_back(_sub); return AssemblyItem(PushSub, m_subs.size() - 1); }

solidity/libevmasm/AssemblyItem.h

Lines 75 to 83 in f369cdd

	AssemblyItem(AssemblyItemType _type, u256 _data = 0, langutil::DebugData::ConstPtr _debugData = langutil::DebugData::create()):
	m_type(_type),
	m_debugData(std::move(_debugData))
	{
	if (m_type == Operation)
	m_instruction = Instruction(uint8_t(_data));
	else
	m_data = std::make_shared<u256>(std::move(_data));
	}

m_instruction is not unsigned narrow character type or std::byte, and will not be default initalized.

solidity/libevmasm/AssemblyItem.h

Lines 217 to 229 in 26d5b3f

	AssemblyItemType m_type;
	Instruction m_instruction; ///< Only valid if m_type == Operation
	std::shared_ptr<u256> m_data; ///< Only valid if m_type != Operation
	/// If m_type == VerbatimBytecode, this holds number of arguments, number of
	/// return variables and verbatim bytecode.
	std::optional<std::tuple<size_t, size_t, bytes>> m_verbatimBytecode;
	langutil::DebugData::ConstPtr m_debugData;
	JumpType m_jumpType = JumpType::Ordinary;
	/// Pushed value for operations with data to be determined during assembly stage,
	/// e.g. PushSubSize, PushTag, PushSub, etc.
	mutable std::shared_ptr<u256> m_pushedValue;
	/// Number of PushImmutable's with the same hash. Only used for AssignImmutable.
	mutable std::optional<size_t> m_immutableOccurrences;

copy constructor

And also found some use of copy constructor before initializing AssemblyItem's member m_instruction.

solidity/libsolidity/codegen/CompilerContext.cpp

Lines 593 to 599 in 26d5b3f

	evmasm::AssemblyItem tag = _context.namedTag(
	labelName,
	params,
	returns,
	_declaration.id()
	);
	m_entryLabels.insert(std::make_pair(&_declaration, tag));

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 :)