Merged
Conversation
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.
jaebaek
approved these changes
Sep 11, 2020
jaebaek
reviewed
Sep 11, 2020
Contributor
jaebaek
left a comment
jaebaek
left a comment
There was a problem hiding this comment.
nit: could you please add some unit tests to check the parser update?
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.
dj2
approved these changes
Sep 14, 2020
archimedus
pushed a commit
to archimedus/amber
that referenced
this pull request
Oct 30, 2023
Roll third_party/clspv/ f67468c36..ac58b342e (1 commit) google/clspv@f67468c...ac58b34 $ git log f67468c36..ac58b342e --date=short --no-merges --format='%ad %ae %s' 2019-11-28 alanbaker Update llvm (google#444) Roll third_party/clspv-llvm/ e74b326b1..2c4ca6832 (186941 commits) llvm-mirror/llvm@e74b326...2c4ca68 $ git log e74b326b1..2c4ca6832 --date=short --no-merges --format='%ad %ae %s' 2019-10-22 david.green [InstCombine] Signed saturation patterns 2019-10-22 david.green [InstCombine] Signed saturation tests. NFC 2019-10-22 Petar.Avramovic [MIParser] Set RegClassOrRegBank during instruction parsing 2019-10-22 Petar.Avramovic [MIPS GlobalISel] Select MSA vector generic and builtin add 2019-10-22 eleviant [ThinLTO] Add code comment. NFC 2019-10-22 gchatelet [Alignment][NFC] Convert StoreInst to MaybeAlign 2019-10-22 gchatelet [Alignment][NFC] Convert LoadInst to MaybeAlign 2019-10-22 nemanja.i.ibm [PowerPC] Turn on CR-Logical reducer pass 2019-10-22 gchatelet [Alignment][NFC] Use MaybeAlign in AttrBuilder 2019-10-22 gchatelet [Alignment][NFC] Attributes use Align/MaybeAlign 2019-10-22 eleviant [ThinLTO] Don't internalize during promotion 2019-10-22 grimar [LLVMDebugInfoPDB] - Use cantFail() instead of assert(). 2019-10-22 martin [CMake] [WinMsvc] Look for includes and libs in ${MSVC_BASE}/atlmfc 2019-10-22 martin [CMake] Allow overriding MSVC_DIA_SDK_DIR via CMake 2019-10-22 llvmgnsyncbot gn build: Merge r375483 2019-10-22 jlettner [lit] Move increase_process_limit to ParallelRun 2019-10-21 llvm-dev [X86][BMI] Pull out schedule classes from bmi_andn<> and bmi_bls<> 2019-10-21 llvm-dev [X86][SSE] Add OR(EXTRACTELT(X,0),OR(EXTRACTELT(X,1))) -> MOVMSK+CMP reduction combine 2019-10-21 llvm-dev [X86][SSE] Add OR(EXTRACTELT(X,0),OR(EXTRACTELT(X,1))) movmsk v2X64 tests 2019-10-21 lhames [examples] Add a dependency on ExecutionEngine to LLJITWithObjectCache example. 2019-10-21 Austin.Kerbow AMDGPU/GlobalISel: Legalize fast unsafe FDIV 2019-10-21 jlettner [lit] Simplify test scheduling via multiprocessing.Pool 2019-10-21 Matthew.Arsenault AMDGPU: Select basic interp directly from intrinsics 2019-10-21 jlettner [lit] Remove redundancy from names and comments 2019-10-21 lebedev.ri [CVP] No-wrap deduction for `shl` 2019-10-21 quentin.colombet [GISel][CombinerHelper] Add a combine turning shuffle_vector into concat_vectors 2019-10-21 Matthew.Arsenault AMDGPU: Use CopyToReg for interp intrinsic lowering 2019-10-21 Matthew.Arsenault AMDGPU: Erase redundant redefs of m0 in SIFoldOperands 2019-10-21 Matthew.Arsenault AMDGPU: Stop adding m0 implicit def to SGPR spills 2019-10-21 Matthew.Arsenault AMDGPU: Slightly restructure m0 init code 2019-10-21 Stanislav.Mekhanoshin [AMDGPU] Select AGPR in PHI operand legalization 2019-10-21 llvm-dev [X86] Rename matchBitOpReduction to matchScalarReduction. NFCI. 2019-10-21 sander.desmalen Reverted r375425 as it broke some buildbots. 2019-10-21 lebedev.ri [NFC][CVP] Add `shl` no-wrap deduction test coverage 2019-10-21 bjorn.a.pettersson Prune Pass.h include from DataLayout.h. NFCI 2019-10-21 llvm-dev [PowerPC] Regenerate test for D52431 2019-10-21 teemperor [NFC] Add missing include to fix modules build 2019-10-21 llvm-dev SystemZISelLowering - supportedAddressingMode - silence static analyzer dyn_cast<> null dereference warning. NFCI. 2019-10-21 llvm-dev GVNHoist - silence static analyzer dyn_cast<> null dereference warning in hasEHOrLoadsOnPath call. NFCI. 2019-10-21 llvm-dev GuardWidening - silence static analyzer null dereference warning with assertion. NFCI. 2019-10-21 llvm-dev CrossDSOCFI - silence static analyzer dyn_cast<> null dereference warning. NFCI. 2019-10-21 llvm-dev IndVarSimplify - silence static analyzer dyn_cast<> null dereference warning. NFCI. 2019-10-21 sander.desmalen [AArch64][DebugInfo] Do not recompute CalleeSavedStackSize (Take 2) 2019-10-21 xiangxdh [NFC] Cleanup with variable name IsPPC64 & IsDarwin 2019-10-21 gchatelet [Alignment][NFC] Finish transition for `Loads` 2019-10-21 jay.foad Pre-commit test cases for D64713. 2019-10-21 david.green [Types] Define a getWithNewBitWidth for Types and make use of it 2019-10-21 gchatelet [Alignment][NFC] Instructions::getLoadStoreAlignment 2019-10-21 david.green [ARM] Extra qdadd patterns 2019-10-21 gchatelet [Alignment][NFC] Add a helper function to DataLayout (...) 2001-06-21 sabre Add a space to the PHI node output code to make it look nicer 2001-06-21 sabre Moved printing code to the Assembly/Writer library. Code now detects looping intervals 2001-06-21 sabre Implement the new Interval::isLoop method Implement destructor to free memory 2001-06-21 sabre New header file defined with neeto utilities put in one place 2001-06-21 sabre Modified to use the new reduce_apply algorithm 2001-06-21 sabre * Added capability to print out an interval 2001-06-21 sabre * Added comments * Added prototype for new Interval::isLoop method * Added destructor to free memory * Added IntervalPartition::isDegeneratePartition method * Added IntervalPartition::size() method 2001-06-21 sabre Add a test case: an irreducible flow graph. 2001-06-20 sabre Get rid of a silly printout that isn't needed right now 2001-06-20 sabre Add note 2001-06-20 sabre New test case 2001-06-20 sabre Add capability to print a derived interval graph 2001-06-20 sabre Add capability to build a derived interval graph 2001-06-20 sabre Factor the predeclarations of the CFG.h functionality into a seperate, new header file: CFGdecls.h 2001-06-20 sabre Initial Checking of Interval handling code 2001-06-20 sabre Add stub for induction variable code 2001-06-20 sabre Add a more complex test case 2001-06-20 sabre Add a test case for interval code 2001-06-20 sabre Add an optimization stub 2001-06-20 sabre New file: Interval analysis support 2001-06-20 sabre Add a note 2001-06-20 sabre Filter out more stuff I don't want all the time 2001-06-13 sabre Removed silly test code 2001-06-13 sabre Added options to print out basic blocks in a variety of different orderings as a testcase for cfg iterators. 2001-06-13 sabre Updates to work with new cfg namespace 2001-06-13 sabre Implement support for writing VCG format output 2001-06-13 sabre Move contents to the cfg namespace. Implement post order and reverse post order iterators 2001-06-11 sabre Updates to support * Changes in PHI node structure * We now run DCE after inlining because it helps clean up LOTS of inlining gunk. 2001-06-11 sabre Updates to support * Changes in PHI node structure 2001-06-11 sabre Updates to support * Changes in PHI node structure * Change to PHI syntax 2001-06-11 sabre Updates to support * Changes in PHI node structure * Fix to Predecessor iterator 2001-06-11 sabre Update documentation to reflect: * Changes in PHI node structure * Intentions for load instruction to work with structures 2001-06-08 sabre Moved getBinaryOperator to the BinaryOperator class and the getUnaryOperator to the UnaryOperator class (from the Instruction class). 2001-06-08 sabre I actually got something done 2001-06-08 sabre Beautify the source a bit. 2001-06-08 sabre Include support for reverse iteration. 2001-06-08 sabre Added a stupid testcase for iterators. 2001-06-08 sabre Added reverse depth first capability, fixed depth first capability 2001-06-07 sabre Updated to work with new CFG.h file. 2001-06-07 sabre Moved iterators to the new CFG.h file. 2001-06-07 sabre New file 2001-06-07 sabre inlining can change methods a second time, so don't rerun inliner when testing for differences in optimizations 2001-06-07 sabre Add extra method to PHI node class 2001-06-07 sabre Significant rework. DCE is still not done (see #ifdef'd out parts) but at least the stuff that is checked in, now works. 2001-06-07 sabre Fixed to print slightly differently. Added use counts for labels 2001-06-07 sabre Fixes for BB iterators, additional methods added for DCE pass 2001-06-07 sabre Extra comments 2001-06-06 sabre Now does not include instruction files... 2001-06-06 sabre Initial revision 2001-06-06 nobody New repository initialized by cvs2svn. Roll third_party/dxc/ 815358229..d0e9147ab (2 commits) microsoft/DirectXShaderCompiler@8153582...d0e9147 $ git log 815358229..d0e9147ab --date=short --no-merges --format='%ad %ae %s' 2019-11-27 hekotas Update version to 1.5.1911 (#2606) 2019-11-27 texr Fix incorrect definition for DXC_PART_REFLECTION_DATA in dxcapi.h (#2605) Roll third_party/glslang/ 38b4db48f..83af46951 (1 commit) KhronosGroup/glslang@38b4db4...83af469 $ git log 38b4db48f..83af46951 --date=short --no-merges --format='%ad %ae %s' 2019-11-27 malcolm.bechard Fix #1981 Roll third_party/googletest/ 34e92be31..b155875f3 (1 commit) google/googletest@34e92be...b155875 $ git log 34e92be31..b155875f3 --date=short --no-merges --format='%ad %ae %s' 2019-11-21 Christoph.Strehle Fix compile break for Microsoft Visual Studio 2017 v141 Roll third_party/shaderc/ 1d6155d86..f205775e9 (3 commits) google/shaderc@1d6155d...f205775 $ git log 1d6155d86..f205775e9 --date=short --no-merges --format='%ad %ae %s' 2019-11-28 9856269+sarahM0 spvc: add --amb flag to glslangValidator in spvc test script (google#915) 2019-11-27 rharrison Remove spvc's dependency on shaderc's util library (google#914) 2019-11-27 rharrison Clean up how include dirs are propegated for spirv_cross (google#912) Roll third_party/spirv-tools/ 52e9cc930..47f3eb426 (2 commits) KhronosGroup/SPIRV-Tools@52e9cc9...47f3eb4 $ git log 52e9cc930..47f3eb426 --date=short --no-merges --format='%ad %ae %s' 2019-11-29 afdx spirv-fuzz: Fix invalid tests (#3079) 2019-11-27 alanbaker Validate nested constructs (#3068) Roll third_party/swiftshader/ 8a6dcf763..b64fbfec4 (17 commits) https://swiftshader.googlesource.com/SwiftShader.git/+log/8a6dcf76315c..b64fbfec4dcd $ git log 8a6dcf763..b64fbfec4 --date=short --no-merges --format='%ad %ae %s' 2019-11-20 bclayton VkPipeline: Replace spirv-opt list with RegisterPerformancePasses() 2019-11-28 paulthomson Regres: refactor for use as a library 2019-11-29 swiftshader.regress Regres: Update test lists @ fb7ca1d5 2019-11-28 bclayton SpirvShaderEnumNames: Use spirv-tools' spvOpcodeString() 2019-11-28 bclayton Rename SpirvShader_dbg.cpp -> SpirvShaderEnumNames.cpp 2019-11-28 bclayton SpirvShader: Remove now unused includes 2019-11-28 bclayton SpirvShader: Move arithmetic ops to new cpp file 2019-11-28 bclayton SpirvShader: Move image handling to new cpp file 2019-11-28 bclayton SpirvShader: Move spec ops to new cpp file 2019-11-28 bclayton SpirvShader: Move group ops to new cpp file 2019-11-28 bclayton SpirvShader: Move memory ops to new cpp file 2019-11-28 bclayton SpirvShader: Move control flow handling to new cpp file 2019-11-28 bclayton SpirvShader: Move GLSLstd450 handling to new cpp file 2019-11-28 bclayton Pipeline: Move utility functions to ShaderCore 2019-11-28 jmadill Fix ICD generation (again). 2019-11-28 bclayton Regres: Use android.googlesource.com for dEQP 2019-11-27 sugoi Support multisampled Bresenham lines Roll third_party/vulkan-validationlayers/ 567954684..4fde9b750 (5 commits) KhronosGroup/Vulkan-ValidationLayers@5679546...4fde9b7 $ git log 567954684..4fde9b750 --date=short --no-merges --format='%ad %ae %s' 2019-11-27 pdaniell layers: Fix VK_KHR_separate_depth_stencil_layouts bugs 2019-11-27 camden layers: Fix naming convention in PD_STATE 2019-11-06 camden layers: Remove CALLSTATE everywhere except BP 2019-11-20 jasuarez tests: Implement test for VK_KHR_timeline_semaphore 2019-10-31 jasuarez layers: Implement VK_KHR_timeline_semaphore validations Created with: roll-dep third_party/clspv third_party/clspv-llvm third_party/dxc third_party/glslang third_party/googletest third_party/lodepng third_party/shaderc third_party/spirv-headers third_party/spirv-tools third_party/swiftshader third_party/vulkan-headers third_party/vulkan-loader third_party/vulkan-validationlayers
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR contains two changes:
dxc: Use a CComPtr for the include handler to prevent use-after-release crash.
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
CComPtrto 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 theVIRTUAL_FILEpath, or a generated path from the shader name for embedded shaders.The parser also adds the shader's source to the
VirtualFileStoreso 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.