tint/resolver: Fix ICE when failing to materialize

If the abstract numeric cannot materialize to the target type, we could
ICE before the validation handles this bad case. To fix, simply hoist
the validation earlier.

Bug: chromium:1337524
Change-Id: Icc603b056900131cfdb029b517f1c3d030b2ecb4
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/94322
Reviewed-by: Dan Sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
This commit is contained in:
Ben Clayton 2022-06-20 19:23:02 +00:00 committed by Dawn LUCI CQ
parent e4e4854b77
commit 22d8dea091
4 changed files with 44 additions and 14 deletions

View File

@ -33,6 +33,7 @@ using f16V = builder::vec<3, f16>;
using i32V = builder::vec<3, i32>;
using u32V = builder::vec<3, u32>;
using f32M = builder::mat<3, 2, f32>;
using i32Varr = builder::array<3, i32>;
constexpr double kHighestU32 = static_cast<double>(u32::kHighest);
constexpr double kLowestU32 = static_cast<double>(u32::kLowest);
@ -530,6 +531,10 @@ INSTANTIATE_TEST_SUITE_P(InvalidConversion,
Types<u32, AFloat>(), //
Types<i32V, AFloatV>(), //
Types<u32V, AFloatV>(), //
Types<i32Varr, AInt>(), //
Types<i32Varr, AIntV>(), //
Types<i32Varr, AFloat>(), //
Types<i32Varr, AFloatV>(), //
})));
INSTANTIATE_TEST_SUITE_P(ScalarValueCannotBeRepresented,
@ -959,5 +964,25 @@ INSTANTIATE_TEST_SUITE_P(ArrayLengthValueCannotBeRepresented,
} // namespace materialize_abstract_numeric_to_default_type
using MaterializeAbstractNumericToUnrelatedType = resolver::ResolverTest;
TEST_F(MaterializeAbstractNumericToUnrelatedType, AIntToStructVarCtor) {
Structure("S", {Member("a", ty.i32())});
WrapInFunction(Decl(Var("v", ty.type_name("S"), Expr(Source{{12, 34}}, 1_a))));
ASSERT_FALSE(r()->Resolve());
EXPECT_THAT(
r()->error(),
testing::HasSubstr("error: cannot convert value of type 'abstract-int' to type 'S'"));
}
TEST_F(MaterializeAbstractNumericToUnrelatedType, AIntToStructLetCtor) {
Structure("S", {Member("a", ty.i32())});
WrapInFunction(Decl(Let("v", ty.type_name("S"), Expr(Source{{12, 34}}, 1_a))));
ASSERT_FALSE(r()->Resolve());
EXPECT_THAT(
r()->error(),
testing::HasSubstr("error: cannot convert value of type 'abstract-int' to type 'S'"));
}
} // namespace
} // namespace tint::resolver

View File

@ -1243,7 +1243,11 @@ const sem::Expression* Resolver::Materialize(const sem::Expression* expr,
// Helper for actually creating the the materialize node, performing the constant cast, updating
// the ast -> sem binding, and performing validation.
auto materialize = [&](const sem::Type* target_ty) -> sem::Materialize* {
auto* src_ty = expr->Type();
auto* decl = expr->Declaration();
if (!validator_.Materialize(target_ty, src_ty, decl->source)) {
return nullptr;
}
auto expr_val = EvaluateConstantValue(decl, expr->Type());
if (!expr_val) {
return nullptr;
@ -1252,7 +1256,7 @@ const sem::Expression* Resolver::Materialize(const sem::Expression* expr,
TINT_ICE(Resolver, builder_->Diagnostics())
<< decl->source
<< "EvaluateConstantValue() returned invalid value for materialized value of type: "
<< builder_->FriendlyName(expr->Type());
<< builder_->FriendlyName(src_ty);
return nullptr;
}
auto materialized_val = ConvertValue(expr_val.Get(), target_ty, decl->source);
@ -1269,7 +1273,7 @@ const sem::Expression* Resolver::Materialize(const sem::Expression* expr,
builder_->create<sem::Materialize>(expr, current_statement_, materialized_val.Get());
m->Behaviors() = expr->Behaviors();
builder_->Sem().Replace(decl, m);
return validator_.Materialize(m) ? m : nullptr;
return m;
};
// Helpers for constructing semantic types

View File

@ -307,14 +307,13 @@ bool Validator::MultisampledTexture(const sem::MultisampledTexture* t, const Sou
return true;
}
bool Validator::Materialize(const sem::Materialize* m) const {
auto* from = m->Expr()->Type();
auto* to = m->Type();
bool Validator::Materialize(const sem::Type* to,
const sem::Type* from,
const Source& source) const {
if (sem::Type::ConversionRank(from, to) == sem::Type::kNoConversion) {
AddError("cannot convert value of type '" + sem_.TypeNameOf(from) + "' to type '" +
sem_.TypeNameOf(to) + "'",
m->Expr()->Declaration()->source);
source);
return false;
}
return true;

View File

@ -283,10 +283,12 @@ class Validator {
/// @returns true on success, false otherwise.
bool LoopStatement(const sem::LoopStatement* stmt) const;
/// Validates a materialize of an abstract numeric value
/// @param m the materialize to validate
/// Validates a materialize of an abstract numeric value from the type `from` to the type `to`.
/// @param to the target type
/// @param from the abstract numeric type
/// @param source the source of the materialization
/// @returns true on success, false otherwise
bool Materialize(const sem::Materialize* m) const;
bool Materialize(const sem::Type* to, const sem::Type* from, const Source& source) const;
/// Validates a matrix
/// @param ty the matrix to validate