Remove program executable backup during link

This was added in
https://chromium-review.googlesource.com/c/angle/angle/+/2181450/30 to
support the case where a program fails to relink, but needs to still be
usable.

However, this does not seem to be an issue anymore.  New tests are
specifically added for this, and they, along with every other test,
pass.

If this needs to be reintroduced, it needs to be rethought.  It does not
play well with parallel link as it changes the executable pointer while
link is in progress (and it was done on the assumption that everything
needing the executable is linked serially).  A better solution would
likely be an `mLastSuccessfullyLinkedExecutable` that normally points to
`mExecutable`, but not during link.  On `resolveLink`, it would either
make `mExecutable` point back to `mLastSuccessfullyLinkedExecutable`, or
the other way around based on whether the link was successful or not.

Bug: angleproject:8297
Change-Id: Ic9d55bccb75fff0253fe299a244bf1e4bbc416a6
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4781632
Reviewed-by: Yuxin Hu <yuxinhu@google.com>
Auto-Submit: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
Reviewed-by: Charlie Lao <cclao@google.com>

src/libANGLE/Program.cpp

@@ -563,7 +563,6 @@ void LoadInterfaceBlock(BinaryInputStream *stream, InterfaceBlock *block)
 // Saves the linking context for later use in resolveLink().
 struct Program::LinkingState
 {
-    std::shared_ptr<ProgramExecutable> linkedExecutable;
     ProgramLinkedResources resources;
     egl::BlobCache::Key programHash;
     std::unique_ptr<rx::LinkEvent> linkEvent;
@@ -1242,13 +1241,6 @@ angle::Result Program::link(const Context *context)
 
     angle::Result result = linkImpl(context);
 
-    // Avoid having two ProgramExecutables if the link failed and the Program had successfully
-    // linked previously.
-    if (mLinkingState && mLinkingState->linkedExecutable)
-    {
-        mState.mExecutable = mLinkingState->linkedExecutable;
-    }
-
     return result;
 }
 
@@ -1435,11 +1427,6 @@ angle::Result Program::linkImpl(const Context *context)
     mState.updateProgramInterfaceInputs();
     mState.updateProgramInterfaceOutputs();
 
-    if (mState.mSeparable)
-    {
-        mLinkingState->linkedExecutable = mState.mExecutable;
-    }
-
     return angle::Result::Continue;
 }
 
@@ -1584,12 +1571,6 @@ void ProgramState::updateProgramInterfaceOutputs()
 // Returns the program object to an unlinked state, before re-linking, or at destruction
 void Program::unlink()
 {
-    if (mLinkingState && mLinkingState->linkedExecutable)
-    {
-        // The new ProgramExecutable that we'll attempt to link with needs to start from a copy of
-        // the last successfully linked ProgramExecutable, so we don't lose any state information.
-        mState.mExecutable.reset(new ProgramExecutable(*mLinkingState->linkedExecutable));
-    }
     mState.mExecutable->reset(true);
 
     mState.mUniformLocations.clear();

src/tests/gl_tests/LinkAndRelinkTest.cpp

@@ -21,18 +21,18 @@ class LinkAndRelinkTest : public ANGLETest<>
     LinkAndRelinkTest() {}
 };
 
+class LinkAndRelinkTestES3 : public ANGLETest<>
+{
+  protected:
+    LinkAndRelinkTestES3() {}
+};
+
 class LinkAndRelinkTestES31 : public ANGLETest<>
 {
   protected:
     LinkAndRelinkTestES31() {}
 };
 
-class LinkAndRelinkTestES32 : public ANGLETest<>
-{
-  protected:
-    LinkAndRelinkTestES32() {}
-};
-
 // When a program link or relink fails, if you try to install the unsuccessfully
 // linked program (via UseProgram) and start rendering or dispatch compute,
 // We can not always report INVALID_OPERATION for rendering/compute pipeline.
@@ -611,8 +611,195 @@ TEST_P(LinkAndRelinkTest, AttachShaderThenCompile)
     ASSERT_GL_NO_ERROR();
 }
 
+// If a program is linked successfully once, it should retain its executable if a relink fails.
+TEST_P(LinkAndRelinkTestES3, SucessfullLinkThenFailingRelink)
+{
+    // Install a render program in current GL state via UseProgram, then render.
+    // It should succeed.
+    constexpr char kVS[]    = R"(#version 300 es
+out vec4 color;
+void main()
+{
+    vec2 position = vec2(-1, -1);
+    if (gl_VertexID == 1)
+        position = vec2(3, -1);
+    else if (gl_VertexID == 2)
+        position = vec2(-1, 3);
+
+    gl_Position = vec4(position, 0, 1);
+    color = vec4(0, 1, 0, 1);
+})";
+    constexpr char kBadFS[] = R"(#version 300 es
+flat in uvec2 color;
+out mediump vec4 colorOut;
+void main()
+{
+    colorOut = vec4(1, color, 1);
+})";
+
+    GLuint program = glCreateProgram();
+    GLuint vs      = CompileShader(GL_VERTEX_SHADER, kVS);
+    GLuint fs      = CompileShader(GL_FRAGMENT_SHADER, essl3_shaders::fs::Green());
+    GLuint badfs   = CompileShader(GL_FRAGMENT_SHADER, kBadFS);
+
+    EXPECT_NE(0u, vs);
+    EXPECT_NE(0u, fs);
+    EXPECT_NE(0u, badfs);
+
+    glAttachShader(program, vs);
+    glAttachShader(program, fs);
+
+    glLinkProgram(program);
+
+    GLint linkStatus;
+    glGetProgramiv(program, GL_LINK_STATUS, &linkStatus);
+    EXPECT_GL_TRUE(linkStatus);
+
+    EXPECT_GL_NO_ERROR();
+
+    const int w = getWindowWidth();
+    const int h = getWindowHeight();
+
+    glViewport(0, 0, w, h);
+
+    glClearColor(0, 0, 0, 1);
+    glClear(GL_COLOR_BUFFER_BIT);
+
+    glEnable(GL_SCISSOR_TEST);
+    glScissor(0, 0, w / 2, h / 2);
+
+    glUseProgram(program);
+    glDrawArrays(GL_TRIANGLES, 0, 3);
+    EXPECT_GL_NO_ERROR();
+
+    // Cause the program to fail linking
+    glDetachShader(program, fs);
+    glAttachShader(program, badfs);
+
+    glLinkProgram(program);
+
+    glGetProgramiv(program, GL_LINK_STATUS, &linkStatus);
+    EXPECT_GL_FALSE(linkStatus);
+
+    // Link failed, but the program should still be usable.
+    glScissor(w / 2, h / 2, w / 2, h / 2);
+    glDrawArrays(GL_TRIANGLES, 0, 3);
+    EXPECT_GL_NO_ERROR();
+
+    EXPECT_PIXEL_RECT_EQ(0, 0, w / 2, h / 2, GLColor::green);
+    EXPECT_PIXEL_RECT_EQ(w / 2, 0, w / 2, h / 2, GLColor::black);
+    EXPECT_PIXEL_RECT_EQ(0, h / 2, w / 2, h / 2, GLColor::black);
+    EXPECT_PIXEL_RECT_EQ(w / 2, h / 2, w / 2, h / 2, GLColor::green);
+
+    glDeleteShader(vs);
+    glDeleteShader(fs);
+    glDeleteShader(badfs);
+    glDeleteProgram(program);
+}
+
+// Same as SucessfullLinkThenFailingRelink, but with PPOs.
+TEST_P(LinkAndRelinkTestES31, SucessfullLinkThenFailingRelinkWithPPO)
+{
+    // Only the Vulkan backend supports PPOs.
+    ANGLE_SKIP_TEST_IF(!IsVulkan());
+    ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_geometry_shader"));
+
+    // Install a render program in current GL state via UseProgram, then render.
+    // It should succeed.
+    constexpr char kVS[]    = R"(#version 300 es
+void main()
+{
+    vec2 position = vec2(-1, -1);
+    if (gl_VertexID == 1)
+        position = vec2(3, -1);
+    else if (gl_VertexID == 2)
+        position = vec2(-1, 3);
+
+    gl_Position = vec4(position, 0, 1);
+})";
+    constexpr char kBadGS[] = R"(#version 310 es
+#extension GL_EXT_geometry_shader : require
+layout (invocations = 3, triangles) in;
+layout (triangle_strip, max_vertices = 3) out;
+void main()
+{
+})";
+
+    GLuint vs    = CompileShader(GL_VERTEX_SHADER, kVS);
+    GLuint fs    = CompileShader(GL_FRAGMENT_SHADER, essl3_shaders::fs::Green());
+    GLuint badgs = CompileShader(GL_GEOMETRY_SHADER, kBadGS);
+
+    EXPECT_NE(0u, vs);
+    EXPECT_NE(0u, fs);
+    EXPECT_NE(0u, badgs);
+
+    GLuint vsProg = glCreateProgram();
+    GLuint fsProg = glCreateProgram();
+
+    glProgramParameteri(vsProg, GL_PROGRAM_SEPARABLE, GL_TRUE);
+    glAttachShader(vsProg, vs);
+    glLinkProgram(vsProg);
+    EXPECT_GL_NO_ERROR();
+
+    glProgramParameteri(fsProg, GL_PROGRAM_SEPARABLE, GL_TRUE);
+    glAttachShader(fsProg, fs);
+    glLinkProgram(fsProg);
+    EXPECT_GL_NO_ERROR();
+
+    GLuint pipeline;
+    glGenProgramPipelines(1, &pipeline);
+    glUseProgramStages(pipeline, GL_VERTEX_SHADER_BIT, vsProg);
+    glUseProgramStages(pipeline, GL_FRAGMENT_SHADER_BIT, fsProg);
+    EXPECT_GL_NO_ERROR();
+
+    const int w = getWindowWidth();
+    const int h = getWindowHeight();
+
+    glViewport(0, 0, w, h);
+
+    glClearColor(0, 0, 0, 1);
+    glClear(GL_COLOR_BUFFER_BIT);
+
+    glEnable(GL_SCISSOR_TEST);
+    glScissor(0, 0, w / 2, h / 2);
+
+    glUseProgram(0);
+    glBindProgramPipeline(pipeline);
+    glDrawArrays(GL_TRIANGLES, 0, 3);
+    EXPECT_GL_NO_ERROR();
+
+    // Cause the fs program to fail linking
+    glAttachShader(fsProg, badgs);
+    glLinkProgram(fsProg);
+    glUseProgramStages(pipeline, GL_GEOMETRY_SHADER_BIT, fsProg);
+    EXPECT_GL_ERROR(GL_INVALID_OPERATION);
+
+    GLint linkStatus;
+    glGetProgramiv(fsProg, GL_LINK_STATUS, &linkStatus);
+    EXPECT_GL_FALSE(linkStatus);
+
+    // Program link failed, but the program pipeline should still be usable.
+    glScissor(w / 2, h / 2, w / 2, h / 2);
+    glDrawArrays(GL_TRIANGLES, 0, 3);
+    EXPECT_GL_NO_ERROR();
+
+    EXPECT_PIXEL_RECT_EQ(0, 0, w / 2, h / 2, GLColor::green);
+    EXPECT_PIXEL_RECT_EQ(w / 2, 0, w / 2, h / 2, GLColor::black);
+    EXPECT_PIXEL_RECT_EQ(0, h / 2, w / 2, h / 2, GLColor::black);
+    EXPECT_PIXEL_RECT_EQ(w / 2, h / 2, w / 2, h / 2, GLColor::green);
+
+    glDeleteShader(vs);
+    glDeleteShader(fs);
+    glDeleteShader(badgs);
+    glDeleteProgram(vsProg);
+    glDeleteProgram(fsProg);
+    glDeleteProgramPipelines(1, &pipeline);
+}
+
 ANGLE_INSTANTIATE_TEST_ES2_AND_ES3(LinkAndRelinkTest);
 
+ANGLE_INSTANTIATE_TEST_ES3(LinkAndRelinkTestES3);
+
 GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(LinkAndRelinkTestES31);
 ANGLE_INSTANTIATE_TEST_ES31(LinkAndRelinkTestES31);