Skip to content

Commit 25407ad

Browse files
Fix #934: segfault when substituting a SubTree node (#1083)
* Fix #934: prevent segfault when substituting a SubTree node When a substitution rule replaced a SubTree with a TestNode, dynamic_cast<SubTreeNode*> returned nullptr but setSubtreeID was called unconditionally, causing a null pointer dereference. Add a null-check so setSubtreeID is only called on actual SubTree nodes. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Fix clang-tidy readability-implicit-bool-conversion warnings Use explicit != nullptr comparisons for pointer-to-bool conversions flagged by clang-tidy. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 1217c7e commit 25407ad

File tree

2 files changed

+42
-1
lines changed

2 files changed

+42
-1
lines changed

src/xml_parsing.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -867,8 +867,13 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
867867
config.input_ports = port_remap;
868868
new_node =
869869
factory->instantiateTreeNode(instance_name, toStr(NodeType::SUBTREE), config);
870+
// If a substitution rule replaced the SubTree with a different node
871+
// (e.g. a TestNode), the dynamic_cast will return nullptr.
870872
auto subtree_node = dynamic_cast<SubTreeNode*>(new_node.get());
871-
subtree_node->setSubtreeID(type_ID);
873+
if(subtree_node != nullptr)
874+
{
875+
subtree_node->setSubtreeID(type_ID);
876+
}
872877
}
873878
else
874879
{

tests/gtest_substitution.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,39 @@ TEST(Substitution, Parser)
5050

5151
ASSERT_EQ(*std::get_if<std::string>(&rules.at("actionC")), "NotAConfig");
5252
}
53+
54+
// Regression test for issue #934: segfault when substituting a SubTree node
55+
TEST(Substitution, SubTreeNodeSubstitution)
56+
{
57+
static const char* parent_xml = R"(
58+
<root BTCPP_format="4">
59+
<BehaviorTree ID="Parent">
60+
<SubTree ID="Child" name="child" />
61+
</BehaviorTree>
62+
</root>
63+
)";
64+
65+
static const char* child_xml = R"(
66+
<root BTCPP_format="4">
67+
<BehaviorTree ID="Child">
68+
<AlwaysSuccess />
69+
</BehaviorTree>
70+
</root>
71+
)";
72+
73+
BehaviorTreeFactory factory;
74+
factory.registerBehaviorTreeFromText(parent_xml);
75+
factory.registerBehaviorTreeFromText(child_xml);
76+
77+
BT::TestNodeConfig config;
78+
config.return_status = BT::NodeStatus::SUCCESS;
79+
factory.addSubstitutionRule("child", config);
80+
81+
// This should not crash (was a segfault before the fix)
82+
Tree tree;
83+
ASSERT_NO_THROW(tree = factory.createTree("Parent"));
84+
85+
// The substituted tree should tick successfully
86+
auto status = tree.tickWhileRunning();
87+
ASSERT_EQ(status, BT::NodeStatus::SUCCESS);
88+
}

0 commit comments

Comments
 (0)