lumberyard icon indicating copy to clipboard operation
lumberyard copied to clipboard

Always call IDataConverter::ConvertToType in ObjectStreamImpl::GetElementStorageAddress

Open yuriy0 opened this issue 6 years ago • 1 comments

Issue #, if available:

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.

yuriy0 avatar Aug 27 '19 15:08 yuriy0

Hi @yuriy0, thanks for contributing to LY we'll take a look at the change.

sconel avatar Aug 27 '19 18:08 sconel