From 6d35ae89c0914de8088ce5a56741f9f575a3b93a Mon Sep 17 00:00:00 2001 From: Ryan Harrison Date: Wed, 28 Aug 2019 11:25:12 -0400 Subject: [PATCH 1/2] Return nullptr after assert to avoid uninitialized variables In the current version of the code on non-debug builds these cases will fallthrough, since assert is a no-op, and eventually make a call passing in |op| which hasn't been initialized. clang is currently throwing a warning about this behaviour when integrating downstream. This patch changes the behaviour, so that in any branch that has an assert now has a return nullptr, to indicate failure after it and avoid the uninitialized variable usage. --- glslang/MachineIndependent/ParseHelper.cpp | 26 ++++++++++++++++------ 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/glslang/MachineIndependent/ParseHelper.cpp b/glslang/MachineIndependent/ParseHelper.cpp index 8441a39d..20c2c8e2 100644 --- a/glslang/MachineIndependent/ParseHelper.cpp +++ b/glslang/MachineIndependent/ParseHelper.cpp @@ -7081,7 +7081,7 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T switch (type.getBasicType()) { default: assert(0); - break; + return nullptr; case EbtInt: { switch (node->getType().getBasicType()) { @@ -7090,7 +7090,9 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T case EbtUint8: op = EOpConvUint8ToInt; break; case EbtInt8: op = EOpConvInt8ToInt; break; case EbtUint: op = EOpConvUintToInt; break; - default: assert(0); + default: + assert(0); + return nullptr; } } @@ -7104,7 +7106,9 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T case EbtInt8: op = EOpConvInt8ToUint; break; case EbtInt: op = EOpConvIntToUint; break; case EbtUint: op = EOpConvUintToInt8; break; - default: assert(0); + default: + assert(0); + return nullptr; } } @@ -7118,7 +7122,9 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T case EbtUint8: op = EOpConvUint8ToInt8; break; case EbtInt: op = EOpConvIntToInt8; break; case EbtUint: op = EOpConvUintToInt8; break; - default: assert(0); + default: + assert(0); + return nullptr; } } @@ -7130,7 +7136,9 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T case EbtInt8: op = EOpConvInt8ToUint8; break; case EbtInt: op = EOpConvIntToUint8; break; case EbtUint: op = EOpConvUintToUint8; break; - default: assert(0); + default: + assert(0); + return nullptr; } } break; @@ -7142,7 +7150,9 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T case EbtUint8: op = EOpConvUint8ToFloat; break; case EbtInt: op = EOpConvIntToFloat; break; case EbtUint: op = EOpConvUintToFloat; break; - default: assert(0); + default: + assert(0); + return nullptr; } } break; @@ -7154,7 +7164,9 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T case EbtUint8: op = EOpConvUint8ToFloat16; break; case EbtInt: op = EOpConvIntToFloat16; break; case EbtUint: op = EOpConvUintToFloat16; break; - default: assert(0); + default: + assert(0); + return nullptr; } } break; From 8b91ecbac99d80cbcdfec678502b9dee2e742ac2 Mon Sep 17 00:00:00 2001 From: Ryan Harrison Date: Wed, 28 Aug 2019 13:15:15 -0400 Subject: [PATCH 2/2] Change to initializing the variable --- glslang/MachineIndependent/ParseHelper.cpp | 28 +++++++--------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/glslang/MachineIndependent/ParseHelper.cpp b/glslang/MachineIndependent/ParseHelper.cpp index 20c2c8e2..bb6ab24a 100644 --- a/glslang/MachineIndependent/ParseHelper.cpp +++ b/glslang/MachineIndependent/ParseHelper.cpp @@ -7077,11 +7077,11 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T } node = intermediate.setAggregateOperator(node, EOpConstructCooperativeMatrix, type, node->getLoc()); } else { - TOperator op; + TOperator op = EOpNull; switch (type.getBasicType()) { default: assert(0); - return nullptr; + break; case EbtInt: { switch (node->getType().getBasicType()) { @@ -7090,9 +7090,7 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T case EbtUint8: op = EOpConvUint8ToInt; break; case EbtInt8: op = EOpConvInt8ToInt; break; case EbtUint: op = EOpConvUintToInt; break; - default: - assert(0); - return nullptr; + default: assert(0); } } @@ -7106,9 +7104,7 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T case EbtInt8: op = EOpConvInt8ToUint; break; case EbtInt: op = EOpConvIntToUint; break; case EbtUint: op = EOpConvUintToInt8; break; - default: - assert(0); - return nullptr; + default: assert(0); } } @@ -7122,9 +7118,7 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T case EbtUint8: op = EOpConvUint8ToInt8; break; case EbtInt: op = EOpConvIntToInt8; break; case EbtUint: op = EOpConvUintToInt8; break; - default: - assert(0); - return nullptr; + default: assert(0); } } @@ -7136,9 +7130,7 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T case EbtInt8: op = EOpConvInt8ToUint8; break; case EbtInt: op = EOpConvIntToUint8; break; case EbtUint: op = EOpConvUintToUint8; break; - default: - assert(0); - return nullptr; + default: assert(0); } } break; @@ -7150,9 +7142,7 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T case EbtUint8: op = EOpConvUint8ToFloat; break; case EbtInt: op = EOpConvIntToFloat; break; case EbtUint: op = EOpConvUintToFloat; break; - default: - assert(0); - return nullptr; + default: assert(0); } } break; @@ -7164,9 +7154,7 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T case EbtUint8: op = EOpConvUint8ToFloat16; break; case EbtInt: op = EOpConvIntToFloat16; break; case EbtUint: op = EOpConvUintToFloat16; break; - default: - assert(0); - return nullptr; + default: assert(0); } } break;