dump: More alignment warning

* dump: Add more alignment
* dump: Cleanup warning logic
* dump: Warn about bad indirect buffers

Author: spencer-lunarg

layers/gpu_dump/gpu_dump.cpp

@@ -38,12 +38,21 @@ std::vector<uint8_t> GpuDump::CopyDataFromMemory(VkDeviceAddress memory_addresss
     auto buffer_state = *buffer_list.begin();
     const vvl::DeviceMemory& memory_state = *buffer_state->MemoryState();
 
+    // Prevent copying OOB of a buffer
+    if ((memory_addresss + copy_size) > buffer_state->DeviceAddressRange().end) {
+        return result;
+    }
+
     VkDeviceSize offset = memory_addresss - buffer_state->DeviceAddressRange().begin;
     if (memory_state.mappable) {
         uint8_t* data_ptr = static_cast<uint8_t*>(memory_state.p_driver_data);
 
         if (!memory_state.p_driver_data) {
-            DispatchMapMemory(device, memory_state.VkHandle(), offset, copy_size, 0, (void**)&data_ptr);
+            // Just use WHOLE_SIZE to avoid issues with partial mappings
+            // Example:
+            //  The |memory_address| is 0x1001 and |copy_size| is 4, the driver (or at least TestICD) will return back something
+            //  aligned to a value like 64, so if the buffer is only 64 bytes, you will now be accessing data over it
+            DispatchMapMemory(device, memory_state.VkHandle(), offset, VK_WHOLE_SIZE, 0, (void**)&data_ptr);
 
             // Skip checking if coherent memory or not
             VkMappedMemoryRange memory_range = vku::InitStructHelper();
@@ -62,9 +71,44 @@ std::vector<uint8_t> GpuDump::CopyDataFromMemory(VkDeviceAddress memory_addresss
         }
     } else {
         // TODO - Handle non-host visible memory
+        // When we add, we need to guard against if trying to read non-aligned data
+        //   (which should have a warning already)
     }
 
     return result;
 }
 
+// return for warning if no buffer found
+bool GpuDump::ListBuffers(std::ostringstream& ss, VkDeviceAddress address, uint32_t indents, bool new_line_start) {
+    if (new_line_start) {
+        ss << "\n";
+    }
+
+    auto buffer_states = GetBuffersByAddress(address);
+    for (uint32_t i = 0; i < indents; i++) {
+        ss << "    ";
+    }
+
+    for (uint32_t i = 0; i < buffer_states.size(); i++) {
+        if (i != 0) {
+            ss << '\n';
+            for (uint32_t j = 0; j < indents; j++) {
+                ss << "    ";
+            }
+        }
+        auto& buffer_state = buffer_states[i];
+        ss << "- " << buffer_state->Describe(*this);
+    }
+
+    if (buffer_states.empty()) {
+        ss << "- [WARNING] No VkBuffer found at 0x" << std::hex << address;
+    }
+
+    if (!new_line_start) {
+        ss << "\n";
+    }
+
+    return buffer_states.empty();
+}
+
 }  // namespace gpudump

layers/gpu_dump/gpu_dump.h

@@ -38,6 +38,8 @@ class GpuDump : public vvl::DeviceProxy {
     void Created(vvl::CommandBuffer& cb_state) override;
 
     std::vector<uint8_t> CopyDataFromMemory(VkDeviceAddress memory_addresss, VkDeviceSize copy_size);
+
+    bool ListBuffers(std::ostringstream& ss, VkDeviceAddress address, uint32_t indents, bool new_line_start = false);
 };
 
 }  // namespace gpudump

layers/gpu_dump/gpu_dump_copy_memory_indirect.cpp

@@ -23,32 +23,15 @@
 #include <cstdint>
 #include <iostream>
 #include <sstream>
-#include "state_tracker/buffer_state.h"
 #include "state_tracker/cmd_buffer_state.h"
 
 namespace gpudump {
 
-// return for warning if no buffer found
-static bool ListBuffers(std::ostringstream& ss, const GpuDump& dev_data, VkDeviceAddress address, uint32_t indents) {
-    auto buffer_states = dev_data.GetBuffersByAddress(address);
-    for (uint32_t i = 0; i < indents; i++) {
-        ss << "    ";
-    }
-    for (auto& buffer_state : buffer_states) {
-        ss << "- " << buffer_state->Describe(dev_data) << "\n";
-    }
-    if (buffer_states.empty()) {
-        ss << "- [WARNING] No VkBuffer found at 0x" << std::hex << address << "\n";
-        return true;
-    }
-    return false;
-}
-
 bool CommandBufferSubState::DumpCopyMemoryIndirectCommon(std::ostringstream& ss, uint32_t copy_count,
                                                          VkStridedDeviceAddressRangeKHR copy_address_range) const {
     ss << "copyCount = " << copy_count << ", starting address = 0x" << std::hex << copy_address_range.address
        << ", size = " << std::dec << copy_address_range.size << ", stride = " << copy_address_range.stride << "\n";
-    return ListBuffers(ss, dev_data, copy_address_range.address, 1);
+    return dev_data.ListBuffers(ss, copy_address_range.address, 1);
 }
 
 void CommandBufferSubState::DumpCopyMemoryIndirect(const VkCopyMemoryIndirectInfoKHR& info, const Location& loc) const {
@@ -70,9 +53,9 @@ void CommandBufferSubState::DumpCopyMemoryIndirect(const VkCopyMemoryIndirectInf
             auto command = *(VkCopyMemoryIndirectCommandKHR*)(indirect_data.data() + stride_i);
             ss << "    - size: " << std::dec << command.size << '\n';
             ss << "    - srcAddress: 0x" << std::hex << command.srcAddress << '\n';
-            found_warning |= ListBuffers(ss, dev_data, command.srcAddress, 2);
+            found_warning |= dev_data.ListBuffers(ss, command.srcAddress, 2);
             ss << "    - dstAddress: 0x" << std::hex << command.dstAddress << '\n';
-            found_warning |= ListBuffers(ss, dev_data, command.dstAddress, 2);
+            found_warning |= dev_data.ListBuffers(ss, command.dstAddress, 2);
         }
     }
 
@@ -106,7 +89,7 @@ void CommandBufferSubState::DumpCopyMemoryToImageIndirect(const VkCopyMemoryToIm
         if (know_data) {
             auto command = *(VkCopyMemoryToImageIndirectCommandKHR*)(indirect_data.data() + stride_i);
             ss << "    - srcAddress: 0x" << std::hex << command.srcAddress << '\n';
-            found_warning |= ListBuffers(ss, dev_data, command.srcAddress, 2);
+            found_warning |= dev_data.ListBuffers(ss, command.srcAddress, 2);
             ss << "    - bufferRowLength: " << std::dec << command.bufferRowLength << '\n';
             ss << "    - bufferImageHeight: " << command.bufferImageHeight << '\n';
             ss << "    - imageSubresource: " << string_VkImageSubresourceLayers(command.imageSubresource) << '\n';