Description of changes:
The implementation of serialization of AZStd::variant relies on storing the alternative data on serialization via ObjectStreamWriteElementOverride attribute; then on deserialization it relies on VariantDataConverter::ConvertFromType being called to offset a pointer to AZStd::variant<T1, .., Tn> to a pointer to Ti if the serialized value is Ti.
The problem is this function is not called for non-pointer datums by ObjectStream. This results in a pointer to a AZStd::variant<T1, .., Tn> being reinterpreted as a pointer to Ti - at worst this causes a segfault so it’s a pretty severe issue. In theory this might work for some implementations of variant and for some platform, but it’s clear that:
this won’t always be the case - some platforms/implementations will have a non-zero offset to the data member
AZStd::variant is currently the only type making use of a IDataConverter. Even if we rely on the data member of variant being at offset=0, consumers of IDataConverter (i.e. ObjectStream) cannot make such an assumption (indeed, if it could, there would be no point in converting the pointer by IDataConverter - the conversion would be the identity!).
This merge request changes ObjectStreamImpl::GetElementStorageAddress to always call ConvertFromType if CanConvertFromType returns true.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Issue #, if available:
Description of changes: The implementation of serialization of
AZStd::variant
relies on storing the alternative data on serialization viaObjectStreamWriteElementOverride
attribute; then on deserialization it relies onVariantDataConverter::ConvertFromType
being called to offset a pointer toAZStd::variant<T1, .., Tn>
to a pointer to Ti if the serialized value isTi
.The problem is this function is not called for non-pointer datums by
ObjectStream
. This results in a pointer to aAZStd::variant<T1, .., Tn>
being reinterpreted as a pointer toTi
- at worst this causes a segfault so it’s a pretty severe issue. In theory this might work for some implementations of variant and for some platform, but it’s clear that:AZStd::variant
is currently the only type making use of aIDataConverter
. Even if we rely on the data member of variant being at offset=0, consumers ofIDataConverter
(i.e.ObjectStream
) cannot make such an assumption (indeed, if it could, there would be no point in converting the pointer byIDataConverter
- the conversion would be the identity!).This merge request changes
ObjectStreamImpl::GetElementStorageAddress
to always callConvertFromType
ifCanConvertFromType
returns true.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.