Created attachment 116709 [details] glx: fix unit tests build on osx It fails because of missing symbols and some duplicated symbols. The attached patch adds missing dependencies in tests/Makefile.am to fix some missing symbols. The attached patch also marks some functions with attribute weak to fix some duplicated symbols due to redefinitions in the unit tests. It allows to fix the build. Then it passes some tests but crash around the 30th test (glXCreateContextAttribARB_test.does_send_protocol): It seems to crash when calling apple_cgl.choose_pixel_format. I investigated a bit and glx/apple/apple_cgl.c::apple_cgl_init is not called (so apple_cgl.choose_pixel_format = sym(h, "CGLChoosePixelFormat"); is not called). backtrace with lldb: * thread #1: tid = 0x86cd39, 0x0000000000000000, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0) frame #0: 0x0000000000000000 (lldb) bt * thread #1: tid = 0x86cd39, 0x0000000000000000, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0) * frame #0: 0x0000000000000000 frame #1: 0x00000001000879fa glx-test`apple_visual_create_pfobj(pfobj=0x0000000100c22e88, mode=0x0000000100c22328, double_buffered=0x0000000100c22ea4, uses_stereo=0x0000000100c22ea5, offscreen=false) + 1610 at apple_visual.c:169 frame #2: 0x00000001000833b9 glx-test`apple_glx_create_context(ptr=0x0000000100c22de8, dpy=0x0000000000000000, screen=0, mode=0x0000000100c22328, sharedContext=0x0000000000000000, errorptr=0x00007fff5fbff09c, x11errorptr=0x00007fff5fbff09b) + 377 at apple_glx_context.c:161 frame #3: 0x00000001000823ec glx-test`applegl_create_context(psc=0x0000000100c22c90, config=0x0000000100c22328, shareList=0x0000000000000000, renderType=0) + 332 at applegl_glx.c:150 frame #4: 0x00000001000439f3 glx-test`glXCreateContextAttribsARB(dpy=0x000000010100e400, config=0x0000000100c22328, share_context=0x0000000000000000, direct=0, attrib_list=0x0000000000000000) + 451 at create_context.c:86 frame #5: 0x00000001000155c1 glx-test`glXCreateContextAttribARB_test_does_send_protocol_Test::TestBody(this=0x0000000100c22310) + 97 at create_context_unittest.cpp:221 frame #6: 0x00000001004a1f13 glx-test`void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(object=0x0000000100c22310, method=0x0000000000000021, location=0x000000010057f3fd)(), char const*) + 131 at gtest.cc:2078 frame #7: 0x000000010048c7b7 glx-test`void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(object=0x0000000100c22310, method=0x0000000000000021, location=0x000000010057f3fd)(), char const*) + 119 at gtest.cc:2114 frame #8: 0x0000000100465b05 glx-test`testing::Test::Run(this=0x0000000100c22310) + 197 at gtest.cc:2150 frame #9: 0x0000000100466ddb glx-test`testing::TestInfo::Run(this=0x0000000100c1a420) + 219 at gtest.cc:2326 frame #10: 0x0000000100467d07 glx-test`testing::TestCase::Run(this=0x0000000100c1a040) + 231 at gtest.cc:2444 frame #11: 0x00000001004747f8 glx-test`testing::internal::UnitTestImpl::RunAllTests(this=0x0000000100c17590) + 952 at gtest.cc:4315 frame #12: 0x000000010049ed93 glx-test`bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(object=0x0000000100c17590, method=0x0000000100474440, location=0x000000010057fabc)(), char const*) + 131 at gtest.cc:2078 frame #13: 0x000000010048efc7 glx-test`bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(object=0x0000000100c17590, method=0x0000000100474440, location=0x000000010057fabc)(), char const*) + 119 at gtest.cc:2114 frame #14: 0x00000001004743b6 glx-test`testing::UnitTest::Run(this=0x00000001005b3458) + 422 at gtest.cc:3926 frame #15: 0x00000001004ac491 glx-test`RUN_ALL_TESTS() + 17 at gtest.h:2288 frame #16: 0x00000001004ac46c glx-test`main(argc=1, argv=0x00007fff5fbffac0) + 60 at gtest_main.cc:37 frame #17: 0x00007fff886ee5c9 libdyld.dylib`start + 1 Also I tried to manually call __glXInitialize (because it is never called) in tests/create_context_unittest.cpp::GetGLXScreenConfigs existing redefinition: GetGLXScreenConfigs(Display * dpy, int scrn) { + if(psc) + __glXInitialize(dpy); But it still does not make "apple_cgl_init" to be called, because it crashes in glxext.c:847: dpyPriv->codes = XInitExtension(dpy, __glXExtensionName); * thread #1: tid = 0x86de8e, 0x0000000100989057 libX11.6.dylib`require_socket + 32, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x40) frame #0: 0x0000000100989057 libX11.6.dylib`require_socket + 32 libX11.6.dylib`require_socket + 32: -> 0x100989057: cmpl $0x0, 0x40(%rax) 0x10098905b: setne %cl 0x10098905e: movzbl %cl, %ecx 0x100989061: movq (%rax), %rdi (lldb) bt * thread #1: tid = 0x86de8e, 0x0000000100989057 libX11.6.dylib`require_socket + 32, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x40) * frame #0: 0x0000000100989057 libX11.6.dylib`require_socket + 32 frame #1: 0x0000000100989018 libX11.6.dylib`_XFlush + 14 frame #2: 0x000000010098a1fb libX11.6.dylib`_XGetRequest + 53 frame #3: 0x000000010098036f libX11.6.dylib`XQueryExtension + 68 frame #4: 0x0000000100975a80 libX11.6.dylib`XInitExtension + 36 frame #5: 0x0000000100048fc4 glx-test`__glXInitialize(dpy=0x0000000101803c00) + 308 at glxext.c:847 frame #6: 0x000000010001490b glx-test`GetGLXScreenConfigs(dpy=0x0000000101803c00, scrn=0) + 75 at create_context_unittest.cpp:70 frame #7: 0x00000001000438bd glx-test`glXCreateContextAttribsARB(dpy=0x0000000101803c00, config=0x0000000100d00098, share_context=0x0000000000000000, direct=0, attrib_list=0x0000000000000000) + 141 at create_context.c:59 frame #8: 0x00000001000155c1 glx-test`glXCreateContextAttribARB_test_does_send_protocol_Test::TestBody(this=0x0000000100d00080) + 97 at create_context_unittest.cpp:221 frame #9: 0x00000001004a1f13 glx-test`void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(object=0x0000000100d00080, method=0x0000000000000021, location=0x000000010057f3fd)(), char const*) + 131 at gtest.cc:2078 frame #10: 0x000000010048c7b7 glx-test`void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(object=0x0000000100d00080, method=0x0000000000000021, location=0x000000010057f3fd)(), char const*) + 119 at gtest.cc:2114 frame #11: 0x0000000100465b05 glx-test`testing::Test::Run(this=0x0000000100d00080) + 197 at gtest.cc:2150 frame #12: 0x0000000100466ddb glx-test`testing::TestInfo::Run(this=0x0000000100c1a420) + 219 at gtest.cc:2326 frame #13: 0x0000000100467d07 glx-test`testing::TestCase::Run(this=0x0000000100c1a040) + 231 at gtest.cc:2444 frame #14: 0x00000001004747f8 glx-test`testing::internal::UnitTestImpl::RunAllTests(this=0x0000000100c17590) + 952 at gtest.cc:4315 frame #15: 0x000000010049ed93 glx-test`bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(object=0x0000000100c17590, method=0x0000000100474440, location=0x000000010057fabc)(), char const*) + 131 at gtest.cc:2078 frame #16: 0x000000010048efc7 glx-test`bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(object=0x0000000100c17590, method=0x0000000100474440, location=0x000000010057fabc)(), char const*) + 119 at gtest.cc:2114 frame #17: 0x00000001004743b6 glx-test`testing::UnitTest::Run(this=0x00000001005b3458) + 422 at gtest.cc:3926 frame #18: 0x00000001004ac491 glx-test`RUN_ALL_TESTS() + 17 at gtest.h:2288 frame #19: 0x00000001004ac46c glx-test`main(argc=1, argv=0x00007fff5fbffac0) + 60 at gtest_main.cc:37 frame #20: 0x00007fff886ee5c9 libdyld.dylib`start + 1 (lldb) Any comment about the patch to fix the build and about the crashes ?
Hi Julien, I'm not sure how many people go bugzilla hunting for patches. Here are some things that stuck out: From 2b424445a8c9028e72e2370da8dbdc969bb8717b Mon Sep 17 00:00:00 2001 From: Julien Isorce <j.isorce@samsung.com> Date: Thu, 25 Jun 2015 08:59:27 +0100 Subject: [PATCH] glx: fix unit tests build on osx It fails because of missing symbols and some duplicated symbols. The patch adds missing dependencies in tests/Makefile.am to fix some missing symbols. The patch also marks some functions with attribute weak to fix some duplicated symbols due to redefinitions in the unit tests. Signed-off-by: Julien Isorce <j.isorce@samsung.com> --- src/glx/glxcmds.c | 10 ++++++++-- src/glx/glxcurrent.c | 8 +++++++- src/glx/glxextensions.c | 7 ++++++- src/glx/indirect_init.h | 2 ++ src/glx/tests/Makefile.am | 9 ++++++++- 5 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/glx/glxcmds.c b/src/glx/glxcmds.c index 26ff804..f073ecb 100644 --- a/src/glx/glxcmds.c +++ b/src/glx/glxcmds.c @@ -55,6 +55,12 @@ #include <xcb/xcb.h> #include <xcb/glx.h> +#if defined(GLX_USE_APPLEGL) +#define _ATTRIBUTE_WEAK __attribute__((weak)) +#else +#define _ATTRIBUTE_WEAK +#endif + (If we end up keeping this) Can we avoid redefining it X times ? Ian (the original author of these tests) should know the details as to why/how we don't end up with duplicated/conflicting symbols. --- a/src/glx/tests/Makefile.am +++ b/src/glx/tests/Makefile.am @@ -21,7 +21,6 @@ glx_test_SOURCES = \ enum_sizes.cpp \ fake_glx_screen.cpp \ fake_glx_screen.h \ - indirect_api.cpp \ mock_xdisplay.h \ query_renderer_unittest.cpp @@ -30,9 +29,17 @@ glx_test_SOURCES += \ query_renderer_implementation_unittest.cpp endif +if !HAVE_APPLEDRI +glx_test_SOURCES += indirect_api.cpp +endif + glx_test_LDADD = \ $(top_builddir)/src/glx/libglx.la \ $(top_builddir)/src/gtest/libgtest.la \ + $(top_builddir)/src/mapi/glapi/libglapi.la \ Which symbols are missing if we omit this ? Noone else seems to need it. $(top_builddir)/src/mapi/shared-glapi/libglapi.la \ + $(SHARED_GLAPI_LIB) \ Unset/empty variable. + $(GL_LIB_DEPS) \ Good catch, I think we can now nuke PTHEADS_LIBS below. Perhaps do this as a separate commit ? + $(X11_LIBS) \ Unset/empty variable. Upon closer look one can do a X11_* variable cleanup though mesa. $(PTHREAD_LIBS) endif -- 1.9.5 (Apple Git-50.3)
Although I would honestly suggest that one takes a closer look into src/glx{,/apple} and give it some much needed love. Iirc Jon had some branches which rework/clean things up which seems like the way forward imho.
(In reply to Emil Velikov from comment #1) > +#if defined(GLX_USE_APPLEGL) > +#define _ATTRIBUTE_WEAK __attribute__((weak)) > +#else > +#define _ATTRIBUTE_WEAK > +#endif > + > (If we end up keeping this) Can we avoid redefining it X times ? Ian (the > original author of these tests) should know the details as to why/how we > don't end up with duplicated/conflicting symbols. These really are duplicates that the test harness provides to override the real versions. Unfortunately, ld on OSX doesn't behave that way, and the option -multiply_defined suppress seems to be obsolete. I'm not very keen on scattering __attribute__(weak) around to fix this, but I guess the only other approach is to split those symbols out into a separate object, which is linked with to produce libglx.a, but not into a convenience library which is used by the tests. > + $(top_builddir)/src/mapi/glapi/libglapi.la \ > Which symbols are missing if we omit this ? Noone else seems to need it. glapi_create_table_from_handle, which only exists on APPLE, if memory serves. But linking with glapi and shared-glapi doesn't seem right, so I think this is a sign that something else is wrong.
(In reply to Emil Velikov from comment #2) > Iirc Jon had some > branches which rework/clean things up which seems like the way forward imho. Sorry, all the clean-up I did for __APPLE__ is already upstream, with the exception of the 2 patches which clearly need to be done in a better way.
There have been verious osx fixes since this bug was filed. Is this still a problem or can we close this bug?
Pretty sure we can call this fixed with Jon's changes from a few years back. If there are still issues, we'll need newer reports.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.