commit 9b606f7a25f66ec4150fb499cf42f5240445ed85
author Ben Clayton <bclayton@google.com> Fri Apr 09 18:22:28 2021 +0000
committer Commit Bot service account <commit-bot@chromium.org> Fri Apr 09 18:22:28 2021 +0000
tree 65cf73a73d779984ad485ef935f3b8879f5212ca
parent 9178d9bf3500e0f03e9681a8d1df5be94c021d06

ShaderModule: Keep the tint::Source::File alive

Tint diagnostics hold a raw pointer to the tint::Source::File, so that source snippets can be printed.
The tint::Source::File pointer was taken from a stack allocation, and the diagnostics were left with a dead pointer.
Have the ShaderModuleParseResult keep the tint::Source::File alive with a PIMPL unique_ptr.

Change-Id: I80d77516afd4012e15992821e060ef3038eba1c1
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/47422
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>

src/dawn_native/ShaderModule.cpp

diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp
index 9090119..7774c61 100644
--- a/src/dawn_native/ShaderModule.cpp
+++ b/src/dawn_native/ShaderModule.cpp
@@ -1047,6 +1047,17 @@
         return tintProgram != nullptr || spirv.size() > 0;
     }
 
+    // TintSource is a PIMPL container for a tint::Source::File, which needs to be kept alive for as
+    // long as tint diagnostics are inspected.
+    class TintSource {
+      public:
+        template <typename... ARGS>
+        TintSource(ARGS&&... args) : file(std::forward<ARGS>(args)...) {
+        }
+
+        tint::Source::File file;
+    };
+
     MaybeError ValidateShaderModuleDescriptor(DeviceBase* device,
                                               const ShaderModuleDescriptor* descriptor,
                                               ShaderModuleParseResult* parseResult) {
@@ -1088,13 +1099,14 @@
                 const auto* wgslDesc =
                     static_cast<const ShaderModuleWGSLDescriptor*>(chainedDescriptor);
 
-                tint::Source::File file("", wgslDesc->source);
+                auto tintSource = std::make_unique<TintSource>("", wgslDesc->source);
 
                 tint::Program program;
-                DAWN_TRY_ASSIGN(program, ParseWGSL(&file, outMessages));
+                DAWN_TRY_ASSIGN(program, ParseWGSL(&tintSource->file, outMessages));
 
                 if (device->IsToggleEnabled(Toggle::UseTintGenerator)) {
                     parseResult->tintProgram = std::make_unique<tint::Program>(std::move(program));
+                    parseResult->tintSource = std::move(tintSource);
                 } else {
                     tint::transform::Manager transformManager;
                     transformManager.append(

src/dawn_native/ShaderModule.h

diff --git a/src/dawn_native/ShaderModule.h b/src/dawn_native/ShaderModule.h
index 2e8c3af..ab3a271 100644
--- a/src/dawn_native/ShaderModule.h
+++ b/src/dawn_native/ShaderModule.h
@@ -55,6 +55,9 @@
     using EntryPointMetadataTable =
         std::unordered_map<std::string, std::unique_ptr<EntryPointMetadata>>;
 
+    // Source for a tint program
+    class TintSource;
+
     struct ShaderModuleParseResult {
         ShaderModuleParseResult();
         ~ShaderModuleParseResult();
@@ -64,6 +67,7 @@
         bool HasParsedShader() const;
 
         std::unique_ptr<tint::Program> tintProgram;
+        std::unique_ptr<TintSource> tintSource;
         std::vector<uint32_t> spirv;
         std::unique_ptr<OwnedCompilationMessages> compilationMessages;
     };