Add maximum limitation for query count in CreateQuerySet
- Limit the maximum query count to 8192 to fit Metal restriction.
- Add unittest tests of query count and remove the test of buffer
size overflow validation on 32-bits.
Bug: dawn:434
Change-Id: Ie573b715cc3f67ec158996119a8b4a49e493680a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/36021
Commit-Queue: Hao Li <hao.x.li@intel.com>
Reviewed-by: Austin Eng <enga@chromium.org>
diff --git a/src/common/Constants.h b/src/common/Constants.h
index d47afc0..289df4b 100644
--- a/src/common/Constants.h
+++ b/src/common/Constants.h
@@ -68,4 +68,9 @@
// on macOS, but we decide to do it on all platforms.
static constexpr uint64_t kCopyBufferToBufferOffsetAlignment = 4u;
+// The maximum size of visibilityResultBuffer is 256KB on Metal, to fit the restriction, limit the
+// maximum size of query set to 64KB. The size of a query is 8-bytes, the maximum query count is 64
+// * 1024 / 8.
+static constexpr uint32_t kMaxQueryCount = 8192u;
+
#endif // COMMON_CONSTANTS_H_
diff --git a/src/dawn_native/QuerySet.cpp b/src/dawn_native/QuerySet.cpp
index bd65ed0..c98c8ca 100644
--- a/src/dawn_native/QuerySet.cpp
+++ b/src/dawn_native/QuerySet.cpp
@@ -43,6 +43,10 @@
return DAWN_VALIDATION_ERROR("nextInChain must be nullptr");
}
+ if (descriptor->count > kMaxQueryCount) {
+ return DAWN_VALIDATION_ERROR("Max query count exceeded");
+ }
+
DAWN_TRY(ValidateQueryType(descriptor->type));
switch (descriptor->type) {
diff --git a/src/tests/BUILD.gn b/src/tests/BUILD.gn
index c76c9b2..342aa80 100644
--- a/src/tests/BUILD.gn
+++ b/src/tests/BUILD.gn
@@ -195,7 +195,7 @@
"unittests/validation/GetBindGroupLayoutValidationTests.cpp",
"unittests/validation/IndexBufferValidationTests.cpp",
"unittests/validation/MinimumBufferSizeValidationTests.cpp",
- "unittests/validation/QuerySetValidationTests.cpp",
+ "unittests/validation/QueryValidationTests.cpp",
"unittests/validation/QueueSubmitValidationTests.cpp",
"unittests/validation/QueueWriteTextureValidationTests.cpp",
"unittests/validation/RenderBundleValidationTests.cpp",
diff --git a/src/tests/unittests/validation/QuerySetValidationTests.cpp b/src/tests/unittests/validation/QueryValidationTests.cpp
similarity index 96%
rename from src/tests/unittests/validation/QuerySetValidationTests.cpp
rename to src/tests/unittests/validation/QueryValidationTests.cpp
index 674ca90..fa0d8c7 100644
--- a/src/tests/unittests/validation/QuerySetValidationTests.cpp
+++ b/src/tests/unittests/validation/QueryValidationTests.cpp
@@ -47,6 +47,15 @@
ASSERT_DEVICE_ERROR(CreateQuerySet(device, wgpu::QueryType::Timestamp, 1));
}
+// Test creating query set with invalid count
+TEST_F(QuerySetValidationTest, InvalidQueryCount) {
+ // Success create a query set with the maximum count
+ CreateQuerySet(device, wgpu::QueryType::Occlusion, kMaxQueryCount);
+
+ // Fail to create a query set with the count which exceeds the maximum
+ ASSERT_DEVICE_ERROR(CreateQuerySet(device, wgpu::QueryType::Occlusion, kMaxQueryCount + 1));
+}
+
// Test creating query set with invalid type
TEST_F(QuerySetValidationTest, InvalidQueryType) {
ASSERT_DEVICE_ERROR(CreateQuerySet(device, static_cast<wgpu::QueryType>(0xFFFFFFFF), 1));
@@ -602,18 +611,3 @@
ASSERT_DEVICE_ERROR(queue.Submit(1, &commands));
}
}
-
-// Check that in 32bit mode the computation of queryCount * sizeof(uint64_t) doesn't overflow (which
-// would skip validation).
-TEST_F(ResolveQuerySetValidationTest, BufferOverflowOn32Bits) {
- // If compiling for 32-bits mode, the data size calculated by queryCount * sizeof(uint64_t)
- // is 8, which is less than the buffer size.
- constexpr uint32_t kQueryCount = std::numeric_limits<uint32_t>::max() / sizeof(uint64_t) + 2;
-
- wgpu::QuerySet querySet = CreateQuerySet(device, wgpu::QueryType::Occlusion, kQueryCount);
- wgpu::Buffer destination = CreateBuffer(device, 1024, wgpu::BufferUsage::QueryResolve);
- wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- encoder.ResolveQuerySet(querySet, 0, kQueryCount, destination, 0);
-
- ASSERT_DEVICE_ERROR(encoder.Finish());
-}
