From 6e2295d34092d7483eb99b0dbd748aca125c34af Mon Sep 17 00:00:00 2001 From: John Kessenich Date: Wed, 24 May 2017 16:02:56 -0600 Subject: [PATCH] HLSL: Fix #902: Incorrect protection against zero arguments. --- Test/baseResults/hlsl.function.frag.out | 119 ++++++++++++++++++++++++++++++++ Test/hlsl.function.frag | 25 +++++++ gtests/Hlsl.FromFile.cpp | 1 + hlsl/hlslParseHelper.cpp | 18 +++-- 4 files changed, 157 insertions(+), 6 deletions(-) create mode 100755 Test/baseResults/hlsl.function.frag.out create mode 100644 Test/hlsl.function.frag diff --git a/Test/baseResults/hlsl.function.frag.out b/Test/baseResults/hlsl.function.frag.out new file mode 100755 index 0000000..00b882d --- /dev/null +++ b/Test/baseResults/hlsl.function.frag.out @@ -0,0 +1,119 @@ +hlsl.function.frag +ERROR: 0:24: 'fun1' : unknown variable +ERROR: 0:24: 'return' : type does not match, or is not convertible to, the function's return type +ERROR: 2 compilation errors. No code generated. + + +Shader version: 500 +gl_FragCoord origin is upper left +ERROR: node is still EOpNull! +0:2 Function Definition: fun0( ( temp 4-component vector of float) +0:2 Function Parameters: +0:? Sequence +0:3 Branch: Return with expression +0:3 Constant: +0:3 1.000000 +0:3 1.000000 +0:3 1.000000 +0:3 1.000000 +0:7 Function Definition: fun2(vf4; ( temp uint) +0:7 Function Parameters: +0:7 'col' ( in 4-component vector of float) +0:? Sequence +0:8 Branch: Return with expression +0:8 Constant: +0:8 7 (const uint) +0:12 Function Definition: fun4(u1;u1; ( temp 4-component vector of float) +0:12 Function Parameters: +0:12 'id1' ( in uint) +0:12 'id2' ( in uint) +0:? Sequence +0:13 Branch: Return with expression +0:13 Construct vec4 ( temp 4-component vector of float) +0:13 Convert uint to float ( temp float) +0:13 component-wise multiply ( temp uint) +0:13 'id1' ( in uint) +0:13 'id2' ( in uint) +0:17 Function Definition: fun1(i1; ( temp 4-component vector of float) +0:17 Function Parameters: +0:17 'index' ( in int) +0:? Sequence +0:18 Sequence +0:18 move second child to first child ( temp uint) +0:18 'entityId' ( temp uint) +0:18 Function Call: fun2(vf4; ( temp uint) +0:18 Function Call: fun0( ( temp 4-component vector of float) +0:19 Branch: Return with expression +0:19 Function Call: fun4(u1;u1; ( temp 4-component vector of float) +0:19 'entityId' ( temp uint) +0:19 'entityId' ( temp uint) +0:23 Function Definition: @main( ( temp int) +0:23 Function Parameters: +0:23 Function Definition: main( ( temp void) +0:23 Function Parameters: +0:? Sequence +0:23 move second child to first child ( temp int) +0:? '@entryPointOutput' (layout( location=0) out int) +0:23 Function Call: @main( ( temp int) +0:? Linker Objects +0:? '@entryPointOutput' (layout( location=0) out int) + + +Linked fragment stage: + + +Shader version: 500 +gl_FragCoord origin is upper left +ERROR: node is still EOpNull! +0:2 Function Definition: fun0( ( temp 4-component vector of float) +0:2 Function Parameters: +0:? Sequence +0:3 Branch: Return with expression +0:3 Constant: +0:3 1.000000 +0:3 1.000000 +0:3 1.000000 +0:3 1.000000 +0:7 Function Definition: fun2(vf4; ( temp uint) +0:7 Function Parameters: +0:7 'col' ( in 4-component vector of float) +0:? Sequence +0:8 Branch: Return with expression +0:8 Constant: +0:8 7 (const uint) +0:12 Function Definition: fun4(u1;u1; ( temp 4-component vector of float) +0:12 Function Parameters: +0:12 'id1' ( in uint) +0:12 'id2' ( in uint) +0:? Sequence +0:13 Branch: Return with expression +0:13 Construct vec4 ( temp 4-component vector of float) +0:13 Convert uint to float ( temp float) +0:13 component-wise multiply ( temp uint) +0:13 'id1' ( in uint) +0:13 'id2' ( in uint) +0:17 Function Definition: fun1(i1; ( temp 4-component vector of float) +0:17 Function Parameters: +0:17 'index' ( in int) +0:? Sequence +0:18 Sequence +0:18 move second child to first child ( temp uint) +0:18 'entityId' ( temp uint) +0:18 Function Call: fun2(vf4; ( temp uint) +0:18 Function Call: fun0( ( temp 4-component vector of float) +0:19 Branch: Return with expression +0:19 Function Call: fun4(u1;u1; ( temp 4-component vector of float) +0:19 'entityId' ( temp uint) +0:19 'entityId' ( temp uint) +0:23 Function Definition: @main( ( temp int) +0:23 Function Parameters: +0:23 Function Definition: main( ( temp void) +0:23 Function Parameters: +0:? Sequence +0:23 move second child to first child ( temp int) +0:? '@entryPointOutput' (layout( location=0) out int) +0:23 Function Call: @main( ( temp int) +0:? Linker Objects +0:? '@entryPointOutput' (layout( location=0) out int) + +SPIR-V is not generated for failed compile or link diff --git a/Test/hlsl.function.frag b/Test/hlsl.function.frag new file mode 100644 index 0000000..4d11678 --- /dev/null +++ b/Test/hlsl.function.frag @@ -0,0 +1,25 @@ +float4 fun0() +{ + return 1.0f; +} + +uint fun2(float4 col) +{ + return 7; +} + +float4 fun4(uint id1, uint id2) +{ + return id1 * id2; +} + +float4 fun1(int index) +{ + uint entityId = fun2(fun0()); + return fun4(entityId, entityId); +} + +int main() : SV_TARGET +{ + return fun1; +} \ No newline at end of file diff --git a/gtests/Hlsl.FromFile.cpp b/gtests/Hlsl.FromFile.cpp index 8f12c55..9da900f 100644 --- a/gtests/Hlsl.FromFile.cpp +++ b/gtests/Hlsl.FromFile.cpp @@ -131,6 +131,7 @@ INSTANTIATE_TEST_CASE_P( {"hlsl.domain.1.tese", "main"}, {"hlsl.domain.2.tese", "main"}, {"hlsl.domain.3.tese", "main"}, + {"hlsl.function.frag", "main"}, {"hlsl.hull.1.tesc", "main"}, {"hlsl.hull.2.tesc", "main"}, {"hlsl.hull.void.tesc", "main"}, diff --git a/hlsl/hlslParseHelper.cpp b/hlsl/hlslParseHelper.cpp index e875cb7..d39e2c5 100755 --- a/hlsl/hlslParseHelper.cpp +++ b/hlsl/hlslParseHelper.cpp @@ -2988,13 +2988,19 @@ void HlslParseContext::decomposeSampleMethods(const TSourceLoc& loc, TIntermType const TOperator op = node->getAsOperator()->getOp(); const TIntermAggregate* argAggregate = arguments ? arguments->getAsAggregate() : nullptr; - // Bail out if not a sampler method + // Bail out if not a sampler method. + // Note though this is odd to do before checking the op, because the op + // could be something that takes the arguments, and the function in question + // takes the result of the op. So, this is not the final word. if (arguments != nullptr) { - if ((argAggregate != nullptr && argAggregate->getSequence()[0]->getAsTyped()->getBasicType() != EbtSampler)) - return; - - if (argAggregate == nullptr && arguments->getAsTyped()->getBasicType() != EbtSampler) - return; + if (argAggregate == nullptr) { + if (arguments->getAsTyped()->getBasicType() != EbtSampler) + return; + } else { + if (argAggregate->getSequence().size() == 0 || + argAggregate->getSequence()[0]->getAsTyped()->getBasicType() != EbtSampler) + return; + } } switch (op) { -- 2.7.4