[Mesa-dev] [PATCH] clover: Check the return value of pipe_loader_probe() when probing for devices

[Hans de Goede]

Hi,

On 29-11-15 17:14, Samuel Pitoiset wrote:
> This patch fixes the issue for me (eg. https://bugs.freedesktop.org/show_bug.cgi?id=93091#c8).
>
> Thanks Tom.
>
> Tested-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>

I'm seeing a similar crash, but for me the problem is caused by
pipe_loader_sw_probe returning 1, but not filling in the devs
pointer. In which case this fix does not help.

I've been using the following fix locally (I've not gotten
around to submitting this upstream yet, sorry about that):

--- a/src/gallium/auxiliary/pipe-loader/pipe_loader_sw.c
+++ b/src/gallium/auxiliary/pipe-loader/pipe_loader_sw.c
@@ -222,7 +222,7 @@ pipe_loader_sw_probe(struct pipe_loader_device **devs, int n
  {
     int i = 1;

-   if (i < ndev) {
+   if (ndev) {
        if (!pipe_loader_sw_probe_null(devs)) {
           i--;
        }


The problem is that if pipe_loader_sw_probe gets called with
ndev=1 it will not set devs, but will still return 1, leading
to ... (continued below)




>
> On 11/28/2015 03:52 AM, Tom Stellard wrote:
>> When probing for devices, clover will call pipe_loader_probe() twice.
>> The first time to retrieve the number of devices, and then second time
>> to retrieve the device structures.
>>
>> We currently assume that the return value of both calls will be the
>> same, but this will not be the case if a device happens to disappear
>> between the two calls.
>>
>> This patch removes this assumption and checks the return value of the
>> second pipe_loader_probe() call to ensure it does not try to initialize
>> devices that no longer exits.
>>
>> CC: <mesa-stable at lists.freedesktop.org>
>> ---
>>   src/gallium/state_trackers/clover/core/platform.cpp | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/gallium/state_trackers/clover/core/platform.cpp b/src/gallium/state_trackers/clover/core/platform.cpp
>> index 328b71c..689d692 100644
>> --- a/src/gallium/state_trackers/clover/core/platform.cpp
>> +++ b/src/gallium/state_trackers/clover/core/platform.cpp
>> @@ -28,9 +28,10 @@ platform::platform() : adaptor_range(evals(), devs) {
>>      int n = pipe_loader_probe(NULL, 0);
>>      std::vector<pipe_loader_device *> ldevs(n);
>>
>> -   pipe_loader_probe(&ldevs.front(), n);
>> +   n = pipe_loader_probe(&ldevs.front(), n);
>>
>> -   for (pipe_loader_device *ldev : ldevs) {
>> +   for (int i = 0; i < n; ++i) {
>> +      pipe_loader_device *ldev = ldevs[i];
>>         try {
>>            devs.push_back(create<device>(*this, ldev));

create getting called with a ldev value of NULL here.

Regards,

Hans


>>         } catch (error &) {
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev