[ASAN] Use after free memory problem when merging geps

[bob80905]

Description

In HLLowerUDT.cpp:ReplaceUsesForLoweredUDT:258, which is a recursive function, there is a block of code that looks like this: ReplaceUsesForLoweredUDT(GEP, NewGEP); dxilutil::MergeGepUse(NewGEP); GEP->eraseFromParent(); There is a problem with this block of code. It turns out, the function recurses on itself twice, then runs MergeGepUse. MergeGepUse will then call DxilUtilDbgInfoAndMisc.cpp:TryMegeWithNestedGEP, and TryMegeWithNestedGEP on line 161 will eraseFromParent() the previous Gep. This previous Gep happens to be equivalent to the NewGEP parameter passed to the original ReplaceUsesForLoweredUDT call before all of the recursion happened. So, when execution returns back to the original ReplaceUsesForLoweredUDT call, and executes the next line MergeGepUse(NewGEP), NewGEP no longer exists, as it's been freed. Where this causes a problem specifically is inside the lambda function. When addUsersToWorkList(V) is called, the lambda function runs, and when it checks if(!V->user_empty()), V is corrupted since memory has already overwritten it. So, there needs to be a way to check that NewGEP hasn't been erased from its parent after the ReplaceUsesForLoweredUDT(GEP, NewGEP); call.

Steps to Reproduce

hctbuild -sanitizer <DXC build-dir>\debug\bin\dxc.exe -Tlib_6_8 tools\clang\test\HLSLFileCheck\hlsl\workgraph\multi_dim_array_record.hlsl

Actual Behavior

Compilation succeeds without errors, but ASAN will catch this issue if enabled.

Environment

• DXC version dxcompiler.dll: 1.8 - 1.7.0.14317 (main, e3c311409675)
• Host Operating System Windows 11 Enterprise, version 22H2

[tex3d]

@tcorringham

Related to comment about a fix for this issue here:

I think the correct fix for this is not to call MergeGepUse inside of ReplaceUsesForLoweredUDT. If merging is required, it should happen at the high-level before (and maybe after if necessary) the top-level call to ReplaceUsesForLoweredUDT.

If there is a reason MergeGepUse was required in this location, then it's calling it on the wrong thing. It should be called on NewV instead of NewGEP I think. Then, NewGEP should be considered potentially replaced and should be discarded.