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 <rharrison@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Alastair Donaldson <afdx@google.com>

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",

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)

fuzzers/tint_ast_fuzzer/fuzzer.cc

@@ -114,7 +114,6 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
     tb.AddTransform<tint::transform::Robustness>();
 
     CommonFuzzer fuzzer(InputFormat::kWGSL, target.output_format);
-    fuzzer.EnableInspector();
     fuzzer.SetTransformManager(tb.manager(), tb.data_map());
 
     fuzzer.Run(data, size);

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<uint32_t>& 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(),
+    FATAL_ERROR(
+        program.Diagnostics(),
         "Fuzzing detected invalid input spirv not being caught by Tint");
   }
 #endif  // TINT_BUILD_SPV_READER
 
-  if (inspector_enabled_) {
+  RunInspector(&program);
-    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;
-      }
-    }
-  }
 
   if (transform_manager_) {
     auto out = transform_manager_->Run(&program, *transform_inputs_);
@@ -258,7 +191,7 @@ 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(),
+          FATAL_ERROR(out.program.Diagnostics(),
                       "Fuzzing detected valid input program being transformed "
                       "into an invalid output program");
         }
@@ -266,6 +199,7 @@ int CommonFuzzer::Run(const uint8_t* data, size_t size) {
     }
 
     program = std::move(out.program);
+    RunInspector(&program);
   }
 
   switch (output_) {
@@ -274,7 +208,7 @@ 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(),
+        FATAL_ERROR(program.Diagnostics(),
                     "WGSL writer failed: " + result.error);
       }
 #endif  // TINT_BUILD_WGSL_WRITER
@@ -285,11 +219,11 @@ 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(),
+        FATAL_ERROR(program.Diagnostics(),
                     "SPIR-V writer failed: " + result.error);
       }
       if (!SPIRVToolsValidationCheck(program, generated_spirv_)) {
-        FatalError(program.Diagnostics(),
+        FATAL_ERROR(program.Diagnostics(),
                     "Fuzzing detected invalid spirv being emitted by Tint");
       }
 
@@ -301,7 +235,7 @@ 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(),
+        FATAL_ERROR(program.Diagnostics(),
                     "HLSL writer failed: " + result.error);
       }
 #endif  // TINT_BUILD_HLSL_WRITER
@@ -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

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<Source::File> file_;
 #endif  // TINT_BUILD_WGSL_READER
+
+  // Run series of reflection operations to exercise the Inspector API.
+  void RunInspector(Program* program);
 };
 
 }  // namespace fuzzers

fuzzers/tint_inspector_fuzzer.cc

@@ -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<transform::Robustness>();
-
-  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

fuzzers/tint_regex_fuzzer/fuzzer.cc

@@ -142,7 +142,6 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
     tb.AddTransform<tint::transform::Robustness>();
 
     CommonFuzzer fuzzer(InputFormat::kWGSL, target.output_format);
-    fuzzer.EnableInspector();
     fuzzer.SetTransformManager(tb.manager(), tb.data_map());
 
     fuzzer.Run(data, size);

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<const uint8_t*>(wgsl.data()), wgsl.size());
     if (fuzzer.HasErrors()) {
       auto error = spv_to_wgsl.Diagnostics().str();

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.