Commit 5d01d538 by Rafael Cintron Committed by Commit Bot

Resolve Bad Binary Link Failures

When ANGLE_PROGRAM_BINARY_LOAD is enabled, Program::loadBinary unconditionally returns angle::Result::Continue to the caller. The caller, gl::Program::link, postpones the resolution of the link until resolveLinkImpl. Unfortunately, resolveLinkImpl is not able to tell whether the link failed because the shader from the developer is bad or because the loaded binary is not compatible with the backend. The former case should fail link. In the latter case, we should fallback to linking the program from the original shader sources. The loaded binary could be read from the on-disk shader cache and be corrupted or serialized with different revision and subsystem id than the currently loaded ANGLE backend. This fix adjusts Program::loadBinary and ProgramD3D::load so that angle::Result::Incomplete is returned to gl::Program::link when the binary is incompatible with the backend. gl::Program:link falls back to compilation from original shader sources. Since no code checks the return value of SizedMRUCache::eraseByKey, modified it to now return void. Bug: chromium:1079497 Change-Id: Id5271d7badad8627563e87859d1c9fdb81de5785 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2197944Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarTim Van Patten <timvp@google.com> Commit-Queue: Rafael Cintron <rafael.cintron@microsoft.com>
parent c55d2b41
...@@ -147,8 +147,7 @@ bool BlobCache::getAt(size_t index, const BlobCache::Key **keyOut, BlobCache::Va ...@@ -147,8 +147,7 @@ bool BlobCache::getAt(size_t index, const BlobCache::Key **keyOut, BlobCache::Va
void BlobCache::remove(const BlobCache::Key &key) void BlobCache::remove(const BlobCache::Key &key)
{ {
bool result = mBlobCache.eraseByKey(key); mBlobCache.eraseByKey(key);
ASSERT(result);
} }
void BlobCache::setBlobCacheFuncs(EGLSetBlobFuncANDROID set, EGLGetBlobFuncANDROID get) void BlobCache::setBlobCacheFuncs(EGLSetBlobFuncANDROID set, EGLGetBlobFuncANDROID get)
......
...@@ -1865,12 +1865,41 @@ angle::Result Program::loadBinary(const Context *context, ...@@ -1865,12 +1865,41 @@ angle::Result Program::loadBinary(const Context *context,
mDirtyBits.set(uniformBlockIndex); mDirtyBits.set(uniformBlockIndex);
} }
mLinkingState.reset(new LinkingState()); // The rx::LinkEvent returned from ProgramImpl::load is a base class with multiple
mLinkingState->linkingFromBinary = true; // implementations. In some implementations, a background thread is used to compile the
mLinkingState->linkEvent = mProgram->load(context, &stream, infoLog); // shaders. Any calls to the LinkEvent object, therefore, are racy and may interfere with
// the operation.
// We do not want to call LinkEvent::wait because that will cause the background thread
// to finish its task before returning, thus defeating the purpose of background compilation.
// We need to defer waiting on background compilation until the very last minute when we
// absolutely need the results, such as when the developer binds the program or queries
// for the completion status.
// If load returns nullptr, we know for sure that the binary is not compatible with the backend.
// The loaded binary could have been read from the on-disk shader cache and be corrupted or
// serialized with different revision and subsystem id than the currently loaded backend.
// Returning 'Incomplete' to the caller results in link happening using the original shader
// sources.
angle::Result result;
std::unique_ptr<LinkingState> linkingState;
std::unique_ptr<rx::LinkEvent> linkEvent = mProgram->load(context, &stream, infoLog);
if (linkEvent)
{
linkingState = std::make_unique<LinkingState>();
linkingState->linkingFromBinary = true;
linkingState->linkEvent = std::move(linkEvent);
result = angle::Result::Continue;
mLinkResolved = false; mLinkResolved = false;
}
else
{
result = angle::Result::Incomplete;
mLinkResolved = true;
}
mLinkingState = std::move(linkingState);
return angle::Result::Continue; return result;
#endif // #if ANGLE_PROGRAM_BINARY_LOAD == ANGLE_ENABLED #endif // #if ANGLE_PROGRAM_BINARY_LOAD == ANGLE_ENABLED
} }
......
...@@ -75,7 +75,7 @@ class SizedMRUCache final : angle::NonCopyable ...@@ -75,7 +75,7 @@ class SizedMRUCache final : angle::NonCopyable
mCurrentSize = 0; mCurrentSize = 0;
} }
bool eraseByKey(const Key &key) void eraseByKey(const Key &key)
{ {
// Check for existing key. // Check for existing key.
auto existing = mStore.Peek(key); auto existing = mStore.Peek(key);
...@@ -83,10 +83,7 @@ class SizedMRUCache final : angle::NonCopyable ...@@ -83,10 +83,7 @@ class SizedMRUCache final : angle::NonCopyable
{ {
mCurrentSize -= existing->second.size; mCurrentSize -= existing->second.size;
mStore.Erase(existing); mStore.Erase(existing);
return true;
} }
return false;
} }
size_t entryCount() const { return mStore.size(); } size_t entryCount() const { return mStore.size(); }
......
...@@ -963,14 +963,14 @@ std::unique_ptr<rx::LinkEvent> ProgramD3D::load(const gl::Context *context, ...@@ -963,14 +963,14 @@ std::unique_ptr<rx::LinkEvent> ProgramD3D::load(const gl::Context *context,
if (memcmp(&identifier, &binaryDeviceIdentifier, sizeof(DeviceIdentifier)) != 0) if (memcmp(&identifier, &binaryDeviceIdentifier, sizeof(DeviceIdentifier)) != 0)
{ {
infoLog << "Invalid program binary, device configuration has changed."; infoLog << "Invalid program binary, device configuration has changed.";
return std::make_unique<LinkEventDone>(angle::Result::Incomplete); return nullptr;
} }
int compileFlags = stream->readInt<int>(); int compileFlags = stream->readInt<int>();
if (compileFlags != ANGLE_COMPILE_OPTIMIZATION_LEVEL) if (compileFlags != ANGLE_COMPILE_OPTIMIZATION_LEVEL)
{ {
infoLog << "Mismatched compilation flags."; infoLog << "Mismatched compilation flags.";
return std::make_unique<LinkEventDone>(angle::Result::Incomplete); return nullptr;
} }
for (int &index : mAttribLocationToD3DSemantic) for (int &index : mAttribLocationToD3DSemantic)
...@@ -1033,7 +1033,7 @@ std::unique_ptr<rx::LinkEvent> ProgramD3D::load(const gl::Context *context, ...@@ -1033,7 +1033,7 @@ std::unique_ptr<rx::LinkEvent> ProgramD3D::load(const gl::Context *context,
if (stream->error()) if (stream->error())
{ {
infoLog << "Invalid program binary."; infoLog << "Invalid program binary.";
return std::make_unique<LinkEventDone>(angle::Result::Incomplete); return nullptr;
} }
ASSERT(mD3DShaderStorageBlocks.empty()); ASSERT(mD3DShaderStorageBlocks.empty());
...@@ -1051,7 +1051,7 @@ std::unique_ptr<rx::LinkEvent> ProgramD3D::load(const gl::Context *context, ...@@ -1051,7 +1051,7 @@ std::unique_ptr<rx::LinkEvent> ProgramD3D::load(const gl::Context *context,
if (stream->error()) if (stream->error())
{ {
infoLog << "Invalid program binary."; infoLog << "Invalid program binary.";
return std::make_unique<LinkEventDone>(angle::Result::Incomplete); return nullptr;
} }
ASSERT(mImage2DUniforms.empty()); ASSERT(mImage2DUniforms.empty());
...@@ -1073,7 +1073,7 @@ std::unique_ptr<rx::LinkEvent> ProgramD3D::load(const gl::Context *context, ...@@ -1073,7 +1073,7 @@ std::unique_ptr<rx::LinkEvent> ProgramD3D::load(const gl::Context *context,
if (stream->error()) if (stream->error())
{ {
infoLog << "Invalid program binary."; infoLog << "Invalid program binary.";
return std::make_unique<LinkEventDone>(angle::Result::Incomplete); return nullptr;
} }
const auto &linkedUniforms = mState.getUniforms(); const auto &linkedUniforms = mState.getUniforms();
...@@ -1100,7 +1100,7 @@ std::unique_ptr<rx::LinkEvent> ProgramD3D::load(const gl::Context *context, ...@@ -1100,7 +1100,7 @@ std::unique_ptr<rx::LinkEvent> ProgramD3D::load(const gl::Context *context,
if (stream->error()) if (stream->error())
{ {
infoLog << "Invalid program binary."; infoLog << "Invalid program binary.";
return std::make_unique<LinkEventDone>(angle::Result::Incomplete); return nullptr;
} }
ASSERT(mD3DUniformBlocks.empty()); ASSERT(mD3DUniformBlocks.empty());
......
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