From b75075ac7bcfd6d8ad29f76cb54ecf34df62af0a Mon Sep 17 00:00:00 2001 From: Austin Eng Date: Fri, 19 Jun 2020 18:47:33 +0000 Subject: [PATCH] Protect against huge buffer allocations on macOS 10.12, 10.13 |MTLDevice maxBufferLength| is not available until 10.14, so on lower versions of macOS, try using |recommendedMaxWorkingSetSize| Bug: dawn:450 Change-Id: I52dc4211cf3d7014771d580a9af9c72abe92a375 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/23263 Commit-Queue: Austin Eng Reviewed-by: Stephen White --- src/dawn_native/metal/BufferMTL.mm | 15 +++++++++++++ src/tests/end2end/BufferTests.cpp | 36 +++++++++--------------------- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/dawn_native/metal/BufferMTL.mm b/src/dawn_native/metal/BufferMTL.mm index 4d00d69341..ccfd3b39ec 100644 --- a/src/dawn_native/metal/BufferMTL.mm +++ b/src/dawn_native/metal/BufferMTL.mm @@ -24,6 +24,10 @@ namespace dawn_native { namespace metal { // largest alignment of supported data types static constexpr uint32_t kMinUniformOrStorageBufferAlignment = 16u; + // The maximum buffer size if querying the maximum buffer size or recommended working set size + // is not available. This is a somewhat arbitrary limit of 1 GiB. + static constexpr uint32_t kMaxBufferSizeFallback = 1024u * 1024u * 1024u; + // static ResultOrError Buffer::Create(Device* device, const BufferDescriptor* descriptor) { Ref buffer = AcquireRef(new Buffer(device, descriptor)); @@ -63,6 +67,17 @@ namespace dawn_native { namespace metal { if (currentSize > maxBufferSize) { return DAWN_OUT_OF_MEMORY_ERROR("Buffer allocation is too large"); } + } else if (@available(macOS 10.12, *)) { + // |maxBufferLength| isn't always available on older systems. If available, use + // |recommendedMaxWorkingSetSize| instead. We can probably allocate more than this, + // but don't have a way to discover a better limit. MoltenVK also uses this heuristic. + uint64_t maxWorkingSetSize = + [ToBackend(GetDevice())->GetMTLDevice() recommendedMaxWorkingSetSize]; + if (currentSize > maxWorkingSetSize) { + return DAWN_OUT_OF_MEMORY_ERROR("Buffer allocation is too large"); + } + } else if (currentSize > kMaxBufferSizeFallback) { + return DAWN_OUT_OF_MEMORY_ERROR("Buffer allocation is too large"); } mMtlBuffer = [ToBackend(GetDevice())->GetMTLDevice() newBufferWithLength:currentSize diff --git a/src/tests/end2end/BufferTests.cpp b/src/tests/end2end/BufferTests.cpp index f29bacb574..989b919156 100644 --- a/src/tests/end2end/BufferTests.cpp +++ b/src/tests/end2end/BufferTests.cpp @@ -535,11 +535,8 @@ TEST_P(BufferTests, CreateBufferOOM) { ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor)); // UINT64_MAX may be special cased. Test a smaller, but really large buffer also fails - // This hangs on the Metal AMD driver - if (!(IsMetal() && IsAMD())) { - descriptor.size = 1ull << 50; - ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor)); - } + descriptor.size = 1ull << 50; + ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor)); } // Test that a very large CreateBufferMapped fails gracefully. @@ -562,11 +559,8 @@ TEST_P(BufferTests, CreateBufferMappedOOM) { ASSERT_DEVICE_ERROR(device.CreateBufferMapped(&descriptor)); // UINT64_MAX may be special cased. Test a smaller, but really large buffer also fails - // This hangs on the Metal AMD driver - if (!(IsMetal() && IsAMD())) { - descriptor.size = 1ull << 50; - ASSERT_DEVICE_ERROR(device.CreateBufferMapped(&descriptor)); - } + descriptor.size = 1ull << 50; + ASSERT_DEVICE_ERROR(device.CreateBufferMapped(&descriptor)); } // Test mappable buffer @@ -582,11 +576,9 @@ TEST_P(BufferTests, CreateBufferMappedOOM) { descriptor.size = std::numeric_limits::max(); ASSERT_DEVICE_ERROR(device.CreateBufferMapped(&descriptor)); - if (!(IsMetal() && IsAMD())) { - // UINT64_MAX may be special cased. Test a smaller, but really large buffer also fails - descriptor.size = 1ull << 50; - ASSERT_DEVICE_ERROR(device.CreateBufferMapped(&descriptor)); - } + // UINT64_MAX may be special cased. Test a smaller, but really large buffer also fails + descriptor.size = 1ull << 50; + ASSERT_DEVICE_ERROR(device.CreateBufferMapped(&descriptor)); } } @@ -624,11 +616,8 @@ TEST_P(BufferTests, CreateBufferOOMMapReadAsync) { RunTest(descriptor); // UINT64_MAX may be special cased. Test a smaller, but really large buffer also fails - // This hangs on the Metal AMD driver - if (!(IsMetal() && IsAMD())) { - descriptor.size = 1ull << 50; - RunTest(descriptor); - } + descriptor.size = 1ull << 50; + RunTest(descriptor); } // Test that mapping an OOM buffer for reading fails gracefully @@ -664,11 +653,8 @@ TEST_P(BufferTests, CreateBufferOOMMapWriteAsync) { RunTest(descriptor); // UINT64_MAX may be special cased. Test a smaller, but really large buffer also fails - // This hangs on the Metal AMD driver - if (!(IsMetal() && IsAMD())) { - descriptor.size = 1ull << 50; - RunTest(descriptor); - } + descriptor.size = 1ull << 50; + RunTest(descriptor); } DAWN_INSTANTIATE_TEST(BufferTests,