[PATCH] Call fbPrepareAccess when creating scratch pixmaps (Re: Xorg segmentation fault when drawing PolyArcs (under kicad))

[Michel Dänzer]

On Tue, 2009-11-03 at 08:39 +0000, Renato Caldas wrote: 
> 2009/11/2 Michel Dänzer <michel at daenzer.net>:
> > On Wed, 2009-10-21 at 14:25 +0100, Renato Caldas wrote:
> >> 2009/10/21 Renato Caldas <seventhguardian at gmail.com>:
> >> > 2009/10/20 Renato Caldas <seventhguardian at gmail.com>:
> >> >> 2009/10/20 Renato Caldas <seventhguardian at gmail.com>:
> >> >>>
> >> >>> 2009/10/20 Michel Dänzer <michel at daenzer.net>:
> >> >>>> On Mon, 2009-10-12 at 20:12 +0100, Renato Caldas wrote:
> >> >>>>>
> >> >>>>> When using (trying to use...), kicad I managed to get consistent Xorg
> >> >>>>> segmentation faults. I've filed a bug report on fedora's bugzilla:
> >> >>>>> https://bugzilla.redhat.com/show_bug.cgi?id=528475. There I've
> >> >>>>> included a very basic test case that works all the time. In the
> >> >>>>> meantime, I've also got my hands dirty, and tried to debug Xorg using
> >> >>>>> gdb.
> >> >>>>>
> >> >>>>> The symptom is that the function fbBltOne is called with src=0x0, so
> >> >>>>> Xorg segfaults as soon as it tries to use src (in LoadBits; at
> >> >>>>> fb/fbbltone.c:292).
> >> >>>>>
> >> >>>>> I've traced its value back to fbPushPixels, where it is created and
> >> >>>>> fed to the function chain, in the macro fbGetStipDrawable (defined at
> >> >>>>> fb/fb.h:720).
> >> >>>>>
> >> >>>>> Here pDrawable->type seems to be DRAWABLE_PIXMAP, so _pPix is simply
> >> >>>>> cast from pDrawable. Then the _pPix -> devPrivate.ptr is used as
> >> >>>>> "src", after a couple of castings and copying around.
> >> >>>>
> >> >>>> Please provide a full backtrace ('bt full' in gdb) from when the crash
> >> >>>> occurs.
> >> >>>
> >> >>> Attached. I've also attached it to the bug report, just in case.
> >> >>>
> >> >>>> If you're using EXA, the problem is most likely that some code path
> >> >>>> isn't calling exaPrepareAccess(Reg) for the pixmap in question before
> >> >>>> calling down to the fb module.
> >> >
> >> > It seems that the code path is calling exaPrepareAccess:
> >> >
> >> > in ExaCheckPolyArc:
> >> >
> >> > (...)
> >> > exaPrepareAccess (pDrawable, EXA_PREPARE_DEST);
> >> > exaPrepareAccessGC (pGC);
> >> > pGC->ops->PolyArc (pDrawable, pGC, narcs, pArcs);
> >> > (...)
> >> >
> >> > but there's possibly something wrong there.
> >>
> >> You're right, I found the problem, and it seems to be tricky! It seems
> >> that the scratch buffers used by a lot of functions never call
> >> exaPrepareAccess.
> >>
> >> I've created a small test program, and it never crashed. It turned out
> >> that kicad used a "GXor" operation, which was flagged as "tricky",
> >> requiring a scratch buffer. This scratch buffer would then be used
> >> without "preparation". There are a lot of functions that create a
> >> scratch buffer, but they're possibly rarely used:
> >>
> >> $ grep CREATE_PIXMAP_USAGE_SCRATCH * -lR
> >> dix/glyphcurs.c
> >> exa/exa_glyphs.c
> >> hw/xfree86/xaa/xaaInit.c
> >> include/scrnintstr.h
> >> mi/miarc.c
> >> mi/midispcur.c
> >> mi/mibitblt.c
> >> mi/miglblt.c
> >> render/render.c
> >> render/glyph.c
> >> render/mirect.c
> >> Xext/mbuf.c
> >> Xext/shm.c
> >> Xext/mbufpx.c
> >>
> >> Some of them may be safe, but at least the mi/ files should be fixed.
> >>
> >> The "tricky" part of the problem is how to call exaPrepareAccess in a
> >> clean way. fbPrepareAccess calls the driver's "PrepareAccess", so it
> >> seems to be the correct solution. But it shouldn't be called directly
> >> either, so I first need to put fbPrepareAccess in the *pGC->ops.
> >>
> >> Does this sound ok?
> >
> > I'm afraid not.
> >
> > exaPrepareAccess* are internal to EXA, no other layer needs to know
> > about them.
> >
> > It sounds like either the logic in exaPrepareAccessGC() is wrong for
> > preparing access to the stipple or tile pixmap, or the pixmap in
> > question comes from yet another place not handled in
> > exaPrepareAccessGC() or ExaCheckPolyArc() yet. Can you find out where in
> > the GC or other data structure the pointer to the pixmap in question is
> > retrieved from?
> 
> The pixmap is created from:
> 
>  pDrawTo = (DrawablePtr)(*pDraw->pScreen->CreatePixmap)
>                                 (pDraw->pScreen, pixmapWidth, pixmapHeight, 1,
>                                  CREATE_PIXMAP_USAGE_SCRATCH);
> 
> This is a scratch pixmap, so ExaCheckPolyArc is never called onto it.
> 
> There is however a call to "ValidateGC(pDrawTo, pGCTo);" shortly after
> the creation. Maybe I should check the logic behind ValidateGC to
> prepare the pixmap there...
> 
> In the meantime, I've came up with a patch that calls fbPrepareAccess
> inside fbCreatePixmapBpp if the usage hint is
> "CREATE_PIXMAP_USAGE_SCRATCH" (on annex). The rationale behind this is
> that whenever a scratch pixmap is created (at least in mi/) it doesn't
> ever seem to be "prepared". I've tested it and it fixes the problem,
> but it may not be the best thing to do... please advise.

I'm not sure how fbPrepareAccess() can make any difference, as in the
normal fb module (not wfb) it's defined as:

#define fbPrepareAccess(pPix)

Again, this is an EXA internal issue and needs to be fixed there.
Apparently Maarten has an idea.


-- 
Earthling Michel Dänzer           |                http://www.vmware.com
Libre software enthusiast         |          Debian, X and DRI developer