commit 722bfb51d59188ee086839b69e00296b2d5d2da3
author Olli Etuaho <oetuaho@nvidia.com> Thu Oct 26 17:00:11 2017 +0300
committer Commit Bot <commit-bot@chromium.org> Tue Oct 31 07:31:35 2017 +0000
tree 66cc46bf773ab18e81bf568248185880af4c6147
parent 9088557fe47e0ce1487b248b60acbc740acd7801

Fix detecting duplicate field names in structures

Previously field names that were listed in the first declarator list
inside a struct declaration were not checked against each other.

BUG=angleproject:2204
TEST=angle_unittests

Change-Id: Ibf821d45556f6dfe0223dae673644f6795daf4cb
Reviewed-on: https://chromium-review.googlesource.com/739825
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>

src/compiler/translator/ParseContext.cpp

diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 3f0ce5a..15a9cd6 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -4573,19 +4573,39 @@
     return new TField(type, identifier, loc);
 }
 
+void TParseContext::checkDoesNotHaveDuplicateFieldName(const TFieldList::const_iterator begin,
+                                                       const TFieldList::const_iterator end,
+                                                       const TString &name,
+                                                       const TSourceLoc &location)
+{
+    for (auto fieldIter = begin; fieldIter != end; ++fieldIter)
+    {
+        if ((*fieldIter)->name() == name)
+        {
+            error(location, "duplicate field name in structure", name.c_str());
+        }
+    }
+}
+
+TFieldList *TParseContext::addStructFieldList(TFieldList *fields, const TSourceLoc &location)
+{
+    for (TFieldList::const_iterator fieldIter = fields->begin(); fieldIter != fields->end();
+         ++fieldIter)
+    {
+        checkDoesNotHaveDuplicateFieldName(fields->begin(), fieldIter, (*fieldIter)->name(),
+                                           location);
+    }
+    return fields;
+}
+
 TFieldList *TParseContext::combineStructFieldLists(TFieldList *processedFields,
                                                    const TFieldList *newlyAddedFields,
                                                    const TSourceLoc &location)
 {
     for (TField *field : *newlyAddedFields)
     {
-        for (TField *oldField : *processedFields)
-        {
-            if (oldField->name() == field->name())
-            {
-                error(location, "duplicate field name in structure", field->name().c_str());
-            }
-        }
+        checkDoesNotHaveDuplicateFieldName(processedFields->begin(), processedFields->end(),
+                                           field->name(), location);
         processedFields->push_back(field);
     }
     return processedFields;

src/compiler/translator/ParseContext.h

diff --git a/src/compiler/translator/ParseContext.h b/src/compiler/translator/ParseContext.h
index f423b83..1f7f5b9 100644
--- a/src/compiler/translator/ParseContext.h
+++ b/src/compiler/translator/ParseContext.h
@@ -306,6 +306,11 @@
                                        unsigned int arraySize,
                                        const TSourceLoc &arraySizeLoc);
 
+    void checkDoesNotHaveDuplicateFieldName(const TFieldList::const_iterator begin,
+                                            const TFieldList::const_iterator end,
+                                            const TString &name,
+                                            const TSourceLoc &location);
+    TFieldList *addStructFieldList(TFieldList *fields, const TSourceLoc &location);
     TFieldList *combineStructFieldLists(TFieldList *processedFields,
                                         const TFieldList *newlyAddedFields,
                                         const TSourceLoc &location);

src/compiler/translator/glslang.y

diff --git a/src/compiler/translator/glslang.y b/src/compiler/translator/glslang.y
index 98f313b..b02710b 100644
--- a/src/compiler/translator/glslang.y
+++ b/src/compiler/translator/glslang.y
@@ -1187,7 +1187,7 @@
 
 struct_declaration_list
     : struct_declaration {
-        $$ = $1;
+        $$ = context->addStructFieldList($1, @1);
     }
     | struct_declaration_list struct_declaration {
         $$ = context->combineStructFieldLists($1, $2, @2);

src/compiler/translator/glslang_tab.cpp

diff --git a/src/compiler/translator/glslang_tab.cpp b/src/compiler/translator/glslang_tab.cpp
index ea56b37..64fac5a 100644
--- a/src/compiler/translator/glslang_tab.cpp
+++ b/src/compiler/translator/glslang_tab.cpp
@@ -4358,7 +4358,7 @@
   case 229:
 
     {
-        (yyval.interm.fieldList) = (yyvsp[0].interm.fieldList);
+        (yyval.interm.fieldList) = context->addStructFieldList((yyvsp[0].interm.fieldList), (yylsp[0]));
     }
 
     break;

src/tests/compiler_tests/ShaderValidation_test.cpp

diff --git a/src/tests/compiler_tests/ShaderValidation_test.cpp b/src/tests/compiler_tests/ShaderValidation_test.cpp
index 61a6945..51b793b 100644
--- a/src/tests/compiler_tests/ShaderValidation_test.cpp
+++ b/src/tests/compiler_tests/ShaderValidation_test.cpp
@@ -5172,4 +5172,25 @@
     {
         FAIL() << "Shader compilation succeeded, expecting failure:\n" << mInfoLog;
     }
-}
\ No newline at end of file
+}
+
+// Test that duplicate field names in a struct declarator list are validated.
+TEST_F(FragmentShaderValidationTest, DuplicateFieldNamesInStructDeclaratorList)
+{
+    const std::string &shaderString =
+        R"(precision mediump float;
+
+        struct S {
+            float f, f;
+        };
+
+        void main()
+        {
+            gl_FragColor = vec4(1.0);
+        })";
+
+    if (compile(shaderString))
+    {
+        FAIL() << "Shader compilation succeeded, expecting failure:\n" << mInfoLog;
+    }
+}