From 9b606f7a25f66ec4150fb499cf42f5240445ed85 Mon Sep 17 00:00:00 2001
From: Ben Clayton <bclayton@google.com>
Date: Fri, 9 Apr 2021 18:22:28 +0000
Subject: [PATCH] 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 | 16 ++++++++++++++--
 src/dawn_native/ShaderModule.h   |  4 ++++
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp
index 9090119ef0..7774c61bf5 100644
--- a/src/dawn_native/ShaderModule.cpp
+++ b/src/dawn_native/ShaderModule.cpp
@@ -1047,6 +1047,17 @@ namespace dawn_native {
         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 @@ namespace dawn_native {
                 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(
diff --git a/src/dawn_native/ShaderModule.h b/src/dawn_native/ShaderModule.h
index 2e8c3af949..ab3a271558 100644
--- a/src/dawn_native/ShaderModule.h
+++ b/src/dawn_native/ShaderModule.h
@@ -55,6 +55,9 @@ namespace dawn_native {
     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 @@ namespace dawn_native {
         bool HasParsedShader() const;
 
         std::unique_ptr<tint::Program> tintProgram;
+        std::unique_ptr<TintSource> tintSource;
         std::vector<uint32_t> spirv;
         std::unique_ptr<OwnedCompilationMessages> compilationMessages;
     };