Commit d3e1a7ff by Tim Van Patten Committed by Commit Bot

Reset mCurrentGraphicsPipeline in ProgramExecutableVk::reset

ContextVk::mCurrentGraphicsPipeline should be reset during ProgramExecutableVk::reset() since programInfo.release() frees the PipelineHelper that it's pointing to. This has resulted in several use-after-free errors, which this CL will prevent in the future. Bug: angleproject:5624 Bug: b/182409935 Change-Id: I847bb7eb5b593c89b84f0fbbca23ea5367f5f55c Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2792861 Commit-Queue: Tim Van Patten <timvp@google.com> Reviewed-by: 's avatarShahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarCody Northrop <cnorthrop@google.com>
parent 54dfb62c
...@@ -1315,6 +1315,7 @@ angle::Result ContextVk::handleDirtyGraphicsPipelineDesc(DirtyBits::Iterator *di ...@@ -1315,6 +1315,7 @@ angle::Result ContextVk::handleDirtyGraphicsPipelineDesc(DirtyBits::Iterator *di
} }
else if (mGraphicsPipelineTransition.any()) else if (mGraphicsPipelineTransition.any())
{ {
ASSERT(mCurrentGraphicsPipeline->valid());
if (!mCurrentGraphicsPipeline->findTransition( if (!mCurrentGraphicsPipeline->findTransition(
mGraphicsPipelineTransition, *mGraphicsPipelineDesc, &mCurrentGraphicsPipeline)) mGraphicsPipelineTransition, *mGraphicsPipelineDesc, &mCurrentGraphicsPipeline))
{ {
...@@ -4604,11 +4605,6 @@ angle::Result ContextVk::updateActiveTextures(const gl::Context *context) ...@@ -4604,11 +4605,6 @@ angle::Result ContextVk::updateActiveTextures(const gl::Context *context)
// TODO(http://anglebug.com/5033): This will recreate the descriptor pools each time, which // TODO(http://anglebug.com/5033): This will recreate the descriptor pools each time, which
// will likely affect performance negatively. // will likely affect performance negatively.
ANGLE_TRY(mExecutable->createPipelineLayout(context, &mActiveTextures)); ANGLE_TRY(mExecutable->createPipelineLayout(context, &mActiveTextures));
invalidateCurrentGraphicsPipeline();
// TODO(http://anglebug.com/5624): rework updateActiveTextures(), createPipelineLayout(),
// and handleDirtyGraphicsPipeline().
mCurrentGraphicsPipeline = nullptr;
// The default uniforms descriptor set was reset during createPipelineLayout(), so mark them // The default uniforms descriptor set was reset during createPipelineLayout(), so mark them
// dirty to get everything reallocated/rebound before the next draw. // dirty to get everything reallocated/rebound before the next draw.
...@@ -5472,6 +5468,37 @@ angle::Result ContextVk::initializeMultisampleTextureToBlack(const gl::Context * ...@@ -5472,6 +5468,37 @@ angle::Result ContextVk::initializeMultisampleTextureToBlack(const gl::Context *
return textureVk->initializeContents(context, gl::ImageIndex::Make2DMultisample()); return textureVk->initializeContents(context, gl::ImageIndex::Make2DMultisample());
} }
void ContextVk::onProgramExecutableReset(ProgramExecutableVk *executableVk)
{
const gl::ProgramExecutable *executable = getState().getProgramExecutable();
if (!executable)
{
return;
}
// Only do this for the currently bound ProgramExecutableVk, since Program A can be linked while
// Program B is currently in use and we don't want to reset/invalidate Program B's pipeline.
if (executableVk != mExecutable)
{
return;
}
// Reset *ContextVk::mCurrentGraphicsPipeline, since programInfo.release() freed the
// PipelineHelper that it's currently pointing to.
// TODO(http://anglebug.com/5624): rework updateActiveTextures(), createPipelineLayout(),
// handleDirtyGraphicsPipeline(), and ProgramPipelineVk::link().
resetCurrentGraphicsPipeline();
if (executable->isCompute())
{
invalidateCurrentComputePipeline();
}
else
{
invalidateCurrentGraphicsPipeline();
}
}
angle::Result ContextVk::updateRenderPassDepthStencilAccess() angle::Result ContextVk::updateRenderPassDepthStencilAccess()
{ {
if (hasStartedRenderPass() && mDrawFramebuffer->getDepthStencilRenderTarget()) if (hasStartedRenderPass() && mDrawFramebuffer->getDepthStencilRenderTarget())
......
...@@ -616,6 +616,12 @@ class ContextVk : public ContextImpl, public vk::Context, public MultisampleText ...@@ -616,6 +616,12 @@ class ContextVk : public ContextImpl, public vk::Context, public MultisampleText
angle::Result initializeMultisampleTextureToBlack(const gl::Context *context, angle::Result initializeMultisampleTextureToBlack(const gl::Context *context,
gl::Texture *glTexture) override; gl::Texture *glTexture) override;
// TODO(http://anglebug.com/5624): rework updateActiveTextures(), createPipelineLayout(),
// handleDirtyGraphicsPipeline(), and ProgramPipelineVk::link().
void resetCurrentGraphicsPipeline() { mCurrentGraphicsPipeline = nullptr; }
void onProgramExecutableReset(ProgramExecutableVk *executableVk);
private: private:
// Dirty bits. // Dirty bits.
enum DirtyBitType : size_t enum DirtyBitType : size_t
......
...@@ -266,6 +266,8 @@ void ProgramExecutableVk::reset(ContextVk *contextVk) ...@@ -266,6 +266,8 @@ void ProgramExecutableVk::reset(ContextVk *contextVk)
programInfo.release(contextVk); programInfo.release(contextVk);
} }
mComputeProgramInfo.release(contextVk); mComputeProgramInfo.release(contextVk);
contextVk->onProgramExecutableReset(this);
} }
std::unique_ptr<rx::LinkEvent> ProgramExecutableVk::load(gl::BinaryInputStream *stream) std::unique_ptr<rx::LinkEvent> ProgramExecutableVk::load(gl::BinaryInputStream *stream)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment