From af0c6a7fe96f6dbfdd59eae1174081a4bffd2679 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Tue, 24 Oct 2023 15:30:12 -0700 Subject: [PATCH] OpenGLBlobCache: be more robust when shader fails to compile - don't call BlobCache if link status false - don't assume glGetProgramiv never fails - don't assume malloc never fails FIXES=[307549547] --- .../backend/src/opengl/OpenGLBlobCache.cpp | 25 +++++++++++-------- .../src/opengl/ShaderCompilerService.cpp | 4 +-- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/filament/backend/src/opengl/OpenGLBlobCache.cpp b/filament/backend/src/opengl/OpenGLBlobCache.cpp index 48155dc23f3..4f65c605867 100644 --- a/filament/backend/src/opengl/OpenGLBlobCache.cpp +++ b/filament/backend/src/opengl/OpenGLBlobCache.cpp @@ -90,18 +90,21 @@ void OpenGLBlobCache::insert(Platform& platform, if (platform.hasBlobFunc()) { SYSTRACE_CONTEXT(); GLenum format; - GLint programBinarySize; + GLint programBinarySize = 0; SYSTRACE_NAME("glGetProgramiv"); glGetProgramiv(program, GL_PROGRAM_BINARY_LENGTH, &programBinarySize); if (programBinarySize) { size_t const size = sizeof(Blob) + programBinarySize; std::unique_ptr blob{ (Blob*)malloc(size), &::free }; - SYSTRACE_NAME("glGetProgramBinary"); - glGetProgramBinary(program, programBinarySize, &programBinarySize, &format, blob->data); - GLenum const error = glGetError(); - if (error == GL_NO_ERROR) { - blob->format = format; - platform.insertBlob(key.data(), key.size(), blob.get(), size); + if (UTILS_LIKELY(blob)) { + SYSTRACE_NAME("glGetProgramBinary"); + glGetProgramBinary(program, programBinarySize, &programBinarySize, &format, + blob->data); + GLenum const error = glGetError(); + if (error == GL_NO_ERROR) { + blob->format = format; + platform.insertBlob(key.data(), key.size(), blob.get(), size); + } } } } @@ -115,9 +118,11 @@ void OpenGLBlobCache::insert(Platform& platform, BlobCacheKey const& key, if (programBinarySize) { size_t const size = sizeof(Blob) + programBinarySize; std::unique_ptr blob{ (Blob*)malloc(size), &::free }; - blob->format = format; - memcpy(blob->data, data, programBinarySize); - platform.insertBlob(key.data(), key.size(), blob.get(), size); + if (UTILS_LIKELY(blob)) { + blob->format = format; + memcpy(blob->data, data, programBinarySize); + platform.insertBlob(key.data(), key.size(), blob.get(), size); + } } } } diff --git a/filament/backend/src/opengl/ShaderCompilerService.cpp b/filament/backend/src/opengl/ShaderCompilerService.cpp index ad6914d2f6f..4d49cca9bfe 100644 --- a/filament/backend/src/opengl/ShaderCompilerService.cpp +++ b/filament/backend/src/opengl/ShaderCompilerService.cpp @@ -249,7 +249,7 @@ ShaderCompilerService::program_token_t ShaderCompilerService::createProgram( // We need to query the link status here to guarantee that the // program is compiled and linked now (we don't want this to be // deferred to later). We don't care about the result at this point. - GLint status; + GLint status = GL_FALSE; glGetProgramiv(glProgram, GL_LINK_STATUS, &status); programData.program = glProgram; @@ -262,7 +262,7 @@ ShaderCompilerService::program_token_t ShaderCompilerService::createProgram( mCallbackManager.put(token->handle); // caching must be the last thing we do - if (token->key) { + if (token->key && status == GL_TRUE) { // Attempt to cache. This calls glGetProgramBinary. OpenGLBlobCache::insert(mDriver.mPlatform, token->key, glProgram); }