<div dir="ltr"><div>Would you please review this fixed version:</div><div><a href="https://cgit.freedesktop.org/~mareko/mesa/commit/?h=master&id=40e4702ef815410f74130f349e2b40cc0524e422">https://cgit.freedesktop.org/~mareko/mesa/commit/?h=master&id=40e4702ef815410f74130f349e2b40cc0524e422</a></div><div> </div><div>It trivially solves the GBM crash by checking that gbm_surf != NULL before using it.</div><div> </div><div>Marek </div></div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, May 20, 2019 at 3:03 PM Mathias Fröhlich <<a href="mailto:Mathias.Froehlich@gmx.net">Mathias.Froehlich@gmx.net</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi, I did not have time to really look into the series, but a quick retest: wflinfo --platform=gbm -a gl still crashes on my site. Can you recheck that? best Mathias On Thursday, 16 May 2019 19:01:38 CEST Emil Velikov wrote: > From: Emil Velikov <<a href="mailto:emil.velikov@collabora.com" target="_blank">emil.velikov@collabora.com</a>> > > Wrap the loader->createNewDrawable() dance into a helper and use it > throughout the codebase. > > This addresses a cases like surfaceless (SL) on swrast (SL on kms_swrast > is fine) where we'd attempt using the wrong driver and crash out. > > v2: fixup quirky GBM (Mathias) > > Cc: <a href="mailto:mesa-stable@lists.freedesktop.org" target="_blank">mesa-stable@lists.freedesktop.org</a> > Cc: Mathias Fröhlich <<a href="mailto:Mathias.Froehlich@web.de" target="_blank">Mathias.Froehlich@web.de</a>> > Reviewed-by: Mathias Fröhlich <<a href="mailto:Mathias.Froehlich@web.de" target="_blank">Mathias.Froehlich@web.de</a>> (v1) > Reviewed-by: Marek Olšák <<a href="mailto:marek.olsak@amd.com" target="_blank">marek.olsak@amd.com</a>> (v1) > Signed-off-by: Emil Velikov <<a href="mailto:emil.velikov@collabora.com" target="_blank">emil.velikov@collabora.com</a>> > --- > src/egl/drivers/dri2/egl_dri2.c | 39 +++++++++++++++++++++ > src/egl/drivers/dri2/egl_dri2.h | 5 +++ > src/egl/drivers/dri2/platform_android.c | 12 +------ > src/egl/drivers/dri2/platform_drm.c | 17 +-------- > src/egl/drivers/dri2/platform_surfaceless.c | 7 +--- > src/egl/drivers/dri2/platform_wayland.c | 14 +------- > src/egl/drivers/dri2/platform_x11.c | 15 +------- > 7 files changed, 49 insertions(+), 60 deletions(-) > > diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c > index 09d6315b19e..17a0176b646 100644 > --- a/src/egl/drivers/dri2/egl_dri2.c > +++ b/src/egl/drivers/dri2/egl_dri2.c > @@ -1425,6 +1425,45 @@ dri2_surf_update_fence_fd(_EGLContext *ctx, > dri2_surface_set_out_fence_fd(surf, fence_fd); > } > > +static inline _EGLDisplay * > +dri2_display_to_egl_display(struct dri2_egl_display *dri2_dpy) > +{ > + const size_t offset = offsetof(_EGLDisplay, DriverData); > + return (_EGLDisplay *)(((void*) dri2_dpy) - offset); > +} > + > +EGLBoolean > +dri2_create_drawable(struct dri2_egl_display *dri2_dpy, > + const __DRIconfig *config, > + struct dri2_egl_surface *dri2_surf) > +{ > + __DRIcreateNewDrawableFunc createNewDrawable; > + _EGLDisplay *disp = dri2_display_to_egl_display(dri2_dpy); > + void *loaderPrivate = dri2_surf; > + > + if (dri2_dpy->image_driver) > + createNewDrawable = dri2_dpy->image_driver->createNewDrawable; > + else if (dri2_dpy->dri2) > + createNewDrawable = dri2_dpy->dri2->createNewDrawable; > + else if (dri2_dpy->swrast) > + createNewDrawable = dri2_dpy->swrast->createNewDrawable; > + else > + return _eglError(EGL_BAD_ALLOC, "no createNewDrawable"); > + > + /* As always gbm is a bit special.. */ > +#ifdef HAVE_DRM_PLATFORM > + if (disp->Platform == _EGL_PLATFORM_DRM) > + loaderPrivate = dri2_surf->gbm_surf; > +#endif > + > + dri2_surf->dri_drawable = (*createNewDrawable)(dri2_dpy->dri_screen, > + config, loaderPrivate); > + if (dri2_surf->dri_drawable == NULL) > + return _eglError(EGL_BAD_ALLOC, "createNewDrawable"); > + > + return EGL_TRUE; > +} > + > /** > * Called via eglMakeCurrent(), drv->API.MakeCurrent(). > */ > diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h > index 4918517b572..5555bcea3ab 100644 > --- a/src/egl/drivers/dri2/egl_dri2.h > +++ b/src/egl/drivers/dri2/egl_dri2.h > @@ -541,6 +541,11 @@ dri2_init_surface(_EGLSurface *surf, _EGLDisplay *disp, EGLint type, > void > dri2_fini_surface(_EGLSurface *surf); > > +EGLBoolean > +dri2_create_drawable(struct dri2_egl_display *dri2_dpy, > + const __DRIconfig *config, > + struct dri2_egl_surface *dri2_surf); > + > static inline uint64_t > combine_u32_into_u64(uint32_t hi, uint32_t lo) > { > diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c > index fece214d939..e44447c7df7 100644 > --- a/src/egl/drivers/dri2/platform_android.c > +++ b/src/egl/drivers/dri2/platform_android.c > @@ -341,7 +341,6 @@ droid_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type, > _EGLConfig *conf, void *native_window, > const EGLint *attrib_list) > { > - __DRIcreateNewDrawableFunc createNewDrawable; > struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp); > struct dri2_egl_config *dri2_conf = dri2_egl_config(conf); > struct dri2_egl_surface *dri2_surf; > @@ -386,17 +385,8 @@ droid_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type, > goto cleanup_surface; > } > > - if (dri2_dpy->image_driver) > - createNewDrawable = dri2_dpy->image_driver->createNewDrawable; > - else > - createNewDrawable = dri2_dpy->dri2->createNewDrawable; > - > - dri2_surf->dri_drawable = (*createNewDrawable)(dri2_dpy->dri_screen, config, > - dri2_surf); > - if (dri2_surf->dri_drawable == NULL) { > - _eglError(EGL_BAD_ALLOC, "createNewDrawable"); > + if (!dri2_create_drawable(dri2_dpy, config, dri2_surf)) > goto cleanup_surface; > - } > > if (window) { > window->common.incRef(&window->common); > diff --git a/src/egl/drivers/dri2/platform_drm.c b/src/egl/drivers/dri2/platform_drm.c > index 7d916fe92b7..c59b9341d87 100644 > --- a/src/egl/drivers/dri2/platform_drm.c > +++ b/src/egl/drivers/dri2/platform_drm.c > @@ -171,23 +171,8 @@ dri2_drm_create_window_surface(_EGLDriver *drv, _EGLDisplay *disp, > dri2_surf->base.Height = surf->base.height; > surf->dri_private = dri2_surf; > > - if (dri2_dpy->dri2) { > - dri2_surf->dri_drawable = > - dri2_dpy->dri2->createNewDrawable(dri2_dpy->dri_screen, config, > - dri2_surf->gbm_surf); > - > - } else { > - assert(dri2_dpy->swrast != NULL); > - > - dri2_surf->dri_drawable = > - dri2_dpy->swrast->createNewDrawable(dri2_dpy->dri_screen, config, > - dri2_surf->gbm_surf); > - > - } > - if (dri2_surf->dri_drawable == NULL) { > - _eglError(EGL_BAD_ALLOC, "createNewDrawable()"); > + if (!dri2_create_drawable(dri2_dpy, config, dri2_surf)) > goto cleanup_surf; > - } > > return &dri2_surf->base; > > diff --git a/src/egl/drivers/dri2/platform_surfaceless.c b/src/egl/drivers/dri2/platform_surfaceless.c > index 27b1d44ebec..ce9f7d0f980 100644 > --- a/src/egl/drivers/dri2/platform_surfaceless.c > +++ b/src/egl/drivers/dri2/platform_surfaceless.c > @@ -136,13 +136,8 @@ dri2_surfaceless_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type, > goto cleanup_surface; > } > > - dri2_surf->dri_drawable = > - dri2_dpy->image_driver->createNewDrawable(dri2_dpy->dri_screen, config, > - dri2_surf); > - if (dri2_surf->dri_drawable == NULL) { > - _eglError(EGL_BAD_ALLOC, "image->createNewDrawable"); > + if (!dri2_create_drawable(dri2_dpy, config, dri2_surf)) > goto cleanup_surface; > - } > > if (conf->RedSize == 5) > dri2_surf->visual = __DRI_IMAGE_FORMAT_RGB565; > diff --git a/src/egl/drivers/dri2/platform_wayland.c b/src/egl/drivers/dri2/platform_wayland.c > index 0f5d85be326..fb5ecdab2c5 100644 > --- a/src/egl/drivers/dri2/platform_wayland.c > +++ b/src/egl/drivers/dri2/platform_wayland.c > @@ -272,7 +272,6 @@ dri2_wl_create_window_surface(_EGLDriver *drv, _EGLDisplay *disp, > _EGLConfig *conf, void *native_window, > const EGLint *attrib_list) > { > - __DRIcreateNewDrawableFunc createNewDrawable; > struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp); > struct dri2_egl_config *dri2_conf = dri2_egl_config(conf); > struct wl_egl_window *window = native_window; > @@ -349,19 +348,8 @@ dri2_wl_create_window_surface(_EGLDriver *drv, _EGLDisplay *disp, > if (dri2_dpy->flush) > dri2_surf->wl_win->resize_callback = resize_callback; > > - if (dri2_dpy->image_driver) > - createNewDrawable = dri2_dpy->image_driver->createNewDrawable; > - else if (dri2_dpy->dri2) > - createNewDrawable = dri2_dpy->dri2->createNewDrawable; > - else > - createNewDrawable = dri2_dpy->swrast->createNewDrawable; > - > - dri2_surf->dri_drawable = (*createNewDrawable)(dri2_dpy->dri_screen, config, > - dri2_surf); > - if (dri2_surf->dri_drawable == NULL) { > - _eglError(EGL_BAD_ALLOC, "createNewDrawable"); > + if (!dri2_create_drawable(dri2_dpy, config, dri2_surf)) > goto cleanup_surf_wrapper; > - } > > dri2_surf->base.SwapInterval = dri2_dpy->default_swap_interval; > > diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/platform_x11.c > index 538cffd6a76..be9eb513243 100644 > --- a/src/egl/drivers/dri2/platform_x11.c > +++ b/src/egl/drivers/dri2/platform_x11.c > @@ -290,21 +290,8 @@ dri2_x11_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type, > goto cleanup_pixmap; > } > > - if (dri2_dpy->dri2) { > - dri2_surf->dri_drawable = > - dri2_dpy->dri2->createNewDrawable(dri2_dpy->dri_screen, config, > - dri2_surf); > - } else { > - assert(dri2_dpy->swrast); > - dri2_surf->dri_drawable = > - dri2_dpy->swrast->createNewDrawable(dri2_dpy->dri_screen, config, > - dri2_surf); > - } > - > - if (dri2_surf->dri_drawable == NULL) { > - _eglError(EGL_BAD_ALLOC, "dri2->createNewDrawable"); > + if (!dri2_create_drawable(dri2_dpy, config, dri2_surf)) > goto cleanup_pixmap; > - } > > if (type != EGL_PBUFFER_BIT) { > cookie = xcb_get_geometry (dri2_dpy->conn, dri2_surf->drawable); > _______________________________________________ mesa-stable mailing list <a href="mailto:mesa-stable@lists.freedesktop.org" target="_blank">mesa-stable@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-stable" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-stable</a></blockquote></div>