Avoid storing temporary `shared_ptr` instances in reference-to-const members

Declaring a struct's data member to be of type `const shared_ptr<Base>&`
has a dangerous possibility of storing a temporary stack-allocated
`shared_ptr` in that reference. Specifically, this code was trying to
initialize a struct's `const shared_ptr<Base>&` member with a
`shared_ptr<Derived>` instance, expecting it not to need to copy the
`shared_ptr` to avoid artifically inflating its reference count. But of
course the type of `shared_ptr<Base>` is not the same as the type of
`shared_ptr<Derived>`, so it has to invoke the copy conversion constructor
of `shared_ptr`. The resulting instance is a temporary, and its lifetime
ends once the constructor of the struct completes, and the struct's member
is now dangling. A subsequent call that uses that struct and expects those
`shared_ptr` references to be valid end up doing invalid reads.

Example code to show this scenario:

```cpp
#include <memory>
#include <iostream>
​
class Base
{
};
class Derived
    : public Base
{
};
​
class BaseContext
{
public:
    BaseContext(const std::shared_ptr<Base>& nodeData)
        : m_graphData(nodeData)
    {
    }
    const std::shared_ptr<Base>& m_graphData;
};
class DerivedContext
    : public BaseContext
{
public:
    DerivedContext(const std::shared_ptr<Base>& nodeData)
        : BaseContext(nodeData)
    {
    }
};
​
void DoStuffWith(DerivedContext& context)
{
    if (context.m_graphData)
    {
        std::cout << context.m_graphData.get() << "\n";
    }
}
​
int main()
{
    auto customPropertyMapData = std::make_shared<Derived>();
    DerivedContext context(customPropertyMapData);
    DoStuffWith(context);
    return 0;
}
```

The disassembly for `main()` shows this order of calls:

```asm
call    std::shared_ptr<Base>::shared_ptr<Derived, void>(std::shared_ptr<Derived> const&)      // conversion constructor
call    DerivedContext::DerivedContext(std::shared_ptr<Base> const&) [base object constructor] // store temporary in struct
call    std::shared_ptr<Base>::~shared_ptr() [base object destructor]                          // destroy the temporary
call    DoStuffWith(DerivedContext&)                                                           // use the destroyed temporary
```

Signed-off-by: Chris Burel <[email protected]>

Code/Tools/SceneAPI/SceneBuilder/ImportContexts/AssImpImportContexts.cpp

@@ -49,9 +49,9 @@ namespace AZ
             }
 
             AssImpSceneDataPopulatedContext::AssImpSceneDataPopulatedContext(AssImpNodeEncounteredContext& parent,
-                const AZStd::shared_ptr<DataTypes::IGraphObject>& graphData, const AZStd::string& dataName)
+                AZStd::shared_ptr<DataTypes::IGraphObject> graphData, const AZStd::string& dataName)
                 : AssImpImportContext(parent.m_sourceScene, parent.m_sourceSceneSystem, parent.m_sourceNode)
-                , SceneDataPopulatedContextBase(parent, graphData, dataName)
+                , SceneDataPopulatedContextBase(parent, AZStd::move(graphData), dataName)
             {
             }
 
@@ -61,9 +61,9 @@ namespace AZ
                 const SceneSystem& sourceSceneSystem,
                 RenamedNodesMap& nodeNameMap,
                 AssImpSDKWrapper::AssImpNodeWrapper& sourceNode,
-                const AZStd::shared_ptr<DataTypes::IGraphObject>& nodeData, const AZStd::string& dataName)
+                AZStd::shared_ptr<DataTypes::IGraphObject> nodeData, const AZStd::string& dataName)
                 : AssImpImportContext(sourceScene, sourceSceneSystem, sourceNode)
-                , SceneDataPopulatedContextBase(scene, currentGraphPosition, nodeNameMap, nodeData, dataName)
+                , SceneDataPopulatedContextBase(scene, currentGraphPosition, nodeNameMap, AZStd::move(nodeData), dataName)
             {
             }
 
@@ -84,9 +84,9 @@ namespace AZ
             {
             }
 
-            AssImpSceneAttributeDataPopulatedContext::AssImpSceneAttributeDataPopulatedContext(AssImpSceneNodeAppendedContext& parent, const AZStd::shared_ptr<DataTypes::IGraphObject>& nodeData, const Containers::SceneGraph::NodeIndex attributeNodeIndex, const AZStd::string& dataName)
+            AssImpSceneAttributeDataPopulatedContext::AssImpSceneAttributeDataPopulatedContext(AssImpSceneNodeAppendedContext& parent, AZStd::shared_ptr<DataTypes::IGraphObject> nodeData, const Containers::SceneGraph::NodeIndex attributeNodeIndex, const AZStd::string& dataName)
                 : AssImpImportContext(parent.m_sourceScene, parent.m_sourceSceneSystem, parent.m_sourceNode)
-                , SceneAttributeDataPopulatedContextBase(parent, nodeData, attributeNodeIndex, dataName)
+                , SceneAttributeDataPopulatedContextBase(parent, AZStd::move(nodeData), attributeNodeIndex, dataName)
             {
             }
 

Code/Tools/SceneAPI/SceneBuilder/ImportContexts/AssImpImportContexts.h

@@ -80,7 +80,7 @@ namespace AZ
                 AZ_RTTI(AssImpSceneDataPopulatedContext, "{888DA37E-4234-4990-AD50-E6E54AFA9C35}", AssImpImportContext, SceneDataPopulatedContextBase);
 
                 AssImpSceneDataPopulatedContext(AssImpNodeEncounteredContext& parent,
-                    const AZStd::shared_ptr<DataTypes::IGraphObject>& nodeData,
+                    AZStd::shared_ptr<DataTypes::IGraphObject> nodeData,
                     const AZStd::string& dataName);
 
                 AssImpSceneDataPopulatedContext(Containers::Scene& scene,
@@ -89,7 +89,7 @@ namespace AZ
                     const SceneSystem& sourceSceneSystem,
                     RenamedNodesMap& nodeNameMap,
                     AssImpSDKWrapper::AssImpNodeWrapper& sourceNode,
-                    const AZStd::shared_ptr<DataTypes::IGraphObject>& nodeData,
+                    AZStd::shared_ptr<DataTypes::IGraphObject> nodeData,
                     const AZStd::string& dataName);
             };
 
@@ -121,7 +121,7 @@ namespace AZ
                 AZ_RTTI(AssImpSceneAttributeDataPopulatedContext, "{A5EFB485-2F36-4214-972B-0EFF4EFBF33D}", AssImpImportContext, SceneAttributeDataPopulatedContextBase);
 
                 AssImpSceneAttributeDataPopulatedContext(AssImpSceneNodeAppendedContext& parent,
-                    const AZStd::shared_ptr<DataTypes::IGraphObject>& nodeData,
+                    AZStd::shared_ptr<DataTypes::IGraphObject> nodeData,
                     const Containers::SceneGraph::NodeIndex attributeNodeIndex,const AZStd::string& dataName);
             };
 

Code/Tools/SceneAPI/SceneBuilder/ImportContexts/ImportContexts.cpp

@@ -46,9 +46,9 @@ namespace AZ
             }
 
             SceneDataPopulatedContextBase::SceneDataPopulatedContextBase(NodeEncounteredContext& parent,
-                const AZStd::shared_ptr<DataTypes::IGraphObject>& graphData, const AZStd::string& dataName)
+                AZStd::shared_ptr<DataTypes::IGraphObject> graphData, const AZStd::string& dataName)
                 : ImportContext(parent.m_scene, parent.m_currentGraphPosition, parent.m_nodeNameMap)
-                , m_graphData(graphData)
+                , m_graphData(AZStd::move(graphData))
                 , m_dataName(dataName)
             {
             }
@@ -56,9 +56,9 @@ namespace AZ
             SceneDataPopulatedContextBase::SceneDataPopulatedContextBase(Containers::Scene& scene,
                 Containers::SceneGraph::NodeIndex currentGraphPosition,
                 RenamedNodesMap& nodeNameMap,
-                const AZStd::shared_ptr<DataTypes::IGraphObject>& nodeData, const AZStd::string& dataName)
+                AZStd::shared_ptr<DataTypes::IGraphObject> nodeData, const AZStd::string& dataName)
                 : ImportContext(scene, currentGraphPosition, nodeNameMap)
-                , m_graphData(nodeData)
+                , m_graphData(AZStd::move(nodeData))
                 , m_dataName(dataName)
             {
             }
@@ -76,10 +76,10 @@ namespace AZ
             }
 
             SceneAttributeDataPopulatedContextBase::SceneAttributeDataPopulatedContextBase(SceneNodeAppendedContextBase& parent,
-                const AZStd::shared_ptr<DataTypes::IGraphObject>& nodeData,
+                AZStd::shared_ptr<DataTypes::IGraphObject> nodeData,
                 const Containers::SceneGraph::NodeIndex attributeNodeIndex, const AZStd::string& dataName)
                 : ImportContext(parent.m_scene, attributeNodeIndex, parent.m_nodeNameMap)
-                , m_graphData(nodeData)
+                , m_graphData(AZStd::move(nodeData))
                 , m_dataName(dataName)
             {
             }

Code/Tools/SceneAPI/SceneBuilder/ImportContexts/ImportContexts.h

@@ -90,15 +90,15 @@ namespace AZ
                 AZ_RTTI(SceneDataPopulatedContextBase, "{5F4CE8D2-EEAC-49F7-8065-0B6372162D6F}", ImportContext);
 
                 SceneDataPopulatedContextBase(NodeEncounteredContext& parent,
-                    const AZStd::shared_ptr<DataTypes::IGraphObject>& nodeData,
+                    AZStd::shared_ptr<DataTypes::IGraphObject> nodeData,
                     const AZStd::string& dataName);
 
                 SceneDataPopulatedContextBase(Containers::Scene& scene,
                     Containers::SceneGraph::NodeIndex currentGraphPosition,
                     RenamedNodesMap& nodeNameMap,
-                    const AZStd::shared_ptr<DataTypes::IGraphObject>& nodeData, const AZStd::string& dataName);
+                    AZStd::shared_ptr<DataTypes::IGraphObject> nodeData, const AZStd::string& dataName);
 
-                const AZStd::shared_ptr<DataTypes::IGraphObject>& m_graphData;
+                AZStd::shared_ptr<DataTypes::IGraphObject> m_graphData;
                 const AZStd::string m_dataName;
             };
 
@@ -124,10 +124,10 @@ namespace AZ
                 AZ_RTTI(SceneAttributeDataPopulatedContextBase, "{DA133E14-0770-435B-9A4E-38679367F56C}", ImportContext);
 
                 SceneAttributeDataPopulatedContextBase(SceneNodeAppendedContextBase& parent,
-                    const AZStd::shared_ptr<DataTypes::IGraphObject>& nodeData,
+                    AZStd::shared_ptr<DataTypes::IGraphObject> nodeData,
                     const Containers::SceneGraph::NodeIndex attributeNodeIndex, const AZStd::string& dataName);
 
-                const AZStd::shared_ptr<DataTypes::IGraphObject>& m_graphData;
+                AZStd::shared_ptr<DataTypes::IGraphObject> m_graphData;
                 const AZStd::string m_dataName;
             };