[Validation] Prevent instructions that accept handle arguments from accepting malformed handle arguments (#5399)
Except for Output Complete, instructions should not accept handle arguments that are undefs / zeroinitializers, and should emit a diagnostic when this happens. This PR adds tests to specifically exercise the cases where a call instruction : 1. receives an undef argument 2. receives a zeroinitializer argument 3. receives a handle argument that is invalid And combines these cases with all different types of handle arguments (resource handle, node handle, and node record handle) This PR intends to at least implement the first work item in [#5356](https://github.com/microsoft/DirectXShaderCompiler/issues/5356)
This commit is contained in:
Joshua Batista
GitHub
Родитель
f09905d0ec
Коммит
9468120e6c
|
|
@ -1131,6 +1131,51 @@ static int GetCBufSize(Value *cbHandle, ValidationContext &ValCtx) {
|
|||
return RP.CBufferSizeInBytes;
|
||||
}
|
||||
|
||||
// Make sure none of the handle arguments are undef / zero-initializer,
|
||||
// Also, do not accept any resource handles with invalid dxil resource
|
||||
// properties
|
||||
void ValidateHandleArgsForInstruction(CallInst *CI, DXIL::OpCode opcode,
|
||||
ValidationContext &ValCtx) {
|
||||
|
||||
for (Value *op : CI->operands()) {
|
||||
const Type *pHandleTy = ValCtx.HandleTy; // This is a resource handle
|
||||
const Type *pNodeHandleTy = ValCtx.DxilMod.GetOP()->GetNodeHandleType();
|
||||
const Type *pNodeRecordHandleTy =
|
||||
ValCtx.DxilMod.GetOP()->GetNodeRecordHandleType();
|
||||
|
||||
const Type *argTy = op->getType();
|
||||
if (argTy == pNodeHandleTy || argTy == pNodeRecordHandleTy ||
|
||||
argTy == pHandleTy) {
|
||||
|
||||
if (isa<UndefValue>(op) || isa<ConstantAggregateZero>(op)) {
|
||||
ValCtx.EmitInstrError(CI, ValidationRule::InstrNoReadingUninitialized);
|
||||
}
|
||||
else if (argTy == pHandleTy) {
|
||||
// GetResourceFromHandle will emit an error on an invalid handle
|
||||
GetResourceFromHandle(op, ValCtx);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void ValidateHandleArgs(CallInst* CI, DXIL::OpCode opcode, ValidationContext &ValCtx) {
|
||||
|
||||
switch (opcode) {
|
||||
// TODO: add case DXIL::OpCode::IndexNodeRecordHandle:
|
||||
|
||||
case DXIL::OpCode::AnnotateHandle:
|
||||
case DXIL::OpCode::AnnotateNodeHandle:
|
||||
case DXIL::OpCode::AnnotateNodeRecordHandle:
|
||||
case DXIL::OpCode::CreateHandleForLib:
|
||||
// TODO: add custom validation for these intrinsics
|
||||
break;
|
||||
|
||||
default:
|
||||
ValidateHandleArgsForInstruction(CI, opcode, ValCtx);
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
static unsigned GetNumVertices(DXIL::InputPrimitive inputPrimitive) {
|
||||
const unsigned InputPrimitiveVertexTab[] = {
|
||||
0, // Undefined = 0,
|
||||
|
|
@ -2113,6 +2158,8 @@ static void ValidateDxilOperationCallInProfile(CallInst *CI,
|
|||
// Is called from a library function
|
||||
bool isLibFunc = shaderKind == DXIL::ShaderKind::Library;
|
||||
|
||||
ValidateHandleArgs(CI, opcode, ValCtx);
|
||||
|
||||
switch (opcode) {
|
||||
// Imm input value validation.
|
||||
case DXIL::OpCode::Asin:
|
||||
|
|
|
|||
|
|
@ -0,0 +1,132 @@
|
|||
; RUN: %dxv %s | FileCheck %s
|
||||
|
||||
; The purpose of this test is to make sure that for various dxil ops that take in one
|
||||
; of three handle types: Handle (resource handles), NodeHandle, and NodeRecordHandle,
|
||||
; when the argument given to these types is zeroinitializer or undef, an
|
||||
; error gets emitted, which proves there is validation that handle arguments
|
||||
; aren't ill-formed.
|
||||
|
||||
|
||||
target datalayout = "e-m:e-p:32:32-i1:32-i8:32-i16:32-i32:32-i64:64-f16:32-f32:32-f64:64-n8:16:32:64"
|
||||
target triple = "dxil-ms-dx"
|
||||
|
||||
%dx.types.Handle = type { i8* }
|
||||
%dx.types.NodeHandle = type { i8* }
|
||||
%dx.types.NodeInfo = type { i32, i32 }
|
||||
%dx.types.NodeRecordHandle = type { i8* }
|
||||
%dx.types.NodeRecordInfo = type { i32, i32 }
|
||||
%struct.loadStressRecord.0 = type { [3 x i32], [3 x i32] }
|
||||
|
||||
define void @loadStress_16() {
|
||||
%1 = call i32 @dx.op.flattenedThreadIdInGroup.i32(i32 96)
|
||||
%2 = call %dx.types.NodeHandle @dx.op.createNodeOutputHandle(i32 247, i32 0)
|
||||
%3 = call %dx.types.NodeHandle @dx.op.annotateNodeHandle(i32 249, %dx.types.NodeHandle %2, %dx.types.NodeInfo { i32 6, i32 24 })
|
||||
%4 = call %dx.types.NodeRecordHandle @dx.op.allocateNodeOutputRecords(i32 238, %dx.types.NodeHandle %3, i32 1, i1 true)
|
||||
%5 = call %dx.types.NodeRecordHandle @dx.op.annotateNodeRecordHandle(i32 251, %dx.types.NodeRecordHandle %4, %dx.types.NodeRecordInfo { i32 38, i32 24 })
|
||||
|
||||
; Test a Dxil op with a NodeRecordHandle handle type
|
||||
; CHECK-DAG: error: Instructions should not read uninitialized value.
|
||||
; CHECK-DAG: note: at '%6 = call %struct.loadStressRecord.0 addrspace(6)* @dx.op.getNodeRecordPtr.struct.loadStressRecord.0
|
||||
%6 = call %struct.loadStressRecord.0 addrspace(6)* @dx.op.getNodeRecordPtr.struct.loadStressRecord.0(i32 239, %dx.types.NodeRecordHandle zeroinitializer, i32 0)
|
||||
|
||||
; Test that OutputComplete fails when it receives an undef.
|
||||
; OutputComplete also serves as a test for NodeRecordHandle handle types.
|
||||
; CHECK-DAG: error: Instructions should not read uninitialized value.
|
||||
; CHECK-DAG: note: at 'call void @dx.op.outputComplete(i32 241, %dx.types.NodeRecordHandle undef)
|
||||
call void @dx.op.outputComplete(i32 241, %dx.types.NodeRecordHandle undef)
|
||||
|
||||
|
||||
; Test that OutputComplete fails when it receives an undef.
|
||||
; CHECK-DAG: error: Instructions should not read uninitialized value.
|
||||
; CHECK-DAG: note: at 'call void @dx.op.outputComplete(i32 241, %dx.types.NodeRecordHandle zeroinitializer)
|
||||
call void @dx.op.outputComplete(i32 241, %dx.types.NodeRecordHandle zeroinitializer)
|
||||
|
||||
; Test a Dxil op with a Resource handle type (mixing in undef and zeroinitializer)
|
||||
; (it should be noted that handles with zeroinitialize / undef values are invalid according to DxilResourceProperties)
|
||||
; CHECK-DAG: error: Instructions should not read uninitialized value.
|
||||
; CHECK-DAG: note: at 'call void @dx.op.writeSamplerFeedbackLevel(i32 176, %dx.types.Handle undef,
|
||||
|
||||
; CHECK-DAG: error: Instructions should not read uninitialized value.
|
||||
; CHECK-DAG: note: at 'call void @dx.op.writeSamplerFeedbackLevel(i32 176, %dx.types.Handle undef, %dx.types.Handle zeroinitializer,
|
||||
|
||||
; CHECK-DAG: error: Instructions should not read uninitialized value.
|
||||
; CHECK-DAG: note: at 'call void @dx.op.writeSamplerFeedbackLevel(i32 176, %dx.types.Handle undef, %dx.types.Handle zeroinitializer, %dx.types.Handle undef
|
||||
call void @dx.op.writeSamplerFeedbackLevel(i32 176, %dx.types.Handle undef, %dx.types.Handle zeroinitializer, %dx.types.Handle undef, float 2.100000e+01, float 2.200000e+01, float undef, float undef, float 6.000000e+00) ; WriteSamplerFeedbackLevel(feedbackTex,sampledTex,sampler,c0,c1,c2,c3,lod)
|
||||
|
||||
; Test a dxil op with a NodeHandle handle type as undef
|
||||
; CHECK-DAG: error: Instructions should not read uninitialized value.
|
||||
; CHECK-DAG: note: at 'call void @dx.op.incrementOutputCount(i32 240, %dx.types.NodeHandle undef, i32 1, i1 false)
|
||||
call void @dx.op.incrementOutputCount(i32 240, %dx.types.NodeHandle undef, i32 1, i1 false)
|
||||
|
||||
; Test a dxil op with a NodeHandle handle type as zeroinitializer
|
||||
; CHECK-DAG: error: Instructions should not read uninitialized value.
|
||||
; CHECK-DAG: note: at 'call void @dx.op.incrementOutputCount(i32 240, %dx.types.NodeHandle zeroinitializer, i32 1, i1 false)
|
||||
call void @dx.op.incrementOutputCount(i32 240, %dx.types.NodeHandle zeroinitializer, i32 1, i1 false)
|
||||
|
||||
br label %7
|
||||
; <label>:7 ; preds = %0
|
||||
ret void
|
||||
}
|
||||
|
||||
; Function Attrs: nounwind readnone
|
||||
declare i32 @dx.op.flattenedThreadIdInGroup.i32(i32) #0
|
||||
|
||||
; Function Attrs: nounwind readnone
|
||||
declare %struct.loadStressRecord.0 addrspace(6)* @dx.op.getNodeRecordPtr.struct.loadStressRecord.0(i32, %dx.types.NodeRecordHandle, i32) #0
|
||||
|
||||
; Function Attrs: nounwind
|
||||
declare void @dx.op.writeSamplerFeedbackLevel(i32, %dx.types.Handle, %dx.types.Handle, %dx.types.Handle, float, float, float, float, float) #0
|
||||
|
||||
; Function Attrs: nounwind
|
||||
declare %dx.types.NodeRecordHandle @dx.op.allocateNodeOutputRecords(i32, %dx.types.NodeHandle, i32, i1) #1
|
||||
|
||||
; Function Attrs: nounwind
|
||||
declare void @dx.op.outputComplete(i32, %dx.types.NodeRecordHandle) #1
|
||||
|
||||
; Function Attrs: nounwind readnone
|
||||
declare %dx.types.NodeRecordHandle @dx.op.annotateNodeRecordHandle(i32, %dx.types.NodeRecordHandle, %dx.types.NodeRecordInfo) #0
|
||||
|
||||
; Function Attrs: nounwind readnone
|
||||
declare %dx.types.NodeHandle @dx.op.createNodeOutputHandle(i32, i32) #0
|
||||
|
||||
; Function Attrs: nounwind readnone
|
||||
declare %dx.types.NodeHandle @dx.op.annotateNodeHandle(i32, %dx.types.NodeHandle, %dx.types.NodeInfo) #0
|
||||
|
||||
; Function Attrs: nounwind readnone
|
||||
declare void @dx.op.incrementOutputCount(i32, %dx.types.NodeHandle, i32, i1) #0
|
||||
|
||||
attributes #0 = { nounwind readnone }
|
||||
attributes #1 = { nounwind }
|
||||
|
||||
!llvm.ident = !{!0}
|
||||
!dx.version = !{!1}
|
||||
!dx.valver = !{!1}
|
||||
!dx.shaderModel = !{!2}
|
||||
!dx.typeAnnotations = !{!3}
|
||||
!dx.entryPoints = !{!7, !8}
|
||||
|
||||
!0 = !{!"dxc(private) 1.7.0.4790 (work-graphs, 35d890870)"}
|
||||
!1 = !{i32 1, i32 8}
|
||||
!2 = !{!"lib", i32 6, i32 8}
|
||||
!3 = !{i32 1, void ()* @loadStress_16, !4}
|
||||
!4 = !{!5}
|
||||
!5 = !{i32 0, !6, !6}
|
||||
!6 = !{}
|
||||
!7 = !{null, !"", null, null, null}
|
||||
!8 = !{void ()* @loadStress_16, !"loadStress_16", null, null, !9}
|
||||
!9 = !{i32 8, i32 15, i32 13, i32 1, i32 15, !10, i32 16, i32 -1, i32 22, !11, i32 20, !12, i32 21, !14, i32 4, !19, i32 5, !20}
|
||||
!10 = !{!"loadStress_16", i32 0}
|
||||
!11 = !{i32 3, i32 1, i32 1}
|
||||
!12 = !{!13}
|
||||
!13 = !{i32 1, i32 9}
|
||||
!14 = !{!15}
|
||||
!15 = !{i32 1, i32 6, i32 2, !16, i32 3, i32 0, i32 0, !18}
|
||||
!16 = !{i32 0, i32 24, i32 1, !17}
|
||||
!17 = !{i32 0, i32 5, i32 3}
|
||||
!18 = !{!"loadStressChild", i32 0}
|
||||
!19 = !{i32 1, i32 1, i32 1}
|
||||
!20 = !{i32 0}
|
||||
!21 = !{!22, !22, i64 0}
|
||||
!22 = !{!"int", !23, i64 0}
|
||||
!23 = !{!"omnipotent char", !24, i64 0}
|
||||
!24 = !{!"Simple C/C++ TBAA"}
|
||||
|
|
@ -1,12 +1,8 @@
|
|||
// RUN: %dxc -T lib_6_8 %s | FileCheck %s
|
||||
|
||||
// CHECK: call void @dx.op.outputComplete(i32 241, %dx.types.NodeRecordHandle %{{[0-9]+}})
|
||||
|
||||
// TBD: Prevent this use of null handle
|
||||
// (%dx.types.NodeRecordHandle zeroinitializer) by failing validation.
|
||||
// Next, prevent this even earlier if possible.
|
||||
// CHECK: call %struct.loadStressRecord.0 addrspace(6)* @dx.op.getNodeRecordPtr.struct.loadStressRecord.0(i32 239, %dx.types.NodeRecordHandle zeroinitializer, i32 0)
|
||||
|
||||
// TBD: Prevent (%dx.types.NodeRecordHandle zeroinitializer) even earlier if possible.
|
||||
// CHECK: error: Instructions should not read uninitialized value.
|
||||
// CHECK-NEXT: call %struct.loadStressRecord.0 addrspace(6)* @dx.op.getNodeRecordPtr.struct.loadStressRecord.0(i32 239, %dx.types.NodeRecordHandle zeroinitializer,
|
||||
#define LOAD_STRESS_MAX_GRID_SIZE 3
|
||||
#define GROUP_SHARED_SIZE 128
|
||||
|
||||
|
|
@ -37,6 +33,7 @@ void loadStressWorker(
|
|||
outRec.OutputComplete();
|
||||
if (threadIndex % 3) {
|
||||
// error: Invalid use of completed record handle.
|
||||
// here is where the zeroinitializer is used as a handle value
|
||||
outRec.Get().data[0] = 0;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче