diff --git a/src/tree_node.cpp b/src/tree_node.cpp index d464d6af9..16171a135 100644 --- a/src/tree_node.cpp +++ b/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 diff --git a/src/xml_parsing.cpp b/src/xml_parsing.cpp index a11383041..7c2e6004c 100644 --- a/src/xml_parsing.cpp +++ b/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(new_node.get()); - subtree_node->setSubtreeID(type_ID); + if(subtree_node != nullptr) + { + subtree_node->setSubtreeID(type_ID); + } } else { diff --git a/tests/gtest_substitution.cpp b/tests/gtest_substitution.cpp index 44f6f37c3..cb97570bc 100644 --- a/tests/gtest_substitution.cpp +++ b/tests/gtest_substitution.cpp @@ -50,3 +50,39 @@ TEST(Substitution, Parser) ASSERT_EQ(*std::get_if(&rules.at("actionC")), "NotAConfig"); } + +// Regression test for issue #934: segfault when substituting a SubTree node +TEST(Substitution, SubTreeNodeSubstitution) +{ + static const char* parent_xml = R"( + + + + + + )"; + + static const char* child_xml = R"( + + + + + + )"; + + 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); +}