From 8645953be246fd2cef3fb0f089d2c4e598deb711 Mon Sep 17 00:00:00 2001 From: Ryan Harrison Date: Wed, 20 Oct 2021 05:01:03 +0000 Subject: [PATCH] Refactor Inspector fuzzing It is always on now when using tint::CommonFuzzer, and runs before & after the transform step. This CL also adds missing API coverage to the Inspector fuzzing code. Errors found with the Inspector are now reported as fuzzer failures and should generate bug reports. BUG=tint:1250,tint:1251,tint:1250 Change-Id: I1c1bcbddf81a35620f89c5b7a648c44e6a1f2952 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/66980 Auto-Submit: Ryan Harrison Kokoro: Kokoro Commit-Queue: Ryan Harrison Reviewed-by: Alastair Donaldson --- fuzzers/BUILD.gn | 10 -- fuzzers/CMakeLists.txt | 1 - fuzzers/tint_ast_fuzzer/fuzzer.cc | 1 - fuzzers/tint_common_fuzzer.cc | 203 +++++++++++----------- fuzzers/tint_common_fuzzer.h | 5 +- fuzzers/tint_inspector_fuzzer.cc | 35 ---- fuzzers/tint_regex_fuzzer/fuzzer.cc | 1 - fuzzers/tint_spirv_tools_fuzzer/fuzzer.cc | 2 - src/inspector/inspector.h | 2 +- 9 files changed, 109 insertions(+), 151 deletions(-) delete mode 100644 fuzzers/tint_inspector_fuzzer.cc diff --git a/fuzzers/BUILD.gn b/fuzzers/BUILD.gn index 0e59d230df..ebcfde35c6 100644 --- a/fuzzers/BUILD.gn +++ b/fuzzers/BUILD.gn @@ -159,15 +159,6 @@ if (build_with_chromium) { seed_corpus_deps = [ ":tint_generate_wgsl_corpus" ] } - fuzzer_test("tint_inspector_fuzzer") { - sources = [ "tint_inspector_fuzzer.cc" ] - deps = [ ":tint_fuzzer_common_with_init" ] - dict = "dictionary.txt" - libfuzzer_options = tint_fuzzer_common_libfuzzer_options - seed_corpus = fuzzer_corpus_wgsl_dir - seed_corpus_deps = [ ":tint_generate_wgsl_corpus" ] - } - fuzzer_test("tint_regex_spv_writer_fuzzer") { sources = [ "tint_regex_fuzzer/tint_regex_spv_writer_fuzzer.cc" ] deps = [ "tint_regex_fuzzer:tint_regex_fuzzer" ] @@ -303,7 +294,6 @@ if (build_with_chromium) { ":tint_ast_spv_writer_fuzzer", ":tint_binding_remapper_fuzzer", ":tint_first_index_offset_fuzzer", - ":tint_inspector_fuzzer", ":tint_regex_spv_writer_fuzzer", ":tint_renamer_fuzzer", ":tint_robustness_fuzzer", diff --git a/fuzzers/CMakeLists.txt b/fuzzers/CMakeLists.txt index 40af3a5df5..25e53a3e2e 100644 --- a/fuzzers/CMakeLists.txt +++ b/fuzzers/CMakeLists.txt @@ -40,7 +40,6 @@ if (${TINT_BUILD_WGSL_READER} AND ${TINT_BUILD_SPV_WRITER}) add_tint_fuzzer(tint_all_transforms_fuzzer) add_tint_fuzzer(tint_binding_remapper_fuzzer) add_tint_fuzzer(tint_first_index_offset_fuzzer) - add_tint_fuzzer(tint_inspector_fuzzer) add_tint_fuzzer(tint_renamer_fuzzer) add_tint_fuzzer(tint_robustness_fuzzer) add_tint_fuzzer(tint_single_entry_point_fuzzer) diff --git a/fuzzers/tint_ast_fuzzer/fuzzer.cc b/fuzzers/tint_ast_fuzzer/fuzzer.cc index b6b238f6f6..a0befa580b 100644 --- a/fuzzers/tint_ast_fuzzer/fuzzer.cc +++ b/fuzzers/tint_ast_fuzzer/fuzzer.cc @@ -114,7 +114,6 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { tb.AddTransform(); CommonFuzzer fuzzer(InputFormat::kWGSL, target.output_format); - fuzzer.EnableInspector(); fuzzer.SetTransformManager(tb.manager(), tb.data_map()); fuzzer.Run(data, size); diff --git a/fuzzers/tint_common_fuzzer.cc b/fuzzers/tint_common_fuzzer.cc index fab1782fac..d2100dbe46 100644 --- a/fuzzers/tint_common_fuzzer.cc +++ b/fuzzers/tint_common_fuzzer.cc @@ -37,11 +37,11 @@ namespace fuzzers { namespace { -// A macro is used to avoid FatalError creating its own stack frame. This leads +// A macro is used to avoid FATAL_ERROR creating its own stack frame. This leads // to better de-duplication of bug reports, because ClusterFuzz only uses the -// top few stack frames for de-duplication, and a FatalError stack frame +// top few stack frames for de-duplication, and a FATAL_ERROR stack frame // provides no useful information. -#define FatalError(diags, msg_string) \ +#define FATAL_ERROR(diags, msg_string) \ do { \ std::string msg = msg_string; \ auto printer = tint::diag::Printer::create(stderr, true); \ @@ -54,9 +54,20 @@ namespace { [[noreturn]] void TintInternalCompilerErrorReporter( const tint::diag::List& diagnostics) { - FatalError(diagnostics, ""); + FATAL_ERROR(diagnostics, ""); } +// Wrapping this in a macro so it can be a one-liner in the code, but not +// introducing another level in the stack trace. This will help with de-duping +// ClusterFuzz issues. +#define CHECK_INSPECTOR(inspector) \ + do { \ + if (inspector.has_error()) { \ + FATAL_ERROR(program->Diagnostics(), \ + "Inspector failed: " + inspector.error()); \ + } \ + } while (false) + bool SPIRVToolsValidationCheck(const tint::Program& program, const std::vector& spirv) { spvtools::SpirvTools tools(SPV_ENV_VULKAN_1_1); @@ -164,91 +175,13 @@ int CommonFuzzer::Run(const uint8_t* data, size_t size) { #if TINT_BUILD_SPV_READER if (input_ == InputFormat::kSpv && !SPIRVToolsValidationCheck(program, spirv_input)) { - FatalError(program.Diagnostics(), - "Fuzzing detected invalid input spirv not being caught by Tint"); + FATAL_ERROR( + program.Diagnostics(), + "Fuzzing detected invalid input spirv not being caught by Tint"); } #endif // TINT_BUILD_SPV_READER - if (inspector_enabled_) { - inspector::Inspector inspector(&program); - - auto entry_points = inspector.GetEntryPoints(); - if (inspector.has_error()) { - diagnostics_.add_error(tint::diag::System::Inspector, inspector.error()); - return 0; - } - - for (auto& ep : entry_points) { - auto remapped_name = inspector.GetRemappedNameForEntryPoint(ep.name); - if (inspector.has_error()) { - diagnostics_.add_error(tint::diag::System::Inspector, - inspector.error()); - return 0; - } - - auto constant_ids = inspector.GetConstantIDs(); - if (inspector.has_error()) { - diagnostics_.add_error(tint::diag::System::Inspector, - inspector.error()); - return 0; - } - - auto uniform_bindings = - inspector.GetUniformBufferResourceBindings(ep.name); - if (inspector.has_error()) { - diagnostics_.add_error(tint::diag::System::Inspector, - inspector.error()); - return 0; - } - - auto storage_bindings = - inspector.GetStorageBufferResourceBindings(ep.name); - if (inspector.has_error()) { - diagnostics_.add_error(tint::diag::System::Inspector, - inspector.error()); - return 0; - } - - auto readonly_bindings = - inspector.GetReadOnlyStorageBufferResourceBindings(ep.name); - if (inspector.has_error()) { - diagnostics_.add_error(tint::diag::System::Inspector, - inspector.error()); - return 0; - } - - auto sampler_bindings = inspector.GetSamplerResourceBindings(ep.name); - if (inspector.has_error()) { - diagnostics_.add_error(tint::diag::System::Inspector, - inspector.error()); - return 0; - } - - auto comparison_sampler_bindings = - inspector.GetComparisonSamplerResourceBindings(ep.name); - if (inspector.has_error()) { - diagnostics_.add_error(tint::diag::System::Inspector, - inspector.error()); - return 0; - } - - auto sampled_texture_bindings = - inspector.GetSampledTextureResourceBindings(ep.name); - if (inspector.has_error()) { - diagnostics_.add_error(tint::diag::System::Inspector, - inspector.error()); - return 0; - } - - auto multisampled_texture_bindings = - inspector.GetMultisampledTextureResourceBindings(ep.name); - if (inspector.has_error()) { - diagnostics_.add_error(tint::diag::System::Inspector, - inspector.error()); - return 0; - } - } - } + RunInspector(&program); if (transform_manager_) { auto out = transform_manager_->Run(&program, *transform_inputs_); @@ -258,14 +191,15 @@ int CommonFuzzer::Run(const uint8_t* data, size_t size) { for (const auto& diag : out.program.Diagnostics()) { if (diag.severity > diag::Severity::Error || diag.system != diag::System::Transform) { - FatalError(out.program.Diagnostics(), - "Fuzzing detected valid input program being transformed " - "into an invalid output program"); + FATAL_ERROR(out.program.Diagnostics(), + "Fuzzing detected valid input program being transformed " + "into an invalid output program"); } } } program = std::move(out.program); + RunInspector(&program); } switch (output_) { @@ -274,8 +208,8 @@ int CommonFuzzer::Run(const uint8_t* data, size_t size) { auto result = writer::wgsl::Generate(&program, options_wgsl_); generated_wgsl_ = std::move(result.wgsl); if (!result.success) { - FatalError(program.Diagnostics(), - "WGSL writer failed: " + result.error); + FATAL_ERROR(program.Diagnostics(), + "WGSL writer failed: " + result.error); } #endif // TINT_BUILD_WGSL_WRITER break; @@ -285,12 +219,12 @@ int CommonFuzzer::Run(const uint8_t* data, size_t size) { auto result = writer::spirv::Generate(&program, options_spirv_); generated_spirv_ = std::move(result.spirv); if (!result.success) { - FatalError(program.Diagnostics(), - "SPIR-V writer failed: " + result.error); + FATAL_ERROR(program.Diagnostics(), + "SPIR-V writer failed: " + result.error); } if (!SPIRVToolsValidationCheck(program, generated_spirv_)) { - FatalError(program.Diagnostics(), - "Fuzzing detected invalid spirv being emitted by Tint"); + FATAL_ERROR(program.Diagnostics(), + "Fuzzing detected invalid spirv being emitted by Tint"); } #endif // TINT_BUILD_SPV_WRITER @@ -301,8 +235,8 @@ int CommonFuzzer::Run(const uint8_t* data, size_t size) { auto result = writer::hlsl::Generate(&program, options_hlsl_); generated_hlsl_ = std::move(result.hlsl); if (!result.success) { - FatalError(program.Diagnostics(), - "HLSL writer failed: " + result.error); + FATAL_ERROR(program.Diagnostics(), + "HLSL writer failed: " + result.error); } #endif // TINT_BUILD_HLSL_WRITER break; @@ -312,7 +246,8 @@ int CommonFuzzer::Run(const uint8_t* data, size_t size) { auto result = writer::msl::Generate(&program, options_msl_); generated_msl_ = std::move(result.msl); if (!result.success) { - FatalError(program.Diagnostics(), "MSL writer failed: " + result.error); + FATAL_ERROR(program.Diagnostics(), + "MSL writer failed: " + result.error); } #endif // TINT_BUILD_MSL_WRITER break; @@ -322,5 +257,77 @@ int CommonFuzzer::Run(const uint8_t* data, size_t size) { return 0; } +void CommonFuzzer::RunInspector(Program* program) { + inspector::Inspector inspector(program); + + auto entry_points = inspector.GetEntryPoints(); + CHECK_INSPECTOR(inspector); + + auto constant_ids = inspector.GetConstantIDs(); + CHECK_INSPECTOR(inspector); + + auto constant_name_to_id = inspector.GetConstantNameToIdMap(); + CHECK_INSPECTOR(inspector); + + for (auto& ep : entry_points) { + auto remapped_name = inspector.GetRemappedNameForEntryPoint(ep.name); + CHECK_INSPECTOR(inspector); + + auto storage_size = inspector.GetStorageSize(ep.name); + storage_size = 0; // Silencing unused variable warning + CHECK_INSPECTOR(inspector); + + auto resource_bindings = inspector.GetResourceBindings(ep.name); + CHECK_INSPECTOR(inspector); + + auto uniform_bindings = inspector.GetUniformBufferResourceBindings(ep.name); + CHECK_INSPECTOR(inspector); + + auto storage_bindings = inspector.GetStorageBufferResourceBindings(ep.name); + CHECK_INSPECTOR(inspector); + + auto readonly_bindings = + inspector.GetReadOnlyStorageBufferResourceBindings(ep.name); + CHECK_INSPECTOR(inspector); + + auto sampler_bindings = inspector.GetSamplerResourceBindings(ep.name); + CHECK_INSPECTOR(inspector); + + auto comparison_sampler_bindings = + inspector.GetComparisonSamplerResourceBindings(ep.name); + CHECK_INSPECTOR(inspector); + + auto sampled_texture_bindings = + inspector.GetSampledTextureResourceBindings(ep.name); + CHECK_INSPECTOR(inspector); + + auto multisampled_texture_bindings = + inspector.GetMultisampledTextureResourceBindings(ep.name); + CHECK_INSPECTOR(inspector); + + auto writeonly__bindings = + inspector.GetWriteOnlyStorageTextureResourceBindings(ep.name); + CHECK_INSPECTOR(inspector); + + auto depth_bindings = inspector.GetDepthTextureResourceBindings(ep.name); + CHECK_INSPECTOR(inspector); + + auto depth_multisampled_bindings = + inspector.GetDepthMultisampledTextureResourceBindings(ep.name); + CHECK_INSPECTOR(inspector); + + auto external_bindings = + inspector.GetExternalTextureResourceBindings(ep.name); + CHECK_INSPECTOR(inspector); + + auto sampler_texture_uses = inspector.GetSamplerTextureUses(ep.name); + CHECK_INSPECTOR(inspector); + + auto workgroup_storage = inspector.GetWorkgroupStorageSize(ep.name); + workgroup_storage = 0; // Silencing unused variable warning + CHECK_INSPECTOR(inspector); + } +} + } // namespace fuzzers } // namespace tint diff --git a/fuzzers/tint_common_fuzzer.h b/fuzzers/tint_common_fuzzer.h index b5281a5758..faa9c93bee 100644 --- a/fuzzers/tint_common_fuzzer.h +++ b/fuzzers/tint_common_fuzzer.h @@ -48,7 +48,6 @@ class CommonFuzzer { transform_manager_ = tm; transform_inputs_ = inputs; } - void EnableInspector() { inspector_enabled_ = true; } void SetDumpInput(bool enabled) { dump_input_ = enabled; } @@ -89,7 +88,6 @@ class CommonFuzzer { OutputFormat output_; transform::Manager* transform_manager_ = nullptr; transform::DataMap* transform_inputs_ = nullptr; - bool inspector_enabled_ = false; bool dump_input_ = false; tint::diag::List diagnostics_; @@ -107,6 +105,9 @@ class CommonFuzzer { /// The source file needs to live at least as long as #diagnostics_ std::unique_ptr file_; #endif // TINT_BUILD_WGSL_READER + + // Run series of reflection operations to exercise the Inspector API. + void RunInspector(Program* program); }; } // namespace fuzzers diff --git a/fuzzers/tint_inspector_fuzzer.cc b/fuzzers/tint_inspector_fuzzer.cc deleted file mode 100644 index 0fa71982ab..0000000000 --- a/fuzzers/tint_inspector_fuzzer.cc +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright 2021 The Tint Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "fuzzers/fuzzer_init.h" -#include "fuzzers/tint_common_fuzzer.h" -#include "fuzzers/transform_builder.h" - -namespace tint { -namespace fuzzers { - -extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { - TransformBuilder tb(data, size); - tb.AddTransform(); - - fuzzers::CommonFuzzer fuzzer(InputFormat::kWGSL, OutputFormat::kWGSL); - fuzzer.EnableInspector(); - fuzzer.SetTransformManager(tb.manager(), tb.data_map()); - fuzzer.SetDumpInput(GetCliParams().dump_input); - - return fuzzer.Run(data, size); -} - -} // namespace fuzzers -} // namespace tint diff --git a/fuzzers/tint_regex_fuzzer/fuzzer.cc b/fuzzers/tint_regex_fuzzer/fuzzer.cc index 0a557501d4..6d79a0b93f 100644 --- a/fuzzers/tint_regex_fuzzer/fuzzer.cc +++ b/fuzzers/tint_regex_fuzzer/fuzzer.cc @@ -142,7 +142,6 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { tb.AddTransform(); CommonFuzzer fuzzer(InputFormat::kWGSL, target.output_format); - fuzzer.EnableInspector(); fuzzer.SetTransformManager(tb.manager(), tb.data_map()); fuzzer.Run(data, size); diff --git a/fuzzers/tint_spirv_tools_fuzzer/fuzzer.cc b/fuzzers/tint_spirv_tools_fuzzer/fuzzer.cc index 4d3b8aa99a..9e0e8fa654 100644 --- a/fuzzers/tint_spirv_tools_fuzzer/fuzzer.cc +++ b/fuzzers/tint_spirv_tools_fuzzer/fuzzer.cc @@ -224,7 +224,6 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { } CommonFuzzer spv_to_wgsl(InputFormat::kSpv, OutputFormat::kWGSL); - spv_to_wgsl.EnableInspector(); spv_to_wgsl.Run(data, size); if (spv_to_wgsl.HasErrors()) { auto error = spv_to_wgsl.Diagnostics().str(); @@ -247,7 +246,6 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { } CommonFuzzer fuzzer(InputFormat::kWGSL, target.second); - fuzzer.EnableInspector(); fuzzer.Run(reinterpret_cast(wgsl.data()), wgsl.size()); if (fuzzer.HasErrors()) { auto error = spv_to_wgsl.Diagnostics().str(); diff --git a/src/inspector/inspector.h b/src/inspector/inspector.h index 8a8f582ee8..3fca877d8b 100644 --- a/src/inspector/inspector.h +++ b/src/inspector/inspector.h @@ -63,7 +63,7 @@ class Inspector { /// @param entry_point name of the entry point to get information about. /// @returns the total size of shared storage required by an entry point, - // including all uniforms storage buffers. + /// including all uniform storage buffers. uint32_t GetStorageSize(const std::string& entry_point); /// @param entry_point name of the entry point to get information about.