Commit f827123d by Olli Etuaho Committed by Commit Bot

Handle negative float to uint conversion robustly

Converting a negative float to uint is undefined in the GLSL ES 3.00.6 spec. However, it improves portability if we don't trigger undefined results in C++ in this case. To do this, we cast negative floats first to signed integer before casting them to unsigned integer. We also issue a warning about an undefined conversion in case a negative float was converted to uint. BUG=chromium:835868 TEST=angle_unittests Change-Id: I9835a739ec80699d420a4f91a3bfa112c9a13604 Reviewed-on: https://chromium-review.googlesource.com/1026681Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarCorentin Wallez <cwallez@chromium.org> Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
parent f9749eaf
...@@ -157,7 +157,16 @@ bool TConstantUnion::cast(TBasicType newType, const TConstantUnion &constant) ...@@ -157,7 +157,16 @@ bool TConstantUnion::cast(TBasicType newType, const TConstantUnion &constant)
setUConst(static_cast<unsigned int>(constant.getBConst())); setUConst(static_cast<unsigned int>(constant.getBConst()));
break; break;
case EbtFloat: case EbtFloat:
setUConst(static_cast<unsigned int>(constant.getFConst())); if (constant.getFConst() < 0.0f)
{
// Avoid undefined behavior in C++ by first casting to signed int.
setUConst(
static_cast<unsigned int>(static_cast<int>(constant.getFConst())));
}
else
{
setUConst(static_cast<unsigned int>(constant.getFConst()));
}
break; break;
default: default:
return false; return false;
......
...@@ -1711,6 +1711,32 @@ TIntermTyped *TIntermAggregate::fold(TDiagnostics *diagnostics) ...@@ -1711,6 +1711,32 @@ TIntermTyped *TIntermAggregate::fold(TDiagnostics *diagnostics)
if (mType.canReplaceWithConstantUnion()) if (mType.canReplaceWithConstantUnion())
{ {
constArray = getConstantValue(); constArray = getConstantValue();
if (constArray && mType.getBasicType() == EbtUInt)
{
// Check if we converted a negative float to uint and issue a warning in that case.
size_t sizeRemaining = mType.getObjectSize();
for (TIntermNode *arg : mArguments)
{
TIntermTyped *typedArg = arg->getAsTyped();
if (typedArg->getBasicType() == EbtFloat)
{
const TConstantUnion *argValue = typedArg->getConstantValue();
size_t castSize =
std::min(typedArg->getType().getObjectSize(), sizeRemaining);
for (size_t i = 0; i < castSize; ++i)
{
if (argValue[i].getFConst() < 0.0f)
{
// ESSL 3.00.6 section 5.4.1.
diagnostics->warning(
mLine, "casting a negative float to uint is undefined",
mType.getBuiltInTypeNameString());
}
}
}
sizeRemaining -= typedArg->getType().getObjectSize();
}
}
} }
} }
else if (CanFoldAggregateBuiltInOp(mOp)) else if (CanFoldAggregateBuiltInOp(mOp))
......
...@@ -1607,3 +1607,41 @@ TEST_F(ConstantFoldingTest, FoldArrayOfArraysConstructorEquality) ...@@ -1607,3 +1607,41 @@ TEST_F(ConstantFoldingTest, FoldArrayOfArraysConstructorEquality)
ASSERT_TRUE(constantFoundInAST(5.0f)); ASSERT_TRUE(constantFoundInAST(5.0f));
ASSERT_FALSE(constantFoundInAST(4.0f)); ASSERT_FALSE(constantFoundInAST(4.0f));
} }
// Test that casting a negative float to uint results in a warning. ESSL 3.00.6 section 5.4.1
// specifies this as an undefined conversion.
TEST_F(ConstantFoldingExpressionTest, FoldNegativeFloatToUint)
{
const std::string &uintString = "uint(-1.0)";
evaluateUint(uintString);
ASSERT_TRUE(constantFoundInAST(std::numeric_limits<unsigned int>::max()));
ASSERT_TRUE(hasWarning());
}
// Test that casting a negative float to uint inside a uvec constructor results in a warning. ESSL
// 3.00.6 section 5.4.1 specifies this as an undefined conversion.
TEST_F(ConstantFoldingExpressionTest, FoldNegativeFloatToUvec)
{
const std::string &uintString = "uvec4(2.0, 1.0, vec2(0.0, -1.0)).w";
evaluateUint(uintString);
ASSERT_TRUE(constantFoundInAST(std::numeric_limits<unsigned int>::max()));
ASSERT_TRUE(hasWarning());
}
// Test that a negative float doesn't result in a warning when it is inside a constructor but isn't
// actually converted.
TEST_F(ConstantFoldingExpressionTest, NegativeFloatInsideUvecConstructorButOutOfRange)
{
const std::string &uintString = "uvec2(1.0, vec2(0.0, -1.0)).x";
evaluateUint(uintString);
ASSERT_FALSE(hasWarning());
}
// Test that a large float (above max int32_t) is converted to unsigned integer correctly.
TEST_F(ConstantFoldingExpressionTest, LargeFloatToUint)
{
const std::string &uintString = "uint(3221225472.0)";
evaluateUint(uintString);
ASSERT_TRUE(constantFoundInAST(3221225472u));
ASSERT_FALSE(hasWarning());
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment