Skip to content

Commit ea8dafa

Browse files
fix undefined double to int64 cast in the script complement operator (#1239)
The ~ operator in ExprUnaryArithmetic::evaluate guarded the cast of its double operand to int64_t with a hand-written range comparison. NaN passes it (every comparison is false) and so does exactly 2^63, because double(INT64_MAX) rounds up to 2^63; the following static_cast is then undefined behavior (UBSan float-cast-overflow). Fractional operands were also truncated silently, while the binary bitwise operators reject them. Use the checked Any::cast<int64_t> the binary bitwise operators already use, so these operands raise the existing RuntimeError instead.
1 parent 03ec870 commit ea8dafa

2 files changed

Lines changed: 31 additions & 3 deletions

File tree

‎include/behaviortree_cpp/scripting/operators.hpp‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,12 +129,20 @@ struct ExprUnaryArithmetic : ExprBase
129129
case negate:
130130
return Any(-rv);
131131
case complement:
132-
if(rv > static_cast<double>(std::numeric_limits<int64_t>::max()) ||
133-
rv < static_cast<double>(std::numeric_limits<int64_t>::min()))
132+
// Use the checked conversion, like the binary bitwise operators do.
133+
// A plain static_cast is undefined for NaN and for values at or
134+
// beyond the int64_t range: the hand-written comparison used here
135+
// before let NaN (every comparison is false) and 2^63 (not greater
136+
// than double(INT64_MAX), which rounds up to 2^63) through, and
137+
// silently truncated fractional values.
138+
try
139+
{
140+
return Any(static_cast<double>(~rhs_v.cast<int64_t>()));
141+
}
142+
catch(const std::exception&)
134143
{
135144
throw RuntimeError("Number out of range for bitwise operation");
136145
}
137-
return Any(static_cast<double>(~static_cast<int64_t>(rv)));
138146
case logical_not:
139147
return Any(static_cast<double>(!static_cast<bool>(rv)));
140148
}

‎tests/script_parser_test.cpp‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -670,6 +670,26 @@ TEST(ParserTest, UnaryOperators)
670670
EXPECT_EQ(Parse("10 + -3").value().cast<double>(), 7.0);
671671
}
672672

673+
TEST(ParserTest, BitwiseComplementRejectsNonIntegers)
674+
{
675+
BT::Ast::Environment env = { BT::Blackboard::create(), {} };
676+
auto Parse = [&env](const char* str) { return BT::ParseScriptAndExecute(env, str); };
677+
678+
// The operand was cast to int64_t after a hand-written range check that
679+
// accepted NaN and exactly 2^63 (undefined conversion, reported by UBSan)
680+
// and silently truncated fractional values.
681+
EXPECT_FALSE(Parse("~(0.0 / 0.0)").has_value());
682+
EXPECT_FALSE(Parse("~9223372036854775808.0").has_value()); // 2^63
683+
EXPECT_FALSE(Parse("~1.5").has_value());
684+
EXPECT_FALSE(Parse("~(1.0 / 0.0)").has_value());
685+
686+
// Integral values, including the int64_t boundaries, still work.
687+
EXPECT_EQ(Parse("~0").value().cast<int64_t>(), ~int64_t(0));
688+
EXPECT_EQ(Parse("~(2 * 3)").value().cast<int64_t>(), ~int64_t(6));
689+
EXPECT_EQ(Parse("~-9223372036854775808.0").value().cast<double>(),
690+
static_cast<double>(~std::numeric_limits<int64_t>::min()));
691+
}
692+
673693
TEST(ParserTest, TernaryExpressions)
674694
{
675695
BT::Ast::Environment env = { BT::Blackboard::create(), {} };

0 commit comments

Comments
 (0)