[Mesa-dev] [PATCH 0/4] egl/*: Set disp->DriverData to NULL on error

[Nicolas Boichat]

On Thu, Aug 4, 2016 at 10:43 PM, Martin Peres
<martin.peres at linux.intel.com> wrote:
>
>
> On 04/08/16 15:17, Nicolas Boichat wrote:
>>
>> On Thu, Aug 4, 2016 at 7:17 PM, Martin Peres
>> <martin.peres at linux.intel.com> wrote:
>>>
>>>
>>>
>>> On 04/08/16 13:08, Nicolas Boichat wrote:
>>>>
>>>>
>>>> On Thu, Aug 4, 2016 at 5:52 PM, Martin Peres
>>>> <martin.peres at linux.intel.com> wrote:
>>>>>
>>>>>
>>>>> On 04/08/16 05:07, Nicolas Boichat wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> egl/x11 fixup has already be sent here:
>>>>>> https://patchwork.freedesktop.org/patch/101934/
>>>>>>
>>>>>> Compile-tested only (on android only).
>>>>>>
>>>>>> I was first thinking of modifying most functions in egl_dri2.c to take
>>>>>> in
>>>>>> dri2_egl_display *dri2_dpy instead of _EGLDisplay *disp as parameter,
>>>>>> so
>>>>>> we do not need to set disp->DriverData early in the dri_initialize_*
>>>>>> functions.
>>>>>>
>>>>>> But there are some functions that do need disp as well:
>>>>>>  - dri2_add_config, to call _eglInitConfig (disp->DriverData does not
>>>>>> need
>>>>>> to be
>>>>>>    set), and to check if extensions are supported.
>>>>>>  - dri2_setup_screen (similar reasons)
>>>>>>
>>>>>> Ideas to untangle this are welcome. In the mean time, this patch
>>>>>> series
>>>>>> should
>>>>>> fix crashes and a piglit test.
>>>>>>
>>>>>> Nicolas Boichat (4):
>>>>>>   egl/wayland: Set disp->DriverData to NULL on error
>>>>>>   egl/surfaceless: Set disp->DriverData to NULL on error
>>>>>>   egl/drm: Set disp->DriverData to NULL on error
>>>>>>   egl/android: Set dpy->DriverData to NULL on error
>>>>>>
>>>>>
>>>>> This patch series fixes egl_khr_fence_sync for me:
>>>>> Tested-by: Martin Peres <martin.peres at linux.intel.com>
>>>>
>>>>
>>>>
>>>> Thanks!
>>>>
>>>>> However, egl-create-context-verify-gl-flavor is still crashing, even
>>>>> with
>>>>> "egl/dri2: dri2_initialize: Do not reference-count TestOnly display"
>>>>> applied. The issue is that eglGetProcAddress("glGetString") returns
>>>>> NULL.
>>>>
>>>>
>>>>
>>>> That's surprising, Michel said that my patch fixed that issue. Did you
>>>> try reverting "9ee683f877 (egl/dri2: Add reference count for
>>>> dri2_egl_display)" to confirm that my original patch broke it?
>>>
>>>
>>>
>>> Yes, reverting the patch fixes the issue.
>>
>>
>> Ok, let's see how I understand eglGetProcAddress should work in this case:
>> - eglapi.c:eglGetProcAddress
>> - egldriver.c:_eglGetDriverProc
>> - egldriver.c:_eglMatchDriver, dpy->Options.TestOnly = EGL_TRUE
>> - egldriver.c:_eglMatchAndInitialize
>> - Calls drv->API.Initialize = egl_dri2.c:dri2_initialize
>> - Returns EGL_TRUE as TestOnly is true (before "egl/dri2:
>> dri2_initialize: Do not reference-count TestOnly display", it would
>> return EGL_FALSE, so eglGetProcAddress would fail)
>>
>> So I don't really see how 9ee683f877 + "egl/dri2: dri2_initialize: Do
>> not reference-count TestOnly display" would fail (while reverting
>> 9ee683f877 would work).
>>
>> Any chance you could help tracing through the code? Or at least enable
>> debugging output?
>
>
> Sorry for the noise, a make clean fixed the issue :s
>
> I can confirm that "egl/dri2: dri2_initialize: Do not reference-count
> TestOnly display" fixes the issue, you can add my Tested-by!

Great! Thanks Martin.