Add validation for pipeline overrides value type error for i32/u32/f32

Follow WebIDL TypeError for float and EnforceRange. Generate
a validation error is the number is not representable.

Bug: dawn:1597
Change-Id: I9a683f65ed0bfadb936d5de358670b01a2036848
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/112920
Reviewed-by: Austin Eng <enga@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Shrek Shao <shrekshao@google.com>
This commit is contained in:
shrekshao 2022-12-07 23:32:33 +00:00 committed by Dawn LUCI CQ
parent 7017ec264c
commit ad71dc1ab2
7 changed files with 105 additions and 39 deletions

View File

@ -51,4 +51,18 @@ inline Dst checked_cast(const Src& value) {
return static_cast<Dst>(value);
}
template <typename T>
bool IsDoubleValueRepresentable(double value) {
if constexpr (std::is_same_v<T, float> || std::is_integral_v<T>) {
// Following WebIDL 3.3.6.[EnforceRange] for integral
// Following WebIDL 3.2.5.float for float
// TODO(crbug.com/1396194): now follows what blink does but may need revisit.
constexpr double kLowest = static_cast<double>(std::numeric_limits<T>::lowest());
constexpr double kMax = static_cast<double>(std::numeric_limits<T>::max());
return kLowest <= value && value <= kMax;
} else {
static_assert(sizeof(T) != sizeof(T), "Unsupported type");
}
}
#endif // SRC_DAWN_COMMON_NUMERIC_H_

View File

@ -30,10 +30,13 @@ MaybeError ValidateComputePipelineDescriptor(DeviceBase* device,
DAWN_TRY(device->ValidateObject(descriptor->layout));
}
return ValidateProgrammableStage(
device, descriptor->compute.module, descriptor->compute.entryPoint,
descriptor->compute.constantCount, descriptor->compute.constants, descriptor->layout,
SingleShaderStage::Compute);
DAWN_TRY_CONTEXT(ValidateProgrammableStage(
device, descriptor->compute.module, descriptor->compute.entryPoint,
descriptor->compute.constantCount, descriptor->compute.constants,
descriptor->layout, SingleShaderStage::Compute),
"validating compute stage (%s, entryPoint: %s).", descriptor->compute.module,
descriptor->compute.entryPoint);
return {};
}
// ComputePipelineBase

View File

@ -68,8 +68,38 @@ MaybeError ValidateProgrammableStage(DeviceBase* device,
"Pipeline overridable constant \"%s\" not found in %s.", constants[i].key,
module);
DAWN_INVALID_IF(!std::isfinite(constants[i].value),
"Pipeline overridable constant \"%s\" with value (%f) is not finite",
constants[i].key, constants[i].value);
"Pipeline overridable constant \"%s\" with value (%f) is not finite in %s",
constants[i].key, constants[i].value, module);
// Validate if constant value can be represented by the given scalar type in shader
auto type = metadata.overrides.at(constants[i].key).type;
switch (type) {
case EntryPointMetadata::Override::Type::Float32:
DAWN_INVALID_IF(!IsDoubleValueRepresentable<float>(constants[i].value),
"Pipeline overridable constant \"%s\" with value (%f) is not "
"representable in type (%s)",
constants[i].key, constants[i].value, "f32");
break;
case EntryPointMetadata::Override::Type::Int32:
DAWN_INVALID_IF(!IsDoubleValueRepresentable<int32_t>(constants[i].value),
"Pipeline overridable constant \"%s\" with value (%f) is not "
"representable in type (%s)",
constants[i].key, constants[i].value,
type == EntryPointMetadata::Override::Type::Int32 ? "i32" : "b");
break;
case EntryPointMetadata::Override::Type::Uint32:
DAWN_INVALID_IF(!IsDoubleValueRepresentable<uint32_t>(constants[i].value),
"Pipeline overridable constant \"%s\" with value (%f) is not "
"representable in type (%s)",
constants[i].key, constants[i].value, "u32");
break;
case EntryPointMetadata::Override::Type::Boolean:
// Conversion to boolean can't fail
// https://webidl.spec.whatwg.org/#es-boolean
break;
default:
UNREACHABLE();
}
if (stageInitializedConstantIdentifiers.count(constants[i].key) == 0) {
if (metadata.uninitializedOverrides.count(constants[i].key) > 0) {
@ -79,8 +109,7 @@ MaybeError ValidateProgrammableStage(DeviceBase* device,
} else {
// There are duplicate initializations
return DAWN_VALIDATION_ERROR(
"Pipeline overridable constants \"%s\" is set more than once in %s",
constants[i].key, module);
"Pipeline overridable constants \"%s\" is set more than once", constants[i].key);
}
}
@ -102,9 +131,9 @@ MaybeError ValidateProgrammableStage(DeviceBase* device,
}
return DAWN_VALIDATION_ERROR(
"There are uninitialized pipeline overridable constants in shader module %s, their "
"There are uninitialized pipeline overridable constants, their "
"identifiers:[%s]",
module, uninitializedConstantsArray);
uninitializedConstantsArray);
}
return {};

View File

@ -128,7 +128,7 @@ MaybeError ValidateVertexState(DeviceBase* device,
DAWN_TRY_CONTEXT(ValidateProgrammableStage(device, descriptor->module, descriptor->entryPoint,
descriptor->constantCount, descriptor->constants,
layout, SingleShaderStage::Vertex),
"validating vertex stage (module: %s, entryPoint: %s).", descriptor->module,
"validating vertex stage (%s, entryPoint: %s).", descriptor->module,
descriptor->entryPoint);
const EntryPointMetadata& vertexMetadata =
descriptor->module->GetEntryPoint(descriptor->entryPoint);
@ -341,7 +341,7 @@ MaybeError ValidateFragmentState(DeviceBase* device,
DAWN_TRY_CONTEXT(ValidateProgrammableStage(device, descriptor->module, descriptor->entryPoint,
descriptor->constantCount, descriptor->constants,
layout, SingleShaderStage::Fragment),
"validating fragment stage (module: %s, entryPoint: %s).", descriptor->module,
"validating fragment stage (%s, entryPoint: %s).", descriptor->module,
descriptor->entryPoint);
uint32_t maxColorAttachments = device->GetLimits().v1.maxColorAttachments;

View File

@ -533,24 +533,8 @@ ResultOrError<std::unique_ptr<EntryPointMetadata>> ReflectEntryPointUsingTint(
for (auto& c : entryPoint.overrides) {
auto id = name2Id.at(c.name);
OverrideScalar defaultValue;
if (c.is_initialized) {
// if it is initialized, the scalar must exist
const auto& scalar = id2Scalar.at(id);
if (scalar.IsBool()) {
defaultValue.b = scalar.AsBool();
} else if (scalar.IsU32()) {
defaultValue.u32 = scalar.AsU32();
} else if (scalar.IsI32()) {
defaultValue.i32 = scalar.AsI32();
} else if (scalar.IsFloat()) {
defaultValue.f32 = scalar.AsFloat();
} else {
UNREACHABLE();
}
}
EntryPointMetadata::Override override = {id, FromTintOverrideType(c.type),
c.is_initialized, defaultValue};
c.is_initialized};
std::string identifier = c.is_id_specified ? std::to_string(override.id.value) : c.name;
metadata->overrides[identifier] = override;

View File

@ -227,11 +227,6 @@ struct EntryPointMetadata {
// Then it is required for the pipeline stage to have a constant record to initialize a
// value
bool isInitialized;
// Store the default initialized value in shader
// This is used by metal backend as the function_constant does not have dafault values
// Initialized when isInitialized == true
OverrideScalar defaultValue;
};
using OverridesMap = std::unordered_map<std::string, Override>;

View File

@ -223,31 +223,72 @@ TEST_F(ComputePipelineOverridableConstantsValidationTest, ConstantsIdentifierUni
TEST_F(ComputePipelineOverridableConstantsValidationTest, InvalidValue) {
SetUpShadersWithDefaultValueConstants();
{
// Error:: NaN
// Error: NaN
std::vector<wgpu::ConstantEntry> constants{{nullptr, "c3", std::nan("")}};
ASSERT_DEVICE_ERROR(TestCreatePipeline(constants));
}
{
// Error:: -NaN
// Error: -NaN
std::vector<wgpu::ConstantEntry> constants{{nullptr, "c3", -std::nan("")}};
ASSERT_DEVICE_ERROR(TestCreatePipeline(constants));
}
{
// Error:: Inf
// Error: Inf
std::vector<wgpu::ConstantEntry> constants{
{nullptr, "c3", std::numeric_limits<double>::infinity()}};
ASSERT_DEVICE_ERROR(TestCreatePipeline(constants));
}
{
// Error:: -Inf
// Error: -Inf
std::vector<wgpu::ConstantEntry> constants{
{nullptr, "c3", -std::numeric_limits<double>::infinity()}};
ASSERT_DEVICE_ERROR(TestCreatePipeline(constants));
}
{
// Valid:: Max
// Valid: Max
std::vector<wgpu::ConstantEntry> constants{
{nullptr, "c3", std::numeric_limits<float>::max()}};
TestCreatePipeline(constants);
}
{
// Valid: Lowest
std::vector<wgpu::ConstantEntry> constants{
{nullptr, "c3", std::numeric_limits<float>::lowest()}};
TestCreatePipeline(constants);
}
}
// Test that values that are not representable by WGSL type i32/u32/f16/f32
TEST_F(ComputePipelineOverridableConstantsValidationTest, OutofRangeValue) {
SetUpShadersWithDefaultValueConstants();
{
// Error: 1.79769e+308 cannot be represented by f32
std::vector<wgpu::ConstantEntry> constants{
{nullptr, "c3", std::numeric_limits<double>::max()}};
ASSERT_DEVICE_ERROR(TestCreatePipeline(constants));
}
{
// Error: i32 out of range
std::vector<wgpu::ConstantEntry> constants{
{nullptr, "c5", static_cast<double>(std::numeric_limits<int32_t>::max()) + 1.0}};
ASSERT_DEVICE_ERROR(TestCreatePipeline(constants));
}
{
// Error: i32 out of range (negative)
std::vector<wgpu::ConstantEntry> constants{
{nullptr, "c5", static_cast<double>(std::numeric_limits<int32_t>::lowest()) - 1.0}};
ASSERT_DEVICE_ERROR(TestCreatePipeline(constants));
}
{
// Error: u32 out of range
std::vector<wgpu::ConstantEntry> constants{
{nullptr, "c8", static_cast<double>(std::numeric_limits<uint32_t>::max()) + 1.0}};
ASSERT_DEVICE_ERROR(TestCreatePipeline(constants));
}
{
// Valid: conversion to boolean can't fail
std::vector<wgpu::ConstantEntry> constants{
{nullptr, "c0", static_cast<double>(std::numeric_limits<int32_t>::max()) + 1.0}};
TestCreatePipeline(constants);
}
}