Prevent diagnostic memory leak (#2110)

Fixes https://crbug.com/906669

* Don't free diagnostics in spvBinaryParse
* When invoking the parser we wish to ignore the error messages from,
instead create a hijacked context and replace the message consumer with
a null consumer
This commit is contained in:
alan-baker 2018-11-26 16:58:09 -05:00 committed by GitHub
parent 72d4e5414b
commit e799bfb923
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 26 additions and 9 deletions

View File

@ -565,8 +565,8 @@ SPIRV_TOOLS_EXPORT void spvReducerOptionsSetSeed(spv_reducer_options options,
// Encodes the given SPIR-V assembly text to its binary representation. The
// length parameter specifies the number of bytes for text. Encoded binary will
// be stored into *binary. Any error will be written into *diagnostic if
// diagnostic is non-null. The generated binary is independent of the context
// and may outlive it.
// diagnostic is non-null, otherwise the context's message consumer will be
// used. The generated binary is independent of the context and may outlive it.
SPIRV_TOOLS_EXPORT spv_result_t spvTextToBinary(const spv_const_context context,
const char* text,
const size_t length,
@ -588,7 +588,8 @@ SPIRV_TOOLS_EXPORT void spvTextDestroy(spv_text text);
// word_count parameter specifies the number of words for binary. The options
// parameter is a bit field of spv_binary_to_text_options_t. Decoded text will
// be stored into *text. Any error will be written into *diagnostic if
// diagnostic is non-null.
// diagnostic is non-null, otherwise the context's message consumer will be
// used.
SPIRV_TOOLS_EXPORT spv_result_t spvBinaryToText(const spv_const_context context,
const uint32_t* binary,
const size_t word_count,
@ -601,20 +602,22 @@ SPIRV_TOOLS_EXPORT spv_result_t spvBinaryToText(const spv_const_context context,
SPIRV_TOOLS_EXPORT void spvBinaryDestroy(spv_binary binary);
// Validates a SPIR-V binary for correctness. Any errors will be written into
// *diagnostic if diagnostic is non-null.
// *diagnostic if diagnostic is non-null, otherwise the context's message
// consumer will be used.
SPIRV_TOOLS_EXPORT spv_result_t spvValidate(const spv_const_context context,
const spv_const_binary binary,
spv_diagnostic* diagnostic);
// Validates a SPIR-V binary for correctness. Uses the provided Validator
// options. Any errors will be written into *diagnostic if diagnostic is
// non-null.
// non-null, otherwise the context's message consumer will be used.
SPIRV_TOOLS_EXPORT spv_result_t spvValidateWithOptions(
const spv_const_context context, const spv_const_validator_options options,
const spv_const_binary binary, spv_diagnostic* diagnostic);
// Validates a raw SPIR-V binary for correctness. Any errors will be written
// into *diagnostic if diagnostic is non-null.
// into *diagnostic if diagnostic is non-null, otherwise the context's message
// consumer will be used.
SPIRV_TOOLS_EXPORT spv_result_t
spvValidateBinary(const spv_const_context context, const uint32_t* words,
const size_t num_words, spv_diagnostic* diagnostic);
@ -661,7 +664,8 @@ typedef spv_result_t (*spv_parsed_instruction_fn_t)(
// is supplied as context to the callbacks. Returns SPV_SUCCESS on successful
// parse where the callbacks always return SPV_SUCCESS. For an invalid parse,
// returns a status code other than SPV_SUCCESS, and if diagnostic is non-null
// also emits a diagnostic. If a callback returns anything other than
// also emits a diagnostic. If diagnostic is null the context's message consumer
// will be used to emit any errors. If a callback returns anything other than
// SPV_SUCCESS, then that status code is returned, no further callbacks are
// issued, and no additional diagnostics are emitted.
SPIRV_TOOLS_EXPORT spv_result_t spvBinaryParse(

View File

@ -232,8 +232,15 @@ spv_result_t ValidateBinaryUsingContextAndValidationState(
<< "Invalid SPIR-V. The id bound is larger than the max id bound "
<< vstate->options()->universal_limits_.max_id_bound << ".";
}
// Look for OpExtension instructions and register extensions.
spvBinaryParse(&context, vstate, words, num_words,
// This parse should not produce any error messages. Hijack the context and
// replace the message consumer so that we do not pollute any state in input
// consumer.
spv_context_t hijacked_context = context;
hijacked_context.consumer = [](spv_message_level_t, const char*,
const spv_position_t&, const char*) {};
spvBinaryParse(&hijacked_context, vstate, words, num_words,
/* parsed_header = */ nullptr, ProcessExtensions,
/* diagnostic = */ nullptr);

View File

@ -197,7 +197,13 @@ ValidationState_t::ValidationState_t(const spv_const_context ctx,
// fail and generate an error.
if (num_words > 0) {
// Count the number of instructions in the binary.
spvBinaryParse(ctx, this, words, num_words,
// This parse should not produce any error messages. Hijack the context and
// replace the message consumer so that we do not pollute any state in input
// consumer.
spv_context_t hijacked_context = *ctx;
hijacked_context.consumer = [](spv_message_level_t, const char*,
const spv_position_t&, const char*) {};
spvBinaryParse(&hijacked_context, this, words, num_words,
/* parsed_header = */ nullptr, CountInstructions,
/* diagnostic = */ nullptr);
preallocateStorage();