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.

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())

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

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())

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,

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_;
 };
 

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