[Mesa-dev] [PATCH] clover: Handle NULL pipe_loader_device returned by pipe_loader_probe()

[Tom Stellard]

On Mon, Nov 30, 2015 at 07:57:32PM +0200, Francisco Jerez wrote:
> Tom Stellard <thomas.stellard at amd.com> writes:
> 
> > pipe_loader_probe() may initalize an entry in the device list to NULL,
> > while still counting this device in the number of devices that it
> > returns, so we need to handle this situation.
> 
> If this is related to the patch you sent last Saturday
> (1448679128-20276-1-git-send-email-thomas.stellard at amd.com), I don't
> think that's what happens.  What happens is that pipe_loader_sw_probe()
> returns an incorrect device count the first time around (one regardless
> of whether the software null device is actually available), so Clover
> allocates and zero-initializes a pointer in the ldevs array for a device
> which is never returned by pipe-loader, and then crashes.
> 
> Please mention in the commit message that this is actually working
> around a pipe-loader bug, but it makes sense to do it anyway because it
> fixes the theoretical race condition you pointed out in your last patch.
> 

Sorry, please disregard this.  I got branches mixed up while working on
this and I thought that this was required in addition to 
1448679128-20276-1-git-send-email-thomas.stellard at amd.com.

-Tom

> > ---
> >
> > This is the most simple fix possible to get clover working again.  We can
> > discuss fixing the other issues in clover in a follow on patch.
> >
> > src/gallium/state_trackers/clover/core/platform.cpp | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/src/gallium/state_trackers/clover/core/platform.cpp b/src/gallium/state_trackers/clover/core/platform.cpp
> > index 328b71c..871b90e 100644
> > --- a/src/gallium/state_trackers/clover/core/platform.cpp
> > +++ b/src/gallium/state_trackers/clover/core/platform.cpp
> > @@ -31,6 +31,9 @@ platform::platform() : adaptor_range(evals(), devs) {
> >     pipe_loader_probe(&ldevs.front(), n);
> >  
> >     for (pipe_loader_device *ldev : ldevs) {
> > +      if (!ldev) {
> > +         continue;
> > +      }
> >        try {
> 
> Just nitpicking now, but I'd prefer to simplify this even more by doing
> the following here:
> 
> +        if (ldev)
> 
> >           devs.push_back(create<device>(*this, ldev));
> >        } catch (error &) {
> > -- 
> > 2.0.4




> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev