graphics: consolidate validation guest checks

src/graphics/host_gpu/memoryTracker.cpp

@@ -18,8 +18,8 @@ MemoryTracker::~MemoryTracker() = default;
 #if KYTY_BUILD == KYTY_BUILD_DEBUG
 void MemoryTracker::ValidateGpuDirtyPages(const RangeSet& dirty, uint64_t vaddr, uint64_t size,
                                           const char* operation) const noexcept {
-	if (vaddr == 0 || size == 0 || size > UINT64_MAX - vaddr ||
+	if (!GuestRange {vaddr, size}.Valid() || (vaddr & (TRACKER_PAGE_SIZE - 1)) != 0 ||
-	    (vaddr & (TRACKER_PAGE_SIZE - 1)) != 0 || (size & (TRACKER_PAGE_SIZE - 1)) != 0) {
+	    (size & (TRACKER_PAGE_SIZE - 1)) != 0) {
 		EXIT("MemoryTracker: invalid dirty-page validation range\n");
 	}
 	for (auto page = vaddr; page < vaddr + size; page += TRACKER_PAGE_SIZE) {
@@ -37,9 +37,6 @@ void MemoryTracker::ValidateGpuDirtyPages(const RangeSet& dirty, uint64_t vaddr,
 void MemoryTracker::ValidateGpuDirtyOwnership(const RangeSet& dirty, uint64_t vaddr, uint64_t size,
                                               const char* operation) {
 	ValidateRange(vaddr, size);
-	if (vaddr + size > UINT64_MAX - (TRACKER_PAGE_SIZE - 1)) {
-		EXIT("MemoryTracker: dirty ownership range alignment overflow\n");
-	}
 	const auto begin = vaddr & ~(TRACKER_PAGE_SIZE - 1);
 	const auto end   = (vaddr + size + TRACKER_PAGE_SIZE - 1) & ~(TRACKER_PAGE_SIZE - 1);
 	for (auto page = begin; page < end; page += TRACKER_PAGE_SIZE) {
@@ -56,8 +53,7 @@ void MemoryTracker::ValidateGpuDirtyOwnership(const RangeSet& dirty, uint64_t va
 #endif
 
 void MemoryTracker::ValidateRange(uint64_t vaddr, uint64_t size) {
-	if (vaddr == 0 || size == 0 || vaddr >= TRACKER_ADDRESS_SIZE ||
+	if (!GuestRange {vaddr, size}.Valid()) {
-	    size > TRACKER_ADDRESS_SIZE - vaddr) {
 		EXIT("invalid memory tracker range\n");
 	}
 }

src/graphics/host_gpu/pageManager.cpp

@@ -110,7 +110,7 @@ private:
 };
 
 void ValidateRange(uint64_t vaddr, uint64_t size) {
-	if (vaddr == 0 || size == 0 || vaddr >= ADDRESS_SIZE || size > ADDRESS_SIZE - vaddr) {
+	if (!GuestRange {vaddr, size}.Valid()) {
 		Fatal("invalid range vaddr=0x%016" PRIx64 ", size=0x%016" PRIx64, vaddr, size);
 	}
 }

src/graphics/host_gpu/regionDefinitions.h

@@ -4,6 +4,8 @@
 #include "common/bitArray.h"
 #include "common/common.h"
 
+#include <compare>
+
 namespace Libs::Graphics {
 
 constexpr uint64_t TRACKER_PAGE_SIZE    = 4ull * 1024ull;
@@ -11,6 +13,20 @@ constexpr uint64_t TRACKER_REGION_SIZE  = 4ull * 1024ull * 1024ull;
 constexpr uint64_t TRACKER_ADDRESS_SIZE = 1ull << 40u;
 constexpr size_t   TRACKER_REGION_PAGES = TRACKER_REGION_SIZE / TRACKER_PAGE_SIZE;
 
+struct GuestRange {
+	uint64_t address = 0;
+	uint64_t size    = 0;
+
+	[[nodiscard]] constexpr bool Empty() const noexcept { return address == 0 && size == 0; }
+	[[nodiscard]] constexpr bool Valid() const noexcept {
+		return address != 0 && size != 0 && address < TRACKER_ADDRESS_SIZE &&
+		       size <= TRACKER_ADDRESS_SIZE - address;
+	}
+	[[nodiscard]] constexpr bool     ValidOrEmpty() const noexcept { return Empty() || Valid(); }
+	[[nodiscard]] constexpr uint64_t End() const noexcept { return address + size; }
+	auto                             operator<=>(const GuestRange&) const = default;
+};
+
 enum class DirtySource { Cpu, Gpu };
 using RegionBits = Common::BitArray<TRACKER_REGION_PAGES>;
 static_assert(sizeof(RegionBits) == TRACKER_REGION_PAGES / 8);

src/graphics/host_gpu/renderer/cache/bufferCache.cpp

@@ -241,8 +241,7 @@ BufferCache::~BufferCache() {
 }
 
 void BufferCache::InvalidateMemory(uint64_t vaddr, uint64_t size) {
-	if (vaddr == 0 || size == 0 || vaddr >= TRACKER_ADDRESS_SIZE ||
+	if (!GuestRange {vaddr, size}.Valid()) {
-	    size > TRACKER_ADDRESS_SIZE - vaddr) {
 		EXIT("BufferCache: invalid memory-invalidation range\n");
 	}
 	m_memory_tracker.InvalidateRegion(vaddr, size,
@@ -314,7 +313,7 @@ BufferId BufferCache::FindBuffer(uint64_t vaddr, uint64_t size) {
 	if (vaddr == 0) {
 		return NULL_BUFFER_ID;
 	}
-	if (size == 0 || vaddr >= TRACKER_ADDRESS_SIZE || size > TRACKER_ADDRESS_SIZE - vaddr) {
+	if (!GuestRange {vaddr, size}.Valid()) {
 		EXIT("BufferCache: invalid buffer discovery request\n");
 	}
 	const auto* owner = m_page_table.Find(vaddr >> PageTable::kPageBits);
@@ -443,8 +442,7 @@ std::pair<Buffer*, uint64_t> BufferCache::ObtainBuffer(uint64_t vaddr, uint64_t
                                                        bool is_written, bool is_texel_buffer,
                                                        BufferId id) {
 	auto& command = m_scheduler.Current();
-	if (command.IsInvalid() || vaddr == 0 || size == 0 ||
+	if (command.IsInvalid() || !GuestRange {vaddr, size}.Valid()) {
-	    vaddr >= TRACKER_ADDRESS_SIZE || size > TRACKER_ADDRESS_SIZE - vaddr) {
 		EXIT("BufferCache: buffer request requires a recording command buffer\n");
 	}
 
@@ -474,8 +472,7 @@ std::pair<Buffer*, uint64_t> BufferCache::ObtainBuffer(uint64_t vaddr, uint64_t
 }
 
 std::pair<Buffer*, uint64_t> BufferCache::ObtainBufferForImage(uint64_t vaddr, uint64_t size) {
-	if (vaddr == 0 || size == 0 || vaddr >= TRACKER_ADDRESS_SIZE ||
+	if (!GuestRange {vaddr, size}.Valid()) {
-	    size > TRACKER_ADDRESS_SIZE - vaddr) {
 		EXIT("BufferCache: invalid image source\n");
 	}
 	auto find_owner = [&]() -> Buffer* {
@@ -661,8 +658,7 @@ void BufferCache::CopyBuffer(uint64_t dst_vaddr, uint64_t src_vaddr, uint64_t si
 }
 
 bool BufferCache::IsRegionRegistered(uint64_t vaddr, uint64_t size) {
-	if (vaddr == 0 || size == 0 || vaddr >= TRACKER_ADDRESS_SIZE ||
+	if (!GuestRange {vaddr, size}.Valid()) {
-	    size > TRACKER_ADDRESS_SIZE - vaddr) {
 		EXIT("BufferCache: invalid registered-region query\n");
 	}
 	// Cached buffers are ordered and non-overlapping. The last buffer beginning before the query

src/graphics/host_gpu/renderer/cache/gpuResourceManager.cpp

@@ -35,8 +35,7 @@ bool GpuResourceManager::InvalidateMemory(uint64_t vaddr, uint64_t size) {
 }
 
 bool GpuResourceManager::IsMapped(uint64_t vaddr, uint64_t size) const noexcept {
-	if (vaddr == 0 || size == 0 || vaddr >= TRACKER_ADDRESS_SIZE ||
+	if (!GuestRange {vaddr, size}.Valid()) {
-	    size > TRACKER_ADDRESS_SIZE - vaddr) {
 		return false;
 	}
 	std::shared_lock lock(m_mapped_ranges_mutex);

src/graphics/host_gpu/renderer/colorRenderTarget.cpp

@@ -309,7 +309,7 @@ void RenderExecutor::ResolveRenderColorTarget(uint64_t submit_id, CommandBuffer&
 	if (backing_size == 0) {
 		EXIT("render-target backing is empty\n");
 	}
-	if (backing_size > TRACKER_ADDRESS_SIZE - rt.base.addr) {
+	if (!GuestRange {rt.base.addr, backing_size}.Valid()) {
 		EXIT("render-target backing range is invalid\n");
 	}
 

src/graphics/host_gpu/renderer/depthRenderTarget.cpp

@@ -260,9 +260,9 @@ void RenderExecutor::ResolveRenderDepthTarget(uint64_t submit_id, CommandBuffer&
 	const auto depth_backing_size   = depth_size.size * view.image_layers;
 	const auto stencil_backing_size = stencil_size.size * view.image_layers;
 	const auto htile_backing_size   = htile_size.size * view.image_layers;
-	if (depth_backing_size > TRACKER_ADDRESS_SIZE - z.z_read_base_addr ||
+	if (!GuestRange {z.z_read_base_addr, depth_backing_size}.Valid() ||
-	    (has_stencil && stencil_backing_size > TRACKER_ADDRESS_SIZE - z.stencil_read_base_addr) ||
+	    (has_stencil && !GuestRange {z.stencil_read_base_addr, stencil_backing_size}.Valid()) ||
-	    (has_htile && htile_backing_size > TRACKER_ADDRESS_SIZE - z.htile_data_base_addr)) {
+	    (has_htile && !GuestRange {z.htile_data_base_addr, htile_backing_size}.Valid())) {
 		DepthFatal("layered depth backing range is invalid");
 	}
 	r.htile                   = has_htile;

src/graphics/host_gpu/renderer/image/image.cpp

@@ -73,10 +73,8 @@ namespace {
 	return usage;
 }
 
-void ValidateRange(GuestRange range, const char* name) {
+void ValidateOptionalRange(GuestRange range, const char* name) {
-	if ((range.address == 0) != (range.size == 0) ||
+	if (!range.ValidOrEmpty()) {
-	    (range.address != 0 && (range.address >= TRACKER_ADDRESS_SIZE ||
-	                            range.size > TRACKER_ADDRESS_SIZE - range.address))) {
 		EXIT("invalid %s image range: address=0x%016llx size=0x%016llx\n", name,
 		     static_cast<unsigned long long>(range.address),
 		     static_cast<unsigned long long>(range.size));
@@ -567,13 +565,13 @@ void Image::CopyMip(Image& source, uint32_t mip, uint32_t layer) {
 namespace ImageOps {
 
 void Validate(const ImageInfo& info) {
-	ValidateRange(info.data, "data");
+	ValidateOptionalRange(info.data, "data");
-	ValidateRange(info.stencil, "stencil");
+	ValidateOptionalRange(info.stencil, "stencil");
 
 	if (info.pixel_format == vk::Format::eUndefined) {
 		const bool metadata_empty =
-		    info.metadata.range.address == 0 && info.metadata.range.size == 0 &&
+		    info.metadata.range.Empty() && info.metadata.kind == ImageMetadataKind::None &&
-		    info.metadata.kind == ImageMetadataKind::None && info.metadata.control == 0 &&
+		    info.metadata.control == 0 &&
 		    info.metadata.compression == VideoOutCompression::Uncompressed &&
 		    !info.metadata.stencil_compressed;
 		if (info.data.Empty() || info.HasStencil() || !metadata_empty || info.extent.width == 0 ||
@@ -620,16 +618,14 @@ void Validate(const ImageInfo& info) {
 	}
 	switch (info.metadata.kind) {
 		case ImageMetadataKind::None:
-			if (info.metadata.range.address != 0 || info.metadata.range.size != 0 ||
+			if (!info.metadata.range.Empty() || info.metadata.control != 0 ||
-			    info.metadata.control != 0 ||
 			    info.metadata.compression != VideoOutCompression::Uncompressed ||
 			    info.metadata.stencil_compressed) {
 				EXIT("metadata-free image has metadata state\n");
 			}
 			break;
 		case ImageMetadataKind::Htile:
-			ValidateRange(info.metadata.range, "HTILE");
+			if (!info.metadata.range.Valid() ||
-			if (info.metadata.range.Empty() ||
 			    info.metadata.compression != VideoOutCompression::Uncompressed) {
 				EXIT("invalid HTILE metadata\n");
 			}

src/graphics/host_gpu/renderer/image/imageInfo.h

@@ -15,18 +15,6 @@
 
 namespace Libs::Graphics {
 
-struct GuestRange {
-	uint64_t address = 0;
-	uint64_t size    = 0;
-
-	[[nodiscard]] constexpr bool Empty() const noexcept { return address == 0 || size == 0; }
-	[[nodiscard]] constexpr bool Valid() const noexcept {
-		return !Empty() && address < TRACKER_ADDRESS_SIZE && size <= TRACKER_ADDRESS_SIZE - address;
-	}
-	[[nodiscard]] constexpr uint64_t End() const noexcept { return address + size; }
-	auto                             operator<=>(const GuestRange&) const = default;
-};
-
 enum class VideoOutCompression : uint8_t { Uncompressed, Dcc256_256_0, Dcc256_64_64, Unsupported };
 
 enum class ImageMetadataKind : uint8_t { None, Htile, Dcc };

tests/MemoryTrackerTests.cpp

@@ -29,9 +29,11 @@
 
 namespace {
 
+using Libs::Graphics::GuestRange;
 using Libs::Graphics::MemoryTracker;
 using Libs::Graphics::PageManager;
 using Libs::Graphics::RangeSet;
+using Libs::Graphics::TRACKER_ADDRESS_SIZE;
 
 void Check(bool value, const char *text) {
   if (!value) {
@@ -208,6 +210,27 @@ void TestRangeSet() {
         "range set subtraction did not preserve both exact tails");
 }
 
+void TestGuestRange() {
+  constexpr GuestRange empty{};
+  constexpr GuestRange first_byte{1, 1};
+  constexpr GuestRange last_byte{TRACKER_ADDRESS_SIZE - 1, 1};
+
+  static_assert(empty.Empty() && !empty.Valid() && empty.ValidOrEmpty());
+  static_assert(!first_byte.Empty() && first_byte.Valid() &&
+                first_byte.ValidOrEmpty() && first_byte.End() == 2);
+  static_assert(last_byte.Valid() && last_byte.End() == TRACKER_ADDRESS_SIZE);
+
+  Check(!GuestRange{0, 1}.Empty() && !GuestRange{0, 1}.ValidOrEmpty(),
+        "zero-address nonempty guest range is rejected");
+  Check(!GuestRange{1, 0}.Empty() && !GuestRange{1, 0}.ValidOrEmpty(),
+        "nonzero-address empty guest range is rejected");
+  Check(!GuestRange{TRACKER_ADDRESS_SIZE, 1}.Valid(),
+        "first address beyond the guest range is rejected");
+  Check(!GuestRange{TRACKER_ADDRESS_SIZE - 1, 2}.Valid(),
+        "guest range crossing the address-space end is rejected");
+  Check(!GuestRange{UINT64_MAX, 2}.Valid(), "wrapping guest range is rejected");
+}
+
 void TestQueriesDoNotRequireMappedOwnership() {
   constexpr uint64_t address = 0x0000000203000000ull;
   TrackerHarness harness;
@@ -829,6 +852,7 @@ int main(int argc, char **argv) {
   if (argc == 3 && std::strcmp(argv[1], "--death") == 0) {
     RunDeathCase(argv[2]);
   }
+  TestGuestRange();
   TestRangeSet();
   TestQueriesDoNotRequireMappedOwnership();
   TestConcurrentRegionPublication();