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]
google · Oct 24, 2023 · af0c6a7 · af0c6a7
1 parent 7b7dfad
commit af0c6a7
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 12 deletions.
diff --git 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, decltype(&::free)> 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, decltype(&::free)> 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
@@ -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);
                     }