From bdbbc68e29ce5d1c0c377732d59943b6c8dee948 Mon Sep 17 00:00:00 2001 From: John Kessenich <cepheus@frii.com> Date: Thu, 14 Sep 2017 19:45:28 -0600 Subject: [PATCH] HLSL: Add bounds checking, shared with GLSL. Partially address #1032. --- Test/baseResults/hlsl.constantbuffer.frag.out | 6 ++--- Test/hlsl.constantbuffer.frag | 2 +- .../MachineIndependent/ParseContextBase.cpp | 25 +++++++++++++++++++ glslang/MachineIndependent/ParseHelper.cpp | 23 ----------------- glslang/MachineIndependent/ParseHelper.h | 3 ++- hlsl/hlslParseHelper.cpp | 19 ++++++-------- hlsl/hlslParseHelper.h | 1 - 7 files changed, 39 insertions(+), 40 deletions(-) diff --git a/Test/baseResults/hlsl.constantbuffer.frag.out b/Test/baseResults/hlsl.constantbuffer.frag.out index 2c7dafe82..33c672736 100644 --- a/Test/baseResults/hlsl.constantbuffer.frag.out +++ b/Test/baseResults/hlsl.constantbuffer.frag.out @@ -45,7 +45,7 @@ gl_FragCoord origin is upper left 0:24 direct index (layout( row_major std140) temp 4-element array of block{layout( row_major std140) uniform bool x, layout( row_major std140) uniform float y}) 0:24 'cb3' (layout( row_major std140) uniform 2-element array of 4-element array of block{layout( row_major std140) uniform bool x, layout( row_major std140) uniform float y}) 0:24 Constant: -0:24 2 (const int) +0:24 1 (const int) 0:24 Constant: 0:24 3 (const int) 0:24 Constant: @@ -113,7 +113,7 @@ gl_FragCoord origin is upper left 0:24 direct index (layout( row_major std140) temp 4-element array of block{layout( row_major std140) uniform bool x, layout( row_major std140) uniform float y}) 0:24 'cb3' (layout( row_major std140) uniform 2-element array of 4-element array of block{layout( row_major std140) uniform bool x, layout( row_major std140) uniform float y}) 0:24 Constant: -0:24 2 (const int) +0:24 1 (const int) 0:24 Constant: 0:24 3 (const int) 0:24 Constant: @@ -231,7 +231,7 @@ gl_FragCoord origin is upper left 54: 7(fvec4) FAdd 45 53 ReturnValue 54 56: Label - 58: 41(ptr) AccessChain 18(cb3) 21 57 20 + 58: 41(ptr) AccessChain 18(cb3) 20 57 20 59: 6(float) Load 58 60: 7(fvec4) CompositeConstruct 59 59 59 59 ReturnValue 60 diff --git a/Test/hlsl.constantbuffer.frag b/Test/hlsl.constantbuffer.frag index d7a6ef529..c2b3a00ca 100644 --- a/Test/hlsl.constantbuffer.frag +++ b/Test/hlsl.constantbuffer.frag @@ -21,6 +21,6 @@ float4 main() : SV_Target0 if (cb3[1][2].x) return cb1.x + cb2[1].y + c1; else - return cb3[2][3].y; + return cb3[1][3].y; } diff --git a/glslang/MachineIndependent/ParseContextBase.cpp b/glslang/MachineIndependent/ParseContextBase.cpp index 44fc0b47b..447c038f3 100644 --- a/glslang/MachineIndependent/ParseContextBase.cpp +++ b/glslang/MachineIndependent/ParseContextBase.cpp @@ -234,6 +234,31 @@ void TParseContextBase::trackLinkage(TSymbol& symbol) linkageSymbols.push_back(&symbol); } +// Ensure index is in bounds, correct if necessary. +// Give an error if not. +void TParseContextBase::checkIndex(const TSourceLoc& loc, const TType& type, int& index) +{ + if (index < 0) { + error(loc, "", "[", "index out of range '%d'", index); + index = 0; + } else if (type.isArray()) { + if (type.isExplicitlySizedArray() && index >= type.getOuterArraySize()) { + error(loc, "", "[", "array index out of range '%d'", index); + index = type.getOuterArraySize() - 1; + } + } else if (type.isVector()) { + if (index >= type.getVectorSize()) { + error(loc, "", "[", "vector index out of range '%d'", index); + index = type.getVectorSize() - 1; + } + } else if (type.isMatrix()) { + if (index >= type.getMatrixCols()) { + error(loc, "", "[", "matrix index out of range '%d'", index); + index = type.getMatrixCols() - 1; + } + } +} + // Make a shared symbol have a non-shared version that can be edited by the current // compile, such that editing its type will not change the shared version and will // effect all nodes already sharing it (non-shallow type), diff --git a/glslang/MachineIndependent/ParseHelper.cpp b/glslang/MachineIndependent/ParseHelper.cpp index 42a2dc027..32cc4bf88 100644 --- a/glslang/MachineIndependent/ParseHelper.cpp +++ b/glslang/MachineIndependent/ParseHelper.cpp @@ -429,29 +429,6 @@ TIntermTyped* TParseContext::handleBracketDereference(const TSourceLoc& loc, TIn return result; } -void TParseContext::checkIndex(const TSourceLoc& loc, const TType& type, int& index) -{ - if (index < 0) { - error(loc, "", "[", "index out of range '%d'", index); - index = 0; - } else if (type.isArray()) { - if (type.isExplicitlySizedArray() && index >= type.getOuterArraySize()) { - error(loc, "", "[", "array index out of range '%d'", index); - index = type.getOuterArraySize() - 1; - } - } else if (type.isVector()) { - if (index >= type.getVectorSize()) { - error(loc, "", "[", "vector index out of range '%d'", index); - index = type.getVectorSize() - 1; - } - } else if (type.isMatrix()) { - if (index >= type.getMatrixCols()) { - error(loc, "", "[", "matrix index out of range '%d'", index); - index = type.getMatrixCols() - 1; - } - } -} - // for ES 2.0 (version 100) limitations for almost all index operations except vertex-shader uniforms void TParseContext::handleIndexLimits(const TSourceLoc& /*loc*/, TIntermTyped* base, TIntermTyped* index) { diff --git a/glslang/MachineIndependent/ParseHelper.h b/glslang/MachineIndependent/ParseHelper.h index 4bdd14910..fb6d0bc2b 100644 --- a/glslang/MachineIndependent/ParseHelper.h +++ b/glslang/MachineIndependent/ParseHelper.h @@ -102,6 +102,8 @@ public: virtual void setLimits(const TBuiltInResource&) = 0; + void checkIndex(const TSourceLoc&, const TType&, int& index); + EShLanguage getLanguage() const { return language; } void setScanContext(TScanContext* c) { scanContext = c; } TScanContext* getScanContext() const { return scanContext; } @@ -283,7 +285,6 @@ public: void handlePragma(const TSourceLoc&, const TVector<TString>&) override; TIntermTyped* handleVariable(const TSourceLoc&, TSymbol* symbol, const TString* string); TIntermTyped* handleBracketDereference(const TSourceLoc&, TIntermTyped* base, TIntermTyped* index); - void checkIndex(const TSourceLoc&, const TType&, int& index); void handleIndexLimits(const TSourceLoc&, TIntermTyped* base, TIntermTyped* index); void makeEditable(TSymbol*&) override; diff --git a/hlsl/hlslParseHelper.cpp b/hlsl/hlslParseHelper.cpp index b49022bdd..1fbef6d6a 100755 --- a/hlsl/hlslParseHelper.cpp +++ b/hlsl/hlslParseHelper.cpp @@ -859,10 +859,8 @@ TIntermTyped* HlslParseContext::handleBracketDereference(const TSourceLoc& loc, bool flattened = false; int indexValue = 0; - if (index->getQualifier().storage == EvqConst) { + if (index->getQualifier().isFrontEndConstant()) indexValue = index->getAsConstantUnion()->getConstArray()[0].getIConst(); - checkIndex(loc, base->getType(), indexValue); - } variableCheck(base); if (! base->isArray() && ! base->isMatrix() && ! base->isVector()) { @@ -871,9 +869,11 @@ TIntermTyped* HlslParseContext::handleBracketDereference(const TSourceLoc& loc, base->getAsSymbolNode()->getName().c_str(), ""); else error(loc, " left of '[' is not of type array, matrix, or vector ", "expression", ""); - } else if (base->getType().getQualifier().storage == EvqConst && index->getQualifier().storage == EvqConst) + } else if (base->getType().getQualifier().storage == EvqConst && index->getQualifier().storage == EvqConst) { + // both base and index are front-end constants + checkIndex(loc, base->getType(), indexValue); return intermediate.foldDereference(base, indexValue, loc); - else { + } else { // at least one of base and index is variable... if (base->getAsSymbolNode() && wasFlattened(base)) { @@ -883,9 +883,11 @@ TIntermTyped* HlslParseContext::handleBracketDereference(const TSourceLoc& loc, result = flattenAccess(base, indexValue); flattened = (result != base); } else { - if (index->getQualifier().storage == EvqConst) { + if (index->getQualifier().isFrontEndConstant()) { if (base->getType().isImplicitlySizedArray()) updateImplicitArraySize(loc, base, indexValue); + else + checkIndex(loc, base->getType(), indexValue); result = intermediate.addIndex(EOpIndexDirect, base, index, loc); } else { result = intermediate.addIndex(EOpIndexIndirect, base, index, loc); @@ -914,11 +916,6 @@ TIntermTyped* HlslParseContext::handleBracketDereference(const TSourceLoc& loc, return result; } -void HlslParseContext::checkIndex(const TSourceLoc& /*loc*/, const TType& /*type*/, int& /*index*/) -{ - // HLSL todo: any rules for index fixups? -} - // Handle seeing a binary node with a math operation. TIntermTyped* HlslParseContext::handleBinaryMath(const TSourceLoc& loc, const char* str, TOperator op, TIntermTyped* left, TIntermTyped* right) diff --git a/hlsl/hlslParseHelper.h b/hlsl/hlslParseHelper.h index 19e462673..3c8106754 100755 --- a/hlsl/hlslParseHelper.h +++ b/hlsl/hlslParseHelper.h @@ -73,7 +73,6 @@ public: TIntermTyped* handleVariable(const TSourceLoc&, const TString* string); TIntermTyped* handleBracketDereference(const TSourceLoc&, TIntermTyped* base, TIntermTyped* index); TIntermTyped* handleBracketOperator(const TSourceLoc&, TIntermTyped* base, TIntermTyped* index); - void checkIndex(const TSourceLoc&, const TType&, int& index); TIntermTyped* handleBinaryMath(const TSourceLoc&, const char* str, TOperator op, TIntermTyped* left, TIntermTyped* right); TIntermTyped* handleUnaryMath(const TSourceLoc&, const char* str, TOperator op, TIntermTyped* childNode); -- GitLab