Commit 07c03b6d by Nicolas Capens Committed by Commit Bot

Validate GLSL attribute location range

The location index of a vertex shader input's layout qualifier must be less than GL_MAX_VERTEX_ATTRIBS to link successfully. We can check for this during shader compilation to not rely on other layers to handle pathological cases. While strictly speaking only 'active' attributes are considered during shader linking, this is merely intended to allow for 'uber' shaders to declare more inputs than GL_MAX_VERTEX_ATTRIBS but only use a subset of them. There is no known reasonable use case for a manually specified location to exceed the valid range. Note that according to http://opengl.gpuinfo.org, the highest GL_MAX_VERTEX_ATTRIBS value at the time of writing is 32. Also, D3D12_VS_INPUT_REGISTER_COUNT = 32. Hence the unit test's value of 1000 should be sufficiently future proof. Also address the case where the uniform location might be close to INT_MAX and not be detected as out-of-range due to numeric overflow. Bug: chromium:1110800 Change-Id: I9985c8eab3bb8a2a59b8f985e8f5b6884756383c Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2405368Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org> Commit-Queue: Nicolas Capens <capn@chromium.org>
parent 23cff4e1
...@@ -210,6 +210,7 @@ TParseContext::TParseContext(TSymbolTable &symt, ...@@ -210,6 +210,7 @@ TParseContext::TParseContext(TSymbolTable &symt,
mMaxCombinedTextureImageUnits(resources.MaxCombinedTextureImageUnits), mMaxCombinedTextureImageUnits(resources.MaxCombinedTextureImageUnits),
mMaxUniformLocations(resources.MaxUniformLocations), mMaxUniformLocations(resources.MaxUniformLocations),
mMaxUniformBufferBindings(resources.MaxUniformBufferBindings), mMaxUniformBufferBindings(resources.MaxUniformBufferBindings),
mMaxVertexAttribs(resources.MaxVertexAttribs),
mMaxAtomicCounterBindings(resources.MaxAtomicCounterBindings), mMaxAtomicCounterBindings(resources.MaxAtomicCounterBindings),
mMaxShaderStorageBufferBindings(resources.MaxShaderStorageBufferBindings), mMaxShaderStorageBufferBindings(resources.MaxShaderStorageBufferBindings),
mDeclaringFunction(false), mDeclaringFunction(false),
...@@ -1541,6 +1542,18 @@ void TParseContext::nonEmptyDeclarationErrorCheck(const TPublicType &publicType, ...@@ -1541,6 +1542,18 @@ void TParseContext::nonEmptyDeclarationErrorCheck(const TPublicType &publicType,
} }
} }
if (mShaderVersion >= 300 && publicType.qualifier == EvqVertexIn)
{
// Valid vertex input declarations can't be unsized arrays since they can't be initialized.
// But invalid shaders may still reach here with an unsized array declaration.
TType type(publicType);
if (!type.isUnsizedArray())
{
checkAttributeLocationInRange(identifierLocation, type.getLocationCount(),
publicType.layoutQualifier);
}
}
// check for layout qualifier issues // check for layout qualifier issues
const TLayoutQualifier layoutQualifier = publicType.layoutQualifier; const TLayoutQualifier layoutQualifier = publicType.layoutQualifier;
...@@ -1788,9 +1801,30 @@ void TParseContext::checkUniformLocationInRange(const TSourceLoc &location, ...@@ -1788,9 +1801,30 @@ void TParseContext::checkUniformLocationInRange(const TSourceLoc &location,
const TLayoutQualifier &layoutQualifier) const TLayoutQualifier &layoutQualifier)
{ {
int loc = layoutQualifier.location; int loc = layoutQualifier.location;
if (loc >= 0 && loc + objectLocationCount > mMaxUniformLocations) if (loc >= 0) // Shader-specified location
{
if (loc >= mMaxUniformLocations || objectLocationCount > mMaxUniformLocations ||
static_cast<unsigned int>(loc) + static_cast<unsigned int>(objectLocationCount) >
static_cast<unsigned int>(mMaxUniformLocations))
{
error(location, "Uniform location out of range", "location");
}
}
}
void TParseContext::checkAttributeLocationInRange(const TSourceLoc &location,
int objectLocationCount,
const TLayoutQualifier &layoutQualifier)
{
int loc = layoutQualifier.location;
if (loc >= 0) // Shader-specified location
{ {
error(location, "Uniform location out of range", "location"); if (loc >= mMaxVertexAttribs || objectLocationCount > mMaxVertexAttribs ||
static_cast<unsigned int>(loc) + static_cast<unsigned int>(objectLocationCount) >
static_cast<unsigned int>(mMaxVertexAttribs))
{
error(location, "Attribute location out of range", "location");
}
} }
} }
......
...@@ -545,6 +545,9 @@ class TParseContext : angle::NonCopyable ...@@ -545,6 +545,9 @@ class TParseContext : angle::NonCopyable
void checkUniformLocationInRange(const TSourceLoc &location, void checkUniformLocationInRange(const TSourceLoc &location,
int objectLocationCount, int objectLocationCount,
const TLayoutQualifier &layoutQualifier); const TLayoutQualifier &layoutQualifier);
void checkAttributeLocationInRange(const TSourceLoc &location,
int objectLocationCount,
const TLayoutQualifier &layoutQualifier);
void checkYuvIsNotSpecified(const TSourceLoc &location, bool yuv); void checkYuvIsNotSpecified(const TSourceLoc &location, bool yuv);
...@@ -653,6 +656,7 @@ class TParseContext : angle::NonCopyable ...@@ -653,6 +656,7 @@ class TParseContext : angle::NonCopyable
int mMaxCombinedTextureImageUnits; int mMaxCombinedTextureImageUnits;
int mMaxUniformLocations; int mMaxUniformLocations;
int mMaxUniformBufferBindings; int mMaxUniformBufferBindings;
int mMaxVertexAttribs;
int mMaxAtomicCounterBindings; int mMaxAtomicCounterBindings;
int mMaxShaderStorageBufferBindings; int mMaxShaderStorageBufferBindings;
......
...@@ -5164,6 +5164,26 @@ TEST_F(VertexShaderValidationTest, LocationConflictsOnStructElement) ...@@ -5164,6 +5164,26 @@ TEST_F(VertexShaderValidationTest, LocationConflictsOnStructElement)
} }
} }
// Test that declaring inputs of a vertex shader with a location larger than GL_MAX_VERTEX_ATTRIBS
// causes a compile error.
TEST_F(VertexShaderValidationTest, AttributeLocationOutOfRange)
{
// Assumes 1000 >= GL_MAX_VERTEX_ATTRIBS.
// Current OpenGL and Direct3D implementations support up to 32.
const std::string &shaderString =
"#version 300 es\n"
"layout (location = 1000) in float i_value;\n"
"void main()\n"
"{\n"
"}\n";
if (compile(shaderString))
{
FAIL() << "Shader compilation succeeded, expecting failure:\n" << mInfoLog;
}
}
// Test that a block can follow the final case in a switch statement. // Test that a block can follow the final case in a switch statement.
// GLSL ES 3.00.5 section 6 and the grammar suggest that an empty block is a statement. // GLSL ES 3.00.5 section 6 and the grammar suggest that an empty block is a statement.
TEST_F(FragmentShaderValidationTest, SwitchFinalCaseHasEmptyBlock) TEST_F(FragmentShaderValidationTest, SwitchFinalCaseHasEmptyBlock)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment