From 9a3dfd107c89a6ffd4ed2f2ace339b9cefcc661e Mon Sep 17 00:00:00 2001 From: baldurk Date: Mon, 9 Jan 2017 11:19:52 +0000 Subject: [PATCH] Implement Map() pointer verification for OpenGL --- renderdoc/driver/d3d11/d3d11_context_wrap.cpp | 11 ++-- renderdoc/driver/gl/gl_resources.cpp | 4 ++ renderdoc/driver/gl/gl_resources.h | 24 ++++++++- .../driver/gl/wrappers/gl_buffer_funcs.cpp | 52 +++++++++++++++++++ 4 files changed, 85 insertions(+), 6 deletions(-) diff --git a/renderdoc/driver/d3d11/d3d11_context_wrap.cpp b/renderdoc/driver/d3d11/d3d11_context_wrap.cpp index b95f09ea5..279084b28 100644 --- a/renderdoc/driver/d3d11/d3d11_context_wrap.cpp +++ b/renderdoc/driver/d3d11/d3d11_context_wrap.cpp @@ -24,6 +24,7 @@ ******************************************************************************/ #include "driver/d3d11/d3d11_context.h" +#include "3rdparty/tinyfiledialogs/tinyfiledialogs.h" #include "driver/d3d11/d3d11_renderstate.h" #include "driver/d3d11/d3d11_resources.h" #include "driver/dx/official/dxgi1_3.h" @@ -7687,10 +7688,12 @@ bool WrappedID3D11DeviceContext::Serialise_Unmap(ID3D11Resource *pResource, UINT { if(!record->VerifyShadowStorage(ctxMapID)) { - int res = - MessageBoxA(NULL, "Breakpoint now to see callstack,\nor click 'Yes' to debugbreak.", - "Map() overwrite detected!", MB_YESNO | MB_ICONERROR); - if(res == IDYES) + string msg = StringFormat::Fmt( + "Overwrite of %llu byte Map()'d buffer detected\n" + "Breakpoint now to see callstack,\nor click 'Yes' to debugbreak.", + record->Length); + int res = tinyfd_messageBox("Map() overwrite detected!", msg.c_str(), "yesno", "error", 1); + if(res == 1) { OS_DEBUG_BREAK(); } diff --git a/renderdoc/driver/gl/gl_resources.cpp b/renderdoc/driver/gl/gl_resources.cpp index c596e4533..bb30e8563 100644 --- a/renderdoc/driver/gl/gl_resources.cpp +++ b/renderdoc/driver/gl/gl_resources.cpp @@ -26,6 +26,10 @@ #include "gl_resources.h" #include "gl_hookset.h" +byte GLResourceRecord::markerValue[32] = { + 0xaa, 0xbb, 0xcc, 0xdd, 0x88, 0x77, 0x66, 0x55, 0x01, 0x23, 0x45, 0x67, 0x98, 0x76, 0x54, 0x32, +}; + size_t GetCompressedByteSize(GLsizei w, GLsizei h, GLsizei d, GLenum internalformat, int mip) { if(!IsCompressedFormat(internalformat)) diff --git a/renderdoc/driver/gl/gl_resources.h b/renderdoc/driver/gl/gl_resources.h index a2144dfd9..752232794 100644 --- a/renderdoc/driver/gl/gl_resources.h +++ b/renderdoc/driver/gl/gl_resources.h @@ -196,6 +196,8 @@ struct GLResourceRecord : public ResourceRecord { static const NullInitialiser NullResource = MakeNullResource; + static byte markerValue[32]; + GLResourceRecord(ResourceId id) : ResourceRecord(id, true), datatype(eGL_NONE), usage(eGL_NONE) { RDCEraseEl(ShadowPtr); @@ -218,6 +220,7 @@ struct GLResourceRecord : public ResourceRecord GLbitfield access; MapStatus status; bool invalidate; + bool verifyWrite; byte *ptr; byte *persistentPtr; @@ -271,11 +274,27 @@ struct GLResourceRecord : public ResourceRecord { if(ShadowPtr[0] == NULL) { - ShadowPtr[0] = Serialiser::AllocAlignedBuffer(size); - ShadowPtr[1] = Serialiser::AllocAlignedBuffer(size); + ShadowPtr[0] = Serialiser::AllocAlignedBuffer(size + sizeof(markerValue)); + ShadowPtr[1] = Serialiser::AllocAlignedBuffer(size + sizeof(markerValue)); + + memcpy(ShadowPtr[0] + size, markerValue, sizeof(markerValue)); + memcpy(ShadowPtr[1] + size, markerValue, sizeof(markerValue)); + + ShadowSize = size; } } + bool VerifyShadowStorage() + { + if(ShadowPtr[0] && memcmp(ShadowPtr[0] + ShadowSize, markerValue, sizeof(markerValue))) + return false; + + if(ShadowPtr[1] && memcmp(ShadowPtr[1] + ShadowSize, markerValue, sizeof(markerValue))) + return false; + + return true; + } + void FreeShadowStorage() { if(ShadowPtr[0] != NULL) @@ -289,4 +308,5 @@ struct GLResourceRecord : public ResourceRecord byte *GetShadowPtr(int p) { return ShadowPtr[p]; } private: byte *ShadowPtr[2]; + size_t ShadowSize; }; diff --git a/renderdoc/driver/gl/wrappers/gl_buffer_funcs.cpp b/renderdoc/driver/gl/wrappers/gl_buffer_funcs.cpp index a46c32cef..9990caaa6 100644 --- a/renderdoc/driver/gl/wrappers/gl_buffer_funcs.cpp +++ b/renderdoc/driver/gl/wrappers/gl_buffer_funcs.cpp @@ -24,6 +24,7 @@ ******************************************************************************/ #include "../gl_driver.h" +#include "3rdparty/tinyfiledialogs/tinyfiledialogs.h" #include "common/common.h" #include "serialise/string_utils.h" @@ -1822,6 +1823,12 @@ void *WrappedOpenGL::glMapNamedBufferRangeEXT(GLuint buffer, GLintptr offset, GL if((access & GL_MAP_PERSISTENT_BIT) || record->Map.persistentPtr) directMap = false; + bool verifyWrite = (RenderDoc::Inst().GetCaptureOptions().VerifyMapWrites != 0); + + // must also intercept to verify writes + if(verifyWrite) + directMap = false; + if(directMap) { m_HighTrafficResources.insert(record->GetResourceID()); @@ -1832,6 +1839,7 @@ void *WrappedOpenGL::glMapNamedBufferRangeEXT(GLuint buffer, GLintptr offset, GL record->Map.length = length; record->Map.access = access; record->Map.invalidate = invalidateMap; + record->Map.verifyWrite = verifyWrite; // store a list of all persistent maps, and subset of all coherent maps if(access & GL_MAP_PERSISTENT_BIT) @@ -1941,6 +1949,30 @@ void *WrappedOpenGL::glMapNamedBufferRangeEXT(GLuint buffer, GLintptr offset, GL } else if(m_State == WRITING_IDLE) { + if(verifyWrite) + { + byte *shadow = record->GetShadowPtr(0); + + GLint buflength; + m_Real.glGetNamedBufferParameterivEXT(buffer, eGL_BUFFER_SIZE, &buflength); + + // if we don't have a shadow pointer, need to allocate & initialise + if(shadow == NULL) + { + // allocate our shadow storage + record->AllocShadowStorage(buflength); + shadow = (byte *)record->GetShadowPtr(0); + } + + // if we're not invalidating, we need the existing contents + if(!invalidateMap) + memcpy(shadow, record->GetDataPtr(), buflength); + else + memset(shadow + offset, 0xcc, length); + + ptr = shadow; + } + // return buffer backing store pointer, offsetted ptr += offset; @@ -2187,6 +2219,26 @@ GLboolean WrappedOpenGL::glUnmapNamedBufferEXT(GLuint buffer) break; case GLResourceRecord::Mapped_Write: { + if(record->Map.verifyWrite) + { + if(!record->VerifyShadowStorage()) + { + string msg = StringFormat::Fmt( + "Overwrite of %llu byte Map()'d buffer detected\n" + "Breakpoint now to see callstack,\nor click 'Yes' to debugbreak.", + record->Length); + int res = + tinyfd_messageBox("Map() overwrite detected!", msg.c_str(), "yesno", "error", 1); + if(res == 1) + { + OS_DEBUG_BREAK(); + } + } + + // copy from shadow to backing store, so we're consistent + memcpy(record->GetDataPtr() + record->Map.offset, record->Map.ptr, record->Map.length); + } + if(record->Map.access & GL_MAP_FLUSH_EXPLICIT_BIT) { // do nothing, any flushes that happened were handled,