Discussion:
[PATCH] egl: link libEGL against the dynamic version of libglapi
Add Reply
Eric Engestrom
2017-12-18 16:33:18 UTC
Reply
Permalink
Raw Message
From: Brendan King <***@imgtec.com>

DRI modules store the address of the dispatch table in a TLS variable,
_glapi_tls_Dispatch.

Changes to the way libEGL is built in d884d8d0077c16d459b1 resulted in
it being statically linked against libglapi, and thus containing its own
copy of _glapi_tls_Dispatch. The result was that some applications would
fail to work (e.g. deqp-egl, which dynamically loads libEGL), due to the
DRI module storing the dispatch table address in one copy of
_glapi_tls_Dispatch, and libEGL obtaining the address from another copy
of the variable.

This applies to autotools builds with --enable-glx-tls (on by default),
and Meson builds (unconditional).

Fixes: d884d8d0077c16d459b1 "egl/dri: link directly to libglapi.so"
Signed-off-by: Brendan King <***@imgtec.com>
Signed-off-by: Eric Engestrom <***@imgtec.com>
---
This issue was noticed in the PowerVR driver. It's unclear whether other
DRI drivers are affected as well.
---
src/egl/Makefile.am | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/egl/Makefile.am b/src/egl/Makefile.am
index 66ba455..58db2c3 100644
--- a/src/egl/Makefile.am
+++ b/src/egl/Makefile.am
@@ -46,7 +46,6 @@ libEGL_common_la_SOURCES = \
$(LIBEGL_C_FILES)

libEGL_common_la_LIBADD = \
- $(top_builddir)/src/mapi/shared-glapi/libglapi.la \
$(top_builddir)/src/util/libmesautil.la \
$(EGL_LIB_DEPS)

@@ -171,7 +170,9 @@ libEGL_mesa_la_SOURCES = \
main/egldispatchstubs.c \
g_egldispatchstubs.c \
g_egldispatchstubs.h
-libEGL_mesa_la_LIBADD = libEGL_common.la
+libEGL_mesa_la_LIBADD = \
+ libEGL_common.la \
+ $(top_builddir)/src/mapi/shared-glapi/libglapi.la
libEGL_mesa_la_LDFLAGS = \
-no-undefined \
-version-number 0 \
@@ -183,7 +184,9 @@ else # USE_LIBGLVND

lib_LTLIBRARIES = libEGL.la
libEGL_la_SOURCES =
-libEGL_la_LIBADD = libEGL_common.la
+libEGL_la_LIBADD = \
+ libEGL_common.la \
+ $(top_builddir)/src/mapi/shared-glapi/libglapi.la
libEGL_la_LDFLAGS = \
-no-undefined \
-version-number 1:0 \
--
2.7.4
Emil Velikov
2017-12-18 16:42:31 UTC
Reply
Permalink
Raw Message
Post by Eric Engestrom
DRI modules store the address of the dispatch table in a TLS variable,
_glapi_tls_Dispatch.
Changes to the way libEGL is built in d884d8d0077c16d459b1 resulted in
it being statically linked against libglapi, and thus containing its own
copy of _glapi_tls_Dispatch. The result was that some applications would
fail to work (e.g. deqp-egl, which dynamically loads libEGL), due to the
DRI module storing the dispatch table address in one copy of
_glapi_tls_Dispatch, and libEGL obtaining the address from another copy
of the variable.
This applies to autotools builds with --enable-glx-tls (on by default),
and Meson builds (unconditional).
Fixes: d884d8d0077c16d459b1 "egl/dri: link directly to libglapi.so"
---
This issue was noticed in the PowerVR driver. It's unclear whether other
DRI drivers are affected as well.
Are you sure any of the extra patches isn't causing the issue?
Just checked the binary and the shared link is there.
Post by Eric Engestrom
---
src/egl/Makefile.am | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/egl/Makefile.am b/src/egl/Makefile.am
index 66ba455..58db2c3 100644
--- a/src/egl/Makefile.am
+++ b/src/egl/Makefile.am
@@ -46,7 +46,6 @@ libEGL_common_la_SOURCES = \
$(LIBEGL_C_FILES)
libEGL_common_la_LIBADD = \
- $(top_builddir)/src/mapi/shared-glapi/libglapi.la \
$(top_builddir)/src/util/libmesautil.la \
$(EGL_LIB_DEPS)
@@ -171,7 +170,9 @@ libEGL_mesa_la_SOURCES = \
main/egldispatchstubs.c \
g_egldispatchstubs.c \
g_egldispatchstubs.h
-libEGL_mesa_la_LIBADD = libEGL_common.la
+libEGL_mesa_la_LIBADD = \
+ libEGL_common.la \
+ $(top_builddir)/src/mapi/shared-glapi/libglapi.la
libEGL_mesa_la_LDFLAGS = \
-no-undefined \
-version-number 0 \
@@ -183,7 +184,9 @@ else # USE_LIBGLVND
lib_LTLIBRARIES = libEGL.la
libEGL_la_SOURCES =
-libEGL_la_LIBADD = libEGL_common.la
+libEGL_la_LIBADD = \
+ libEGL_common.la \
+ $(top_builddir)/src/mapi/shared-glapi/libglapi.la
The exact same object is moved from libEGL_common.la to the parent
(final) shared library.
There is no (obvious) reorder, thus the patch in itself should be a noop.

-Emil
Brendan King
2017-12-19 08:46:37 UTC
Reply
Permalink
Raw Message
Post by Emil Velikov
Post by Eric Engestrom
DRI modules store the address of the dispatch table in a TLS variable,
_glapi_tls_Dispatch.
Changes to the way libEGL is built in d884d8d0077c16d459b1 resulted in
it being statically linked against libglapi, and thus containing its own
copy of _glapi_tls_Dispatch. The result was that some applications would
fail to work (e.g. deqp-egl, which dynamically loads libEGL), due to the
DRI module storing the dispatch table address in one copy of
_glapi_tls_Dispatch, and libEGL obtaining the address from another copy
of the variable.
This applies to autotools builds with --enable-glx-tls (on by default),
and Meson builds (unconditional).
Fixes: d884d8d0077c16d459b1 "egl/dri: link directly to libglapi.so"
---
This issue was noticed in the PowerVR driver. It's unclear whether other
DRI drivers are affected as well.
Are you sure any of the extra patches isn't causing the issue?
Just checked the binary and the shared link is there.
We use slibtool, as libtool no longer works for us (we cross build Mesa,
and libtool fails when trying to re-link libraries). It may be that this
patch fixes an slibtool specific issue. Although slibtool produces .la
files, it doesn't use them itself.

We carry another patch that fixes an slibtool issue. We found it wasn't
possible to build Mesa with Wayland support using slibtool; the link of
libEGL fails with multiple symbol definition errors.
Post by Emil Velikov
Post by Eric Engestrom
---
src/egl/Makefile.am | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/egl/Makefile.am b/src/egl/Makefile.am
index 66ba455..58db2c3 100644
--- a/src/egl/Makefile.am
+++ b/src/egl/Makefile.am
@@ -46,7 +46,6 @@ libEGL_common_la_SOURCES = \
$(LIBEGL_C_FILES)
libEGL_common_la_LIBADD = \
- $(top_builddir)/src/mapi/shared-glapi/libglapi.la \
$(top_builddir)/src/util/libmesautil.la \
$(EGL_LIB_DEPS)
@@ -171,7 +170,9 @@ libEGL_mesa_la_SOURCES = \
main/egldispatchstubs.c \
g_egldispatchstubs.c \
g_egldispatchstubs.h
-libEGL_mesa_la_LIBADD = libEGL_common.la
+libEGL_mesa_la_LIBADD = \
+ libEGL_common.la \
+ $(top_builddir)/src/mapi/shared-glapi/libglapi.la
libEGL_mesa_la_LDFLAGS = \
-no-undefined \
-version-number 0 \
@@ -183,7 +184,9 @@ else # USE_LIBGLVND
lib_LTLIBRARIES = libEGL.la
libEGL_la_SOURCES =
-libEGL_la_LIBADD = libEGL_common.la
+libEGL_la_LIBADD = \
+ libEGL_common.la \
+ $(top_builddir)/src/mapi/shared-glapi/libglapi.la
The exact same object is moved from libEGL_common.la to the parent
(final) shared library.
There is no (obvious) reorder, thus the patch in itself should be a noop.
-Emil
Emil Velikov
2017-12-19 11:23:41 UTC
Reply
Permalink
Raw Message
Post by Emil Velikov
Post by Eric Engestrom
DRI modules store the address of the dispatch table in a TLS variable,
_glapi_tls_Dispatch.
Changes to the way libEGL is built in d884d8d0077c16d459b1 resulted in
it being statically linked against libglapi, and thus containing its own
copy of _glapi_tls_Dispatch. The result was that some applications would
fail to work (e.g. deqp-egl, which dynamically loads libEGL), due to the
DRI module storing the dispatch table address in one copy of
_glapi_tls_Dispatch, and libEGL obtaining the address from another copy
of the variable.
This applies to autotools builds with --enable-glx-tls (on by default),
and Meson builds (unconditional).
Fixes: d884d8d0077c16d459b1 "egl/dri: link directly to libglapi.so"
---
This issue was noticed in the PowerVR driver. It's unclear whether other
DRI drivers are affected as well.
Are you sure any of the extra patches isn't causing the issue?
Just checked the binary and the shared link is there.
We use slibtool, as libtool no longer works for us (we cross build Mesa, and
libtool fails when trying to re-link libraries). It may be that this patch
fixes an slibtool specific issue. Although slibtool produces .la files, it
doesn't use them itself.
We carry another patch that fixes an slibtool issue. We found it wasn't
possible to build Mesa with Wayland support using slibtool; the link of
libEGL fails with multiple symbol definition errors.
I've mentioned this to Eric over IRC - but adding here for posterity.

I would suspect that issue to be similar in nature to the one here.
S does not parse the LT_INIT([disable-static]) hence, unless
explicitly annotated it produces both static and shared lib.

Above seemed to be the case, not too long ago. Now comes the speculation:
As it's creating a static lib - it pulls the static instance of A. In
this case - moving A to the final link stage (aka constructing shared
lib) leads it to pick the glapi DSO.

Quick way to confirm - annotated glapi as shared (only):

--- a/src/mapi/Makefile.am
+++ b/src/mapi/Makefile.am
@@ -77,6 +77,7 @@ shared_glapi_libglapi_la_LIBADD = \
$(PTHREAD_LIBS) \
$(SELINUX_LIBS)
shared_glapi_libglapi_la_LDFLAGS = \
+ -shared \
-no-undefined \
$(GC_SECTIONS) \
$(LD_NO_UNDEFINED)

-Emil
Dylan Baker
2017-12-18 19:14:02 UTC
Reply
Permalink
Raw Message
Quoting Eric Engestrom (2017-12-18 08:33:18)
Post by Eric Engestrom
DRI modules store the address of the dispatch table in a TLS variable,
_glapi_tls_Dispatch.
Changes to the way libEGL is built in d884d8d0077c16d459b1 resulted in
it being statically linked against libglapi, and thus containing its own
copy of _glapi_tls_Dispatch. The result was that some applications would
fail to work (e.g. deqp-egl, which dynamically loads libEGL), due to the
DRI module storing the dispatch table address in one copy of
_glapi_tls_Dispatch, and libEGL obtaining the address from another copy
of the variable.
This applies to autotools builds with --enable-glx-tls (on by default),
and Meson builds (unconditional).
Does this actually apply to the meson build? We don't have an intermediate
convenience library in meson.
Post by Eric Engestrom
Fixes: d884d8d0077c16d459b1 "egl/dri: link directly to libglapi.so"
---
This issue was noticed in the PowerVR driver. It's unclear whether other
DRI drivers are affected as well.
---
src/egl/Makefile.am | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/egl/Makefile.am b/src/egl/Makefile.am
index 66ba455..58db2c3 100644
--- a/src/egl/Makefile.am
+++ b/src/egl/Makefile.am
@@ -46,7 +46,6 @@ libEGL_common_la_SOURCES = \
$(LIBEGL_C_FILES)
libEGL_common_la_LIBADD = \
- $(top_builddir)/src/mapi/shared-glapi/libglapi.la \
$(top_builddir)/src/util/libmesautil.la \
$(EGL_LIB_DEPS)
@@ -171,7 +170,9 @@ libEGL_mesa_la_SOURCES = \
main/egldispatchstubs.c \
g_egldispatchstubs.c \
g_egldispatchstubs.h
-libEGL_mesa_la_LIBADD = libEGL_common.la
+libEGL_mesa_la_LIBADD = \
+ libEGL_common.la \
+ $(top_builddir)/src/mapi/shared-glapi/libglapi.la
libEGL_mesa_la_LDFLAGS = \
-no-undefined \
-version-number 0 \
@@ -183,7 +184,9 @@ else # USE_LIBGLVND
lib_LTLIBRARIES = libEGL.la
libEGL_la_SOURCES =
-libEGL_la_LIBADD = libEGL_common.la
+libEGL_la_LIBADD = \
+ libEGL_common.la \
+ $(top_builddir)/src/mapi/shared-glapi/libglapi.la
libEGL_la_LDFLAGS = \
-no-undefined \
-version-number 1:0 \
--
2.7.4
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Brendan King
2017-12-19 08:48:00 UTC
Reply
Permalink
Raw Message
Post by Dylan Baker
Quoting Eric Engestrom (2017-12-18 08:33:18)
Post by Eric Engestrom
DRI modules store the address of the dispatch table in a TLS variable,
_glapi_tls_Dispatch.
Changes to the way libEGL is built in d884d8d0077c16d459b1 resulted in
it being statically linked against libglapi, and thus containing its own
copy of _glapi_tls_Dispatch. The result was that some applications would
fail to work (e.g. deqp-egl, which dynamically loads libEGL), due to the
DRI module storing the dispatch table address in one copy of
_glapi_tls_Dispatch, and libEGL obtaining the address from another copy
of the variable.
This applies to autotools builds with --enable-glx-tls (on by default),
and Meson builds (unconditional).
Does this actually apply to the meson build? We don't have an intermediate
convenience library in meson.
Eric rewrote the commit message to make it less PowerVR specific, the
comment regarding meson crept in at that point. I think the comment
applies to the enable-glx-tls build option, not to the applicability of
the patch to meson builds.
Post by Dylan Baker
Post by Eric Engestrom
Fixes: d884d8d0077c16d459b1 "egl/dri: link directly to libglapi.so"
---
This issue was noticed in the PowerVR driver. It's unclear whether other
DRI drivers are affected as well.
---
src/egl/Makefile.am | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/egl/Makefile.am b/src/egl/Makefile.am
index 66ba455..58db2c3 100644
--- a/src/egl/Makefile.am
+++ b/src/egl/Makefile.am
@@ -46,7 +46,6 @@ libEGL_common_la_SOURCES = \
$(LIBEGL_C_FILES)
libEGL_common_la_LIBADD = \
- $(top_builddir)/src/mapi/shared-glapi/libglapi.la \
$(top_builddir)/src/util/libmesautil.la \
$(EGL_LIB_DEPS)
@@ -171,7 +170,9 @@ libEGL_mesa_la_SOURCES = \
main/egldispatchstubs.c \
g_egldispatchstubs.c \
g_egldispatchstubs.h
-libEGL_mesa_la_LIBADD = libEGL_common.la
+libEGL_mesa_la_LIBADD = \
+ libEGL_common.la \
+ $(top_builddir)/src/mapi/shared-glapi/libglapi.la
libEGL_mesa_la_LDFLAGS = \
-no-undefined \
-version-number 0 \
@@ -183,7 +184,9 @@ else # USE_LIBGLVND
lib_LTLIBRARIES = libEGL.la
libEGL_la_SOURCES =
-libEGL_la_LIBADD = libEGL_common.la
+libEGL_la_LIBADD = \
+ libEGL_common.la \
+ $(top_builddir)/src/mapi/shared-glapi/libglapi.la
libEGL_la_LDFLAGS = \
-no-undefined \
-version-number 1:0 \
--
2.7.4
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Emil Velikov
2017-12-27 10:54:03 UTC
Reply
Permalink
Raw Message
Post by Eric Engestrom
DRI modules store the address of the dispatch table in a TLS variable,
_glapi_tls_Dispatch.
Changes to the way libEGL is built in d884d8d0077c16d459b1 resulted in
it being statically linked against libglapi, and thus containing its own
copy of _glapi_tls_Dispatch. The result was that some applications would
fail to work (e.g. deqp-egl, which dynamically loads libEGL), due to the
DRI module storing the dispatch table address in one copy of
_glapi_tls_Dispatch, and libEGL obtaining the address from another copy
of the variable.
This applies to autotools builds with --enable-glx-tls (on by default),
and Meson builds (unconditional).
Fixes: d884d8d0077c16d459b1 "egl/dri: link directly to libglapi.so"
---
This issue was noticed in the PowerVR driver. It's unclear whether other
DRI drivers are affected as well.
---
As the issue described is a Slibtool bug, and the Meson mention it
misleading I'll amend the commit message and push later on today:


Note: the following happens only when using slibtool.
Since this is a very serious breakage, we will keep the workaround until a
better solution is available.

DRI modules store the address of the dispatch table in a TLS variable,
_glapi_tls_Dispatch.

Changes to the way libEGL is built in d884d8d0077c16d459b1 resulted in
it being statically linked against libglapi, and thus containing its own
copy of _glapi_tls_Dispatch. The result was that some applications would
fail to work (e.g. deqp-egl, which dynamically loads libEGL), due to the
DRI module storing the dispatch table address in one copy of
_glapi_tls_Dispatch, and libEGL obtaining the address from another copy
of the variable.

Fixes: ...
...
Reviewed-by: Emil Velikov <***@collabora.com>

-Emil
Eric Engestrom
2017-12-27 12:20:59 UTC
Reply
Permalink
Raw Message
Post by Emil Velikov
Post by Eric Engestrom
DRI modules store the address of the dispatch table in a TLS variable,
_glapi_tls_Dispatch.
Changes to the way libEGL is built in d884d8d0077c16d459b1 resulted in
it being statically linked against libglapi, and thus containing its own
copy of _glapi_tls_Dispatch. The result was that some applications would
fail to work (e.g. deqp-egl, which dynamically loads libEGL), due to the
DRI module storing the dispatch table address in one copy of
_glapi_tls_Dispatch, and libEGL obtaining the address from another copy
of the variable.
This applies to autotools builds with --enable-glx-tls (on by default),
and Meson builds (unconditional).
Fixes: d884d8d0077c16d459b1 "egl/dri: link directly to libglapi.so"
---
This issue was noticed in the PowerVR driver. It's unclear whether other
DRI drivers are affected as well.
---
As the issue described is a Slibtool bug, and the Meson mention it
Ack, and sorry for mentioning meson when it was irrelevant :)
Post by Emil Velikov
Note: the following happens only when using slibtool.
Since this is a very serious breakage, we will keep the workaround until a
better solution is available.
DRI modules store the address of the dispatch table in a TLS variable,
_glapi_tls_Dispatch.
Changes to the way libEGL is built in d884d8d0077c16d459b1 resulted in
it being statically linked against libglapi, and thus containing its own
copy of _glapi_tls_Dispatch. The result was that some applications would
fail to work (e.g. deqp-egl, which dynamically loads libEGL), due to the
DRI module storing the dispatch table address in one copy of
_glapi_tls_Dispatch, and libEGL obtaining the address from another copy
of the variable.
Fixes: ...
...
-Emil
Loading...