optimizer: dominator analysis depends on CFG analysis being valid, so that invalidating CFG analysis but not dominator analysis can lead to use-after-free #2889
Description
I have found that invalidating all analysis except for the dominator analysis can lead to problems, due to the dominator analysis holding references to the pseudo-entry and exit blocks computed by the CFG analysis, which will become invalid if the CFG analysis is invalidated.
The following test (which hopefully it is easy to put into whichever test class is most convenient) exposes the problem when compiled and executed with asan:
TEST(PutInYourFavouriteTestClass, AsanErrorTest) {
std::string shader = R"(
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %4 "main"
OpExecutionMode %4 OriginUpperLeft
OpSource ESSL 310
OpName %4 "main"
OpName %8 "x"
OpName %10 "y"
OpDecorate %8 RelaxedPrecision
OpDecorate %10 RelaxedPrecision
OpDecorate %11 RelaxedPrecision
%2 = OpTypeVoid
%3 = OpTypeFunction %2
%6 = OpTypeInt 32 1
%7 = OpTypePointer Function %6
%9 = OpConstant %6 1
%4 = OpFunction %2 None %3
%5 = OpLabel
%8 = OpVariable %7 Function
%10 = OpVariable %7 Function
OpStore %8 %9
%11 = OpLoad %6 %8
OpBranch %20
%20 = OpLabel
%21 = OpPhi %6 %11 %5
OpStore %10 %21
OpReturn
OpFunctionEnd
)";
const auto env = SPV_ENV_UNIVERSAL_1_3;
const auto consumer = nullptr;
const auto context = BuildModule(env, consumer, shader, kFuzzAssembleOption);
opt::Function* fun = context->cfg()->block(5)->GetParent(); // Computes the CFG analysis
opt::DominatorAnalysis* dom = nullptr;
dom = context->GetDominatorAnalysis(fun); // Computes the dominator analysis, which depends on the CFG analysis
context->InvalidateAnalysesExceptFor(opt::IRContext::Analysis::kAnalysisDominatorAnalysis); // Invalidates the CFG analysis
dom = context->GetDominatorAnalysis(fun); // Does *not* recompute the CFG analysis - depends on the now invalid CFG analysis
auto bb = dom->ImmediateDominator(5);
std::cout << bb->id(); // asan complains of use-after-free
}
The problem seems to be that InitializeTree uses cfg.pseudo_exit_block() and cfg.pseudo_entry_block() to get a dummy start node for the dominator tree for a given function, and these dummy nodes then become invalid if the CFG is invalidated.
One way to guard against this would be to have the dominator analysis require the CFG analysis to be valid, along the lines of:
DominatorAnalysis* IRContext::GetDominatorAnalysis(const Function* f) {
if (!AreAnalysesValid(kAnalysisDominatorAnalysis | kAnalysisCFG)) {
ResetDominatorAnalysis();
}
This would not be a proper fix, though, because it only guards against the problem if one always tries recomputing the dominator analysis before using it. If you just hang on to a dominator analysis and use it across calls that invalidate the CFG analysis, the use-after-free will still be there.
Thus it might be better to make it so that the dominator analysis does not keep references to data coming from the CFG analysis. Or to have it recompute such data if it can detect that the CFG analysis has become invalid.