From b14e3f2ef9cdf74fddc864b79bb97584d438336e Mon Sep 17 00:00:00 2001 From: baldurk Date: Tue, 19 Feb 2019 13:53:56 +0000 Subject: [PATCH] Fix valgrind issues found running unit tests * GLContextTLSData default constructor should be initialised. * Add some missing deletes in shader reflection * Call freeaddrinfo after getaddrinfo * Don't leak if we're reserving 0 bytes in rdcstr over the top of an already empty rdcstr --- renderdoc/api/replay/basic_types.h | 2 +- renderdoc/driver/gl/gl_resources.h | 2 +- renderdoc/driver/gl/gl_shader_refl.cpp | 5 +++++ renderdoc/driver/shaders/spirv/spirv_compile.cpp | 6 +++++- renderdoc/os/posix/posix_network.cpp | 13 ++++++++++--- renderdoc/os/posix/posix_process.cpp | 7 +++++++ renderdoc/os/win32/win32_network.cpp | 7 +++++++ 7 files changed, 36 insertions(+), 6 deletions(-) diff --git a/renderdoc/api/replay/basic_types.h b/renderdoc/api/replay/basic_types.h index 73d660fc9..69b33eb10 100644 --- a/renderdoc/api/replay/basic_types.h +++ b/renderdoc/api/replay/basic_types.h @@ -327,7 +327,7 @@ public: { // if we're empty then normally reserving s==0 would do nothing, but if we need to append a null // terminator then we do actually need to allocate - if(s == 0 && capacity() == 0 && null_terminator::allocCount(0) > 0) + if(s == 0 && capacity() == 0 && elems == NULL && null_terminator::allocCount(0) > 0) { elems = allocate(null_terminator::allocCount(0)); return; diff --git a/renderdoc/driver/gl/gl_resources.h b/renderdoc/driver/gl/gl_resources.h index 744173d69..32d872dad 100644 --- a/renderdoc/driver/gl/gl_resources.h +++ b/renderdoc/driver/gl/gl_resources.h @@ -312,7 +312,7 @@ private: struct GLContextTLSData { - GLContextTLSData() {} + GLContextTLSData() : ctxPair({NULL, NULL}), ctxRecord(NULL) {} GLContextTLSData(ContextPair p, GLResourceRecord *r) : ctxPair(p), ctxRecord(r) {} ContextPair ctxPair; GLResourceRecord *ctxRecord; diff --git a/renderdoc/driver/gl/gl_shader_refl.cpp b/renderdoc/driver/gl/gl_shader_refl.cpp index b24a7a780..040fe6e9b 100644 --- a/renderdoc/driver/gl/gl_shader_refl.cpp +++ b/renderdoc/driver/gl/gl_shader_refl.cpp @@ -1625,6 +1625,8 @@ void MakeShaderReflection(GLenum shadType, GLuint sepProg, ShaderReflection &ref string name = namebuf; + delete[] namebuf; + res.name = name; rdcarray &reslist = (res.isReadOnly ? roresources : rwresources); @@ -2016,7 +2018,10 @@ void MakeShaderReflection(GLenum shadType, GLuint sepProg, ShaderReflection &ref } if(unused) + { + delete[] nm; continue; + } // VS built-in inputs if(IS_BUILTIN("gl_VertexID")) diff --git a/renderdoc/driver/shaders/spirv/spirv_compile.cpp b/renderdoc/driver/shaders/spirv/spirv_compile.cpp index e71c2543d..6b69b2fbd 100644 --- a/renderdoc/driver/shaders/spirv/spirv_compile.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_compile.cpp @@ -239,7 +239,11 @@ glslang::TShader *CompileShaderForReflection(SPIRVShaderStage stage, shader->setStrings(strs, (int)sources.size()); - if(shader->parse(&DefaultResources, 100, false, EShMsgRelaxedErrors)) + bool success = shader->parse(&DefaultResources, 100, false, EShMsgRelaxedErrors); + + delete[] strs; + + if(success) { allocatedShaders->push_back(shader); return shader; diff --git a/renderdoc/os/posix/posix_network.cpp b/renderdoc/os/posix/posix_network.cpp index b69c43693..d7e5089c0 100644 --- a/renderdoc/os/posix/posix_network.cpp +++ b/renderdoc/os/posix/posix_network.cpp @@ -424,15 +424,18 @@ Socket *CreateClientSocket(const char *host, uint16_t port, int timeoutMS) hints.ai_socktype = SOCK_STREAM; hints.ai_protocol = IPPROTO_TCP; - addrinfo *result = NULL; - getaddrinfo(host, portstr, &hints, &result); + addrinfo *addrResult = NULL; + getaddrinfo(host, portstr, &hints, &addrResult); - for(addrinfo *ptr = result; ptr != NULL; ptr = ptr->ai_next) + for(addrinfo *ptr = addrResult; ptr != NULL; ptr = ptr->ai_next) { int s = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); if(s == -1) + { + freeaddrinfo(addrResult); return NULL; + } int flags = fcntl(s, F_GETFL, 0); fcntl(s, F_SETFL, flags | O_NONBLOCK); @@ -475,9 +478,13 @@ Socket *CreateClientSocket(const char *host, uint16_t port, int timeoutMS) int nodelay = 1; setsockopt(s, IPPROTO_TCP, TCP_NODELAY, (char *)&nodelay, sizeof(nodelay)); + freeaddrinfo(addrResult); + return new Socket((ptrdiff_t)s); } + freeaddrinfo(addrResult); + RDCDEBUG("Failed to connect to %s:%d", host, port); return NULL; } diff --git a/renderdoc/os/posix/posix_process.cpp b/renderdoc/os/posix/posix_process.cpp index c8b448bbc..406fa90b9 100644 --- a/renderdoc/os/posix/posix_process.cpp +++ b/renderdoc/os/posix/posix_process.cpp @@ -974,6 +974,13 @@ TEST_CASE("Test PID Node list handling", "[osspecific]") CHECK(list1.head->next->next->next == d); CHECK(list1.head->next->next->next->next == e); CHECK(list1.head->next->next->next->next->next == f); + + delete a; + delete b; + delete c; + delete d; + delete e; + delete f; }; #endif // ENABLED(ENABLE_UNIT_TESTS) diff --git a/renderdoc/os/win32/win32_network.cpp b/renderdoc/os/win32/win32_network.cpp index 05713870d..5b4dd4139 100644 --- a/renderdoc/os/win32/win32_network.cpp +++ b/renderdoc/os/win32/win32_network.cpp @@ -394,7 +394,10 @@ Socket *CreateClientSocket(const char *host, uint16_t port, int timeoutMS) WSA_FLAG_NO_HANDLE_INHERIT | WSA_FLAG_OVERLAPPED); if(s == INVALID_SOCKET) + { + FreeAddrInfoW(addrResult); return NULL; + } u_long enable = 1; ioctlsocket(s, FIONBIO, &enable); @@ -446,9 +449,13 @@ Socket *CreateClientSocket(const char *host, uint16_t port, int timeoutMS) BOOL nodelay = TRUE; setsockopt(s, IPPROTO_TCP, TCP_NODELAY, (const char *)&nodelay, sizeof(nodelay)); + FreeAddrInfoW(addrResult); + return new Socket((ptrdiff_t)s); } + FreeAddrInfoW(addrResult); + RDCDEBUG("Failed to connect to %s:%d", host, port); return NULL; }