MovieSceneTransformOriginSystem memory corruption (crash with -stompmalloc)

Development Cinematics & Media
, ,
1

Hello !

One of our QA is testing our game (5.5.4) with -stompmalloc and we get a 100% crash in a cinematic inside MovieSceneTransformOriginSystem.cpp on this line:

TransformOriginsByInstanceID->Insert(Mapping.Child.InstanceID, (*TransformOriginsByInstanceID)[Mapping.Parent.InstanceID]);

It look like the sparse array is reallocated while doing insert but the transform we want to insert is a reference inside the array itself (the operator return a ref and the Insert function take a ConstInitType that could be a reference). Doing a copy of the transform before inserting it remove the crash:

const FTransform ParentTransform = (*TransformOriginsByInstanceID)[Mapping.Parent.InstanceID]; TransformOriginsByInstanceID->Insert(Mapping.Child.InstanceID, ParentTransform);Does the fix seems correct to you or did we miss something ?

Thx for your time.

Vincent

2

Hello Austin,

I’m sorry I don’t have a vanilla repro for this bug. Until recently this crash didn’t even happen on our side with -stompmalloc. Something change recently in the data and trigger this. Our cinematic designer love tweaking a ton of stuff in Sequencer so it’s hard to track the data that may result in this issue (and tbh I don’t really have the time to do it).

But the fix is pretty safe, we already integrated it on our side.

Thx for your time.

Vincent

3

Hi Vincent,

Thanks for bringing this to our attention. It does seem like there is a potential here for errors if the array is reallocated during insertion, and passing by value would resolve that issue.

I haven’t been able to get a repro case myself unfortunately. I tried a level sequence with 40+ sub-sequences in it, all with transform origin overrides to see if this was related to growing the sparse array, but that didn’t trip anything with -stompmalloc, so I’m curious what your repro is, and if it’s something I could easily replicate for a test case around this.

Either way, the fix seems valid, so I’ll try to integrate it soon into main.

Thanks!

Austin

4

Fair enough. Thanks again for bringing it to our attention.