diff options
| author | venkataram-nv <vedavamadath@nvidia.com> | 2024-07-09 16:18:36 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-07-09 16:18:36 -0700 |
| commit | 0e6c5c518953141f31c09e5f10d3939054f9b1ee (patch) | |
| tree | 932bddc80408e211e1128ca5901b301c154e67bd /source | |
| parent | 1caef5907d0b0f16f686a8fcca479c6afc09f146 (diff) | |
Warnings for uninitialized values (#4530)
This extends the code for handling uninitialized output parameters.
Still needs to handle generic templates and assignment of uninitialized
values more carefully.
The file containing the relevant code are now in
source/slang/slang-ir-use-uninitialized-values.cpp
rather than the previous
source/slang/slang-ir-use-uninitialized-out-param.h
and the top-level function is now checkForUsingUinitializedValues.
Additionally a rudimentary test shader has been added for this case, which replaces the old file for out params only; tests/diagnositcs/uninitialized-out.slang becomes tests/diagnositcs/uninitialized.slang.
What this does not implement (could be future PRs):
* Checking uninitialized fields within constructors
* Partially uninitialized values with respect to data structure (e.g. arrays/structs/vector types)
* Partially uninitialized values with respect to control flow (e.g. if/else/loop)
Diffstat (limited to 'source')
| -rw-r--r-- | source/slang/slang-diagnostic-defs.h | 10 | ||||
| -rw-r--r-- | source/slang/slang-ir-use-uninitialized-out-param.cpp | 150 | ||||
| -rw-r--r-- | source/slang/slang-ir-use-uninitialized-values.cpp | 382 | ||||
| -rw-r--r-- | source/slang/slang-ir-use-uninitialized-values.h (renamed from source/slang/slang-ir-use-uninitialized-out-param.h) | 2 | ||||
| -rw-r--r-- | source/slang/slang-lower-to-ir.cpp | 6 |
5 files changed, 392 insertions, 158 deletions
diff --git a/source/slang/slang-diagnostic-defs.h b/source/slang/slang-diagnostic-defs.h index 496fb7e32..7ebe77a8f 100644 --- a/source/slang/slang-diagnostic-defs.h +++ b/source/slang/slang-diagnostic-defs.h @@ -598,7 +598,7 @@ DIAGNOSTIC(39999, Error, unableToFindSymbolInModule, "unable to find the mangled DIAGNOSTIC(39999, Error, overloadedParameterToHigherOrderFunction, "passing overloaded functions to higher order functions is not supported") -// 38xxx +// 38xxx DIAGNOSTIC(38000, Error, entryPointFunctionNotFound, "no function found matching entry point name '$0'") DIAGNOSTIC(38001, Error, ambiguousEntryPoint, "more than one function matches entry point name '$0'") @@ -735,9 +735,11 @@ DIAGNOSTIC(41010, Warning, missingReturn, "control flow may reach end of non-'vo DIAGNOSTIC(41011, Error, profileIncompatibleWithTargetSwitch, "__target_switch has no compatable target with current profile '$0'") DIAGNOSTIC(41012, Warning, profileImplicitlyUpgraded, "user set `profile` had an implicit upgrade applied to it, atoms added: '$0'") DIAGNOSTIC(41012, Error, profileImplicitlyUpgradedRestrictive, "user set `profile` had an implicit upgrade applied to it, atoms added: '$0'") -DIAGNOSTIC(41015, Error, usingUninitializedValue, "use of uninitialized value '$0'") -DIAGNOSTIC(41016, Warning, returningWithUninitializedOut, "returning without initializing out parameter '$0'") -DIAGNOSTIC(41017, Warning, returningWithPartiallyUninitializedOut, "returning without fully initializing out parameter '$0'") +DIAGNOSTIC(41015, Warning, usingUninitializedOut, "use of uninitialized out parameter '$0'") +DIAGNOSTIC(41016, Warning, usingUninitializedVariable, "use of uninitialized variable '$0'") +DIAGNOSTIC(41017, Warning, usingUninitializedGlobalVariable, "use of uninitialized global variable '$0'") +DIAGNOSTIC(41018, Warning, returningWithUninitializedOut, "returning without initializing out parameter '$0'") +DIAGNOSTIC(41019, Warning, returningWithPartiallyUninitializedOut, "returning without fully initializing out parameter '$0'") DIAGNOSTIC(41011, Error, typeDoesNotFitAnyValueSize, "type '$0' does not fit in the size required by its conforming interface.") DIAGNOSTIC(41012, Note, typeAndLimit, "sizeof($0) is $1, limit is $2") diff --git a/source/slang/slang-ir-use-uninitialized-out-param.cpp b/source/slang/slang-ir-use-uninitialized-out-param.cpp deleted file mode 100644 index 7e3ef9ca2..000000000 --- a/source/slang/slang-ir-use-uninitialized-out-param.cpp +++ /dev/null @@ -1,150 +0,0 @@ -#include "slang-ir-use-uninitialized-out-param.h" -#include "slang-ir-util.h" -#include "slang-ir-reachability.h" - -namespace Slang -{ - class DiagnosticSink; - struct IRModule; - - struct StoreSite - { - IRInst* storeInst; - IRInst* address; - }; - - void checkForUsingUninitializedOutParams(IRFunc* func, DiagnosticSink* sink) - { - List<IRInst*> outParams; - auto firstBlock = func->getFirstBlock(); - if (!firstBlock) - return; - - ReachabilityContext reachability(func); - - for (auto param : firstBlock->getParams()) - { - if (auto outType = as<IROutType>(param->getFullType())) - { - // Don't check `out Vertices<T>` or `out Indices<T>` parameters - // in mesh shaders. - // TODO: we should find a better way to represent these mesh shader - // parameters so they conform to the initialize before use convention. - // For example, we can use a `OutputVetices` and `OutputIndices` type - // to represent an output, like `OutputPatch` in domain shader. - // For now, we just skip the check for these parameters. - switch (outType->getValueType()->getOp()) - { - case kIROp_VerticesType: - case kIROp_IndicesType: - case kIROp_PrimitivesType: - continue; - default: - break; - } - } - else - { - continue; - } - List<IRInst*> addresses; - addresses.add(param); - List<StoreSite> stores; - // Collect all sub-addresses from the param. - for (Index i = 0; i < addresses.getCount(); i++) - { - auto addr = addresses[i]; - for (auto use = addr->firstUse; use; use = use->nextUse) - { - switch (use->getUser()->getOp()) - { - case kIROp_GetElementPtr: - case kIROp_FieldAddress: - addresses.add(use->getUser()); - break; - case kIROp_Store: - case kIROp_SwizzledStore: - // If we see a store of this address, add it to stores set. - if (use == use->getUser()->getOperands()) - stores.add(StoreSite{ use->getUser(), addr }); - break; - case kIROp_Call: - case kIROp_SPIRVAsm: - // If we see a call using this address, treat it as a store. - stores.add(StoreSite{ use->getUser(), addr }); - break; - case kIROp_SPIRVAsmOperandInst: - stores.add(StoreSite{ use->getUser()->getParent(), addr}); - break; - } - } - } - // Check all address loads. - List<IRInst*> loadsAndReturns; - for (auto addr : addresses) - { - for (auto use = addr->firstUse; use; use = use->nextUse) - { - if (auto load = as<IRLoad>(use->getUser())) - loadsAndReturns.add(load); - } - } - for(const auto& b : func->getBlocks()) - { - auto t = as<IRReturn>(b->getTerminator()); - if (!t) continue; - loadsAndReturns.add(t); - } - - for (auto store : stores) - { - // Remove insts from `loads` that is reachable from the store. - for (Index i = 0; i < loadsAndReturns.getCount();) - { - auto load = as<IRLoad>(loadsAndReturns[i]); - if (load && !canAddressesPotentiallyAlias(func, store.address, load->getPtr())) - continue; - if (reachability.isInstReachable(store.storeInst, loadsAndReturns[i])) - { - loadsAndReturns.fastRemoveAt(i); - } - else - { - i++; - } - } - } - // If there are any loads left, it means they are using uninitialized out params. - for (auto load : loadsAndReturns) - { - sink->diagnose( - load, - load->m_op == kIROp_Return - ? Diagnostics::returningWithUninitializedOut - : Diagnostics::usingUninitializedValue, - param); - } - } - } - - void checkForUsingUninitializedOutParams( - IRModule* module, - DiagnosticSink* sink) - { - for (auto inst : module->getGlobalInsts()) - { - if (auto func = as<IRFunc>(inst)) - { - checkForUsingUninitializedOutParams(func, sink); - } - else if (auto generic = as<IRGeneric>(inst)) - { - auto retVal = findGenericReturnVal(generic); - if (auto funcVal = as<IRFunc>(retVal)) - { - checkForUsingUninitializedOutParams(funcVal, sink); - } - } - } - } -} diff --git a/source/slang/slang-ir-use-uninitialized-values.cpp b/source/slang/slang-ir-use-uninitialized-values.cpp new file mode 100644 index 000000000..762773ad4 --- /dev/null +++ b/source/slang/slang-ir-use-uninitialized-values.cpp @@ -0,0 +1,382 @@ +#include "slang-ir-use-uninitialized-values.h" +#include "slang-ir-insts.h" +#include "slang-ir-reachability.h" +#include "slang-ir.h" + +namespace Slang +{ + static bool isMetaOp(IRInst* inst) + { + switch (inst->getOp()) + { + // These instructions only look at the parameter's type, + // so passing an undefined value to them is permissible + case kIROp_IsBool: + case kIROp_IsInt: + case kIROp_IsUnsignedInt: + case kIROp_IsSignedInt: + case kIROp_IsHalf: + case kIROp_IsFloat: + case kIROp_IsVector: + case kIROp_GetNaturalStride: + case kIROp_TypeEquals: + return true; + default: + break; + } + + return false; + } + + // Casting to IRUndefined is currently vacuous + // (e.g. any IRInst can be cast to IRUndefined) + static bool isUndefinedValue(IRInst* inst) + { + return (inst->m_op == kIROp_undefined); + } + + static bool isUndefinedParam(IRParam* param) + { + auto outType = as<IROutType>(param->getFullType()); + if (!outType) + return false; + + // Don't check `out Vertices<T>` or `out Indices<T>` parameters + // in mesh shaders. + // TODO: we should find a better way to represent these mesh shader + // parameters so they conform to the initialize before use convention. + // For example, we can use a `OutputVetices` and `OutputIndices` type + // to represent an output, like `OutputPatch` in domain shader. + // For now, we just skip the check for these parameters. + switch (outType->getValueType()->getOp()) + { + case kIROp_VerticesType: + case kIROp_IndicesType: + case kIROp_PrimitivesType: + return false; + default: + break; + } + + return true; + } + + static bool isAliasable(IRInst* inst) + { + switch (inst->getOp()) + { + // These instructions generate (implicit) references to inst + case kIROp_FieldExtract: + case kIROp_FieldAddress: + case kIROp_GetElement: + case kIROp_GetElementPtr: + return true; + default: + break; + } + + return false; + } + + static bool isDifferentiableFunc(IRInst* func) + { + for (auto decor = func->getFirstDecoration(); decor; decor = decor->getNextDecoration()) + { + switch (decor->getOp()) + { + case kIROp_ForwardDerivativeDecoration: + case kIROp_ForwardDifferentiableDecoration: + case kIROp_BackwardDerivativeDecoration: + case kIROp_BackwardDifferentiableDecoration: + case kIROp_UserDefinedBackwardDerivativeDecoration: + return true; + default: + break; + } + } + + return false; + } + + static bool canIgnoreType(IRType* type) + { + if (as<IRVoidType>(type)) + return true; + + // For structs, ignore if its empty + if (as<IRStructType>(type)) + return (type->getFirstChild() == nullptr); + + // Nothing to initialize for a pure interface + if (as<IRInterfaceType>(type)) + return true; + + // For pointers, check the value type (primarily for globals) + if (auto ptr = as<IRPtrType>(type)) + return canIgnoreType(ptr->getValueType()); + + // In the case of specializations, check returned type + if (auto spec = as<IRSpecialize>(type)) + { + IRInst* base = spec->getBase(); + IRGeneric* generic = as<IRGeneric>(base); + IRInst* inner = findInnerMostGenericReturnVal(generic); + IRType* innerType = as<IRType>(inner); + return canIgnoreType(innerType); + } + + return false; + } + + static List<IRInst*> getAliasableInstructions(IRInst* inst) + { + List<IRInst*> addresses; + + addresses.add(inst); + for (auto use = inst->firstUse; use; use = use->nextUse) + { + IRInst* user = use->getUser(); + + // Meta instructions only use the argument type + if (isMetaOp(user) || !isAliasable(user)) + continue; + + addresses.addRange(getAliasableInstructions(user)); + } + + return addresses; + } + + static void collectLoadStore(List<IRInst*>& stores, List<IRInst*>& loads, IRInst* user) + { + // Meta intrinsics (which evaluate on type) do nothing + if (isMetaOp(user)) + return; + + // Ignore instructions generating more aliases + if (isAliasable(user)) + return; + + switch (user->getOp()) + { + case kIROp_loop: + case kIROp_unconditionalBranch: + // TODO: Ignore branches for now + return; + + // These instructions will store data... + case kIROp_Store: + case kIROp_SwizzledStore: + // TODO: for calls, should make check that the + // function is passing as an out param + case kIROp_Call: + case kIROp_SPIRVAsm: + case kIROp_GenericAsm: + // For now assume that __intrinsic_asm blocks will do the right thing... + stores.add(user); + break; + + case kIROp_SPIRVAsmOperandInst: + // For SPIRV asm instructions, need to check out the entire + // block when doing reachability checks + stores.add(user->getParent()); + break; + + case kIROp_MakeExistential: + case kIROp_MakeExistentialWithRTTI: + // For specializing generic structs + stores.add(user); + break; + + // ... and the rest will load/use them + default: + loads.add(user); + break; + } + } + + static void cancelLoads(ReachabilityContext &reachability, const List<IRInst*>& stores, List<IRInst*>& loads) + { + // Remove all loads which are reachable from stores + for (auto store : stores) + { + for (Index i = 0; i < loads.getCount(); ) + { + if (reachability.isInstReachable(store, loads[i])) + loads.fastRemoveAt(i); + else + i++; + } + } + } + + static List<IRInst*> getUnresolvedParamLoads(ReachabilityContext &reachability, IRFunc* func, IRInst* inst) + { + // Collect all aliasable addresses + auto addresses = getAliasableInstructions(inst); + + // Partition instructions + List<IRInst*> stores; + List<IRInst*> loads; + + for (auto alias : addresses) + { + // TODO: Mark specific parts assigned to for partial initialization checks + for (auto use = alias->firstUse; use; use = use->nextUse) + { + IRInst* user = use->getUser(); + collectLoadStore(stores, loads, user); + } + } + + // Only for out params we shall add all returns + for (const auto& b : func->getBlocks()) + { + auto t = as<IRReturn>(b->getTerminator()); + if (!t) + continue; + + loads.add(t); + } + + cancelLoads(reachability, stores, loads); + + return loads; + } + + static List<IRInst*> getUnresolvedVariableLoads(ReachabilityContext &reachability, IRInst* inst) + { + auto addresses = getAliasableInstructions(inst); + + // Partition instructions + List<IRInst*> stores; + List<IRInst*> loads; + + for (auto alias : addresses) + { + for (auto use = alias->firstUse; use; use = use->nextUse) + { + IRInst* user = use->getUser(); + collectLoadStore(stores, loads, user); + } + } + + cancelLoads(reachability, stores, loads); + + return loads; + } + + static void checkUninitializedValues(IRFunc* func, DiagnosticSink* sink) + { + if (isDifferentiableFunc(func)) + return; + + auto firstBlock = func->getFirstBlock(); + if (!firstBlock) + return; + + ReachabilityContext reachability(func); + + // Check out parameters + for (auto param : firstBlock->getParams()) + { + if (!isUndefinedParam(param)) + continue; + + auto loads = getUnresolvedParamLoads(reachability, func, param); + for (auto load : loads) + { + sink->diagnose(load, + as <IRReturn> (load) + ? Diagnostics::returningWithUninitializedOut + : Diagnostics::usingUninitializedOut, + param); + } + } + + // Check ordinary instructions + for (auto inst = firstBlock->getFirstInst(); inst; inst = inst->getNextInst()) + { + if (!isUndefinedValue(inst)) + continue; + + IRType* type = inst->getFullType(); + if (canIgnoreType(type)) + continue; + + auto loads = getUnresolvedVariableLoads(reachability, inst); + for (auto load : loads) + { + sink->diagnose(load, + Diagnostics::usingUninitializedVariable, + inst); + } + } + } + + static void checkUninitializedGlobals(IRGlobalVar* variable, DiagnosticSink* sink) + { + IRType* type = variable->getFullType(); + if (canIgnoreType(type)) + return; + + // Check for semantic decorations + // (e.g. globals like gl_GlobalInvocationID) + if (variable->findDecoration<IRSemanticDecoration>()) + return; + + // Check for initialization blocks + for (auto inst : variable->getChildren()) + { + if (as<IRBlock>(inst)) + return; + } + + auto addresses = getAliasableInstructions(variable); + + List<IRInst*> stores; + List<IRInst*> loads; + + for (auto alias : addresses) + { + for (auto use = alias->firstUse; use; use = use->nextUse) + { + IRInst* user = use->getUser(); + collectLoadStore(stores, loads, user); + + // Disregard if there is at least one store, + // since we cannot tell what the control flow is + if (stores.getCount()) + return; + } + } + + for (auto load : loads) + { + sink->diagnose(load, + Diagnostics::usingUninitializedGlobalVariable, + variable); + } + } + + void checkForUsingUninitializedValues(IRModule* module, DiagnosticSink* sink) + { + for (auto inst : module->getGlobalInsts()) + { + if (auto func = as<IRFunc>(inst)) + { + checkUninitializedValues(func, sink); + } + else if (auto generic = as<IRGeneric>(inst)) + { + auto retVal = findGenericReturnVal(generic); + if (auto funcVal = as<IRFunc>(retVal)) + checkUninitializedValues(funcVal, sink); + } + else if (auto global = as<IRGlobalVar>(inst)) + { + checkUninitializedGlobals(global, sink); + } + } + } +} diff --git a/source/slang/slang-ir-use-uninitialized-out-param.h b/source/slang/slang-ir-use-uninitialized-values.h index fd090c4f9..9b6867a3b 100644 --- a/source/slang/slang-ir-use-uninitialized-out-param.h +++ b/source/slang/slang-ir-use-uninitialized-values.h @@ -6,7 +6,7 @@ namespace Slang class DiagnosticSink; struct IRModule; - void checkForUsingUninitializedOutParams( + void checkForUsingUninitializedValues( IRModule* module, DiagnosticSink* sink); } diff --git a/source/slang/slang-lower-to-ir.cpp b/source/slang/slang-lower-to-ir.cpp index c946072f9..6fa2ce67f 100644 --- a/source/slang/slang-lower-to-ir.cpp +++ b/source/slang/slang-lower-to-ir.cpp @@ -33,7 +33,7 @@ #include "slang-ir-clone.h" #include "slang-ir-lower-error-handling.h" #include "slang-ir-obfuscate-loc.h" -#include "slang-ir-use-uninitialized-out-param.h" +#include "slang-ir-use-uninitialized-values.h" #include "slang-ir-peephole.h" #include "slang-mangle.h" #include "slang-type-layout.h" @@ -10925,8 +10925,8 @@ RefPtr<IRModule> generateIRForTranslationUnit( // call graph) based on constraints imposed by different instructions. propagateConstExpr(module, compileRequest->getSink()); - // Check for using uninitialized out parameters. - checkForUsingUninitializedOutParams(module, compileRequest->getSink()); + // Check for using uninitialized values + checkForUsingUninitializedValues(module, compileRequest->getSink()); // TODO: give error messages if any `undefined` or // instructions remain. |
