Hotfix/crash invalid vk binding (#875)

* Add diagnostic for vk::binding failure.

* Add test for vk::binding failure.

* Add the expected output for glsl-layout-define.hlsl

* * Copy over initialize expr if available when validating unchecked
* Fix unloop - because now it always has one parameter (when before it could have none)
* Split vk::binding and layout tests with invalid parameters
* Removed the diagnostic for 2 ints expected
* Added vk::binding that doesn't specify set in vk-bindings.slang

* * Fix typo
* Improve comments.

3 files changed, 38 insertions, 5 deletions

diff --git a/source/slang/check.cpp b/source/slang/check.cpp
index 998324612..37c0d5baf 100644
--- a/source/slang/check.cpp
+++ b/source/slang/check.cpp
@@ -2603,10 +2603,24 @@ namespace Slang
                 }
                 else if (auto bindingAttr = as<GLSLBindingAttribute>(attr))
                 {
-                    SLANG_ASSERT(attr->args.Count() == 2);
+                    // This must be vk::binding or gl::binding (as specified in core.meta.slang under vk_binding/gl_binding)
+                    // Must have 2 int parameters. Ideally this would all be checked from the specification
+                    // in core.meta.slang, but that's not completely implemented. So for now we check here.
+                    if (attr->args.Count() != 2)
+                    {
+                        return false;
+                    }
+
+                    // TODO(JS): Prior validation currently doesn't ensure both args are ints (as specified in core.meta.slang), so check here
+                    // to make sure they both are
                     auto binding = checkConstantIntVal(attr->args[0]);
                     auto set = checkConstantIntVal(attr->args[1]);
 
+                    if (binding == nullptr || set == nullptr)
+                    {
+                        return false;
+                    }
+                    
                     bindingAttr->binding = int32_t(binding->value);
                     bindingAttr->set = int32_t(set->value);
                 }
@@ -2703,6 +2717,13 @@ namespace Slang
                         return false;
                     }
                 }
+                else if (auto unrollAttr = as<UnrollAttribute>(attr))
+                {
+                    // Check has an argument. We need this because default behavior is to give an error
+                    // if an attribute has arguments, but not handled explicitly (and the default param will come through
+                    // as 1 arg if nothing is specified)
+                    SLANG_ASSERT(attr->args.Count() == 1);
+                }
                 else if (auto userDefAttr = as<UserDefinedAttribute>(attr))
                 {
                     // check arguments against attribute parameters defined in attribClassDecl
@@ -2813,7 +2834,6 @@ namespace Slang
                 {
                     // TODO: support checking the argument against the declared
                     // type for the parameter.
-                    
                 }
                 else
                 {
@@ -2826,6 +2846,9 @@ namespace Slang
                         //
                         // TODO: we need to figure out how to hook up
                         // default arguments as needed.
+                        // For now just copy the expression over.
+
+                        attr->args.Add(paramDecl->initExpr);
                     }
                     else
                     {
diff --git a/source/slang/diagnostic-defs.h b/source/slang/diagnostic-defs.h
index a00c69dba..c773cc4ed 100644
--- a/source/slang/diagnostic-defs.h
+++ b/source/slang/diagnostic-defs.h
@@ -259,7 +259,6 @@ DIAGNOSTIC(31005, Error, expectedSingleStringArg, "attribute '$0' expects a sing
 
 DIAGNOSTIC(31006, Error, attributeFunctionNotFound, "Could not find function '$0' for attribute'$1'")
 
-
 DIAGNOSTIC(31100, Error, unknownStageName, "unknown stage name '$0'")
 DIAGNOSTIC(31120, Error, invalidAttributeTarget, "invalid syntax target for user defined attribute")
 
diff --git a/source/slang/parser.cpp b/source/slang/parser.cpp
index 72e682a63..a7ccb7b8a 100644
--- a/source/slang/parser.cpp
+++ b/source/slang/parser.cpp
@@ -91,7 +91,7 @@ namespace Slang
         RefPtr<Scope> currentScope;
 
         TokenReader tokenReader;
-        DiagnosticSink * sink;
+        DiagnosticSink* sink;
         int genericDepth = 0;
 
         // Have we seen any `import` declarations? If so, we need
@@ -819,8 +819,12 @@ namespace Slang
         auto keywordToken = advanceToken(parser);
 
         RefPtr<RefObject> parsedObject = syntaxDecl->parseCallback(parser, syntaxDecl->parseUserData);
-        auto syntax = as<T>(parsedObject);
+        if (!parsedObject)
+        {
+            return false;
+        }
 
+        auto syntax = as<T>(parsedObject);
         if (syntax)
         {
             if (!syntax->loc.isValid())
@@ -4440,7 +4444,14 @@ namespace Slang
 
                 parser->ReadToken(TokenType::OpAssign);
 
+                // If the token asked for is not returned found will put in recovering state, and return token found
                 Token valToken = parser->ReadToken(TokenType::IntegerLiteral);
+                // If wasn't the desired IntegerLiteral return that couldn't parse
+                if (valToken.type != TokenType::IntegerLiteral)
+                {
+                    return nullptr;
+                }
+
                 // Work out the value
                 auto value = getIntegerLiteralValue(valToken);