Fix #934: segfault when substituting a SubTree node

src/tree_node.cpp

@@ -481,7 +481,7 @@ AnyPtrLocked BT::TreeNode::getLockedPortContent(const std::string& key)
   {
     const auto bb_key = std::string(*remapped_key);
     auto result = _p->config.blackboard->getAnyLocked(bb_key);
-    if(!result && _p->config.manifest)
+    if(!result && _p->config.manifest != nullptr)
     {
       // Entry doesn't exist yet. Create it using the port's type info
       // from the manifest so that getLockedPortContent works even when

src/xml_parsing.cpp

@@ -867,8 +867,13 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
     config.input_ports = port_remap;
     new_node =
         factory->instantiateTreeNode(instance_name, toStr(NodeType::SUBTREE), config);
+    // If a substitution rule replaced the SubTree with a different node
+    // (e.g. a TestNode), the dynamic_cast will return nullptr.
     auto subtree_node = dynamic_cast<SubTreeNode*>(new_node.get());
-    subtree_node->setSubtreeID(type_ID);
+    if(subtree_node != nullptr)
+    {
+      subtree_node->setSubtreeID(type_ID);
+    }
   }
   else
   {

tests/gtest_substitution.cpp

@@ -50,3 +50,39 @@ TEST(Substitution, Parser)
 
   ASSERT_EQ(*std::get_if<std::string>(&rules.at("actionC")), "NotAConfig");
 }
+
+// Regression test for issue #934: segfault when substituting a SubTree node
+TEST(Substitution, SubTreeNodeSubstitution)
+{
+  static const char* parent_xml = R"(
+  <root BTCPP_format="4">
+    <BehaviorTree ID="Parent">
+      <SubTree ID="Child" name="child" />
+    </BehaviorTree>
+  </root>
+  )";
+
+  static const char* child_xml = R"(
+  <root BTCPP_format="4">
+    <BehaviorTree ID="Child">
+      <AlwaysSuccess />
+    </BehaviorTree>
+  </root>
+  )";
+
+  BehaviorTreeFactory factory;
+  factory.registerBehaviorTreeFromText(parent_xml);
+  factory.registerBehaviorTreeFromText(child_xml);
+
+  BT::TestNodeConfig config;
+  config.return_status = BT::NodeStatus::SUCCESS;
+  factory.addSubstitutionRule("child", config);
+
+  // This should not crash (was a segfault before the fix)
+  Tree tree;
+  ASSERT_NO_THROW(tree = factory.createTree("Parent"));
+
+  // The substituted tree should tick successfully
+  auto status = tree.tickWhileRunning();
+  ASSERT_EQ(status, BT::NodeStatus::SUCCESS);
+}