Recognizing wl_display pointer for EGL (Re: Smart comparing of wl_display_interfaces)

[Pekka Paalanen]

On Thu, 16 Oct 2014 10:40:20 +0300
Pekka Paalanen <ppaalanen at gmail.com> wrote:

> On Thu, 16 Oct 2014 09:46:39 +0300
> Pekka Paalanen <ppaalanen at gmail.com> wrote:
> 
> > On Wed, 15 Oct 2014 22:44:25 +0400
> > Dmitry Cherkassov <dcherkassov at gmail.com> wrote:
> > 
> > > Hi list!
> > > 
> > > The definition of wl_display_interface symbol can come from
> > > libwayland-server.so or libwayland-client.so.
> > > 
> > > There is a code in closed source EGL implementation that does
> > > interfaces comparison via pointers, yielding negative results when
> > > addresses are different (one address is saved by QtWayland, the other
> > > one is from &wl_display_interface in EGL source code).
> > > 
> > > Is there a smarter way to compare wl_display_interface's?
> > > 
> > > There is this function:
> > > 
> > > wl_interface_equal(const struct wl_interface *a, const struct wl_interface *b)
> > > {
> > >     return a == b || strcmp(a->name, b->name) == 0;
> > > }
> > > 
> > > But as you can see it's not safe to use it alone because in case of
> > > bad pointer we will get undefined results.
> > > 
> > > Currently struct wl_interface is the following:
> > >  struct wl_interface {
> > >      const char *name;
> > >      int version;
> > >      int method_count;
> > >      const struct wl_message *methods;
> > >      int event_count;
> > >      const struct wl_message *events;
> > >  };
> > > 
> > > Since there is no magic field at the beginning it is not possible to
> > > see whether a pointer points to the valid wl_interface structure.
> > > 
> > > So my question is: how can we compare wl_interfaces in more clever and
> > > safer way?
> > 
> > Hmm, why would you ever have a bad pointer?
> > 
> > Bad pointers in general tend to cause crashes and stuff, so you
> > shouldn't be having a bad one in the first place.
> > 
> > What is the background for your question/problem?
> 
> Ok, I suppose this was discussed in IRC last night: the actual problem
> behind the question is how to detect whether the void pointer
> (EGLNativeDisplayType) given to eglCreateDisplay is actually a
> wl_display.
> 
> That's tough.
> 
> A couple of hacks come to mind.
> 
> bool
> _eglNativePlatformDetectWayland(void *nativeDisplay)
> {
> 	void *f;
> 
> 	if (!dereferencable(nativeDisplay))
> 		return false;
> 
> 	f = *(void **)nativeDisplay;
> 
> The first is to dlopen both libwayland-client and libwayland-server in
> turn with RTLD_NOLOAD | RTLD_LOCAL, and fetching the
> wl_display_interface symbol from each, comparing those to 'f'. If
> either matches, return true.

While discussing in IRC, I realized half of this is nonsense.

eglGetDisplay() can only get a wl_display created by libwayland-client,
and never one created by libwayland-server. Assuming the dynamic
linking is sane, the interface pointer in wl_display is *always* the
one from libwayland-client.so.

Now we just need to make sure, that we compare 'f' to the interface
pointer from libwayland-client.so. That could probably be done with the
dlopen/dlsym trick, or...

> Another would be to check if 'f' is dereferencable (and not equal to
> the default wl_display_interface), and check if (char *)f starts with
> "wl_d". If yes, do a full string comparison with "wl_display". It's
> not reliable, but maybe works enough.
> 
> The proper solution is for apps to use EGL_KHR_platform_wayland to
> explicitly say the pointer is a wl_display. That of course doesn't work
> on old apps, so you might implement a workaround through environment
> like Mesa does: EGL_PLATFORM=wayland bypasses all autodetection and
> assumes it is a wl_display.
> 
> In any case, EGL really should implement EGL_KHR_platform_wayland so we
> have a chance to get rid of the inherently unreliable autodetect. And we
> should start using that in our demo apps...
> 
> If we consider autodetecting client wl_display pointers worth to solve
> properly, we should probably add a function for that in
> libwayland-client.so. However, implementing that portably might be
> difficult.

...adding a function int/bool wl_is_wl_display_pointer(void *p) into
libwayland-client.so. As that function would be built into
libwayland-client.so, it naturally uses the client.so version of
wl_display_interface symbol (unless dynamic linking miraculously
manages to mess that up?).

'p' would be the tentative 'struct wl_display *', so that the
implementation details of struct wl_display would be contained in
libwayland-client.so, and not leaked elsewhere like e.g. in Mesa.

All this means we should be able to get away with just one pointer
comparison, after assuring that the first sizeof(void*) bytes pointed
to by the given unknown pointer (EGLNativeDisplayType) are
dereferencable without a segfault or other fatal error.

So, a quick fix for EGL implementations right now would be to try the
dlopen hack, until they can start to depend on a new
libwayland-client.so providing wl_is_wl_display_pointer().

I am unsure about the exact definition of wl_is_wl_display_pointer():
can it do the dereferencability check itself, or should we require the
caller to guarantee that dereferencing cannot crash. The latter would
leave the portability burden on the callers rather than libwayland.

How's that sound?

Is it possible to have several instances of libwayland-client in a
process address space? Meaning we would have several different
client-side wl_display_interface pointers?

If yes, it's back to the string comparisons...


Thanks,
pq