diff --git a/src/liboslexec/batched_llvm_instance.cpp b/src/liboslexec/batched_llvm_instance.cpp index b9bf7423a..5d81ec018 100644 --- a/src/liboslexec/batched_llvm_instance.cpp +++ b/src/liboslexec/batched_llvm_instance.cpp @@ -138,6 +138,9 @@ static ustring op_aref("aref"); static ustring op_compref("compref"); static ustring op_mxcompref("mxcompref"); static ustring op_useparam("useparam"); +static ustring op_pointcloud_get("pointcloud_get"); +static ustring op_spline("spline"); +static ustring op_splineinverse("splineinverse"); static ustring unknown_shader_group_name(""); @@ -1620,7 +1623,8 @@ BatchedBackendLLVM::llvm_generate_debug_uninit(const Opcode& op) } llvm::Value* ncheck = ll.constant(int(t.numelements() * t.aggregate)); - llvm::Value* offset = ll.constant(0); + llvm::Value* varying_nchecks = nullptr; + llvm::Value* offset = ll.constant(0); BatchedBackendLLVM::TempScope temp_scope(*this); llvm::Value* loc_varying_offsets = nullptr; @@ -1701,6 +1705,31 @@ BatchedBackendLLVM::llvm_generate_debug_uninit(const Opcode& op) ll.op_store(comp, loc_varying_offsets); } ncheck = ll.constant(1); + } else if (op.opname() == op_pointcloud_get && i == 2) { + // int pointcloud_get (string ptcname, int indices[], int count, string attr, type data[]) + // will only read indices[0..count-1], so limit the check to count + OSL_ASSERT(3 < op.nargs()); + Symbol& count_sym = *opargsym(op, 3); + if (count_sym.is_uniform()) { + ncheck = llvm_load_value(count_sym); + } else { + // Don't think we can have a uniform indices array that + // has a varying index count + if (!sym.is_uniform()) { + varying_nchecks = ll.void_ptr(llvm_get_pointer(count_sym)); + } + } + } else if (((op.opname() == op_spline) + || (op.opname() == op_splineinverse)) + && i == 4) { + // If an explicit knot count was provided to spline|splineinverse we should + // limit our check of knot values to that count + bool has_knot_count = (op.nargs() == 5); + if (has_knot_count) { + Symbol& knot_count_sym = *opargsym(op, 3); + OSL_ASSERT(knot_count_sym.is_uniform()); + ncheck = llvm_load_value(knot_count_sym); + } } if (loc_varying_offsets != nullptr) { @@ -1724,15 +1753,15 @@ BatchedBackendLLVM::llvm_generate_debug_uninit(const Opcode& op) if (sym.is_uniform()) { ll.call_function(build_name( FuncSpec("uninit_check_values_offset") - .arg_uniform(TypeDesc::PTR) - .arg_varying(TypeInt) + .arg_uniform(TypeDesc::PTR) // vals + .arg_varying(TypeInt) // firstcheck .mask()), args); } else { ll.call_function(build_name( FuncSpec("uninit_check_values_offset") - .arg_varying(TypeDesc::PTR) - .arg_varying(TypeInt) + .arg_varying(TypeDesc::PTR) // vals + .arg_varying(TypeInt) // firstcheck .mask()), args); } @@ -1756,33 +1785,60 @@ BatchedBackendLLVM::llvm_generate_debug_uninit(const Opcode& op) ncheck }; ll.call_function(build_name( FuncSpec("uninit_check_values_offset") - .arg_uniform(TypeDesc::PTR) - .arg_uniform(TypeInt)), + .arg_uniform(TypeDesc::PTR) // vals + .arg_uniform(TypeInt)), // firstcheck args); } else { - llvm::Value* args[] - = { ll.mask_as_int(ll.current_mask()), - ll.constant(t), - llvm_void_ptr(sym), - sg_void_ptr(), - ll.constant(op.sourcefile()), - ll.constant(op.sourceline()), - ll.constant(group().name()), - ll.constant(layer()), - ll.constant(inst()->layername()), - ll.constant(inst()->shadername().c_str()), - ll.constant(int(&op - &inst()->ops()[0])), - ll.constant(op.opname()), - ll.constant(i), - ll.constant(sym.unmangled()), - offset, - ncheck }; - ll.call_function(build_name( - FuncSpec("uninit_check_values_offset") - .arg_varying(TypeDesc::PTR) - .arg_uniform(TypeInt) - .mask()), - args); + if (varying_nchecks != nullptr) { + llvm::Value* args[] + = { ll.mask_as_int(ll.current_mask()), + ll.constant(t), + llvm_void_ptr(sym), + sg_void_ptr(), + ll.constant(op.sourcefile()), + ll.constant(op.sourceline()), + ll.constant(group().name()), + ll.constant(layer()), + ll.constant(inst()->layername()), + ll.constant(inst()->shadername().c_str()), + ll.constant(int(&op - &inst()->ops()[0])), + ll.constant(op.opname()), + ll.constant(i), + ll.constant(sym.unmangled()), + offset, + varying_nchecks }; + ll.call_function( + build_name(FuncSpec("uninit_check_values_offset") + .arg_varying(TypeDesc::PTR) // vals + .arg_uniform(TypeInt) // firstcheck + .arg_varying(TypeInt) // nchecks + .mask()), + args); + } else { + llvm::Value* args[] + = { ll.mask_as_int(ll.current_mask()), + ll.constant(t), + llvm_void_ptr(sym), + sg_void_ptr(), + ll.constant(op.sourcefile()), + ll.constant(op.sourceline()), + ll.constant(group().name()), + ll.constant(layer()), + ll.constant(inst()->layername()), + ll.constant(inst()->shadername().c_str()), + ll.constant(int(&op - &inst()->ops()[0])), + ll.constant(op.opname()), + ll.constant(i), + ll.constant(sym.unmangled()), + offset, + ncheck }; + ll.call_function( + build_name(FuncSpec("uninit_check_values_offset") + .arg_varying(TypeDesc::PTR) // vals + .arg_uniform(TypeInt) // firstcheck + .mask()), + args); + } } } } diff --git a/src/liboslexec/builtindecl_wide_xmacro.h b/src/liboslexec/builtindecl_wide_xmacro.h index f35889bf5..1a2314a4c 100644 --- a/src/liboslexec/builtindecl_wide_xmacro.h +++ b/src/liboslexec/builtindecl_wide_xmacro.h @@ -373,6 +373,8 @@ DECL(__OSL_MASKED_OP(range_check), "xXiisXsisiss") DECL(__OSL_OP2(uninit_check_values_offset, X, i), "xLXXsisissisisii") DECL(__OSL_MASKED_OP2(uninit_check_values_offset, X, Wi), "xiLXXsisissisisXi") DECL(__OSL_MASKED_OP2(uninit_check_values_offset, WX, i), "xiLXXsisissisisii") +DECL(__OSL_MASKED_OP3(uninit_check_values_offset, WX, i, Wi), + "xiLXXsisissisisiX") DECL(__OSL_MASKED_OP2(uninit_check_values_offset, WX, Wi), "xiLXXsisissisisXi") DECL(__OSL_OP1(get_attribute, s), "iXissiiXXi") diff --git a/src/liboslexec/llvm_instance.cpp b/src/liboslexec/llvm_instance.cpp index f92f889ef..974f95b17 100644 --- a/src/liboslexec/llvm_instance.cpp +++ b/src/liboslexec/llvm_instance.cpp @@ -143,6 +143,9 @@ static ustring op_aref("aref"); static ustring op_compref("compref"); static ustring op_mxcompref("mxcompref"); static ustring op_useparam("useparam"); +static ustring op_pointcloud_get("pointcloud_get"); +static ustring op_spline("spline"); +static ustring op_splineinverse("splineinverse"); static ustring unknown_shader_group_name(""); @@ -1135,8 +1138,10 @@ BackendLLVM::llvm_generate_debug_uninit(const Opcode& op) // don't generate uninit test code for it. continue; } - if (op.opname() == Strings::op_dowhile && i == 0) { - // The first argument of 'dowhile' is the condition temp, but + if (((op.opname() == Strings::op_dowhile) + || (op.opname() == Strings::op_while)) + && i == 0) { + // The first argument of 'dowhile' or "while" is the condition temp, but // most likely its initializer has not run yet. Unless there is // no "condition" code block, in that case we should still test // it for uninit. @@ -1185,6 +1190,20 @@ BackendLLVM::llvm_generate_debug_uninit(const Opcode& op) comp = ll.op_add(comp, col_ind); offset = comp; ncheck = ll.constant(1); + } else if (op.opname() == op_pointcloud_get && i == 2) { + // int pointcloud_get (string ptcname, int indices[], int count, string attr, type data[]) + // will only read indices[0..count-1], so limit the check to count + OSL_ASSERT(3 < op.nargs()); + ncheck = llvm_load_value(*opargsym(op, 3)); + } else if (((op.opname() == op_spline) + || (op.opname() == op_splineinverse)) + && i == 4) { + // If an explicit knot count was provided to spline|splineinverse we should + // limit our check of knot values to that count + bool has_knot_count = (op.nargs() == 5); + if (has_knot_count) { + ncheck = llvm_load_value(*opargsym(op, 3)); + } } llvm::Value* args[] = { ll.constant(t), diff --git a/src/liboslexec/wide/wide_shadingsys.cpp b/src/liboslexec/wide/wide_shadingsys.cpp index 847705fb2..e25c665c5 100644 --- a/src/liboslexec/wide/wide_shadingsys.cpp +++ b/src/liboslexec/wide/wide_shadingsys.cpp @@ -169,10 +169,10 @@ __OSL_OP2(uninit_check_values_offset, X, ustring layername = USTR(layername_); ctx->errorfmt( "Detected possible use of uninitialized value in {} {} at {}:{} (group {}, layer {} {}, shader {}, op {} '{}', arg {})", - typedesc, symbolname, sourcefile, sourceline, + typedesc, USTR(symbolname), USTR(sourcefile), sourceline, groupname.empty() ? "" : groupname.c_str(), layer, layername.empty() ? "" : layername.c_str(), - shadername, opnum, opname, argnum); + USTR(shadername), opnum, USTR(opname), argnum); } } @@ -233,10 +233,82 @@ __OSL_MASKED_OP2(uninit_check_values_offset, WX, ustring layername = USTR(layername_); ctx->errorfmt( "Detected possible use of uninitialized value in {} {} at {}:{} (group {}, layer {} {}, shader {}, op {} '{}', arg {}) for lanes({:x}) of batch", - typedesc, symbolname, sourcefile, sourceline, + typedesc, USTR(symbolname), USTR(sourcefile), sourceline, groupname.empty() ? "" : groupname.c_str(), layer, layername.empty() ? "" : layername.c_str(), - shadername, opnum, opname, argnum, lanes_uninit.value()); + USTR(shadername), opnum, USTR(opname), argnum, + lanes_uninit.value()); + } +} + + + +// Many parameters, but the 3 parameters used in the function name +// correspond to: "vals", "firstcheck", and "nchecks" +OSL_BATCHOP void +__OSL_MASKED_OP3(uninit_check_values_offset, WX, i, + Wi)(int mask_value, long long typedesc_, void* vals_, + void* bsg_, ustring_pod sourcefile, int sourceline, + ustring_pod groupname_, int layer, ustring_pod layername_, + ustring_pod shadername, int opnum, ustring_pod opname, + int argnum, ustring_pod symbolname, int firstcheck, + int* nchecks_) +{ + TypeDesc typedesc = TYPEDESC(typedesc_); + auto* bsg = reinterpret_cast(bsg_); + ShadingContext* ctx = bsg->uniform.context; + const Mask mask(mask_value); + + Mask lanes_uninit(false); + + if (typedesc.basetype == TypeDesc::FLOAT) { + float* vals = (float*)vals_; + mask.foreach ([=, &lanes_uninit](ActiveLane lane) -> void { + int nchecks = nchecks_[lane]; + for (int c = firstcheck, e = firstcheck + nchecks; c < e; ++c) { + if (!std::isfinite(vals[c * __OSL_WIDTH + lane])) { + lanes_uninit.set_on(lane); + vals[c * __OSL_WIDTH + lane] = 0; + } + } + }); + } + if (typedesc.basetype == TypeDesc::INT) { + int* vals = (int*)vals_; + mask.foreach ([=, &lanes_uninit](ActiveLane lane) -> void { + int nchecks = nchecks_[lane]; + for (int c = firstcheck, e = firstcheck + nchecks; c < e; ++c) { + if (vals[c * __OSL_WIDTH + lane] + == std::numeric_limits::min()) { + lanes_uninit.set_on(lane); + vals[c * __OSL_WIDTH + lane] = 0; + } + } + }); + } + if (typedesc.basetype == TypeDesc::STRING) { + ustring* vals = (ustring*)vals_; + mask.foreach ([=, &lanes_uninit](ActiveLane lane) -> void { + int nchecks = nchecks_[lane]; + for (int c = firstcheck, e = firstcheck + nchecks; c < e; ++c) { + if (vals[c * __OSL_WIDTH + lane] + == Strings::uninitialized_string) { + lanes_uninit.set_on(lane); + vals[c * __OSL_WIDTH + lane] = ustring(); + } + } + }); + } + if (lanes_uninit.any_on()) { + ustring groupname = USTR(groupname_); + ustring layername = USTR(layername_); + ctx->errorfmt( + "Detected possible use of uninitialized value in {} {} at {}:{} (group {}, layer {} {}, shader {}, op {} '{}', arg {}) for lanes({:x}) of batch", + typedesc, USTR(symbolname), USTR(sourcefile), sourceline, + groupname.empty() ? "" : groupname.c_str(), layer, + layername.empty() ? "" : layername.c_str(), + USTR(shadername), opnum, USTR(opname), argnum, + lanes_uninit.value()); } } @@ -299,10 +371,11 @@ __OSL_MASKED_OP2(uninit_check_values_offset, X, ustring layername = USTR(layername_); ctx->errorfmt( "Detected possible use of uninitialized value in {} {} at {}:{} (group {}, layer {} {}, shader {}, op {} '{}', arg {}) for lanes({:x}) of batch", - typedesc, symbolname, sourcefile, sourceline, + typedesc, USTR(symbolname), USTR(sourcefile), sourceline, groupname.empty() ? "" : groupname.c_str(), layer, layername.empty() ? "" : layername.c_str(), - shadername, opnum, opname, argnum, lanes_uninit.value()); + USTR(shadername), opnum, USTR(opname), argnum, + lanes_uninit.value()); } } @@ -367,10 +440,11 @@ __OSL_MASKED_OP2(uninit_check_values_offset, WX, ustring layername = USTR(layername_); ctx->errorfmt( "Detected possible use of uninitialized value in {} {} at {}:{} (group {}, layer {} {}, shader {}, op {} '{}', arg {}) for lanes({:x}) of batch", - typedesc, symbolname, sourcefile, sourceline, + typedesc, USTR(symbolname), USTR(sourcefile), sourceline, groupname.empty() ? "" : groupname.c_str(), layer, layername.empty() ? "" : layername.c_str(), - shadername, opnum, opname, argnum, lanes_uninit.value()); + USTR(shadername), opnum, USTR(opname), argnum, + lanes_uninit.value()); } } diff --git a/testsuite/debug-uninit/ref/out-opt2.txt b/testsuite/debug-uninit/ref/out-opt2.txt index 25d3b5860..fdf901da6 100644 --- a/testsuite/debug-uninit/ref/out-opt2.txt +++ b/testsuite/debug-uninit/ref/out-opt2.txt @@ -8,3 +8,5 @@ ERROR: [RendererServices::texture] ImageInput::create() called with no filename ERROR: Detected possible use of uninitialized value in float[3] A at test.osl:22 (group unnamed_group_1, layer 0 test_0, shader test, op 7 'aref', arg 1) ERROR: Detected possible use of uninitialized value in color C at test.osl:28 (group unnamed_group_1, layer 0 test_0, shader test, op 12 'compref', arg 1) ERROR: Detected possible use of uninitialized value in matrix M at test.osl:34 (group unnamed_group_1, layer 0 test_0, shader test, op 17 'mxcompref', arg 1) +ERROR: Detected possible use of uninitialized value in float[20] knots3 at test.osl:54 (group unnamed_group_1, layer 0 test_0, shader test, op 44 'spline', arg 4) +ERROR: Detected possible use of uninitialized value in float[20] knots4 at test.osl:55 (group unnamed_group_1, layer 0 test_0, shader test, op 46 'splineinverse', arg 4) diff --git a/testsuite/debug-uninit/ref/out.txt b/testsuite/debug-uninit/ref/out.txt index b549b7390..a0908b811 100644 --- a/testsuite/debug-uninit/ref/out.txt +++ b/testsuite/debug-uninit/ref/out.txt @@ -8,3 +8,5 @@ ERROR: [RendererServices::texture] ImageInput::create() called with no filename ERROR: Detected possible use of uninitialized value in float[3] A at test.osl:22 (group unnamed_group_1, layer 0 test_0, shader test, op 12 'aref', arg 1) ERROR: Detected possible use of uninitialized value in color C at test.osl:28 (group unnamed_group_1, layer 0 test_0, shader test, op 17 'compref', arg 1) ERROR: Detected possible use of uninitialized value in matrix M at test.osl:34 (group unnamed_group_1, layer 0 test_0, shader test, op 22 'mxcompref', arg 1) +ERROR: Detected possible use of uninitialized value in float[20] knots3 at test.osl:54 (group unnamed_group_1, layer 0 test_0, shader test, op 49 'spline', arg 4) +ERROR: Detected possible use of uninitialized value in float[20] knots4 at test.osl:55 (group unnamed_group_1, layer 0 test_0, shader test, op 51 'splineinverse', arg 4) diff --git a/testsuite/debug-uninit/test.osl b/testsuite/debug-uninit/test.osl index 1bb06aec0..92773383d 100644 --- a/testsuite/debug-uninit/test.osl +++ b/testsuite/debug-uninit/test.osl @@ -33,5 +33,29 @@ test (output color Cout = 0) x += M[1][2]; // NOT an error x += M[0][0]; // An error } + float knots[20]; + float knots2[20]; + float knots3[20]; + float knots4[20]; + int index=0; + // Ensure no false positive for temporary condition variable + while (index < 10) { + knots[index] = x*index; + knots2[index] = (1.0-x)*index; + knots3[index] = 2*x*index; + knots4[index] = 2*(1.0-x)*index; + ++index; + } + // Ensure no false positive for accessing initialized portion of a partially initialized array + x += spline ("linear", x, 10, knots); + x += splineinverse ("linear", x, 10, knots2); + + // Accessing uninitialized portion of a partially initialized array + x += spline ("linear", x, 15, knots3); + x += splineinverse ("linear", x, 15, knots4); + + // pointcloud_get could also have false postive and read partially initialized array + // leaving pointcloud_get out, as not all builds are configured for pointcloud support + Cout[0] += x; // force x results to not be optimized away }