Crash fix + debugger improvement (#912)

* dxc: Use a CComPtr for the include handler

The include handler was constructed and passed down to `IDxcCompiler::Compile()`, but was not referenced by the caller.
Within `IDxcCompiler::Compile()`, the include handler may be referenced, released, and an attempt to be referenced again (using deleted memory).

Use `CComPtr` to keep the COM object alive for the full duration of the compile.

* Pass a unique shader filename down to DXC

In order for the debugger to be able to fetch the source of a shader that has its source embedded within an amber script file, we need to have a known and unique file name.

The file name was previously generated just before calling into DXC to compile the file, and could easily have multiple shaders with the same name.

Instead we now add a optional file path property to the `amber::Shader`, which is populated by the parser with either the `VIRTUAL_FILE` path, or a generated path from the shader name for embedded shaders.
The parser also adds the shader's source to the `VirtualFileStore` so it can be found by the debugger engine.

If no file path is specified by the parser, dxc_helper defaults to its previous naming scheme.

6 files changed, 74 insertions, 25 deletions

diff --git a/src/amberscript/parser.cc b/src/amberscript/parser.cc
index 3b75348..0936bcd 100644
--- a/src/amberscript/parser.cc
+++ b/src/amberscript/parser.cc
@@ -515,34 +515,40 @@ Result Parser::ParseShaderBlock() {
     if (!token->IsIdentifier() && !token->IsString())
       return Result("expected virtual file path after VIRTUAL_FILE");
 
-    r = ValidateEndOfStatement("SHADER command");
-    if (!r.IsSuccess())
-      return r;
-
     auto path = token->AsString();
 
     std::string data;
     r = script_->GetVirtualFile(path, &data);
-    if (!r.IsSuccess()) {
+    if (!r.IsSuccess())
       return r;
-    }
 
     shader->SetData(data);
-  } else {
-    r = ValidateEndOfStatement("SHADER command");
+    shader->SetFilePath(path);
+
+    r = script_->AddShader(std::move(shader));
     if (!r.IsSuccess())
       return r;
 
-    std::string data = tokenizer_->ExtractToNext("END");
-    if (data.empty())
-      return Result("SHADER must not be empty");
+    return ValidateEndOfStatement("SHADER command");
+  }
 
-    shader->SetData(data);
+  r = ValidateEndOfStatement("SHADER command");
+  if (!r.IsSuccess())
+    return r;
 
-    token = tokenizer_->NextToken();
-    if (!token->IsIdentifier() || token->AsString() != "END")
-      return Result("SHADER missing END command");
-  }
+  std::string data = tokenizer_->ExtractToNext("END");
+  if (data.empty())
+    return Result("SHADER must not be empty");
+
+  shader->SetData(data);
+
+  auto path = "embedded-shaders/" + shader->GetName();
+  script_->AddVirtualFile(path, data);
+  shader->SetFilePath(path);
+
+  token = tokenizer_->NextToken();
+  if (!token->IsIdentifier() || token->AsString() != "END")
+    return Result("SHADER missing END command");
 
   r = script_->AddShader(std::move(shader));
   if (!r.IsSuccess())
diff --git a/src/amberscript/parser_shader_test.cc b/src/amberscript/parser_shader_test.cc
index 83aaf2b..657cc67 100644
--- a/src/amberscript/parser_shader_test.cc
+++ b/src/amberscript/parser_shader_test.cc
@@ -461,5 +461,42 @@ END
   ASSERT_TRUE(r.IsSuccess());
 }
 
+TEST_F(AmberScriptParserTest, ShaderDefaultFilePath) {
+  std::string in = R"(#!amber
+SHADER fragment shader_name GLSL
+void main() {
+  gl_FragColor = vec3(2, 3, 4);
+}
+END)";
+
+  Parser parser;
+  Result r = parser.Parse(in);
+  ASSERT_TRUE(r.IsSuccess()) << r.Error();
+
+  auto script = parser.GetScript();
+  auto shader = script->GetShader("shader_name");
+  EXPECT_EQ("embedded-shaders/shader_name", shader->GetFilePath());
+}
+
+TEST_F(AmberScriptParserTest, ShaderVirtualFilePath) {
+  std::string in = R"(#!amber
+VIRTUAL_FILE my_fragment_shader
+void main() {
+  gl_FragColor = vec3(2, 3, 4);
+}
+END
+
+SHADER fragment shader_name GLSL VIRTUAL_FILE my_fragment_shader
+)";
+
+  Parser parser;
+  Result r = parser.Parse(in);
+  ASSERT_TRUE(r.IsSuccess()) << r.Error();
+
+  auto script = parser.GetScript();
+  auto shader = script->GetShader("shader_name");
+  EXPECT_EQ("my_fragment_shader", shader->GetFilePath());
+}
+
 }  // namespace amberscript
 }  // namespace amber
diff --git a/src/dxc_helper.cc b/src/dxc_helper.cc
index 7dc4f4e..8fb0f8a 100644
--- a/src/dxc_helper.cc
+++ b/src/dxc_helper.cc
@@ -135,6 +135,7 @@ Result Compile(const std::string& src,
                const std::string& entry,
                const std::string& profile,
                const std::string& spv_env,
+               const std::string& filename,
                const VirtualFileStore* virtual_files,
                std::vector<uint32_t>* generated_binary) {
   if (hlsl::options::initHlslOptTable()) {
@@ -163,8 +164,8 @@ Result Compile(const std::string& src,
     return Result("DXC compile failure: CreateIncludeHandler");
   }
 
-  IDxcIncludeHandler* include_handler =
-      new IncludeHandler(virtual_files, dxc_lib, fallback_include_handler);
+  CComPtr<IDxcIncludeHandler> include_handler(
+      new IncludeHandler(virtual_files, dxc_lib, fallback_include_handler));
 
   IDxcCompiler* compiler;
   if (DxcCreateInstance(CLSID_DxcCompiler, __uuidof(IDxcCompiler),
@@ -173,9 +174,7 @@ Result Compile(const std::string& src,
     return Result("DXCCreateInstance for DXCCompiler failed");
   }
 
-  IDxcOperationResult* result;
-  std::wstring src_filename =
-      L"amber." + std::wstring(profile.begin(), profile.end());
+  std::string filepath = filename.empty() ? ("amber." + profile) : filename;
 
   std::vector<const wchar_t*> dxc_flags(kDxcFlags, &kDxcFlags[kDxcFlagsCount]);
   const wchar_t* target_env = nullptr;
@@ -191,9 +190,11 @@ Result Compile(const std::string& src,
   if (target_env)
     dxc_flags.push_back(target_env);
 
+  IDxcOperationResult* result;
   if (compiler->Compile(
-          source,               /* source text */
-          src_filename.c_str(), /* original file source */
+          source, /* source text */
+          std::wstring(filepath.begin(), filepath.end())
+              .c_str(), /* original file source */
           std::wstring(entry.begin(), entry.end())
               .c_str(), /* entry point name */
           std::wstring(profile.begin(), profile.end())
diff --git a/src/dxc_helper.h b/src/dxc_helper.h
index 22082ce..eb83216 100644
--- a/src/dxc_helper.h
+++ b/src/dxc_helper.h
@@ -28,7 +28,8 @@ namespace dxchelper {
 
 // Passes the HLSL source code to the DXC compiler with SPIR-V CodeGen.
 // Returns the generated SPIR-V binary via |generated_binary| argument.
-Result Compile(const std::string& src_str,
+Result Compile(const std::string& src,
+               const std::string& filename,
                const std::string& entry_str,
                const std::string& profile_str,
                const std::string& spv_env,
diff --git a/src/shader.h b/src/shader.h
index 1052662..29d6e5e 100644
--- a/src/shader.h
+++ b/src/shader.h
@@ -33,6 +33,9 @@ class Shader {
   void SetName(const std::string& name) { name_ = name; }
   const std::string& GetName() const { return name_; }
 
+  void SetFilePath(const std::string& path) { file_path_ = path; }
+  const std::string& GetFilePath() const { return file_path_; }
+
   void SetFormat(ShaderFormat fmt) { shader_format_ = fmt; }
   ShaderFormat GetFormat() const { return shader_format_; }
 
@@ -49,6 +52,7 @@ class Shader {
   ShaderFormat shader_format_;
   std::string data_;
   std::string name_;
+  std::string file_path_;
   std::string target_env_;
 };
 
diff --git a/src/shader_compiler.cc b/src/shader_compiler.cc
index 8f0d8a4..285dd97 100644
--- a/src/shader_compiler.cc
+++ b/src/shader_compiler.cc
@@ -276,7 +276,7 @@ Result ShaderCompiler::CompileHlsl(const Shader* shader,
     return Result("Unknown shader type");
 
   return dxchelper::Compile(shader->GetData(), "main", target, spv_env_,
-                            virtual_files_, result);
+                            shader->GetFilePath(), virtual_files_, result);
 }
 #else
 Result ShaderCompiler::CompileHlsl(const Shader*,