Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix infinite recursion in ParseDescriptorBlockVariableUsage() #73

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions common/output_stream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,26 @@ std::string ToStringDecorationFlags(SpvReflectDecorationFlags decoration_flags)
return sstream.str();
}

std::string ToStringVariableFlags(SpvReflectVariableFlags variable_flags) {
if (variable_flags == SPV_REFLECT_VARIABLE_FLAGS_NONE) {
return "NONE";
}

#define PRINT_AND_CLEAR_VARIABLE_FLAG(stream, flags, bit) \
if (( (flags) & (SPV_REFLECT_VARIABLE_FLAGS_##bit) ) == (SPV_REFLECT_VARIABLE_FLAGS_##bit)) { \
stream << #bit << " "; \
flags ^= SPV_REFLECT_VARIABLE_FLAGS_##bit; \
}
std::stringstream sstream;
PRINT_AND_CLEAR_VARIABLE_FLAG(sstream, variable_flags, UNUSED);
#undef PRINT_AND_CLEAR_DECORATION_FLAG
if (variable_flags != 0) {
// Unhandled SpvReflectVariableFlags bit
sstream << "???";
}
return sstream.str();
}

std::string ToStringFormat(SpvReflectFormat fmt) {
switch(fmt) {
case SPV_REFLECT_FORMAT_UNDEFINED : return "VK_FORMAT_UNDEFINED";
Expand Down Expand Up @@ -1001,6 +1021,7 @@ void WriteReflection(const spv_reflect::ShaderModule& obj, bool flatten_cbuffers
}
}
}
(void)result;
}

//////////////////////////////////
Expand Down Expand Up @@ -1189,6 +1210,9 @@ void SpvReflectToYaml::WriteBlockVariable(std::ostream& os, const SpvReflectBloc
// } SpvReflectArrayTraits;
os << " }" << std::endl;

// SpvReflectVariableFlags flags;
os << t1 << "flags: " << AsHexString(bv.flags) << " # " << ToStringVariableFlags(bv.flags) << std::endl;

// uint32_t member_count;
os << t1 << "member_count: " << bv.member_count << std::endl;
// struct SpvReflectBlockVariable* members;
Expand Down
3 changes: 2 additions & 1 deletion common/output_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ std::string ToStringResourceType(SpvReflectResourceType type);
std::string ToStringDescriptorType(SpvReflectDescriptorType value);
std::string ToStringTypeFlags(SpvReflectTypeFlags type_flags);
std::string ToStringDecorationFlags(SpvReflectDecorationFlags decoration_flags);
std::string ToStringVariableFlags(SpvReflectVariableFlags variable_flags);
std::string ToStringDescriptorType(SpvReflectDescriptorType value);
std::string ToStringFormat(SpvReflectFormat fmt);
std::string ToStringComponentType(const SpvReflectTypeDescription& type, uint32_t member_decoration_flags);
Expand Down Expand Up @@ -69,4 +70,4 @@ class SpvReflectToYaml {
};


#endif
#endif
58 changes: 30 additions & 28 deletions spirv_reflect.c
Original file line number Diff line number Diff line change
Expand Up @@ -2203,7 +2203,11 @@ static SpvReflectResult ParseDescriptorBlockVariableUsage(

// Clear the current variable's USED flag
p_var->flags &= ~SPV_REFLECT_VARIABLE_FLAGS_UNUSED;


// We are done when the end of the access chain is reached
if (index_index >= p_access_chain->index_count)
return SPV_REFLECT_RESULT_SUCCESS;

// Parsing arrays requires overriding the op type for
// for the lowest dim's element type.
SpvOp op_type = p_var->type_description->op;
Expand All @@ -2212,32 +2216,32 @@ static SpvReflectResult ParseDescriptorBlockVariableUsage(
}

switch (op_type) {
default: break;
default: {
// no more variable flags to update (a SpvOpTypeVector access, for example.)
break;
}

case SpvOpTypeArray: {
// Parse through array's type hierarchy to find the actual/non-array element type
SpvReflectTypeDescription* p_type = p_var->type_description;
while ((p_type->op == SpvOpTypeArray) && (index_index < p_access_chain->index_count)) {
// Find the array element type id
Node* p_node = FindNode(p_parser, p_type->id);
if (p_node == NULL) {
return SPV_REFLECT_RESULT_ERROR_SPIRV_INVALID_ID_REFERENCE;
}
uint32_t element_type_id = p_node->array_traits.element_type_id;
// Get the array element type
p_type = FindType(p_module, element_type_id);
if (p_type == NULL) {
return SPV_REFLECT_RESULT_ERROR_SPIRV_INVALID_ID_REFERENCE;
}
// Next access index
index_index += 1;
// Find the array element type id
Node* p_node = FindNode(p_parser, p_type->id);
if (p_node == NULL) {
return SPV_REFLECT_RESULT_ERROR_SPIRV_INVALID_ID_REFERENCE;
}
uint32_t element_type_id = p_node->array_traits.element_type_id;
// Get the array element type
p_type = FindType(p_module, element_type_id);
if (p_type == NULL) {
return SPV_REFLECT_RESULT_ERROR_SPIRV_INVALID_ID_REFERENCE;
}

// Parse current var again with a type override and advanced index index
SpvReflectResult result = ParseDescriptorBlockVariableUsage(
p_parser,
p_module,
p_access_chain,
index_index,
index_index+1,
p_type->op,
p_var);
if (result != SPV_REFLECT_RESULT_SUCCESS) {
Expand All @@ -2259,17 +2263,15 @@ static SpvReflectResult ParseDescriptorBlockVariableUsage(
}

SpvReflectBlockVariable* p_member_var = &p_var->members[index];
if (index_index < p_access_chain->index_count) {
SpvReflectResult result = ParseDescriptorBlockVariableUsage(
p_parser,
p_module,
p_access_chain,
index_index + 1,
(SpvOp)INVALID_VALUE,
p_member_var);
if (result != SPV_REFLECT_RESULT_SUCCESS) {
return result;
}
SpvReflectResult result = ParseDescriptorBlockVariableUsage(
p_parser,
p_module,
p_access_chain,
index_index + 1,
(SpvOp)INVALID_VALUE,
p_member_var);
if (result != SPV_REFLECT_RESULT_SUCCESS) {
return result;
}
}
break;
Expand Down
8 changes: 7 additions & 1 deletion tests/build_all_shaders.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,19 @@ def my_which(cmd):
{'source':"glsl/built_in_format.glsl", 'entry':"main", 'stage':'vert'},
{'source':"glsl/input_attachment.glsl", 'entry':"main", 'stage':'frag'},
{'source':"glsl/texel_buffer.glsl", 'entry':"main", 'stage':'vert'},
{'source':"glsl/io_vars_vs.glsl", 'entry':"main", 'stage':'vert'},

{'source':"hlsl/append_consume.hlsl", 'entry':"main", 'profile':'ps_6_0', 'stage':'frag'},
{'source':"hlsl/array_of_structured_buffer.hlsl", 'entry':"main", 'profile':'cs_6_0', 'stage':'compute'},
{'source':"hlsl/binding_array.hlsl", 'entry':"main", 'profile':'ps_6_0', 'stage':'frag'},
{'source':"hlsl/binding_types.hlsl", 'entry':"main", 'profile':'ps_6_0', 'stage':'frag'},
{'source':"hlsl/cbuffer.hlsl", 'entry':"main", 'profile':'vs_6_0', 'stage':'vert'},
{'source':"hlsl/constantbuffer.hlsl", 'entry':"main", 'profile':'vs_6_0', 'stage':'vert'},
{'source':"hlsl/constantbuffer_nested_structs.hlsl", 'entry':"main", 'profile':'vs_6_0', 'stage':'vert'},
{'source':"hlsl/counter_buffers.hlsl", 'entry':"main", 'profile':'ps_6_0', 'stage':'frag'},
{'source':"hlsl/semantics.hlsl", 'entry':"main", 'profile':'ps_6_0', 'stage':'frag'},
{'source':"hlsl/structuredbuffer.hlsl", 'entry':"main", 'profile':'ps_6_0', 'stage':'frag'},
{'source':"cbuffer_unused/cbuffer_unused_001.hlsl", 'entry':"main", 'profile':'vs_6_0', 'stage':'vert'},
]

if __name__ == "__main__":
Expand All @@ -51,7 +57,7 @@ def my_which(cmd):
if ext.lower() == ".glsl" or (ext.lower() == ".hlsl" and not args.dxc):
compile_cmd_args = [args.glslc, "-g", "-fshader-stage=" + shader['stage'], "-fentry-point=" + shader['entry'], "-o", spv_path, src_path]
elif ext.lower() == ".hlsl":
compile_cmd_args = [args.dxc, "-spirv", "-Zi", "-fspv-reflect", "-O0", "-T", shader['profile'], "-E", shader['entry'], "-Fo", spv_path, src_path]
compile_cmd_args = [args.dxc, "-spirv", "-Zi", "-fspv-reflect", "-fvk-use-dx-layout", "-O0", "-T", shader['profile'], "-E", shader['entry'], "-Fo", spv_path, src_path]

print("%s -> %s" % (src_path, spv_path))
if args.verbose:
Expand Down
3 changes: 2 additions & 1 deletion tests/build_golden_yaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@
yaml_cmd_args = [spirv_reflect_exe, "-y", "-v", "1", spv_path]
if args.verbose:
print(" ".join(yaml_cmd_args))
subprocess.call(yaml_cmd_args, stdout=file(yaml_path, "w"))
with open(yaml_path, "w") as f:
subprocess.call(yaml_cmd_args, stdout=f)
print("%s -> %s" % (spv_path, yaml_path))
except NameError:
print("spirv-reflect application not found; did you build it first?")
Expand Down
Binary file modified tests/cbuffer_unused/cbuffer_unused_001.spv
Binary file not shown.
Loading