[PATCH v7 0/4] MR582: vkd3d-dxbc: Add options to fix a DXBC shader checksum.
-- v7: vkd3d-dxbc: Add an option to choose the output filename. vkd3d-dxbc: Add an option to re-emit the shader with the correct checksum. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/582
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- include/vkd3d_shader.h | 19 ++++++++++++++++--- libs/vkd3d-shader/dxbc.c | 31 +++++++++++++++++-------------- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/include/vkd3d_shader.h b/include/vkd3d_shader.h index 9e663919c..04640e6c2 100644 --- a/include/vkd3d_shader.h +++ b/include/vkd3d_shader.h @@ -212,6 +212,20 @@ enum vkd3d_shader_compile_option_feature_flags VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_COMPILE_OPTION_FEATURE_FLAGS), }; +/** + * Flags for vkd3d_shader_parse_dxbc(). + * + * \since 1.12 + */ +enum vkd3d_shader_parse_dxbc_flags +{ + /** Ignore the checksum and continue parsing even if it is + * incorrect. */ + VKD3D_SHADER_PARSE_DXBC_IGNORE_CHECKSUM = 0x00000001, + + VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_PARSE_DXBC_FLAGS), +}; + enum vkd3d_shader_compile_option_name { /** @@ -2377,9 +2391,8 @@ VKD3D_SHADER_API void vkd3d_shader_free_dxbc(struct vkd3d_shader_dxbc_desc *dxbc * * \param dxbc A vkd3d_shader_code structure containing the DXBC blob to parse. * - * \param flags A set of flags modifying the behaviour of the function. No - * flags are defined for this version of vkd3d-shader, and this parameter - * should be set to 0. + * \param flags A combination of zero or more elements of enum + * vkd3d_shader_parse_dxbc_flags. * * \param desc A vkd3d_shader_dxbc_desc structure describing the contents of * the DXBC blob. Its vkd3d_shader_dxbc_section_desc structures will contain diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index 37ebc73c0..4a8a7b138 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -150,7 +150,7 @@ static const char *shader_get_string(const char *data, size_t data_size, size_t } static int parse_dxbc(const struct vkd3d_shader_code *dxbc, struct vkd3d_shader_message_context *message_context, - const char *source_name, struct vkd3d_shader_dxbc_desc *desc) + const char *source_name, uint32_t flags, struct vkd3d_shader_dxbc_desc *desc) { const struct vkd3d_shader_location location = {.source_name = source_name}; struct vkd3d_shader_dxbc_section_desc *sections, *section; @@ -186,17 +186,20 @@ static int parse_dxbc(const struct vkd3d_shader_code *dxbc, struct vkd3d_shader_ checksum[1] = read_u32(&ptr); checksum[2] = read_u32(&ptr); checksum[3] = read_u32(&ptr); - vkd3d_compute_dxbc_checksum(data, data_size, calculated_checksum); - if (memcmp(checksum, calculated_checksum, sizeof(checksum))) - { - WARN("Checksum {0x%08x, 0x%08x, 0x%08x, 0x%08x} does not match " - "calculated checksum {0x%08x, 0x%08x, 0x%08x, 0x%08x}.\n", - checksum[0], checksum[1], checksum[2], checksum[3], - calculated_checksum[0], calculated_checksum[1], - calculated_checksum[2], calculated_checksum[3]); - vkd3d_shader_error(message_context, &location, VKD3D_SHADER_ERROR_DXBC_INVALID_CHECKSUM, - "Invalid DXBC checksum."); - return VKD3D_ERROR_INVALID_ARGUMENT; + if (!(flags & VKD3D_SHADER_PARSE_DXBC_IGNORE_CHECKSUM)) + { + vkd3d_compute_dxbc_checksum(data, data_size, calculated_checksum); + if (memcmp(checksum, calculated_checksum, sizeof(checksum))) + { + WARN("Checksum {0x%08x, 0x%08x, 0x%08x, 0x%08x} does not match " + "calculated checksum {0x%08x, 0x%08x, 0x%08x, 0x%08x}.\n", + checksum[0], checksum[1], checksum[2], checksum[3], + calculated_checksum[0], calculated_checksum[1], + calculated_checksum[2], calculated_checksum[3]); + vkd3d_shader_error(message_context, &location, VKD3D_SHADER_ERROR_DXBC_INVALID_CHECKSUM, + "Invalid DXBC checksum."); + return VKD3D_ERROR_INVALID_ARGUMENT; + } } version = read_u32(&ptr); @@ -287,7 +290,7 @@ static int for_each_dxbc_section(const struct vkd3d_shader_code *dxbc, unsigned int i; int ret; - if ((ret = parse_dxbc(dxbc, message_context, source_name, &desc)) < 0) + if ((ret = parse_dxbc(dxbc, message_context, source_name, 0, &desc)) < 0) return ret; for (i = 0; i < desc.section_count; ++i) @@ -313,7 +316,7 @@ int vkd3d_shader_parse_dxbc(const struct vkd3d_shader_code *dxbc, *messages = NULL; vkd3d_shader_message_context_init(&message_context, VKD3D_SHADER_LOG_INFO); - ret = parse_dxbc(dxbc, &message_context, NULL, desc); + ret = parse_dxbc(dxbc, &message_context, NULL, flags, desc); vkd3d_shader_message_context_trace_messages(&message_context); if (!vkd3d_shader_message_context_copy_messages(&message_context, messages) && ret >= 0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/582
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- programs/vkd3d-dxbc/main.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/programs/vkd3d-dxbc/main.c b/programs/vkd3d-dxbc/main.c index e76bcb631..6d5d64805 100644 --- a/programs/vkd3d-dxbc/main.c +++ b/programs/vkd3d-dxbc/main.c @@ -37,6 +37,7 @@ enum { OPTION_COLOUR = CHAR_MAX + 1, OPTION_HELP, + OPTION_IGNORE_CHECKSUM, OPTION_LIST, OPTION_LIST_DATA, OPTION_NO_COLOUR, @@ -50,6 +51,7 @@ struct options bool list; bool list_data; bool print_version; + bool ignore_checksum; struct colours { @@ -86,13 +88,14 @@ static bool parse_command_line(int argc, char **argv, struct options *options) static struct option long_options[] = { - {"colour", no_argument, NULL, OPTION_COLOUR}, - {"help", no_argument, NULL, OPTION_HELP}, - {"list", no_argument, NULL, OPTION_LIST}, - {"list-data", no_argument, NULL, OPTION_LIST_DATA}, - {"no-colour", no_argument, NULL, OPTION_NO_COLOUR}, - {"version", no_argument, NULL, OPTION_VERSION}, - {NULL, 0, NULL, 0}, + {"colour", no_argument, NULL, OPTION_COLOUR}, + {"help", no_argument, NULL, OPTION_HELP}, + {"ignore-checksum", no_argument, NULL, OPTION_IGNORE_CHECKSUM}, + {"list", no_argument, NULL, OPTION_LIST}, + {"list-data", no_argument, NULL, OPTION_LIST_DATA}, + {"no-colour", no_argument, NULL, OPTION_NO_COLOUR}, + {"version", no_argument, NULL, OPTION_VERSION}, + {NULL, 0, NULL, 0}, }; static const struct colours colours = @@ -133,6 +136,10 @@ static bool parse_command_line(int argc, char **argv, struct options *options) options->print_help = true; return true; + case OPTION_IGNORE_CHECKSUM: + options->ignore_checksum = true; + break; + case 't': case OPTION_LIST: options->list = true; @@ -173,6 +180,7 @@ static void print_usage(const char *program_name) " --list-data List the data contained in the DXBC sections.\n" " --no-colour Disable colour, even when supported by the output.\n" " -V, --version Display version information and exit.\n" + " --ignore-checksum Do not validate the checksum when parsing the DXBC blob.\n" " -- Stop option processing. Any subsequent argument is\n" " interpreted as a filename.\n" "\n" @@ -376,6 +384,7 @@ int main(int argc, char **argv) struct options options; bool close_input; char *messages; + uint32_t flags; int fail = 1; FILE *input; int ret; @@ -409,7 +418,11 @@ int main(int argc, char **argv) goto done; } - ret = vkd3d_shader_parse_dxbc(&dxbc, 0, &dxbc_desc, &messages); + flags = 0; + if (options.ignore_checksum) + flags |= VKD3D_SHADER_PARSE_DXBC_IGNORE_CHECKSUM; + + ret = vkd3d_shader_parse_dxbc(&dxbc, flags, &dxbc_desc, &messages); if (messages) fputs(messages, stderr); vkd3d_shader_free_messages(messages); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/582
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- programs/vkd3d-dxbc/main.c | 135 +++++++++++++++++++++++++++++++++++-- 1 file changed, 128 insertions(+), 7 deletions(-) diff --git a/programs/vkd3d-dxbc/main.c b/programs/vkd3d-dxbc/main.c index 6d5d64805..36c8983d9 100644 --- a/programs/vkd3d-dxbc/main.c +++ b/programs/vkd3d-dxbc/main.c @@ -33,9 +33,38 @@ #include <term.h> #endif +static bool array_reserve(void **elements, size_t *capacity, size_t element_count, size_t element_size) +{ + size_t new_capacity, max_capacity; + void *new_elements; + + if (element_count <= *capacity) + return true; + + max_capacity = ~(size_t)0 / element_size; + if (max_capacity < element_count) + return false; + + new_capacity = max(*capacity, 4); + while (new_capacity < element_count && new_capacity <= max_capacity / 2) + new_capacity *= 2; + + if (new_capacity < element_count) + new_capacity = element_count; + + if (!(new_elements = realloc(*elements, new_capacity * element_size))) + return false; + + *elements = new_elements; + *capacity = new_capacity; + + return true; +} + enum { OPTION_COLOUR = CHAR_MAX + 1, + OPTION_EMIT, OPTION_HELP, OPTION_IGNORE_CHECKSUM, OPTION_LIST, @@ -44,6 +73,14 @@ enum OPTION_VERSION, }; +struct action +{ + enum action_type + { + ACTION_TYPE_EMIT, + } type; +}; + struct options { const char *input_filename; @@ -53,6 +90,9 @@ struct options bool print_version; bool ignore_checksum; + struct action *actions; + size_t action_count, action_capacity; + struct colours { const char *reset; @@ -62,6 +102,23 @@ struct options } colours; }; +static struct action *action_push(struct options *options, enum action_type type) +{ + struct action *action; + + if (!array_reserve((void **)&options->actions, &options->action_capacity, + options->action_count + 1, sizeof(*options->actions))) + { + fprintf(stderr, "Out of memory.\n"); + exit(1); + } + + action = &options->actions[options->action_count++]; + action->type = type; + + return action; +} + static bool has_colour(void) { #ifdef HAVE_NCURSES @@ -89,6 +146,7 @@ static bool parse_command_line(int argc, char **argv, struct options *options) static struct option long_options[] = { {"colour", no_argument, NULL, OPTION_COLOUR}, + {"emit", no_argument, NULL, OPTION_EMIT}, {"help", no_argument, NULL, OPTION_HELP}, {"ignore-checksum", no_argument, NULL, OPTION_IGNORE_CHECKSUM}, {"list", no_argument, NULL, OPTION_LIST}, @@ -122,7 +180,7 @@ static bool parse_command_line(int argc, char **argv, struct options *options) for (;;) { - if ((option = getopt_long(argc, argv, "htV", long_options, NULL)) == -1) + if ((option = getopt_long(argc, argv, "ehtV", long_options, NULL)) == -1) break; switch (option) @@ -131,6 +189,16 @@ static bool parse_command_line(int argc, char **argv, struct options *options) options->colours = colours; break; + case 'e': + case OPTION_EMIT: + action_push(options, ACTION_TYPE_EMIT); + if (isatty(fileno(stdout))) + { + fprintf(stderr, "Output is a tty and output format is binary, exiting.\n"); + return false; + } + break; + case 'h': case OPTION_HELP: options->print_help = true; @@ -175,6 +243,7 @@ static void print_usage(const char *program_name) "[options...] [file]\n" "Options:\n" " --colour Enable colour, even when not supported by the output.\n" + " -e, --emit Emit the content of the DXBC blob to the standard output.\n" " -h, --help Display this information and exit.\n" " -t, --list List the contents of the DXBC blob.\n" " --list-data List the data contained in the DXBC sections.\n" @@ -185,7 +254,11 @@ static void print_usage(const char *program_name) " interpreted as a filename.\n" "\n" "If the input file is '-' or not specified, input will be read from standard\n" - "input.\n"; + "input.\n" + "\n" + "Currently this tool can only re-emit the same DXBC blob it loaded. However, it\n" + "will recompute the checksum while doing so, so --emit can be useful\n" + "together with --ignore-checksum to fix the checksum for a blob.\n"; fprintf(stderr, "Usage: %s %s", program_name, usage); } @@ -264,6 +337,21 @@ static bool read_input(FILE *f, struct vkd3d_shader_code *dxbc) return true; } +static bool write_output(FILE *f, const struct vkd3d_shader_code *dxbc) +{ + const char *code = dxbc->code; + size_t pos = 0, ret; + + while (pos != dxbc->size) + { + if (!(ret = fwrite(&code[pos], 1, dxbc->size - pos, f))) + return false; + pos += ret; + } + + return true; +} + static const char *dump_tag(char *out, uint32_t tag) { unsigned int i; @@ -379,26 +467,28 @@ static void dump_dxbc(const struct vkd3d_shader_code *dxbc, const struct vkd3d_s int main(int argc, char **argv) { + struct vkd3d_shader_code dxbc, serialized; struct vkd3d_shader_dxbc_desc dxbc_desc; - struct vkd3d_shader_code dxbc; + bool close_input = false; struct options options; - bool close_input; char *messages; uint32_t flags; int fail = 1; FILE *input; + size_t i; int ret; if (!parse_command_line(argc, argv, &options)) { print_usage(argv[0]); - return 1; + goto done; } if (options.print_help || (argc < 2 && isatty(fileno(stdin)))) { print_usage(argv[0]); - return 0; + fail = 0; + goto done; } if (options.print_version) @@ -406,7 +496,8 @@ int main(int argc, char **argv) const char *version = vkd3d_shader_get_version(NULL, NULL); fprintf(stdout, "vkd3d-dxbc version " PACKAGE_VERSION " using %s\n", version); - return 0; + fail = 0; + goto done; } if (!(input = open_input(options.input_filename, &close_input))) @@ -432,10 +523,40 @@ int main(int argc, char **argv) if (options.list || options.list_data) dump_dxbc(&dxbc, &dxbc_desc, &options); + for (i = 0; i < options.action_count; ++i) + { + struct action *action = &options.actions[i]; + + switch (action->type) + { + case ACTION_TYPE_EMIT: + ret = vkd3d_shader_serialize_dxbc(dxbc_desc.section_count, dxbc_desc.sections, &serialized, &messages); + if (messages) + fputs(messages, stderr); + vkd3d_shader_free_messages(messages); + if (ret < 0) + goto done; + + if (!write_output(stdout, &serialized)) + { + fprintf(stderr, "Failed to write output blob.\n"); + vkd3d_shader_free_shader_code(&serialized); + goto done; + } + + vkd3d_shader_free_shader_code(&serialized); + break; + + default: + vkd3d_unreachable(); + } + } + vkd3d_shader_free_dxbc(&dxbc_desc); vkd3d_shader_free_shader_code(&dxbc); fail = 0; done: + free(options.actions); if (close_input) fclose(input); return fail; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/582
From: Giovanni Mascellani <gmascellani(a)codeweavers.com> --- programs/vkd3d-dxbc/main.c | 68 ++++++++++++++++++++++++++++++++------ 1 file changed, 58 insertions(+), 10 deletions(-) diff --git a/programs/vkd3d-dxbc/main.c b/programs/vkd3d-dxbc/main.c index 36c8983d9..efe0a8fd1 100644 --- a/programs/vkd3d-dxbc/main.c +++ b/programs/vkd3d-dxbc/main.c @@ -70,6 +70,7 @@ enum OPTION_LIST, OPTION_LIST_DATA, OPTION_NO_COLOUR, + OPTION_OUTPUT, OPTION_VERSION, }; @@ -79,11 +80,16 @@ struct action { ACTION_TYPE_EMIT, } type; + union + { + const char *output_filename; + } u; }; struct options { const char *input_filename; + const char *output_filename; bool print_help; bool list; bool list_data; @@ -141,6 +147,7 @@ static bool has_colour(void) static bool parse_command_line(int argc, char **argv, struct options *options) { + struct action *action; int option; static struct option long_options[] = @@ -152,6 +159,7 @@ static bool parse_command_line(int argc, char **argv, struct options *options) {"list", no_argument, NULL, OPTION_LIST}, {"list-data", no_argument, NULL, OPTION_LIST_DATA}, {"no-colour", no_argument, NULL, OPTION_NO_COLOUR}, + {"output", required_argument, NULL, OPTION_OUTPUT}, {"version", no_argument, NULL, OPTION_VERSION}, {NULL, 0, NULL, 0}, }; @@ -180,7 +188,7 @@ static bool parse_command_line(int argc, char **argv, struct options *options) for (;;) { - if ((option = getopt_long(argc, argv, "ehtV", long_options, NULL)) == -1) + if ((option = getopt_long(argc, argv, "ehtVo:", long_options, NULL)) == -1) break; switch (option) @@ -191,12 +199,14 @@ static bool parse_command_line(int argc, char **argv, struct options *options) case 'e': case OPTION_EMIT: - action_push(options, ACTION_TYPE_EMIT); - if (isatty(fileno(stdout))) + action = action_push(options, ACTION_TYPE_EMIT); + if (!options->output_filename && isatty(fileno(stdout))) { - fprintf(stderr, "Output is a tty and output format is binary, exiting.\n"); + fprintf(stderr, "Output is a tty and output format is binary, exiting.\n" + "If this is really what you intended, specify the output explicitly.\n"); return false; } + action->u.output_filename = options->output_filename; break; case 'h': @@ -221,6 +231,11 @@ static bool parse_command_line(int argc, char **argv, struct options *options) options->colours = no_colours; break; + case OPTION_OUTPUT: + case 'o': + options->output_filename = optarg; + break; + case 'V': case OPTION_VERSION: options->print_version = true; @@ -243,22 +258,28 @@ static void print_usage(const char *program_name) "[options...] [file]\n" "Options:\n" " --colour Enable colour, even when not supported by the output.\n" - " -e, --emit Emit the content of the DXBC blob to the standard output.\n" + " -e, --emit Emit the content of the DXBC blob.\n" " -h, --help Display this information and exit.\n" " -t, --list List the contents of the DXBC blob.\n" " --list-data List the data contained in the DXBC sections.\n" " --no-colour Disable colour, even when supported by the output.\n" " -V, --version Display version information and exit.\n" " --ignore-checksum Do not validate the checksum when parsing the DXBC blob.\n" + " -o, --output=<file> Set the output filename for --emit.\n" " -- Stop option processing. Any subsequent argument is\n" " interpreted as a filename.\n" "\n" "If the input file is '-' or not specified, input will be read from standard\n" - "input.\n" + "input. Similarly, if the output file is '-' or not specified at the point\n" + "--emit is found, output will be written to the standard output.\n" "\n" "Currently this tool can only re-emit the same DXBC blob it loaded. However, it\n" "will recompute the checksum while doing so, so --emit can be useful\n" - "together with --ignore-checksum to fix the checksum for a blob.\n"; + "together with --ignore-checksum to fix the checksum for a blob.\n" + "\n" + "Options --emit and --output can be specified more than once. The DXBC blob\n" + "will be emitted once for each --emit occurrence, each time using the closest\n" + "previous --output occurrence as output filename.\n"; fprintf(stderr, "Usage: %s %s", program_name, usage); } @@ -337,6 +358,25 @@ static bool read_input(FILE *f, struct vkd3d_shader_code *dxbc) return true; } +static FILE *open_output(const char *filename, bool *close) +{ + FILE *f; + + *close = false; + + if (!filename || !strcmp(filename, "-")) + return stdout; + + if (!(f = fopen(filename, "wb"))) + { + fprintf(stderr, "Unable to open '%s' for writing.\n", filename); + return NULL; + } + + *close = true; + return f; +} + static bool write_output(FILE *f, const struct vkd3d_shader_code *dxbc) { const char *code = dxbc->code; @@ -467,14 +507,14 @@ static void dump_dxbc(const struct vkd3d_shader_code *dxbc, const struct vkd3d_s int main(int argc, char **argv) { + bool close_input = false, close_output = false; struct vkd3d_shader_code dxbc, serialized; struct vkd3d_shader_dxbc_desc dxbc_desc; - bool close_input = false; struct options options; + FILE *input, *output; char *messages; uint32_t flags; int fail = 1; - FILE *input; size_t i; int ret; @@ -537,7 +577,13 @@ int main(int argc, char **argv) if (ret < 0) goto done; - if (!write_output(stdout, &serialized)) + if (!(output = open_output(action->u.output_filename, &close_output))) + { + vkd3d_shader_free_shader_code(&serialized); + goto done; + } + + if (!write_output(output, &serialized)) { fprintf(stderr, "Failed to write output blob.\n"); vkd3d_shader_free_shader_code(&serialized); @@ -559,5 +605,7 @@ done: free(options.actions); if (close_input) fclose(input); + if (close_output) + fclose(output); return fail; } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/582
On Fri Mar 8 21:54:34 2024 +0000, Henri Verbeet wrote:
```diff - if ((option = getopt_long(argc, argv, "htV", long_options,
NULL)) == -1)
+ if ((option = getopt_long(argc, argv, "ehtVo:", long_options, NULL)) == -1) ``` "o:" belongs in the next commit. ```diff + case 'e': + case OPTION_EMIT: + action_push(options, ACTION_TYPE_EMIT); + if (isatty(fileno(stdout))) + { + fprintf(stderr, "Output is a tty and output format is binary, exiting.\n" + "If this is really what you intended, specify the output explicitly.\n"); + return false; + } + break; ``` Relatedly, we can't specify the output explicitly yet in this commit. ```diff + "Currently this tool can only re-emit the same DXBC blob it loaded. However it\n" + "freshly recompute the checksum while doing so, so --emit can be useful\n" + "together with --ignore-checksum to fix the checksum for a blob.\n"; ``` I think the wording is a bit awkward here. I'd propose "However, it will recompute ..." or possibly "However, it recomputes ..." Note that multiple --emit options will cause vkd3d-dxbc to output the DXBC blob multiple times. I think that's fine, perhaps even desirable, but it may be worth explicitly pointing out. ```diff - if (!write_output(stdout, &serialized)) + if (!(output = open_output(options.output_filename, &close_output))) + { + vkd3d_shader_free_shader_code(&serialized); + goto done; + } + + if (!write_output(output, &serialized)) ``` That should use "action->u.output_filename" instead of "options.output_filename", right? Done, together with another couple of minor details I discovered in the meantime.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/582#note_64051
This merge request was approved by Henri Verbeet. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/582
participants (3)
-
Giovanni Mascellani -
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)