command_buffer: Avoid uninitialized read in ShaderTranslatorCache

The key in ShaderTranslatorCache std::map is copied with memcpy and
compared with memcmp. However, the key is a class that does not
guarantee not having any padding.

Padding in ShBuiltInResources causes the comparison be invalid. Valgrind
reports uninitialized reads for this.

Fix by preserving usage of memcmp. Initialize the object
with memset. This should write any possible padding data to a
well-defined state.

Depends on an ANGLE fix where ShBuiltInResources is similarly
initialized with memset. The resource data is copied to the key
implicitly, and this may or may not copy the padding.

Review URL: https://codereview.chromium.org/654253002

Cr-Commit-Position: refs/heads/master@{#303817}

gpu/BUILD.gn

@@ -204,6 +204,7 @@ test("gpu_unittests") {
     "command_buffer/service/program_cache_unittest.cc",
     "command_buffer/service/shader_manager_unittest.cc",
     "command_buffer/service/shader_translator_unittest.cc",
+    "command_buffer/service/shader_translator_cache_unittest.cc",
     "command_buffer/service/test_helper.cc",
     "command_buffer/service/test_helper.h",
     "command_buffer/service/texture_manager_unittest.cc",

gpu/command_buffer/service/shader_translator_cache.h

@@ -41,6 +41,7 @@ class GPU_EXPORT ShaderTranslatorCache
 
  private:
   friend class base::RefCounted<ShaderTranslatorCache>;
+  friend class ShaderTranslatorCacheTest_InitParamComparable_Test;
   ~ShaderTranslatorCache() override;
 
   // Parameters passed into ShaderTranslator::Init
@@ -52,18 +53,18 @@ class GPU_EXPORT ShaderTranslatorCache
         glsl_implementation_type;
     ShCompileOptions driver_bug_workarounds;
 
-    ShaderTranslatorInitParams(
-        sh::GLenum shader_type,
-        ShShaderSpec shader_spec,
-        const ShBuiltInResources& resources,
-        ShaderTranslatorInterface::GlslImplementationType
-            glsl_implementation_type,
-        ShCompileOptions driver_bug_workarounds)
-      : shader_type(shader_type),
-        shader_spec(shader_spec),
-        resources(resources),
-        glsl_implementation_type(glsl_implementation_type),
-        driver_bug_workarounds(driver_bug_workarounds) {
+    ShaderTranslatorInitParams(sh::GLenum shader_type,
+                               ShShaderSpec shader_spec,
+                               const ShBuiltInResources& resources,
+                               ShaderTranslatorInterface::GlslImplementationType
+                                   glsl_implementation_type,
+                               ShCompileOptions driver_bug_workarounds) {
+      memset(this, 0, sizeof(*this));
+      this->shader_type = shader_type;
+      this->shader_spec = shader_spec;
+      this->resources = resources;
+      this->glsl_implementation_type = glsl_implementation_type;
+      this->driver_bug_workarounds = driver_bug_workarounds;
     }
 
     ShaderTranslatorInitParams(const ShaderTranslatorInitParams& params) {
@@ -77,6 +78,10 @@ class GPU_EXPORT ShaderTranslatorCache
     bool operator< (const ShaderTranslatorInitParams& params) const {
       return memcmp(&params, this, sizeof(*this)) < 0;
     }
+
+   private:
+    ShaderTranslatorInitParams();
+    ShaderTranslatorInitParams& operator=(const ShaderTranslatorInitParams&);
   };
 
   typedef std::map<ShaderTranslatorInitParams, ShaderTranslator* > Cache;

gpu/command_buffer/service/shader_translator_cache_unittest.cc

@@ -0,0 +1,56 @@
+// Copyright (c) 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <GLES2/gl2.h>
+
+#include "gpu/command_buffer/service/shader_translator_cache.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace gpu {
+namespace gles2 {
+
+TEST(ShaderTranslatorCacheTest, InitParamComparable) {
+  // Tests that ShaderTranslatorInitParams padding or padding of its
+  // members does not affect the object equality or ordering.
+
+  ShBuiltInResources a_resources;
+  memset(&a_resources, 88, sizeof(a_resources));
+  ShInitBuiltInResources(&a_resources);
+
+  ShBuiltInResources b_resources;
+  memset(&b_resources, 77, sizeof(b_resources));
+  ShInitBuiltInResources(&b_resources);
+
+  EXPECT_TRUE(memcmp(&a_resources, &b_resources, sizeof(a_resources)) == 0);
+
+  ShCompileOptions driver_bug_workarounds = SH_VALIDATE;
+
+  char a_storage[sizeof(ShaderTranslatorCache::ShaderTranslatorInitParams)];
+  memset(a_storage, 55, sizeof(a_storage));
+  ShaderTranslatorCache::ShaderTranslatorInitParams* a =
+      new (&a_storage) ShaderTranslatorCache::ShaderTranslatorInitParams(
+          GL_VERTEX_SHADER,
+          SH_GLES2_SPEC,
+          a_resources,
+          ShaderTranslatorInterface::kGlslES,
+          driver_bug_workarounds);
+
+  ShaderTranslatorCache::ShaderTranslatorInitParams b(
+      GL_VERTEX_SHADER,
+      SH_GLES2_SPEC,
+      b_resources,
+      ShaderTranslatorInterface::kGlslES,
+      driver_bug_workarounds);
+
+  EXPECT_TRUE(*a == b);
+  EXPECT_FALSE(*a < b || b < *a);
+
+  memset(a_storage, 55, sizeof(a_storage));
+  a = new (&a_storage) ShaderTranslatorCache::ShaderTranslatorInitParams(b);
+
+  EXPECT_TRUE(*a == b);
+  EXPECT_FALSE(*a < b || b < *a);
+}
+}  // namespace gles2
+}  // namespace gpu

gpu/gpu.gyp

@@ -249,6 +249,7 @@
         'command_buffer/service/program_cache_unittest.cc',
         'command_buffer/service/shader_manager_unittest.cc',
         'command_buffer/service/shader_translator_unittest.cc',
+        'command_buffer/service/shader_translator_cache_unittest.cc',
         'command_buffer/service/test_helper.cc',
         'command_buffer/service/test_helper.h',
         'command_buffer/service/texture_manager_unittest.cc',