Add more error checking to Fpy arithmetic directives (#4331)

* Add more error checking to arithmetic directives

* sp

* Inadvertant double literal fix

* Add missing std::
This commit is contained in:
M Starch 2025-10-30 10:43:08 -07:00 committed by GitHub
parent cb9f3a982b
commit c51921aca3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 68 additions and 24 deletions

View File

@ -341,7 +341,9 @@ IPCFG
IPHELPER
isf
isgreater
isgreaterequal
isless
islessequal
isr
isunordered
itimerspec

View File

@ -529,19 +529,6 @@ Signal FpySequencer::constCmd_directiveHandler(const FpySequencer_ConstCmdDirect
}
}
I8 floatCmp(F64 lhs, F64 rhs) {
if (std::isunordered(lhs, rhs)) {
// nan is one of the args
// always fail a comparison if nan
return -2;
} else if (std::isgreater(lhs, rhs)) {
return 1;
} else if (std::isless(lhs, rhs)) {
return -1;
}
return 0;
}
DirectiveError FpySequencer::op_or() {
if (this->m_runtime.stackSize < sizeof(U8) * 2) {
return DirectiveError::STACK_ACCESS_OUT_OF_BOUNDS;
@ -649,7 +636,7 @@ DirectiveError FpySequencer::op_feq() {
F64 rhs = this->pop<F64>();
F64 lhs = this->pop<F64>();
// eq is true if they are equal and neither is nan
this->push(static_cast<U8>(floatCmp(lhs, rhs) == 0));
this->push(static_cast<U8>((lhs == rhs) ? 1 : 0));
return DirectiveError::NO_ERROR;
}
DirectiveError FpySequencer::op_fne() {
@ -658,9 +645,8 @@ DirectiveError FpySequencer::op_fne() {
}
F64 rhs = this->pop<F64>();
F64 lhs = this->pop<F64>();
I8 cmp = floatCmp(lhs, rhs);
// ne is true if they are not equal and neither is nan
this->push(static_cast<U8>(cmp != 0 && cmp != -2));
// ne is true if they are not equal or either is nan
this->push(static_cast<U8>((lhs != rhs) ? 1 : 0));
return DirectiveError::NO_ERROR;
}
DirectiveError FpySequencer::op_flt() {
@ -669,7 +655,7 @@ DirectiveError FpySequencer::op_flt() {
}
F64 rhs = this->pop<F64>();
F64 lhs = this->pop<F64>();
this->push(static_cast<U8>(floatCmp(lhs, rhs) == -1));
this->push(static_cast<U8>(std::isless(lhs, rhs)));
return DirectiveError::NO_ERROR;
}
DirectiveError FpySequencer::op_fle() {
@ -678,8 +664,7 @@ DirectiveError FpySequencer::op_fle() {
}
F64 rhs = this->pop<F64>();
F64 lhs = this->pop<F64>();
I8 cmp = floatCmp(lhs, rhs);
this->push(static_cast<U8>(cmp == 0 || cmp == -1));
this->push(static_cast<U8>(std::islessequal(lhs, rhs)));
return DirectiveError::NO_ERROR;
}
DirectiveError FpySequencer::op_fgt() {
@ -688,7 +673,7 @@ DirectiveError FpySequencer::op_fgt() {
}
F64 rhs = this->pop<F64>();
F64 lhs = this->pop<F64>();
this->push(static_cast<U8>(floatCmp(lhs, rhs) == 1));
this->push(static_cast<U8>(std::isgreater(lhs, rhs)));
return DirectiveError::NO_ERROR;
}
DirectiveError FpySequencer::op_fge() {
@ -697,8 +682,7 @@ DirectiveError FpySequencer::op_fge() {
}
F64 rhs = this->pop<F64>();
F64 lhs = this->pop<F64>();
I8 cmp = floatCmp(lhs, rhs);
this->push(static_cast<U8>(cmp == 0 || cmp == 1));
this->push(static_cast<U8>(std::isgreaterequal(lhs, rhs)));
return DirectiveError::NO_ERROR;
}
DirectiveError FpySequencer::op_not() {
@ -758,6 +742,17 @@ DirectiveError FpySequencer::op_iadd() {
}
I64 rhs = this->pop<I64>();
I64 lhs = this->pop<I64>();
// Check for overflow and underflow and return the appropriate error code
// Overflow can only occur with both operands positive and occurs when one operand is greater than the maximum value
// less the other operand. If either operand is negative or zero, overflow cannot occur.
if ((rhs > 0) && (lhs > 0) && ((std::numeric_limits<I64>::max() - rhs) < lhs)) {
return DirectiveError::ARITHMETIC_OVERFLOW;
}
// Underflow can only occur with both operands negative and occurs when one operand is less than the minimum value
// minus the other operand. If either operand is positive or zero, underflow cannot occur.
else if ((rhs < 0) && (lhs < 0) && ((std::numeric_limits<I64>::min() - rhs) > lhs)) {
return DirectiveError::ARITHMETIC_UNDERFLOW;
}
this->push(static_cast<I64>(lhs + rhs));
return DirectiveError::NO_ERROR;
}
@ -767,6 +762,19 @@ DirectiveError FpySequencer::op_isub() {
}
I64 rhs = this->pop<I64>();
I64 lhs = this->pop<I64>();
// Check for overflow and underflow and return the appropriate error code
// Overflow can only occur when the left operand is positive and the right operand is negative. It occurs when the
// left (positive) operand is greater than the maximum value plus the other (negative) operand. If the right
// operand is positive or zero, overflow cannot occur.
if ((rhs < 0) && (lhs > 0) && ((std::numeric_limits<I64>::max() + rhs) < lhs)) {
return DirectiveError::ARITHMETIC_OVERFLOW;
}
// Underflow can only occur when the left operand is negative and the right operand is positive. It occurs when the
// left (negative) operand is less than the minimum value plus the other (positive) operand. If the right operand
// is negative or zero, underflow cannot occur.
else if ((rhs > 0) && (lhs < 0) && ((std::numeric_limits<I64>::min() - rhs) > lhs)) {
return DirectiveError::ARITHMETIC_UNDERFLOW;
}
this->push(static_cast<I64>(lhs - rhs));
return DirectiveError::NO_ERROR;
}
@ -776,6 +784,27 @@ DirectiveError FpySequencer::op_imul() {
}
I64 rhs = this->pop<I64>();
I64 lhs = this->pop<I64>();
// Check for overflow and underflow and return the appropriate error code
// Overflow can only occur with operands of matching signs and occurs when one operand is greater (or less) than the
// maximum value divided by the other operand. Either operand being zero precludes overflow.
// Check the both positive case.
if ((rhs > 0) && (lhs > 0) && ((std::numeric_limits<I64>::max() / rhs) < lhs)) {
return DirectiveError::ARITHMETIC_OVERFLOW;
}
// Check the both negative case
else if ((rhs < 0) && (lhs < 0) && ((std::numeric_limits<I64>::max() / (-1 * rhs)) < (-1 * lhs))) {
return DirectiveError::ARITHMETIC_OVERFLOW;
}
// Underflow can occur with operands of differing signs and occurs when one operand is less than the minimum value
// divided by the other operand. Either operand being zero precludes underflow.
// Check the case where lhs is positive.
else if ((rhs < 0) && (lhs > 0) && ((std::numeric_limits<I64>::min() / lhs) > rhs)) {
return DirectiveError::ARITHMETIC_UNDERFLOW;
}
// Check the case where rhs is positive.
else if ((rhs > 0) && (lhs < 0) && ((std::numeric_limits<I64>::min() / rhs) > lhs)) {
return DirectiveError::ARITHMETIC_UNDERFLOW;
}
this->push(static_cast<I64>(lhs * rhs));
return DirectiveError::NO_ERROR;
}
@ -785,6 +814,10 @@ DirectiveError FpySequencer::op_udiv() {
}
U64 rhs = this->pop<U64>();
U64 lhs = this->pop<U64>();
// Prevent division by zero
if (rhs == 0) {
return DirectiveError::DOMAIN_ERROR;
}
this->push(static_cast<U64>(lhs / rhs));
return DirectiveError::NO_ERROR;
}
@ -794,6 +827,10 @@ DirectiveError FpySequencer::op_sdiv() {
}
I64 rhs = this->pop<I64>();
I64 lhs = this->pop<I64>();
// Prevent division by zero
if (rhs == 0) {
return DirectiveError::DOMAIN_ERROR;
}
this->push(static_cast<I64>(lhs / rhs));
return DirectiveError::NO_ERROR;
}
@ -888,6 +925,9 @@ DirectiveError FpySequencer::op_flog() {
return DirectiveError::STACK_ACCESS_OUT_OF_BOUNDS;
}
F64 val = this->pop<F64>();
if (val <= 0.0) {
return DirectiveError::DOMAIN_ERROR;
}
this->push(static_cast<F64>(log(val)));
return DirectiveError::NO_ERROR;
}

View File

@ -188,5 +188,7 @@ enum DirectiveErrorCode : U8 {
STACK_ACCESS_OUT_OF_BOUNDS
STACK_OVERFLOW
DOMAIN_ERROR
ARITHMETIC_OVERFLOW
ARITHMETIC_UNDERFLOW
FLAG_IDX_OUT_OF_BOUNDS
}

View File

@ -748,7 +748,7 @@ TEST_F(FpySequencerTester, fne) {
tester_push<F64>(std::numeric_limits<F64>::quiet_NaN());
tester_push<F64>(std::numeric_limits<F64>::quiet_NaN());
ASSERT_EQ(tester_op_fne(), DirectiveError::NO_ERROR);
ASSERT_EQ(tester_get_m_runtime_ptr()->stack[0], 0);
ASSERT_EQ(tester_get_m_runtime_ptr()->stack[0], 1);
}
TEST_F(FpySequencerTester, not) {