mesa: Prevent classic swrast crash on a surfaceless context.

Submitted by Mathias Fröhlich on May 15, 2019, 7:26 a.m.

Details

Message ID d01c3d750259ded998fd1ad0fe30272254064ac7.1557904889.git.Mathias.Froehlich@gmx.net
State New
Headers show
Series "mesa: Prevent classic swrast crash on a surfaceless context." ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Mathias Fröhlich May 15, 2019, 7:26 a.m.
From: Mathias Fröhlich <mathias.froehlich@web.de>

Hi all,

One small fix below.

Please review!

best

Mathias





Running swrast with the new device egl extensions piglit test
brings up this failure. Fix that by adding some NULL pointer
checks.

Signed-off-by: Mathias Fröhlich <Mathias.Froehlich@web.de>
---
 src/mesa/main/fbobject.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

--
2.21.0

Patch hide | download patch | download mbox

diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 09143d30af5..225a7e8e9bd 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -1850,10 +1850,10 @@  _mesa_DeleteRenderbuffers(GLsizei n, const GLuint *renderbuffers)
              *     non-bound framebuffers is the responsibility of the
              *     application.
              */
-            if (_mesa_is_user_fbo(ctx->DrawBuffer)) {
+            if (ctx->DrawBuffer && _mesa_is_user_fbo(ctx->DrawBuffer)) {
                _mesa_detach_renderbuffer(ctx, ctx->DrawBuffer, rb);
             }
-            if (_mesa_is_user_fbo(ctx->ReadBuffer)
+            if (ctx->ReadBuffer && _mesa_is_user_fbo(ctx->ReadBuffer)
                 && ctx->ReadBuffer != ctx->DrawBuffer) {
                _mesa_detach_renderbuffer(ctx, ctx->ReadBuffer, rb);
             }
@@ -2881,7 +2881,6 @@  _mesa_bind_framebuffers(struct gl_context *ctx,
    const bool bindDrawBuf = oldDrawFb != newDrawFb;
    const bool bindReadBuf = oldReadFb != newReadFb;

-   assert(newDrawFb);
    assert(newDrawFb != &DummyFramebuffer);

    /*
@@ -2900,7 +2899,8 @@  _mesa_bind_framebuffers(struct gl_context *ctx,
       FLUSH_VERTICES(ctx, _NEW_BUFFERS);

       /* check if old readbuffer was render-to-texture */
-      check_end_texture_render(ctx, oldReadFb);
+      if (oldDrawFb)
+         check_end_texture_render(ctx, oldReadFb);

       _mesa_reference_framebuffer(&ctx->ReadBuffer, newReadFb);
    }
@@ -2914,7 +2914,8 @@  _mesa_bind_framebuffers(struct gl_context *ctx,
          check_end_texture_render(ctx, oldDrawFb);

       /* check if newly bound framebuffer has any texture attachments */
-      check_begin_texture_render(ctx, newDrawFb);
+      if (newDrawFb)
+         check_begin_texture_render(ctx, newDrawFb);

       _mesa_reference_framebuffer(&ctx->DrawBuffer, newDrawFb);
    }

Comments

On Wed, 15 May 2019 at 08:26, <Mathias.Froehlich@gmx.net> wrote:
>
> From: Mathias Fröhlich <mathias.froehlich@web.de>
>
> Hi all,
>
> One small fix below.
>
> Please review!
>
> best
>
> Mathias
>
>
>
>
>
> Running swrast with the new device egl extensions piglit test
> brings up this failure. Fix that by adding some NULL pointer
> checks.
>
From a very quick look this seems to be off.

The driver does not set the buffers as incomplete, thus in MakeCurrent
(and handle_first_current in particular) things go south.

For context grep for _mesa_get_incomplete_framebuffer and see how
i9[16]5 use the function when both read/draw priv. are NULL.
The gallium drivers st/mesa (really) also do that, yet classic swrast
(alongside r100, r200, nouveau_vieux) don't do that. Which is
understandable since those drivers receive far less testing than the
rest.

Can I suggest fleshing out some helper and fixing the drivers instead?

Note !intel classic drivers may also struggle with (draw xor read)
surfaces. It could be handled in the upper layers (dri/common or DRI
loaders - GLX/EGL/GBM) but haven't checked.

Thanks
Emil