Re: [PATCH] glamor: fix crash when drawing nothing
Am 16.10.2015 18:11, schrieb Rob Clark: > For example, in the PolyFillRect() path w/ nrect==0, we end up in > glamor_get_vbo_space(size=0): > >(gdb) bt >#0 0x007fb73df340 in raise () from /lib64/libc.so.6 >#1 0x007fb73e0fb8 in abort () from /lib64/libc.so.6 >#2 0x007fb73d84f4 in __assert_fail_base () from /lib64/libc.so.6 >#3 0x007fb73d859c in __assert_fail () from /lib64/libc.so.6 >#4 0x0045dc38 in glamor_get_vbo_space (screen=0x8dcb60, > size=size@entry=0, vbo_offset=0x7ff250, vbo_offset@entry=0x7ff2d0) at > glamor_vbo.c:112 >#5 0x004541ac in glamor_poly_fill_rect_gl (prect=0x11be650, > nrect=0, gc=0x4cda70 , drawable=0x7ff388) at > glamor_rects.c:72 >#6 glamor_poly_fill_rect (drawable=0x7ff388, gc=0x4cda70 > , nrect=0, prect=0x11be650) at glamor_rects.c:162 >#7 0x00557d0c in damagePolyFillRect (pDrawable=0x112c300, > pGC=0xe81580, nRects=0, pRects=) at damage.c:1194 >#8 0x004cdb20 in miPaintWindow (pWin=, > prgn=0x7ff388, what=) at miexpose.c:540 >#9 0x004db260 in miClearToBackground (pWin=0x112c300, > x=, y=, w=, h=, > generateExposures=0) at miwindow.c:116 >#10 0x004646e4 in ProcClearToBackground (client=0x111e810) at > dispatch.c:1592 >#11 0x0046907c in Dispatch () at dispatch.c:430 >#12 0x0046cf90 in dix_main (argc=5, argv=0x7ff628, > envp=) at main.c:300 >#13 0x007fb73cb68c in __libc_start_main () from /lib64/libc.so.6 >#14 0x0042b3e8 in _start () > > Also fixed a bunch of other call-sites which could in theory trigger the > same issue. > > v2: add back the early return if size==0 in glamor_get_vbo_space() just > in case. We'd hit a GL error in glamor_put_vbo_space() if we ever ended > up with a call path that hit that (so keep the earlier early-returns), > but a GL error is better than a crash so keep this extra safety-net. > > Signed-off-by: Rob Clark > --- > glamor/glamor_copy.c | 3 +++ > glamor/glamor_dash.c | 3 +++ > glamor/glamor_glyphblt.c | 3 +++ > glamor/glamor_points.c | 3 +++ > glamor/glamor_rects.c| 3 +++ > glamor/glamor_render.c | 3 +++ > glamor/glamor_segs.c | 3 +++ > glamor/glamor_spans.c| 3 +++ > glamor/glamor_text.c | 3 +++ > glamor/glamor_vbo.c | 3 +++ > 10 files changed, 30 insertions(+) > > diff --git a/glamor/glamor_copy.c b/glamor/glamor_copy.c > index 028acf2..97db20c 100644 > --- a/glamor/glamor_copy.c > +++ b/glamor/glamor_copy.c > @@ -317,6 +317,9 @@ glamor_copy_fbo_fbo_draw(DrawablePtr src, > const glamor_facet *copy_facet; > int n; > > +if (nbox == 0) > +return TRUE; > + > glamor_make_current(glamor_priv); > > if (gc && !glamor_set_planemask(gc->depth, gc->planemask)) > diff --git a/glamor/glamor_dash.c b/glamor/glamor_dash.c > index 101228e..b961951 100644 > --- a/glamor/glamor_dash.c > +++ b/glamor/glamor_dash.c > @@ -328,6 +328,9 @@ glamor_poly_segment_dash_gl(DrawablePtr drawable, GCPtr > gc, > int add_last; > int i; > > +if (nseg == 0) > +return TRUE; > + > if (!(prog = glamor_dash_setup(drawable, gc))) > return FALSE; > > diff --git a/glamor/glamor_glyphblt.c b/glamor/glamor_glyphblt.c > index 1791f6d..0cd3406 100644 > --- a/glamor/glamor_glyphblt.c > +++ b/glamor/glamor_glyphblt.c > @@ -175,6 +175,9 @@ glamor_push_pixels_gl(GCPtr gc, PixmapPtr bitmap, > INT16 *points = NULL; > char *vbo_offset; > > +if ((w * h) == 0) > +return TRUE; > + ( ) seems a bit superfluous. re, wh > if (w * h > MAXINT / (2 * sizeof(float))) > goto bail; > > diff --git a/glamor/glamor_points.c b/glamor/glamor_points.c > index 3ba4a69..e0aa87e 100644 > --- a/glamor/glamor_points.c > +++ b/glamor/glamor_points.c > @@ -48,6 +48,9 @@ glamor_poly_point_gl(DrawablePtr drawable, GCPtr gc, int > mode, int npt, DDXPoint > char *vbo_offset; > int box_x, box_y; > > +if (npt == 0) > +return TRUE; > + > pixmap_priv = glamor_get_pixmap_private(pixmap); > if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv)) > goto bail; > diff --git a/glamor/glamor_rects.c b/glamor/glamor_rects.c > index c378e4a..26d1401 100644 > --- a/glamor/glamor_rects.c > +++ b/glamor/glamor_rects.c > @@ -53,6 +53,9 @@ glamor_poly_fill_rect_gl(DrawablePtr drawable, > char *vbo_offset; > int box_x, box_y; > > +if (nrect == 0) > +return TRUE; > + > pixmap_priv = glamor_get_pixmap_private(pixmap); > if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv)) > goto bail; > diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c > index c3a8f17..3ad4507 100644 > --- a/glamor/glamor_render.c > +++ b/glamor/glamor_render.c > @@ -621,6 +621,9 @@ glamor_setup_composite_vbo(ScreenPtr screen, int n_verts) > > vert_size = n_verts * glamor_priv->vb_stride; > > +if (vert_size == 0) > +
[PATCH] glamor: fix crash when drawing nothing
For example, in the PolyFillRect() path w/ nrect==0, we end up in glamor_get_vbo_space(size=0): (gdb) bt #0 0x007fb73df340 in raise () from /lib64/libc.so.6 #1 0x007fb73e0fb8 in abort () from /lib64/libc.so.6 #2 0x007fb73d84f4 in __assert_fail_base () from /lib64/libc.so.6 #3 0x007fb73d859c in __assert_fail () from /lib64/libc.so.6 #4 0x0045dc38 in glamor_get_vbo_space (screen=0x8dcb60, size=size@entry=0, vbo_offset=0x7ff250, vbo_offset@entry=0x7ff2d0) at glamor_vbo.c:112 #5 0x004541ac in glamor_poly_fill_rect_gl (prect=0x11be650, nrect=0, gc=0x4cda70 , drawable=0x7ff388) at glamor_rects.c:72 #6 glamor_poly_fill_rect (drawable=0x7ff388, gc=0x4cda70 , nrect=0, prect=0x11be650) at glamor_rects.c:162 #7 0x00557d0c in damagePolyFillRect (pDrawable=0x112c300, pGC=0xe81580, nRects=0, pRects=) at damage.c:1194 #8 0x004cdb20 in miPaintWindow (pWin=, prgn=0x7ff388, what=) at miexpose.c:540 #9 0x004db260 in miClearToBackground (pWin=0x112c300, x=, y=, w=, h=, generateExposures=0) at miwindow.c:116 #10 0x004646e4 in ProcClearToBackground (client=0x111e810) at dispatch.c:1592 #11 0x0046907c in Dispatch () at dispatch.c:430 #12 0x0046cf90 in dix_main (argc=5, argv=0x7ff628, envp=) at main.c:300 #13 0x007fb73cb68c in __libc_start_main () from /lib64/libc.so.6 #14 0x0042b3e8 in _start () Also fixed a bunch of other call-sites which could in theory trigger the same issue. v2: add back the early return if size==0 in glamor_get_vbo_space() just in case. We'd hit a GL error in glamor_put_vbo_space() if we ever ended up with a call path that hit that (so keep the earlier early-returns), but a GL error is better than a crash so keep this extra safety-net. Signed-off-by: Rob Clark --- glamor/glamor_copy.c | 3 +++ glamor/glamor_dash.c | 3 +++ glamor/glamor_glyphblt.c | 3 +++ glamor/glamor_points.c | 3 +++ glamor/glamor_rects.c| 3 +++ glamor/glamor_render.c | 3 +++ glamor/glamor_segs.c | 3 +++ glamor/glamor_spans.c| 3 +++ glamor/glamor_text.c | 3 +++ glamor/glamor_vbo.c | 3 +++ 10 files changed, 30 insertions(+) diff --git a/glamor/glamor_copy.c b/glamor/glamor_copy.c index 028acf2..97db20c 100644 --- a/glamor/glamor_copy.c +++ b/glamor/glamor_copy.c @@ -317,6 +317,9 @@ glamor_copy_fbo_fbo_draw(DrawablePtr src, const glamor_facet *copy_facet; int n; +if (nbox == 0) +return TRUE; + glamor_make_current(glamor_priv); if (gc && !glamor_set_planemask(gc->depth, gc->planemask)) diff --git a/glamor/glamor_dash.c b/glamor/glamor_dash.c index 101228e..b961951 100644 --- a/glamor/glamor_dash.c +++ b/glamor/glamor_dash.c @@ -328,6 +328,9 @@ glamor_poly_segment_dash_gl(DrawablePtr drawable, GCPtr gc, int add_last; int i; +if (nseg == 0) +return TRUE; + if (!(prog = glamor_dash_setup(drawable, gc))) return FALSE; diff --git a/glamor/glamor_glyphblt.c b/glamor/glamor_glyphblt.c index 1791f6d..0cd3406 100644 --- a/glamor/glamor_glyphblt.c +++ b/glamor/glamor_glyphblt.c @@ -175,6 +175,9 @@ glamor_push_pixels_gl(GCPtr gc, PixmapPtr bitmap, INT16 *points = NULL; char *vbo_offset; +if ((w * h) == 0) +return TRUE; + if (w * h > MAXINT / (2 * sizeof(float))) goto bail; diff --git a/glamor/glamor_points.c b/glamor/glamor_points.c index 3ba4a69..e0aa87e 100644 --- a/glamor/glamor_points.c +++ b/glamor/glamor_points.c @@ -48,6 +48,9 @@ glamor_poly_point_gl(DrawablePtr drawable, GCPtr gc, int mode, int npt, DDXPoint char *vbo_offset; int box_x, box_y; +if (npt == 0) +return TRUE; + pixmap_priv = glamor_get_pixmap_private(pixmap); if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv)) goto bail; diff --git a/glamor/glamor_rects.c b/glamor/glamor_rects.c index c378e4a..26d1401 100644 --- a/glamor/glamor_rects.c +++ b/glamor/glamor_rects.c @@ -53,6 +53,9 @@ glamor_poly_fill_rect_gl(DrawablePtr drawable, char *vbo_offset; int box_x, box_y; +if (nrect == 0) +return TRUE; + pixmap_priv = glamor_get_pixmap_private(pixmap); if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv)) goto bail; diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c index c3a8f17..3ad4507 100644 --- a/glamor/glamor_render.c +++ b/glamor/glamor_render.c @@ -621,6 +621,9 @@ glamor_setup_composite_vbo(ScreenPtr screen, int n_verts) vert_size = n_verts * glamor_priv->vb_stride; +if (vert_size == 0) +return TRUE; + glamor_make_current(glamor_priv); vb = glamor_get_vbo_space(screen, vert_size, &vbo_offset); diff --git a/glamor/glamor_segs.c b/glamor/glamor_segs.c index e167325..1c4ee75 100644 --- a/glamor/glamor_segs.c +++ b/glamor/glamor_segs.c @@ -47,6 +47,9 @@ glamor_poly_segment_solid_gl(DrawablePtr drawable, GCPtr gc, int box_x, bo
Re: [PATCH] glamor: fix crash when drawing nothing
On Fri, Oct 16, 2015 at 11:07 AM, Keith Packard wrote: > Rob Clark writes: > >> On Wed, Oct 14, 2015 at 8:10 PM, Eric Anholt wrote: >>> Rob Clark writes: >>> For example, in the PolyFillRect() path w/ nrect==0, we end up in glamor_get_vbo_space(size=0): >>> >>> I wonder instead if we shouldn't just have glamor_get_vbo_space() return >>> NULL on size == 0? >> >> that was my first approach, and is a much smaller patch, but triggers >> GL errors on glamor_put_vbo_space().. so with that approach we'd >> either need some bookkeeping to skip the glUnmapBuffer() or live w/ >> extra gl errors.. > > Might be good to do both in case we end up there again; a gl error is a > better result than a segfault? true, I'll send a v2 w/ the extra if (size == 0).. BR, -R ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] glamor: fix crash when drawing nothing
Rob Clark writes: > On Wed, Oct 14, 2015 at 8:10 PM, Eric Anholt wrote: >> Rob Clark writes: >> >>> For example, in the PolyFillRect() path w/ nrect==0, we end up in >>> glamor_get_vbo_space(size=0): >> >> I wonder instead if we shouldn't just have glamor_get_vbo_space() return >> NULL on size == 0? > > that was my first approach, and is a much smaller patch, but triggers > GL errors on glamor_put_vbo_space().. so with that approach we'd > either need some bookkeeping to skip the glUnmapBuffer() or live w/ > extra gl errors.. Might be good to do both in case we end up there again; a gl error is a better result than a segfault? -- -keith signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] glamor: fix crash when drawing nothing
On Wed, Oct 14, 2015 at 8:10 PM, Eric Anholt wrote: > Rob Clark writes: > >> For example, in the PolyFillRect() path w/ nrect==0, we end up in >> glamor_get_vbo_space(size=0): > > I wonder instead if we shouldn't just have glamor_get_vbo_space() return > NULL on size == 0? that was my first approach, and is a much smaller patch, but triggers GL errors on glamor_put_vbo_space().. so with that approach we'd either need some bookkeeping to skip the glUnmapBuffer() or live w/ extra gl errors.. note that somehow MESA_EXTENSION_OVERRIDE="-GL_ARB_buffer_storage" (so that I could apitrace things) was somehow triggering the initial problem (but that might be somehow timing related) BR, -R ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] glamor: fix crash when drawing nothing
Rob Clark writes: > For example, in the PolyFillRect() path w/ nrect==0, we end up in > glamor_get_vbo_space(size=0): I wonder instead if we shouldn't just have glamor_get_vbo_space() return NULL on size == 0? signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH] glamor: fix crash when drawing nothing
For example, in the PolyFillRect() path w/ nrect==0, we end up in glamor_get_vbo_space(size=0): (gdb) bt #0 0x007fb73df340 in raise () from /lib64/libc.so.6 #1 0x007fb73e0fb8 in abort () from /lib64/libc.so.6 #2 0x007fb73d84f4 in __assert_fail_base () from /lib64/libc.so.6 #3 0x007fb73d859c in __assert_fail () from /lib64/libc.so.6 #4 0x0045dc38 in glamor_get_vbo_space (screen=0x8dcb60, size=size@entry=0, vbo_offset=0x7ff250, vbo_offset@entry=0x7ff2d0) at glamor_vbo.c:112 #5 0x004541ac in glamor_poly_fill_rect_gl (prect=0x11be650, nrect=0, gc=0x4cda70 , drawable=0x7ff388) at glamor_rects.c:72 #6 glamor_poly_fill_rect (drawable=0x7ff388, gc=0x4cda70 , nrect=0, prect=0x11be650) at glamor_rects.c:162 #7 0x00557d0c in damagePolyFillRect (pDrawable=0x112c300, pGC=0xe81580, nRects=0, pRects=) at damage.c:1194 #8 0x004cdb20 in miPaintWindow (pWin=, prgn=0x7ff388, what=) at miexpose.c:540 #9 0x004db260 in miClearToBackground (pWin=0x112c300, x=, y=, w=, h=, generateExposures=0) at miwindow.c:116 #10 0x004646e4 in ProcClearToBackground (client=0x111e810) at dispatch.c:1592 #11 0x0046907c in Dispatch () at dispatch.c:430 #12 0x0046cf90 in dix_main (argc=5, argv=0x7ff628, envp=) at main.c:300 #13 0x007fb73cb68c in __libc_start_main () from /lib64/libc.so.6 #14 0x0042b3e8 in _start () Also fixed a bunch of other call-sites which could in theory trigger the same issue. Signed-off-by: Rob Clark --- glamor/glamor_copy.c | 3 +++ glamor/glamor_dash.c | 3 +++ glamor/glamor_glyphblt.c | 3 +++ glamor/glamor_points.c | 3 +++ glamor/glamor_rects.c| 3 +++ glamor/glamor_render.c | 3 +++ glamor/glamor_segs.c | 3 +++ glamor/glamor_spans.c| 3 +++ glamor/glamor_text.c | 3 +++ 9 files changed, 27 insertions(+) diff --git a/glamor/glamor_copy.c b/glamor/glamor_copy.c index 028acf2..97db20c 100644 --- a/glamor/glamor_copy.c +++ b/glamor/glamor_copy.c @@ -317,6 +317,9 @@ glamor_copy_fbo_fbo_draw(DrawablePtr src, const glamor_facet *copy_facet; int n; +if (nbox == 0) +return TRUE; + glamor_make_current(glamor_priv); if (gc && !glamor_set_planemask(gc->depth, gc->planemask)) diff --git a/glamor/glamor_dash.c b/glamor/glamor_dash.c index 101228e..b961951 100644 --- a/glamor/glamor_dash.c +++ b/glamor/glamor_dash.c @@ -328,6 +328,9 @@ glamor_poly_segment_dash_gl(DrawablePtr drawable, GCPtr gc, int add_last; int i; +if (nseg == 0) +return TRUE; + if (!(prog = glamor_dash_setup(drawable, gc))) return FALSE; diff --git a/glamor/glamor_glyphblt.c b/glamor/glamor_glyphblt.c index 1791f6d..0cd3406 100644 --- a/glamor/glamor_glyphblt.c +++ b/glamor/glamor_glyphblt.c @@ -175,6 +175,9 @@ glamor_push_pixels_gl(GCPtr gc, PixmapPtr bitmap, INT16 *points = NULL; char *vbo_offset; +if ((w * h) == 0) +return TRUE; + if (w * h > MAXINT / (2 * sizeof(float))) goto bail; diff --git a/glamor/glamor_points.c b/glamor/glamor_points.c index 3ba4a69..e0aa87e 100644 --- a/glamor/glamor_points.c +++ b/glamor/glamor_points.c @@ -48,6 +48,9 @@ glamor_poly_point_gl(DrawablePtr drawable, GCPtr gc, int mode, int npt, DDXPoint char *vbo_offset; int box_x, box_y; +if (npt == 0) +return TRUE; + pixmap_priv = glamor_get_pixmap_private(pixmap); if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv)) goto bail; diff --git a/glamor/glamor_rects.c b/glamor/glamor_rects.c index c378e4a..26d1401 100644 --- a/glamor/glamor_rects.c +++ b/glamor/glamor_rects.c @@ -53,6 +53,9 @@ glamor_poly_fill_rect_gl(DrawablePtr drawable, char *vbo_offset; int box_x, box_y; +if (nrect == 0) +return TRUE; + pixmap_priv = glamor_get_pixmap_private(pixmap); if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv)) goto bail; diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c index c3a8f17..3ad4507 100644 --- a/glamor/glamor_render.c +++ b/glamor/glamor_render.c @@ -621,6 +621,9 @@ glamor_setup_composite_vbo(ScreenPtr screen, int n_verts) vert_size = n_verts * glamor_priv->vb_stride; +if (vert_size == 0) +return TRUE; + glamor_make_current(glamor_priv); vb = glamor_get_vbo_space(screen, vert_size, &vbo_offset); diff --git a/glamor/glamor_segs.c b/glamor/glamor_segs.c index e167325..1c4ee75 100644 --- a/glamor/glamor_segs.c +++ b/glamor/glamor_segs.c @@ -47,6 +47,9 @@ glamor_poly_segment_solid_gl(DrawablePtr drawable, GCPtr gc, int box_x, box_y; int add_last; +if (nseg == 0) +return TRUE; + pixmap_priv = glamor_get_pixmap_private(pixmap); if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv)) goto bail; diff --git a/glamor/glamor_spans.c b/glamor/glamor_spans.c index 58da3ed..db5fe7c 100644 --- a/glamor/glamor_spans.c +++ b/gla
