Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions Test/baseResults/spv.longVectorBadParamsError.comp.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
spv.longVectorBadParamsError.comp
ERROR: 0:11: 'void' : invalid element type for vector
ERROR: 0:11: 'noElemType' : expected two type parameters
ERROR: 0:11: 'noElemType' : illegal use of type 'void'
ERROR: 0:12: 'void' : invalid element type for vector
ERROR: 0:12: 'badElemType' : illegal use of type 'void'
ERROR: 0:13: '' : vector type requires exactly 1 element count
ERROR: 0:13: 'missingCount' : expected two type parameters
ERROR: 0:14: '' : vector type requires exactly 1 element count
ERROR: 0:14: 'excessCount' : expected two type parameters
ERROR: 0:16: '' : vector type missing type parameters
ERROR: 0:16: '' : vector type missing type parameters
ERROR: 0:16: '' : syntax error, unexpected IDENTIFIER, expecting COMMA or SEMICOLON
ERROR: 12 compilation errors. No code generated.


SPIR-V is not generated for failed compile or link
20 changes: 20 additions & 0 deletions Test/spv.longVectorBadParamsError.comp
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#version 450 core
#extension GL_KHR_memory_scope_semantics : enable
#extension GL_EXT_long_vector : enable
#extension GL_EXT_shader_explicit_arithmetic_types : enable
#extension GL_EXT_buffer_reference : enable
#extension GL_EXT_expect_assume : enable


void main()
{
vector<5, 5> noElemType;
vector<void, 5> badElemType;
vector<float> missingCount;
vector<float, 5, 7> excessCount;

vector() yep;
vector<void,5>() oof;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't access the original crbug (is there a way to change that?), but I don't think this will exercise the null-dereference. The vector() yep will produce a syntax error and it supresses any other diagnostics. Maybe something like this?

float a = vector<float>(0);
float b = vector<float,5,7>(0);

vector<float>() blam;
vector<float,5,7>() borf;
}
3 changes: 3 additions & 0 deletions glslang/Include/Types.h
Original file line number Diff line number Diff line change
Expand Up @@ -1567,6 +1567,9 @@ class TPublicType {
bool isTensorLayoutNV() const { return basicType == EbtTensorLayoutNV; }
bool isTensorViewNV() const { return basicType == EbtTensorViewNV; }

const TTypeParameters* getTypeParameters() const { return typeParameters; }
const TArraySizes* getArraySizes() const { return arraySizes; }
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getArraySizes does not seem to be called anywhere. Remove?


void initType(const TSourceLoc& l)
{
basicType = EbtVoid;
Expand Down
7 changes: 7 additions & 0 deletions glslang/MachineIndependent/ParseHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4668,6 +4668,10 @@ bool TParseContext::constructorError(const TSourceLoc& loc, TIntermNode* node, T
return true;
}

if (type.isLongVector() && !isValidLongVectorElseError(loc, type)) {
return true;
}

if ((op != EOpConstructStruct && size != 1 && size < type.computeNumComponents()) ||
(op == EOpConstructStruct && size < type.computeNumComponents())) {
error(loc, "not enough data provided for construction", constructorString.c_str(), "");
Expand Down Expand Up @@ -8792,6 +8796,9 @@ void TParseContext::typeParametersCheck(const TSourceLoc& loc, const TPublicType
return;
}
}
if (publicType.isLongVector() && !isValidLongVectorElseError(loc, publicType)) {
return;
}
}

bool TParseContext::vkRelaxedRemapUniformVariable(const TSourceLoc& loc, TString& identifier, const TPublicType& publicType,
Expand Down
26 changes: 26 additions & 0 deletions glslang/MachineIndependent/ParseHelper.h
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,32 @@ class TParseContext : public TParseContextBase {
virtual void setAtomicCounterBlockDefaults(TType& block) const override;
virtual void setInvariant(const TSourceLoc& loc, const char* builtin) override;

// Returns true if the given long vector type is correctly parameterized.
// Otherwise emits an error and returns false.
template<typename TTYPE>
bool isValidLongVectorElseError(const TSourceLoc& loc, const TTYPE& type) {
assert(type.isLongVector());
const TTypeParameters* typeParams = type.getTypeParameters();
if (typeParams == nullptr) {
error(loc, "vector type missing type parameters", "", "");
return false;
}
const auto basicType = typeParams->basicType;
if (!isTypeInt(basicType) && !isTypeFloat(basicType) && basicType != EbtBool) {
error(loc, "invalid element type for vector", TType::getBasicString(typeParams->basicType), "");
return false;
}
if (typeParams->arraySizes == nullptr) {
error(loc, "vector type missing element count", "", "");
return false;
}
if (typeParams->arraySizes->getNumDims() != 1) {
error(loc, "vector type requires exactly 1 element count", "", "");
return false;
}
return true;
}

public:
//
// Generally, bison productions, the scanner, and the PP need read/write access to these; just give them direct access
Expand Down
1 change: 1 addition & 0 deletions gtests/Spv.FromFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,7 @@ INSTANTIATE_TEST_SUITE_P(
"spv.longVectorNotReserved.comp",
"spv.longVectorSpecConst.comp",
"spv.longVectorOperators.comp",
"spv.longVectorBadParamsError.comp",
"spv.longVectorBuiltins.comp",
"spv.longVectorBuiltinsfp16.comp",
"spv.longVectorBuiltinsfp64.comp",
Expand Down
Loading