search

NVIDIA / egl-wayland

The EGLStream-based Wayland external platform
MIT License
275 stars 43 forks source link

Valgrind error running `eglGetDisplay(EGL_DEFAULT_DISPLAY)` #88

Open ids1024 opened 10 months ago

ids1024 commented 10 months ago

I noticed this error running eglinfo in valgrind:

==451064== Syscall param write(buf) points to unaddressable byte(s)
==451064==    at 0x49AAA37: write (write.c:26)
==451064==    by 0x4FA70BC: wlEglMemoryIsReadable (wayland-eglutils.c:74)
==451064==    by 0x4FA0BE8: wlEglIsWaylandDisplay (wayland-egldisplay.c:55)
==451064==    by 0x4FA0BE8: wlEglIsValidNativeDisplayExport (wayland-egldisplay.c:71)
==451064==    by 0x50AD3DB: ??? (in /usr/lib/x86_64-linux-gnu/libEGL_nvidia.so.535.86.05)
==451064==    by 0x50AD467: ??? (in /usr/lib/x86_64-linux-gnu/libEGL_nvidia.so.535.86.05)
==451064==    by 0x5048BDD: ??? (in /usr/lib/x86_64-linux-gnu/libEGL_nvidia.so.535.86.05)
==451064==    by 0x4EC7F64: ??? (in /usr/lib/x86_64-linux-gnu/libEGL.so.1.1.0)
==451064==    by 0x1467AC: glad_egl_find_core_egl (egl.c:922)
==451064==    by 0x146A2D: gladLoadEGLUserPtr (egl.c:959)
==451064==    by 0x147159: gladLoaderLoadEGL (egl.c:1159)
==451064==    by 0x10F4F6: main (eglinfo.c:937)
==451064==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

Technically what wlEglMemoryIsReadable is doing may be correct (on Linux anyway), if unusual. But it's good not to have cluttered Valgrind output distracting from actual issues.

In this particular case, it looks like it's trying to interpret EGL_DEFAULT_DISPLAY (which is 0) as a Wayland display. I guess something should test for EGL_DEFAULT_DISPLAY before calling wlEglMemoryIsReadable?

erik-kz commented 10 months ago

Unfortunately this check is necessary because on systems without GLVND we may be passed other native display types. An Xlib Display, an XCB connection, a GBM device, and so on. It's not just EGL_DEFAULT_DISPLAY.

Valgrind does provide a way to silence errors from particular libraries, see https://valgrind.org/docs/manual/mc-manual.html#mc-manual.suppfiles

ids1024 commented 10 months ago

Ah, so it's when GLVND isn't present that this test is expected to be needed?

It's awkward, but other than the Valgrind message probably will work fine. Though I guess there would be no harm in having a NULL test in wlEglMemoryIsReadable which would avoid it in this case anyway.

I saw a more significant memory error with egl-gbm: https://github.com/NVIDIA/egl-gbm/pull/3.