commit b75075ac7bcfd6d8ad29f76cb54ecf34df62af0a
author Austin Eng <enga@chromium.org> Fri Jun 19 18:47:33 2020 +0000
committer Commit Bot service account <commit-bot@chromium.org> Fri Jun 19 18:47:33 2020 +0000
tree 335be30df55bd9be16a4ebcabd46c49c01c86786
parent 0e9e5ee8aadacf77e5f6a2e91e73ce83e8c0f7b4

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 <enga@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>

src/dawn_native/metal/BufferMTL.mm

diff --git a/src/dawn_native/metal/BufferMTL.mm b/src/dawn_native/metal/BufferMTL.mm
index 4d00d69..ccfd3b3 100644
--- a/src/dawn_native/metal/BufferMTL.mm
+++ b/src/dawn_native/metal/BufferMTL.mm
@@ -24,6 +24,10 @@
     // 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*> Buffer::Create(Device* device, const BufferDescriptor* descriptor) {
         Ref<Buffer> buffer = AcquireRef(new Buffer(device, descriptor));
@@ -63,6 +67,17 @@
             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

src/tests/end2end/BufferTests.cpp

diff --git a/src/tests/end2end/BufferTests.cpp b/src/tests/end2end/BufferTests.cpp
index f29bacb..989b919 100644
--- a/src/tests/end2end/BufferTests.cpp
+++ b/src/tests/end2end/BufferTests.cpp
@@ -535,11 +535,8 @@
     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 @@
         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 @@
         descriptor.size = std::numeric_limits<uint64_t>::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 @@
     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 @@
     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,