Skip to content

Commit 95acbbc

Browse files
johnstiles-googleSkia Commit-Bot
authored andcommitted
Fix crash when comparing against a negated constant vector.
This CL solves the fuzzer crash. Constant propagation of the negative sign into the vector will be investigated in a followup CL. This CL also adds a few cleanups into IRGenerator::constantFold. Change-Id: If73a4fe2a5777265e7d43cc4f482653a38cb59af Bug: oss-fuzz:26830, oss-fuzz:26789 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332261 Commit-Queue: John Stiles <johnstiles@google.com> Auto-Submit: John Stiles <johnstiles@google.com> Reviewed-by: Brian Osman <brianosman@google.com> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
1 parent 6c88ea1 commit 95acbbc

File tree

3 files changed

+47
-26
lines changed

3 files changed

+47
-26
lines changed

src/sksl/SkSLIRGenerator.cpp

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1726,9 +1726,9 @@ std::unique_ptr<Expression> IRGenerator::constantFold(const Expression& left,
17261726
const Expression& right) const {
17271727
// If the left side is a constant boolean literal, the right side does not need to be constant
17281728
// for short circuit optimizations to allow the constant to be folded.
1729-
if (left.kind() == Expression::Kind::kBoolLiteral && !right.isCompileTimeConstant()) {
1729+
if (left.is<BoolLiteral>() && !right.isCompileTimeConstant()) {
17301730
return short_circuit_boolean(fContext, left, op, right);
1731-
} else if (right.kind() == Expression::Kind::kBoolLiteral && !left.isCompileTimeConstant()) {
1731+
} else if (right.is<BoolLiteral>() && !left.isCompileTimeConstant()) {
17321732
// There aren't side effects in SKSL within expressions, so (left OP right) is equivalent to
17331733
// (right OP left) for short-circuit optimizations
17341734
return short_circuit_boolean(fContext, right, op, left);
@@ -1742,8 +1742,7 @@ std::unique_ptr<Expression> IRGenerator::constantFold(const Expression& left,
17421742
// precision to calculate the results and hope the result makes sense. The plan is to move the
17431743
// Skia caps into SkSL, so we have access to all of them including the precisions of the various
17441744
// types, which will let us be more intelligent about this.
1745-
if (left.kind() == Expression::Kind::kBoolLiteral &&
1746-
right.kind() == Expression::Kind::kBoolLiteral) {
1745+
if (left.is<BoolLiteral>() && right.is<BoolLiteral>()) {
17471746
bool leftVal = left.as<BoolLiteral>().value();
17481747
bool rightVal = right.as<BoolLiteral>().value();
17491748
bool result;
@@ -1753,15 +1752,14 @@ std::unique_ptr<Expression> IRGenerator::constantFold(const Expression& left,
17531752
case Token::Kind::TK_LOGICALXOR: result = leftVal ^ rightVal; break;
17541753
default: return nullptr;
17551754
}
1756-
return std::unique_ptr<Expression>(new BoolLiteral(fContext, left.fOffset, result));
1755+
return std::make_unique<BoolLiteral>(fContext, left.fOffset, result);
17571756
}
17581757
#define RESULT(t, op) std::make_unique<t ## Literal>(fContext, left.fOffset, \
17591758
leftVal op rightVal)
17601759
#define URESULT(t, op) std::make_unique<t ## Literal>(fContext, left.fOffset, \
17611760
(uint32_t) leftVal op \
17621761
(uint32_t) rightVal)
1763-
if (left.kind() == Expression::Kind::kIntLiteral &&
1764-
right.kind() == Expression::Kind::kIntLiteral) {
1762+
if (left.is<IntLiteral>() && right.is<IntLiteral>()) {
17651763
int64_t leftVal = left.as<IntLiteral>().value();
17661764
int64_t rightVal = right.as<IntLiteral>().value();
17671765
switch (op) {
@@ -1814,8 +1812,7 @@ std::unique_ptr<Expression> IRGenerator::constantFold(const Expression& left,
18141812
return nullptr;
18151813
}
18161814
}
1817-
if (left.kind() == Expression::Kind::kFloatLiteral &&
1818-
right.kind() == Expression::Kind::kFloatLiteral) {
1815+
if (left.is<FloatLiteral>() && right.is<FloatLiteral>()) {
18191816
SKSL_FLOAT leftVal = left.as<FloatLiteral>().value();
18201817
SKSL_FLOAT rightVal = right.as<FloatLiteral>().value();
18211818
switch (op) {
@@ -1842,19 +1839,25 @@ std::unique_ptr<Expression> IRGenerator::constantFold(const Expression& left,
18421839
if (leftType.typeKind() == Type::TypeKind::kVector && leftType.componentType().isFloat() &&
18431840
leftType == rightType) {
18441841
ExpressionArray args;
1845-
#define RETURN_VEC_COMPONENTWISE_RESULT(op) \
1846-
for (int i = 0; i < leftType.columns(); i++) { \
1847-
SKSL_FLOAT value = left.getFVecComponent(i) op right.getFVecComponent(i); \
1848-
args.push_back(std::make_unique<FloatLiteral>(fContext, /*offset=*/-1, value)); \
1849-
} \
1850-
return std::make_unique<Constructor>(/*offset=*/-1, &leftType, std::move(args))
1842+
#define RETURN_VEC_COMPONENTWISE_RESULT(op) \
1843+
for (int i = 0; i < leftType.columns(); i++) { \
1844+
SKSL_FLOAT value = left.getFVecComponent(i) op right.getFVecComponent(i); \
1845+
args.push_back(std::make_unique<FloatLiteral>(fContext, left.fOffset, value)); \
1846+
} \
1847+
return std::make_unique<Constructor>(left.fOffset, &leftType, std::move(args))
18511848
switch (op) {
18521849
case Token::Kind::TK_EQEQ:
1853-
return std::unique_ptr<Expression>(new BoolLiteral(fContext, -1,
1854-
left.compareConstant(fContext, right)));
1850+
if (left.kind() == right.kind()) {
1851+
return std::make_unique<BoolLiteral>(fContext, left.fOffset,
1852+
left.compareConstant(fContext, right));
1853+
}
1854+
return nullptr;
18551855
case Token::Kind::TK_NEQ:
1856-
return std::unique_ptr<Expression>(new BoolLiteral(fContext, -1,
1857-
!left.compareConstant(fContext, right)));
1856+
if (left.kind() == right.kind()) {
1857+
return std::make_unique<BoolLiteral>(fContext, left.fOffset,
1858+
!left.compareConstant(fContext, right));
1859+
}
1860+
return nullptr;
18581861
case Token::Kind::TK_PLUS: RETURN_VEC_COMPONENTWISE_RESULT(+);
18591862
case Token::Kind::TK_MINUS: RETURN_VEC_COMPONENTWISE_RESULT(-);
18601863
case Token::Kind::TK_STAR: RETURN_VEC_COMPONENTWISE_RESULT(*);
@@ -1872,16 +1875,17 @@ std::unique_ptr<Expression> IRGenerator::constantFold(const Expression& left,
18721875
default:
18731876
return nullptr;
18741877
}
1878+
#undef RETURN_VEC_COMPONENTWISE_RESULT
18751879
}
18761880
if (leftType.typeKind() == Type::TypeKind::kMatrix &&
18771881
rightType.typeKind() == Type::TypeKind::kMatrix &&
18781882
left.kind() == right.kind()) {
18791883
switch (op) {
18801884
case Token::Kind::TK_EQEQ:
1881-
return std::make_unique<BoolLiteral>(fContext, /*offset=*/-1,
1885+
return std::make_unique<BoolLiteral>(fContext, left.fOffset,
18821886
left.compareConstant(fContext, right));
18831887
case Token::Kind::TK_NEQ:
1884-
return std::make_unique<BoolLiteral>(fContext, /*offset=*/-1,
1888+
return std::make_unique<BoolLiteral>(fContext, left.fOffset,
18851889
!left.compareConstant(fContext, right));
18861890
default:
18871891
return nullptr;
Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1-
### Compilation failed:
2-
31

2+
out vec4 sk_FragColor;
3+
void main() {
4+
sk_FragColor.x = 1.0;
5+
sk_FragColor.y = float(vec4(1.0) == -vec4(1.0) ? 1 : 0);
6+
sk_FragColor.z = float(vec4(0.0) == -vec4(0.0) ? 1 : 0);
7+
}
Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
1-
### Compilation failed:
2-
3-
1+
#include <metal_stdlib>
2+
#include <simd/simd.h>
3+
using namespace metal;
4+
struct Inputs {
5+
};
6+
struct Outputs {
7+
float4 sk_FragColor [[color(0)]];
8+
};
9+
fragment Outputs fragmentMain(Inputs _in [[stage_in]], bool _frontFacing [[front_facing]], float4 _fragCoord [[position]]) {
10+
Outputs _outputStruct;
11+
thread Outputs* _out = &_outputStruct;
12+
_out->sk_FragColor.x = 1.0;
13+
_out->sk_FragColor.y = float(all(float4(1.0) == -float4(1.0)) ? 1 : 0);
14+
_out->sk_FragColor.z = float(all(float4(0.0) == -float4(0.0)) ? 1 : 0);
15+
return *_out;
16+
}

0 commit comments

Comments
 (0)