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

[Renato Caldas]

2009/11/5 Michel Dänzer <michel at daenzer.net>:
> 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.

Any news on this? Can I help on anything?

Cheers,
  Renato

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