Correctly insert unmangled function names to symbol table

This fixes detecting some cases of function parameter mismatch
(previously a mangled function name corresponding to no function
parameters was added to the symbol table for each user-defined function,
and this was returned when doing function lookups with no parameters).

Also fixes detection of reusing a function name as a variable/struct
name.

New unit tests are added to ensure that these fixes don't regress.

BUG=angleproject:936
TEST=angle_unittests, WebGL conformance tests

Change-Id: I2dadde9dcc01c7a4a653c1982c36377b89e6d437
Reviewed-on: https://chromium-review.googlesource.com/260800
Tested-by: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
diff --git a/src/compiler/translator/SymbolTable.cpp b/src/compiler/translator/SymbolTable.cpp
index 4b2c19f..0eb663f 100644
--- a/src/compiler/translator/SymbolTable.cpp
+++ b/src/compiler/translator/SymbolTable.cpp
@@ -48,6 +48,16 @@
     return result.second;
 }
 
+bool TSymbolTableLevel::insertUnmangled(TFunction *function)
+{
+    function->setUniqueId(TSymbolTable::nextUniqueId());
+
+    // returning true means symbol was added to the table
+    tInsertResult result = level.insert(tLevelPair(function->getName(), function));
+
+    return result.second;
+}
+
 TSymbol *TSymbolTableLevel::find(const TString &name) const
 {
     tLevel::const_iterator it = level.find(name);
diff --git a/src/compiler/translator/SymbolTable.h b/src/compiler/translator/SymbolTable.h
index 8e2dfd7..397e4d6 100644
--- a/src/compiler/translator/SymbolTable.h
+++ b/src/compiler/translator/SymbolTable.h
@@ -288,6 +288,9 @@
 
     bool insert(TSymbol *symbol);
 
+    // Insert a function using its unmangled name as the key.
+    bool insertUnmangled(TFunction *function);
+
     TSymbol *find(const TString &name) const;
 
   protected:
diff --git a/src/compiler/translator/glslang.y b/src/compiler/translator/glslang.y
index ccfa08b..47143c2 100644
--- a/src/compiler/translator/glslang.y
+++ b/src/compiler/translator/glslang.y
@@ -710,7 +710,7 @@
         {
             // Insert the unmangled name to detect potential future redefinition as a variable.
             TFunction *function = new TFunction(NewPoolTString($1->getName().c_str()), $1->getReturnType());
-            context->symbolTable.getOuterLevel()->insert(function);
+            context->symbolTable.getOuterLevel()->insertUnmangled(function);
         }
 
         //
diff --git a/src/compiler/translator/glslang_tab.cpp b/src/compiler/translator/glslang_tab.cpp
index e5ab8e1..6ef67cf 100644
--- a/src/compiler/translator/glslang_tab.cpp
+++ b/src/compiler/translator/glslang_tab.cpp
@@ -3168,7 +3168,7 @@
         {
             // Insert the unmangled name to detect potential future redefinition as a variable.
             TFunction *function = new TFunction(NewPoolTString((yyvsp[-1].interm.function)->getName().c_str()), (yyvsp[-1].interm.function)->getReturnType());
-            context->symbolTable.getOuterLevel()->insert(function);
+            context->symbolTable.getOuterLevel()->insertUnmangled(function);
         }
 
         //
diff --git a/src/tests/angle_unittests.gypi b/src/tests/angle_unittests.gypi
index d86044f..3e51dc3 100644
--- a/src/tests/angle_unittests.gypi
+++ b/src/tests/angle_unittests.gypi
@@ -25,6 +25,7 @@
             '<(angle_path)/src/tests/compiler_tests/ConstantFolding_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/DebugShaderPrecision_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/ExpressionLimit_test.cpp',
+            '<(angle_path)/src/tests/compiler_tests/MalformedShader_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/ShaderExtension_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/NV_draw_buffers_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/ShaderVariable_test.cpp',
diff --git a/src/tests/compiler_tests/MalformedShader_test.cpp b/src/tests/compiler_tests/MalformedShader_test.cpp
new file mode 100644
index 0000000..0db375e
--- /dev/null
+++ b/src/tests/compiler_tests/MalformedShader_test.cpp
@@ -0,0 +1,123 @@
+//
+// Copyright (c) 2015 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// MalformedShader_test.cpp:
+//   Tests that malformed shaders fail compilation.
+//
+
+#include "angle_gl.h"
+#include "gtest/gtest.h"
+#include "GLSLANG/ShaderLang.h"
+#include "compiler/translator/TranslatorESSL.h"
+
+class MalformedShaderTest : public testing::Test
+{
+  public:
+    MalformedShaderTest() {}
+
+  protected:
+    virtual void SetUp()
+    {
+        ShBuiltInResources resources;
+        ShInitBuiltInResources(&resources);
+
+        mTranslator = new TranslatorESSL(GL_FRAGMENT_SHADER, SH_GLES3_SPEC);
+        ASSERT_TRUE(mTranslator->Init(resources));
+    }
+
+    virtual void TearDown()
+    {
+        delete mTranslator;
+    }
+
+    // Return true when compilation succeeds
+    bool compile(const std::string& shaderString)
+    {
+        const char *shaderStrings[] = { shaderString.c_str() };
+        bool compilationSuccess = mTranslator->compile(shaderStrings, 1, SH_INTERMEDIATE_TREE);
+        TInfoSink &infoSink = mTranslator->getInfoSink();
+        mInfoLog = infoSink.info.c_str();
+        return compilationSuccess;
+    }
+
+  protected:
+    std::string mInfoLog;
+
+  private:
+    TranslatorESSL *mTranslator;
+};
+
+// This is a test for a bug that used to exist in ANGLE:
+// Calling a function with all parameters missing should not succeed.
+TEST_F(MalformedShaderTest, FunctionParameterMismatch)
+{
+    const std::string &shaderString =
+        "precision mediump float;\n"
+        "float fun(float a) {\n"
+        "   return a * 2.0;\n"
+        "}\n"
+        "void main() {\n"
+        "   float ff = fun();\n"
+        "   gl_FragColor = vec4(ff);\n"
+        "}\n";
+    if (compile(shaderString))
+    {
+      FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+    }
+};
+
+// Functions can't be redeclared as variables in the same scope (ESSL 1.00 section 4.2.7)
+TEST_F(MalformedShaderTest, RedeclaringFunctionAsVariable)
+{
+    const std::string &shaderString =
+        "precision mediump float;\n"
+        "float fun(float a) {\n"
+        "   return a * 2.0;\n"
+        "}\n"
+        "float fun;\n"
+        "void main() {\n"
+        "   gl_FragColor = vec4(0.0);\n"
+        "}\n";
+    if (compile(shaderString))
+    {
+      FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+    }
+};
+
+// Functions can't be redeclared as structs in the same scope (ESSL 1.00 section 4.2.7)
+TEST_F(MalformedShaderTest, RedeclaringFunctionAsStruct)
+{
+    const std::string &shaderString =
+        "precision mediump float;\n"
+        "float fun(float a) {\n"
+        "   return a * 2.0;\n"
+        "}\n"
+        "struct fun { float a; };\n"
+        "void main() {\n"
+        "   gl_FragColor = vec4(0.0);\n"
+        "}\n";
+    if (compile(shaderString))
+    {
+      FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+    }
+};
+
+// Functions can't be redeclared with different qualifiers (ESSL 1.00 section 6.1.0)
+TEST_F(MalformedShaderTest, RedeclaringFunctionWithDifferentQualifiers)
+{
+    const std::string &shaderString =
+        "precision mediump float;\n"
+        "float fun(out float a);\n"
+        "float fun(float a) {\n"
+        "   return a * 2.0;\n"
+        "}\n"
+        "void main() {\n"
+        "   gl_FragColor = vec4(0.0);\n"
+        "}\n";
+    if (compile(shaderString))
+    {
+      FAIL() << "Shader compilation succeeded, expecting failure " << mInfoLog;
+    }
+};